[HOLD for payment 2024-10-25] [HOLD for payment 2024-10-24] Worksapce - Workspace member in workspace chat can be removed and then reappears after removal

[IuliiaHerets]

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.49-0
Reproducible in staging?: Y
Reproducible in production?: N/A - new feature, doesn't exist in prod
Issue was found when executing this PR: #50158
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. As admin, create a workspace and invite a member,
3. Go to workspace chat with member.
4. Click on the chat header > Members.
5. Hover over checkbox next to the member.
6. Note that the invited workspace memeber cannot be selected (hence they should not be removable).
7. Click on the same member.
8. Click Remove from chat.
9. Remove the member from chat.
10. Close the RHP and open Members RHP again.
11. Note that the removed member reappears.

Expected Result:

In Step 8, the workspace member should not be removable.

Actual Result:

In Step 8, the workspace member is removable.
When reopening Members page in Step 11, the removed workspace member reappears.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6634874_1728967748348.20241015_124448.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @isabelastisser

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @madmax330 (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[IuliiaHerets]

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

[bernhardoj]

Proposal

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

Workspace expense chat owner can be removed from the member details page.

What is the root cause of that problem?

This happens after #50158. In that PR, we enable the user to open the member detail page but still prevent the user from selecting it. Previously, we couldn't open the workspace expense chat owner details.

Then, on the member details page, we only disable the remove button if the user is the current user.

App/src/pages/RoomMemberDetailsPage.tsx

Lines 88 to 91 in a333676

	<Button
	text={translate('workspace.people.removeRoomMemberButtonTitle')}
	onPress={() => setIsRemoveMemberConfirmModalVisible(true)}
	isDisabled={isSelectedMemberCurrentUser}

So, the problem is on the details page.

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

We should improve the button disabling condition so it covers more cases. We can follow the same condition as below.

App/src/pages/RoomMembersPage.tsx

Lines 220 to 225 in a333676

	const isDisabled = pendingChatMember?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || details.isOptimisticPersonalDetail;
	const isDisabledCheckbox =
	(isPolicyExpenseChat && isAdmin) ||
	accountID === session?.accountID ||
	pendingChatMember?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ||
	details.accountID === report.ownerAccountID;

const policy = usePolicy(report?.policyID);
const isPolicyExpenseChat = useMemo(() => ReportUtils.isPolicyExpenseChat(report), [report]);
const isSelectedMemberCurrentUser = accountID === currentUserPersonalDetails?.accountID;
const isSelectedMemberOwner = accountID === report.ownerAccountID;
const shouldDisableRemoveUser = (isPolicyExpenseChat && PolicyUtils.isPolicyAdmin(policy, details.login)) || isSelectedMemberCurrentUser || isSelectedMemberOwner;

App/src/pages/RoomMemberDetailsPage.tsx

Lines 88 to 91 in a333676

	<Button
	text={translate('workspace.people.removeRoomMemberButtonTitle')}
	onPress={() => setIsRemoveMemberConfirmModalVisible(true)}
	isDisabled={isSelectedMemberCurrentUser}

isDisabled={shouldDisableRemoveUser}

[dominictb]

Checking.

[marcaaron]

This was deployed to staging as far as I can tell. I will re-test this on staging to confirm as this is our last blocker and would be good to get the deploy out. Edit: Realized this is the issue and not PR 😄

[marcaaron]

This is working great. Removing the blocker.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• [CP Staging] fix: workspace member is removable when he is the report owner #50870

If no regressions arise, payment will be issued on 2024-10-24. 🎊

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

• @allgandalf requires payment (Needs manual offer from BZ)
• @dominictb requires payment (Needs manual offer from BZ)

[melvin-bot]

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 / @dominictb] The PR that introduced the bug has been identified. Link to the PR:
• [@allgandalf / @dominictb] 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:
• [@allgandalf / @dominictb] 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:
• [@allgandalf / @dominictb] Determine if we should create a regression test for this bug.
• [@allgandalf / @dominictb] 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.
• [@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

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

• [CP Staging] fix: workspace member is removable when he is the report owner #50870

If no regressions arise, payment will be issued on 2024-10-25. 🎊

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

• @allgandalf requires payment (Needs manual offer from BZ)
• @dominictb requires payment (Needs manual offer from BZ)

[melvin-bot]

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 / @dominictb] The PR that introduced the bug has been identified. Link to the PR:
• [@allgandalf / @dominictb] 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:
• [@allgandalf / @dominictb] 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:
• [@allgandalf / @dominictb] Determine if we should create a regression test for this bug.
• [@allgandalf / @dominictb] 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.
• [@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[allgandalf]

@isabelastisser the payment is on 24th, melvin duplicated the message, can you please update.

Also no checklist here, this was a deploy blocker, only payment remains 😄

[isabelastisser]

Thanks for the heads up, reviewing this now.

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @allgandalf paid $(AMOUNT) via Upwork (LINK)
• ROLE: @dominictb paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@isabelastisser)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[isabelastisser]

@allgandalf @dominictb, please share your Upwork profile and I will hire you there. Thanks!

[isabelastisser]

Bump @allgandalf @dominictb to share your Upwork profiles so I can process the payment.

[isabelastisser]

I DM'd the contributors for visibility.

[dominictb]

@isabelastisser This is a regression fix. No need for payment for me.

[allgandalf]

I am due payment here as I was assigned from slack for the PR review

[isabelastisser]

hey @allgandalf, the Upwork link you shared with me in Slack is not working. Can you please check and share it here? Thanks!

[isabelastisser]

https://www.upwork.com/freelancers/~01c9aad73757d913f7 offer sent!

[isabelastisser]

All set!

[melvin-bot]

@madmax330 @isabelastisser @marcaaron Be sure to fill out the Contact List!