[HOLD for payment 2024-08-02] [$250] Workspace - "Remove member" modal show that the user is an approver when they are not approver

@kadiealexander

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.7-4
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

Action Performed:

Go to staging.new.expensify.com
Create a new workspace
Go to workspace settings > Members
Add some members
Click on any member
Click Remove from workspace

Expected Result:

"Remove member" modal will not show that the user is an approver if they are not an approver

Actual Result:

"Remove member" modal show that the user is an approver when they are not an approver

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6543977_1721137169629.20240716_213707.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b2f905d2ab7918d6
Upwork Job ID: 1813404779499632184
Last Price Increase: 2024-07-17
Automatic offers:

allgandalf | Reviewer | 103193071

Issue Owner

Current Issue Owner: @kadiealexander

@kadiealexander

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.

@kadiealexander

@kadiealexander 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

We think that this bug might be related to #wave-collect - Release 1

Proposal

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

"Remove member" modal show that the user is an approver when they are not approver

What is the root cause of that problem?

When isApprover is false, we are not returning translate('workspace.people.removeMemberPrompt', {memberName: displayName});. Therefore, code execution continues to the next line of code and return translate('workspace.people.removeMembersWarningPrompt', {memberName: displayName, ownerName: policyOwnerDisplayName});

App/src/pages/workspace/members/WorkspaceMemberDetailsPage.tsx

Lines 73 to 77 in e009c43

    
               if (!isApprover) { 
        
                   translate('workspace.people.removeMemberPrompt', {memberName: displayName}); 
        
               } 
        
               return translate('workspace.people.removeMembersWarningPrompt', {memberName: displayName, ownerName: policyOwnerDisplayName}); 
        
           }, [accountID, policy, displayName, policyOwnerDisplayName, translate]);

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

Add return statement before translate('workspace.people.removeMemberPrompt', {memberName: displayName});

return translate('workspace.people.removeMemberPrompt', {memberName: displayName});

Note: isApprover seems to be returning false for approver too. We can fix that too by adding || employeeLogin === policy?.approver below (LN 108)

App/src/libs/actions/Policy/Member.ts

Lines 105 to 108 in 307565a

    
           function isApprover(policy: OnyxEntry<Policy>, employeeAccountID: number) { 
        
               const employeeLogin = allPersonalDetails?.[employeeAccountID]?.login; 
        
               return Object.values(policy?.employeeList ?? {}).some( 
        
                   (employee) => employee?.submitsTo === employeeLogin || employee?.forwardsTo === employeeLogin || employee?.overLimitForwardsTo === employeeLogin,

Doing that will fix similar issues on workspace member page too

What alternative solutions did you explore? (Optional)

Result:

Screen.Recording.2024-07-16.at.10.26.11.at.night.mp4

Proposal Updated:

Added result

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

@allgandalf

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

Proposal

Problem Statement
"Remove member" modal should show that the user is an approver when they are not an approver but it is showing user is an approver even if they are not an approver.

Problem
In the confirm modal prompt, we are getting the false value in isApprover but it is not returned

Solution
return translate('workspace.people.removeMemberPrompt', {memberName: displayName});

Contributor details
Your Expensify account email: akashftg@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~0114eee6cee79fee8e

@allgandalf

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.12-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: "Remove member" modal should display the right message when the removed user is not approver #45878

If no regressions arise, payment will be issued on 2024-08-02. 🎊

For reference, here are some details about the assignees on this issue:

@allgandalf requires payment automatic offer (Reviewer)
@etCoderDysto requires payment (Needs manual offer from BZ)

@allgandalf

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@allgandalf] The PR that introduced the bug has been identified. Link to the PR: feat: Display warning prompt when removing an approver from a control policy #44008
[@allgandalf] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/44008/files#r1694533629
[@allgandalf] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: n/a
[@allgandalf] Determine if we should create a regression test for this bug.
[@allgandalf] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/417762

The PR that introduced the bug has been identified. Link to the PR: feat: Display warning prompt when removing an approver from a control policy #44008
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/44008/files#r1694533629
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal 1

Precondition: Workspace member is an approver of the workspace.

As a admin, Go to workspace settings
Go to members
Click on the profile of a member who is approver and click on remove member

Verify that you see the message : ${memberName} is an approver in this workspace. When you unshare this workspace with them, we’ll replace them in the approval workflow with the workspace owner, ${ownerName}

Regression Test Proposal 2

Precondition: Workspace member is not an approver of the workspace.

As a admin, Go to workspace settings
Go to members
Click on the profile of a member who is approver and click on remove member

Verify that you see the message : Are you sure you want to remove ${memberName}?

Do we agree 👍 or 👎

LGTM, opened internal issue to create a regression test.

Can this be paid out please :)

@etCoderDysto

Payouts due:

Contributor: $250 @etCoderDysto
Contributor+: $250 @allgandalf

Upwork job is here.

@etCoderDysto

@etCoderDysto I can't find you on Upwork, could you please apply to the Upwork job?

@etCoderDysto

@etCoderDysto sent you an offer, thanks!

@kadiealexander

@kadiealexander I have accepted the offer. Thanks!

Navigation Menu

Repository navigation

[HOLD for payment 2024-08-02] [$250] Workspace - "Remove member" modal show that the user is an approver when they are not approver #45510

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Proposal

26 remaining items

Regression Test Proposal 1

Regression Test Proposal 2

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (!isApprover) {
	translate('workspace.people.removeMemberPrompt', {memberName: displayName});
	}
	return translate('workspace.people.removeMembersWarningPrompt', {memberName: displayName, ownerName: policyOwnerDisplayName});
	}, [accountID, policy, displayName, policyOwnerDisplayName, translate]);

	function isApprover(policy: OnyxEntry<Policy>, employeeAccountID: number) {
	const employeeLogin = allPersonalDetails?.[employeeAccountID]?.login;
	return Object.values(policy?.employeeList ?? {}).some(
	(employee) => employee?.submitsTo === employeeLogin \|\| employee?.forwardsTo === employeeLogin \|\| employee?.overLimitForwardsTo === employeeLogin,

[HOLD for payment 2024-08-02] [$250] Workspace - "Remove member" modal show that the user is an approver when they are not approver #45510

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Jul 16, 2024

lanitochka17 commented on Jul 16, 2024

lanitochka17 commented on Jul 16, 2024

etCoderDysto commented on Jul 16, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

etCoderDysto commented on Jul 16, 2024

melvin-bot commented on Jul 17, 2024

melvin-bot commented on Jul 17, 2024

codeapexdev commented on Jul 17, 2024

Proposal

26 remaining items

melvin-bot commented on Jul 26, 2024

melvin-bot commented on Jul 26, 2024

allgandalf commented on Jul 29, 2024

Regression Test Proposal 1

Regression Test Proposal 2

francoisl commented on Jul 29, 2024

allgandalf commented on Aug 4, 2024

kadiealexander commented on Aug 5, 2024

kadiealexander commented on Aug 5, 2024

kadiealexander commented on Aug 7, 2024

etCoderDysto commented on Aug 7, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions