[HOLD for payment 2024-10-25] [HOLD for payment 2024-10-24] [$250] Workspace - Admin is unable to click on the member to view member profile in workspace chat

[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.31-12
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:

1. Go to staging.new.expensify.com
2. [Admin] Invite employee to workspace.
3. [Admin & employee] Go to workspace chat.
4. [Admin & employee] Click on the room header > Members.
5. [Admin & employee] Click on the room members.

Expected Result:

Both admin and employee should be able to click on the member to view the profile.

Actual Result:

Only employee is able to click on the member to view the profile, whereas admin is blocked from viewing the profile.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6598424_1725954809119.bandicam_2024-09-10_15-49-34-142.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021835655904461898964
• Upwork Job ID: 1835655904461898964
• Last Price Increase: 2024-09-30
• Automatic offers:
• dominictb | Contributor | 104233105

Issue Owner

Current Issue Owner: @zanyrenney

[melvin-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.

[IuliiaHerets]

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

[nyomanjyotisa]

Proposal

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

Admin is unable to click on the member to view member profile in workspace chat

What is the root cause of that problem?

We disabled the members item click if isAdmin here

App/src/pages/RoomMembersPage.tsx

Lines 217 to 222 in 5224d91

	const isAdmin = !!(policy && policy.employeeList && details.login && policy.employeeList[details.login]?.role === CONST.POLICY.ROLE.ADMIN);
	const isDisabled =
	(isPolicyExpenseChat && isAdmin) ||
	accountID === session?.accountID ||
	pendingChatMember?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ||
	details.accountID === report.ownerAccountID;

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

create new const isDisabledCheckbox and split the condition on isDisabled to isDisabledCheckbox properly.
isDisabled to disable the click to open member details
isDisabledCheckbox to disable the click on the user checkbox

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

And include the isDisabledCheckbox in the result.push

result.push({
    ...
    isDisabledCheckbox,
    ...
});

What alternative solutions did you explore? (Optional)

[dominictb]

Proposal

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

• Only employee is able to click on the member to view the profile, whereas admin is blocked from viewing the profile.

What is the root cause of that problem?

• The isDisabled prop prevents users from interacting with the option, while isDisabledCheckbox prevents users from selecting the option.

• In this section, we only have the isDisabled prop, but not the isDisabledCheckbox prop as implemented in other pages.

• For options where users can click to view details but can't select, the logic here leads to this issue.

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

• We should update this to:

            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;

• Then return isDisabledCheckbox in here.

• Then update this to:

        const enabledAccounts = memberList.filter((member) => !member.isDisabled && !member.isDisabledCheckbox);

What alternative solutions did you explore? (Optional)

[zanyrenney]

super weird, i agree the admin should have access to this.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[zanyrenney]

have you managed to review thiss yet @eVoloshchak

[eVoloshchak]

Both proposals are similar (which is expected since we're making use of an already existing approach), so we can proceed with the first proposal. @nyomanjyotisa's proposal looks good to me!

🎀👀🎀 C+ reviewed!

[melvin-bot]

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

[dominictb]

@eVoloshchak I saw the selected solution causes the regression: Click on "Select all" will select the disable options. Could you help confirm?

[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]

@tgolen @eVoloshchak @zanyrenney this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[dominictb]

@eVoloshchak Can you check my thoughts above once you have a chance?

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[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:

• fix: admin cannot open room member details #50158
• [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:

• @eVoloshchak requires payment through NewDot Manual Requests
• @dominictb requires payment automatic offer (Contributor)

[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:

• [@eVoloshchak / @dominictb] The PR that introduced the bug has been identified. Link to the PR:
• [@eVoloshchak / @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:
• [@eVoloshchak / @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:
• [@eVoloshchak / @dominictb] Determine if we should create a regression test for this bug.
• [@eVoloshchak / @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.
• [@zanyrenney] 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:

• @eVoloshchak requires payment through NewDot Manual Requests
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

Here's the updated BugZero Checklist incorporating the provided answers:

---

BugZero Checklist:

The PR fixing this issue has been merged! The following checklist (instructions) must be completed before the issue can be closed:

• [@eVoloshchak / @dominictb] The PR that introduced the bug has been identified.
  Link to the PR: I'm unable to pinpoint the exact PR that has caused this, looks like it's a limitation of the original implementation.

• [@eVoloshchak / @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: N/A

• [@eVoloshchak / @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: This is a minor edge case, additional discussion is not necessary.

• [@eVoloshchak / @dominictb] Determine if we should create a regression test for this bug.

• [@eVoloshchak / @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.

• [@zanyrenney] Link the GH issue for creating/updating the regression test once the above steps have been agreed upon:
  Link to this regression test: #51392

---

Let me know if further updates are needed!

[zanyrenney]

payment summary

@eVoloshchak requires payment through NewDot Manual Requests - please request $250 via NewDot.
@dominictb requires payment automatic offer (Contributor)

I have DM's @eVoloshchak to ask them to complete the list above.

[melvin-bot]

@tgolen @zanyrenney Be sure to fill out the Contact List!

[zanyrenney]

Done ^

[eVoloshchak]

• The PR that introduced the bug has been identified. Link to the PR: I'm unable to pinpoint the exact PR that has caused this, looks like it's a limitation of the original implementation
• 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: N/A
• 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: this is a minor edge case, additional discussion is not necessary

Regression Test Proposal

1. [Admin of the workspace] Go to workspace room
2. Press room header >> Members
3. Invite several members to the room
4. Press on the member item in the members list
5. Verify member details page opens

Do we agree 👍 or 👎

[tgolen]

Reopening so @zanyrenney can complete the checklist and fill it out with the answers from @eVoloshchak.

I think the regression test makes sense and would be valuable to have. As an alternative, could an automated test be created for that?

[JmillsExpensify]

$250 approved for @eVoloshchak

[melvin-bot]

@tgolen @zanyrenney Be sure to fill out the Contact List!