[$250] Category rule - Approver field is not empty after the approver is removed from the workspace

[IuliiaHerets]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.39-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

• Rules are enabled.
• "Add approvals" is enabled in Workflows.
• Workspace has at least one member other than yourself.

1. Go to staging.new.expensify.com
2. Go to workspace settings > Categories..
3. Click on any category.
4. Click Approver.
5. Select a member as the approver.
6. Go to Members.
7. Remove the member from the workspace.
8. Go back to the category rule of the category in Step 3.

Expected Result:

Approver field will be empty because the approver is no longer a workspace member.

Actual Result:

Approver field still shows the removed member when the approver is no longer a workspace member.

Workaround:

Unknown

Platforms:

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/d169ef74-d918-435d-832f-8fe111f02d22

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021854279355604338561
• Upwork Job ID: 1854279355604338561
• Last Price Increase: 2024-11-13

Issue Owner

Current Issue Owner: @mollfpr

[melvin-bot[bot]]

Triggered auto assignment to @mallenexpensify (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.

[IuliiaHerets]

We think that this bug might be related to #wave-control

[IuliiaHerets]

@mallenexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[abzokhattab]

Edited by proposal-police: This proposal was edited at 2024-09-22 11:52:04 UTC.

Proposal

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

The approver field is not empty after the approver is removed from the workspace.

What is the root cause of that problem?

We don't reset the approval rule to be empty after deleting a member.

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

Since we retrieve the category rule in the getCategoryApproverRule function, we can reset the rule.approver property to null if the employee is found to be deleted from the policy's employee list.

To achieve this, we need to add the policyEmployeeList as an additional argument to the getCategoryApproverRule function. We then use the employee list to check if the approver has been removed:

function getCategoryApproverRule(approvalRules: ApprovalRule[], categoryName: string, policyEmployeeList?: Policy['employeeList']) {
    const approverRule = approvalRules?.find((rule) =>
        rule.applyWhen.find(({condition, field, value}) => condition === CONST.POLICY.RULE_CONDITIONS.MATCHES && field === CONST.POLICY.FIELDS.CATEGORY && value === categoryName),
    );

    if (!approverRule) {
        return null;
    }

    if (policyEmployeeList) {
        const approverPolicyMember = approverRule.approver ? policyEmployeeList?.[approverRule.approver] : null;
        if (!approverPolicyMember || approverPolicyMember?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) {
            approverRule.approver = '';
        }
    }
    return approverRule;
}

Next, we need to add the policy.employeeList as an argument here: https://github.com/Expensify/App/blob/2fdea8246269e9c1cc6b9ccf2a349fc6aa31d83e/src/pages/workspace/categories/CategorySettingsPage.tsx#L80

optional: if we want to reset the rule completely in this case, we can return null instead of resetting the approverRule.approver in this case. here is the sudo code for the updated code:

    return approvalRules?.find((rule) =>
        rule.applyWhen.find(({condition, field, value}) => condition === CONST.POLICY.RULE_CONDITIONS.MATCHES && field === CONST.POLICY.FIELDS.CATEGORY && value === categoryName) && 
    (!policyEmployeeList || rule.approver == null || policyEmployeeList[rule.approver]?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE)
    )

POC

https://github.com/user-attachments/assets/903f05ed-7ed5-4e36-97ee-8f8152a9abdf

What alternative solutions did you explore? (Optional)

Another solution is to implement both frontend and backend changes when removing a member from a workspace:

1. In the remove member request (DeleteMembersFromWorkspace), the backend should also check if this member is used as an approver in any rules and reset the rule value accordingly.
2. In the frontend, we should set the rule and set the pending fields in the optimisticData, successData, and failureData of the delete member request (DeleteMembersFromWorkspace), ensuring we also handle the offline environment properly. PS: the rule info could be fetched from the policy and we can find the approval rule after looping though the rules object (rules.approvalRules) and check if the approver is used in any of them.

[mallenexpensify]

Checking with Tom about quick

[melvin-bot[bot]]

@mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mallenexpensify]

Posted and bumped in #wave-control room https://expensify.slack.com/archives/C06ML6X0W9L/p1727482508265999?thread_ts=1727123059.243429&cid=C06ML6X0W9L

[mallenexpensify]

Per Dylan, Put on hold pending the beta being built for it

• https://github.com/Expensify/Expensify/issues/431466

[mallenexpensify]

On hold

[mallenexpensify]

Off hold, the below was closed

• https://github.com/Expensify/Expensify/issues/431466

Bumped to Daily, will test soon.

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[melvin-bot[bot]]

@mallenexpensify Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[mallenexpensify]

Was able to reproduce

and

@mollfpr, can you review the proposal above? Thx

[melvin-bot[bot]]

Triggered auto assignment to @zanyrenney (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.

[mallenexpensify]

@zanyrenney I'm off the next week, can you please keep an eye on this issue til I'm back? Thx

[melvin-bot[bot]]

@mallenexpensify, @mollfpr, @zanyrenney Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot[bot]]

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

[zanyrenney]

Sure thing @mallenexpensify !

[zanyrenney]

Seeing as @mallenexpensify successfully reproduced this, I am removing the label.

[zanyrenney]

bumping @mollfpr to review the proposal above.

[melvin-bot[bot]]

@mallenexpensify, @mollfpr, @zanyrenney Still overdue 6 days?! Let's take care of this!

[mollfpr]

We don't reset the approval rule to be empty after deleting a member.

@abzokhattab I agree with RCA, but why is the solution not related to resetting the rules?

[abzokhattab]

Updated the proposal to include an additional point on resetting the rule in case the approval status is changed.

[melvin-bot[bot]]

@mallenexpensify, @mollfpr, @zanyrenney Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[mallenexpensify]

@mollfpr , 👀 above plz. Thanks for watching over Zany, I'm back!

[mollfpr]

We don't reset the approval rule to be empty after deleting a member.

@abzokhattab, I'm still not onboard with the solution. The root cause is in our BE response not updating the policy rules.approvalRules after removing the member. So I think the proper solution is:

1. BE: Fixing the API DeleteMembersFromWorkspace response to have the rules.approvalRules updated with the employee list
2. FE: Follow up with the same approach for the OFFLINE

That's it, no need to update getCategoryApproverRule since we got the correct data for rules.approvalRules.

[abzokhattab]

Thanks @mollfpr for your feedback.

I think i have referred to this approach in the alternative proposal but i have just added more details to it to be more concrete.

please review the alternative proposal and let me know what you think.

[mollfpr]

Ohyeah, you already have that in the alternative solution my bad 🙏

The alternative solution looks good to me and it's pretty much the same as the strategy I suggest. I think we can move forward from here and discuss the BE changes with the selected engineer.

🎀 👀 🎀 C+ reviewed!