[$250] mWeb - Approver - Setting non existing user as approver shows error & then for existing users too

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.25
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Actual Result:

1. Go to https://staging.new.expensify.com/home
2. Tap profile icon -- workspaces -- new workspace
3. Tap member -- invite a existing user from the list
4. Tap more features -- enable workflow
5. Tap work flow -- add approver
6. Select the existing user and set him as approver
7. Note existing user is saved as approver
8. Navigate back and create a new workspace
9. Tap member -- invite a non existing user
10. Close the error
11. Tap more features -- enable workflow
12. Tap work flow -- add approver
13. Select the non existing user and set him as approver
14. Note unable to select and save non existing user as approver
15. Navigate back to settings page and note error "402 approver can only be set for members of the policy. "
16. Navigate back to workspace settings
17. Tap members - invite an existing user from list
18. Tap work flow -- add approver
19. Select the existing user and set him as approver
20. Note now existing user also cannot be selected and saved as approver
21. Navigate back to workspace settings and note error "402 approver can only be set for members of the policy. "

Expected Result:

Selected user must be displayed in workflow as approver without any error

Actual Result:

Non existing user cannot be selected & saved as approver. Error displays on setting non existing users as approver. After this, for setting existing user as approver also same error is shown and unable to save approver

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6585035_1724836009845.Screenrecorder-2024-08-28-14-15-35-263_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021836594715808027566
• Upwork Job ID: 1836594715808027566
• Last Price Increase: 2024-10-03

Issue Owner

Current Issue Owner: @abdulrahuman5196

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[cretadn22]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Assigning a member with an error as an approver triggered another error in the approval workflow

What is the root cause of that problem?

We don't exclude members with errors from the approver list

App/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx

Line 135 in eec5e2b

.filter((approver): approver is SelectionListApprover => !!approver);

What changes do you think we should make in order to solve the problem?

The member errors are stored in policy?.employeeList > employee.errors. We also need to filter out members with errors from the approver list

What alternative solutions did you explore? (Optional)

[kadiealexander]

Android <> ios swap

[jliexpensify]

Hmm I am not able to reproduce this one on Android web v26-4 (staging)

I get up to Step 10 - Close the error - but I don't see any error, and my "non-existent" user is actually saved fine.

@lanitochka17 please re-test on the newest version of Android web and share a new video, thanks!

[melvin-bot]

@jliexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jliexpensify]

@lanitochka17 just pinging you again to re-test on the newest version please.

[trjExpensify]

Where did we get to on that?

[lanitochka17]

Issue is still reproducible

Screenrecorder-2024-09-04-10-02-19-681_compress_1.mp4

[jliexpensify]

@lanitochka17 what version are you testing on? I still cannot reproduce this.

[jliexpensify]

Bump @lanitochka17 - what version are you testing on? I still cannot repro.

[trjExpensify]

@jliexpensify probably better to escalate to #qa at this point.

[jliexpensify]

Posted

[melvin-bot]

@jliexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[jliexpensify]

Waiting on a retest, Kavi has acknowledged.

[melvin-bot]

@jliexpensify Eep! 4 days overdue now. Issues have feelings too...

[jliexpensify]

Bumped again

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021836594715808027566

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @abdulrahuman5196 (External)

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Non existing user cannot be selected & saved as approver. Error displays on setting non existing users as approver. After this, for setting existing user as approver also same error is shown and unable to save approver

What is the root cause of that problem?

1. We don't filter the user with pending action as add and has the error.

App/src/pages/workspace/workflows/approvals/WorkspaceWorkflowsApprovalsApproverPage.tsx

Lines 80 to 83 in f7a4aa1

	const availableApprovers = Object.values(employeeList)
	.map((employee): SelectionListApprover | null => {
	const isAdmin = employee?.role === CONST.REPORT.ROLE.ADMIN;
	const email = employee.email;

2. After we invite a non-existing user, BE returns the error but this user is still stored in employeeList of the policy in the backend. After that we can't change the approver for an existing user.

What changes do you think we should make in order to solve the problem?

1. We should filter the members that are pending add and have the error. We shouldn't always filter the members that has errors because the member can have the error when we remove this issue failure and in this case, this member should appear in the approval list

const availableApprovers = Object.values(employeeList)
  .filter((employee) => !employee.errors || employee.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD)
  .map((employee): SelectionListApprover | null => {

2. BE shouldn't save the non-existing user as a member of the policy if we return an error when inviting this user.

What alternative solutions did you explore? (Optional)

[melvin-bot]

@jliexpensify, @abdulrahuman5196 Eep! 4 days overdue now. Issues have feelings too...

[jliexpensify]

Bump @abdulrahuman5196 for a review of the above proposal!

[melvin-bot]

@jliexpensify @abdulrahuman5196 this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[jliexpensify]

Bumped @abdulrahuman5196 on Slack

[abdulrahuman5196]

hi sorry for the delay. Checking now and will get an update before EOD

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[abdulrahuman5196]

@jliexpensify / @kavimuru I am unable to repro this issue.

Note unable to select and save non existing user as approver
Navigate back to settings page and note error "402 approver can only be set for members of the policy. "

This is not happening for me. The user is added properly.

Screen.Recording.2024-09-26.at.10.48.04.PM.mov

[jliexpensify]

@abdulrahuman5196 it was inconsistent for me as well to reproduce this.

A couple of things:

1. I noticed you may have typed .co at the end of the email, what happens when you type .com instead?
2. After adding the user, can you wait 5 seconds and see if an error generates? It took a little bit of time for me to see it.

@lanitochka17 can you also re-test on the newest version as well and share a video?

[jliexpensify]

Just a heads up I'm OOO from 3rd to 14th, but I don't think anything is needed from me during this period. Feel free to reassign to another B0 if needed though!

[melvin-bot]

@jliexpensify, @abdulrahuman5196 Eep! 4 days overdue now. Issues have feelings too...

[abdulrahuman5196]

A couple of things:
I noticed you may have typed .co at the end of the email, what happens when you type .com instead?
After adding the user, can you wait 5 seconds and see if an error generates? It took a little bit of time for me to see it.

Still weren't able to repro the issue. We need to re-test this.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@jliexpensify, @abdulrahuman5196 Huh... This is 4 days overdue. Who can take care of this?

[kadiealexander]

Requested a retest here.

[kavimuru]

Reproduced by the tester

Screenrecorder-2024-10-09-08-24-55-123_compress_1.mp4

[kadiealexander]

I think this is expected behaviour in the video (there isn't an error that shows, and the approver switches back to the original approver). I'm going to close the issue, but feel free to reopen if you disagree.