Prevent Self-Approval: Fix Unexpected Behavior for Historical Workspaces and Update Existing Approvers

[marcaaron]

cc @JmillsExpensify coming from our DM

While we have the ability to set "Prevent self approval" on a workspace. There are some ways in which this feature works unexpectedly in NewDot. Specifically, the following scenario is possible and should not be:

Historical workspace behavior

Problem

• User has historical workspaces with themselves already set as the approver.
• Self approval is then enabled.
• What should we do after that? They should not be able to approve their own expenses, but we need some way to "fix" the reports currently submitted to themselves.

Solution

• Create a modal that warns the user that any users who are currently approving their own reports will be removed as the manager.
• Set the approver back to the "default approver" as shown here:

[melvin-bot]

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[marcaaron]

Specifically we need design help on:

Create a modal that warns the user that any users who are currently approving their own reports will be removed as the manager.

[dannymcclain]

@marcaaron when will this modal show?

[marcaaron]

QA steps would be something like:

1. As User A enable Rules on a workspace
2. Invite User B
3. In Workflows create a rule for User B that has them set as their own approver
   
4. In Rules select the Prevent Self Approvals option
   
5. Modal is triggered since now User B can't be their own approver. We will be setting their approver to the default approver if the modal is "confirmed".

[dannymcclain]

Perfect! Thank you.

[shawnborton]

Yeah, I tend to agree - just a simple, matter-of-fact dialog seems appropriate.

[dannymcclain]

And actually, I just realized I did this one wrong. I think it should actually be like this:

Where we give you the warning and the choice. @shawnborton @dubielzyk-expensify, the question is, should this button be danger?

[shawnborton]

Hmm I could go either way honestly. What do you think? It doesn't feel quite serious enough to warrant a danger button but I can see why the logic would check out to do so if that's what we decide.

[dannymcclain]

Yeah I feel the same way. Let's make @dubielzyk-expensify weigh in! :gavel:

[dubielzyk-expensify]

Yeah, I'm with ya. Don't feel strongly. Danger button seems perhaps a bit too harsh for what it is. cc @jamesdeanexpensify for copy!?

[dannymcclain]

I think we can go with the green button for this one—even if some workflows are reset to the default approver, no information or anything is actually being destroyed. Nothing is going to stop working. So I agree with Jon the the danger button feels too harsh.

[jamesdeanexpensify]

Slight tweaks!

Prevent self-approvals?
Any members currently approving their own expenses will be replaced with the default approver for this workspace (email/phone).

[dannymcclain]

Update mocks based on new copy from James!

[dubielzyk-expensify]

Looking good, McClain!

[trjExpensify]

👋 I think I have a few questions about this. But maybe to start, it would be helpful if someone could list out the numbered steps of the proposed solution in full? Are the QA steps here the extent of the proposal, or is there more to it?

Thanks!

[JmillsExpensify]

Also I have a question: How are we handling the case where someone enables this feature, they submit reports, and they are the default approver for the workspace? I would imagine we need to create a special approval workflow for that case, right? Asking because I ran across this case in a #migrate thread.

[dannymcclain]

Any update on this one? Do we have clear next steps to move this along?

[JmillsExpensify]

Btw, we need to find someone to pick this now that Marc is on leave.

[JmillsExpensify]

Still need to find a volunteer. This is being highlighted as a hot pick in the weekly update.