[HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [$250] Workspace chat - Unable to remove member from workspace chat

[lanitochka17]

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: 1.4.78-0
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:

1. Go to staging.new.expensify.com
2. Go to Account Settings > Workspace > click on a workspace > Invite Member > Invite a test account as a member
3. Go to workspace chat
4. Click on the chat header
5. Go to Members
6. Remove the new users - verify you see the "removed" notification in the chat
7. Notice the list of members is greater than the number of members (as if the removed member wasn't removed)

Expected Result:

User should be able to remove a member from the workspace
The member count should match the number of members

Actual Result:

When refreshing the browser the user isn't removed from the Members list
The member count isn't accurate

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

Bug6497346_1717152407197.20240531_184248.mp4

2024-06-04_14-44-07.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01008c963ec6d29d78
• Upwork Job ID: 1798248796400459776
• Last Price Increase: 2024-06-05

Issue Owner

Current Issue Owner: @Christinadobrzyn / @Christinadobrzyn

Issue Owner

Current Issue Owner: @strepanier03 / @Christinadobrzyn

[melvin-bot]

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

[lanitochka17]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[bernhardoj]

Proposal

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

Unable to remove qa.guide@team.expensify.com from workspace chat members.

What is the root cause of that problem?

The BE gives us a successful response when removing it, but shows again after refresh.

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

I think we should either disable it or not show the qa.guide@team.expensify.com at all like on the workspace members page,

App/src/pages/workspace/WorkspaceMembersPage.tsx

Lines 325 to 332 in 0cb2dc0

	// If this policy is owned by Expensify then show all support (expensify.com or team.expensify.com) emails
	// We don't want to show guides as policy members unless the user is a guide. Some customers get confused when they
	// see random people added to their policy, but guides having access to the policies help set them up.
	if (PolicyUtils.isExpensifyTeam(details?.login ?? details?.displayName)) {
	if (policyOwner && currentUserLogin && !PolicyUtils.isExpensifyTeam(policyOwner) && !PolicyUtils.isExpensifyTeam(currentUserLogin)) {
	return;
	}
	}

but with an additional check of the owner of the report.

if (!PolicyUtils.isExpensifyTeam(reportOwner) && !PolicyUtils.isExpensifyTeam(policyOwner) && !PolicyUtils.isExpensifyTeam(currentUserPersonalDetails.login)) {

If we want to disable it, we can use the condition and apply it here

App/src/pages/RoomMembersPage.tsx

Line 214 in 0cb2dc0

isDisabled: accountID === session?.accountID || pendingChatMember?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,

If we don't want to show it, we need to use the condition in ReportUtils.getVisibleChatMemberAccountIDs.

so it will affect the member count too.

[kpadmanabhan]

Proposal

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

Unable to remove qa.guide@team.expensify.com from members list in a workspace. The same is the case with removing myself from my workspace. I see this behaviour for any user removal.

What is the root cause of that problem?

Behaviour seen in main branch HEAD.

OpenReport API returns list of members.

Though the RemoveFromRoom API returns 200 response, refresh of page returns back the removed user in the list.
This needs to be fixed in the backend.

However, proposing frontend fixes below.

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

Similar the first user in the screenshot below (with a disabled checkbox), owner of the account and qa.guide@team.expensify.com users can be marked disabled, if they are not to be removable.

This is a call the product has to take. Accordingly frontend changes can be implemented.

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[Christinadobrzyn]

Hum, what I'm able to reproduce is slightly different - the member doesn't always appear again in the chat but the member count is off. Asking QA to confirm what they see. https://expensify.slack.com/archives/C9YU7BX5M/p1717483892088549

[MelvinBot]

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

[kavimuru]

Tester is seeing the same result as in the OP.

bandicam.2024-06-05.03-52-21-698.mp4

[melvin-bot]

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

[melvin-bot]

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

[Christinadobrzyn]

I think this might need to be internal but we'll start with external. I think this is vsp

[jayeshmangwani]

I think for the consistency we can go with what @bernhardoj suggest Of not to display the user if it consist Expensify Team's email domain, just like how we did on the workspace members page, but let's see what Internal engineer thinks

@bernhardoj 's Proposal looks good here

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @techievivek, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[kpadmanabhan]

@jayeshmangwani / @Christinadobrzyn : as mentioned in my proposal, this is not limited to qa.guide@team.expensify.com.
I see this behaviour for all users. The issue is coming from backend as I have explained.

@jayeshmangwani : i am not sure if your conclusion is final. But the selected solution will not solve this issue.

[jayeshmangwani]

I see this behaviour for all users

I am not getting this behavior with any other users except the Expensify domain's users, can you add test steps/video of the reproduction for all users ?

i am not sure if your conclusion is final. But the selected solution will not solve this issue.

No, final call will be of @techievivek, And How selected solution will not solve this ? We have added same logic to WorkspaceMembersPage And that's working

[techievivek]

Quick question regarding the bug: Are we unable to remove only the team Expensify users and the owner, or can we not remove anyone from the workspace?

You can't actually remove team Expensify users since they are usually assigned for customer support. Similarly, owners can't be removed since they own the policy. If the bug is for these 2 categories of users, then we can just hide them in the front end.

[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.5-13 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: workspace chat owner can be removed #44595

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @jayeshmangwani requires payment through NewDot Manual Requests
• @tienifr requires payment through NewDot Manual Requests

[melvin-bot]

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

• fix: workspace chat owner can be removed #44595

If no regressions arise, payment will be issued on 2024-07-22. 🎊

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

• @jayeshmangwani requires payment through NewDot Manual Requests
• @tienifr requires payment through NewDot Manual Requests

[jayeshmangwani]

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: Prevent removal of admin and default members from workspace chats #43065
• [@jayeshmangwani] 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: Prevent removal of admin and default members from workspace chats #43065 (comment)
• [@jayeshmangwani] 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
• [@jayeshmangwani] Determine if we should create a regression test for this bug. Yes
• [@jayeshmangwani] 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. Add a new member to a workspace
2. Go to the workspace chat created for that member
3. Press report details -> Members
4. Verify you cannot remove that member from the workspace chat

Do we agree 👍 or 👎

[strepanier03]

@Christinadobrzyn - I didn't realize we were both assigned to this until I'd already done the the reg test so that parts is finished 😂.

[strepanier03]

It doesn't look like @bernhardoj is due payment so I think we can just post the payment summary for this.

[strepanier03]

Payment Summary

• Contributor: $250 @tienifr (payment through New Expensify)
• Contributor+: $250 @jayeshmangwani (payment through New Expensify)

@JmillsExpensify - Two requests incoming via New Expensify.

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @jayeshmangwani owed $250 via NewDot
• Reviewer: @tienifr owed $250 via NewDot

BugZero Checklist (@JmillsExpensify)

• 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/1798248796400459776/hired)
  - [ ] I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[melvin-bot]

@JmillsExpensify, @strepanier03, @jayeshmangwani, @techievivek, @Christinadobrzyn, @tienifr Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Christinadobrzyn]

I think we can close this since payment will be through New Expensify-

NewDot Payment summary - #42928 (comment)
Regression test - https://github.com/Expensify/Expensify/issues/413575

[jayeshmangwani]

Requested $250

[JmillsExpensify]

$250 approved for @jayeshmangwani

[JmillsExpensify]

$250 approved for @tienifr