[ALREADY PAID, WAIT FOR REGRESSION - 19th] [$250] BUG: checkbox is disabled on members page incorrectly

[davidcardoza]

• Slack conversation for additional background - https://expensify.slack.com/archives/C05LX9D6E07/p1730139501685939

Bug Description:

When inviting a user to a Collect workspace, I’m unable to edit their permissions to make them an admin. Specifically, I invited [email protected] to the workspace from [email protected]. Although Joe’s Expensify account is validated, his account remains grayed out in the permissions list, and the checkbox to make him an admin is not selectable. However, I am able to set him as an admin when using Classic.

Relevant Code:

The code in question WorkspaceMembersPage.tsx states that the checkbox should be disabled if:

• The user is an admin of the policy but NOT the policy owner.
• The user is the one currently logged in.
• In this case, Joe was not an admin on the policy at the time of invitation (confirmed by the UI, as it doesn’t show the "Admin" badge). Therefore, it seems there’s a bug preventing the permissions from being edited as expected.

Expected Behavior:

The invited user’s permissions should be editable to make them an admin, provided they are not currently an admin or the logged-in user.

Steps to Reproduce:

• From a validated Expensify account (e.g., [email protected]), invite a new user (e.g., [email protected]) to a Collect workspace.
• Attempt to edit the new user’s permissions to make them an admin.
• Observe that the checkbox to set admin permissions is grayed out and not selectable.
• Current Workaround:
• Switching to Classic mode allows the permission change to be completed as expected.
• Onyx data from my account:
• onyx-state-2.txt

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021853872777141999546
• Upwork Job ID: 1853872777141999546
• Last Price Increase: 2024-11-05
• Automatic offers:
• nkdengineer | Contributor | 104769213

Issue Owner

Current Issue Owner: @kadiealexander

[melvin-bot]

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

[nkdengineer]

Proposal

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

What is the root cause of that problem?

When we invite a new user, before the Add member API is complete, the new member section is disabled because it's the optimistic data and we don't want to open the member page in this case

App/src/pages/workspace/WorkspaceMembersPage.tsx

Lines 366 to 369 in 65fa00a

	isDisabled:
	!!details.isOptimisticPersonalDetail ||
	(isPolicyAdmin && (policyEmployee.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || !isEmptyObject(policyEmployee.errors))),
	cursorStyle: details.isOptimisticPersonalDetail ? styles.cursorDefault : {},

The checkbox is also disabled when the item is disabled.

App/src/components/SelectionList/TableListItem.tsx

Line 72 in 65fa00a

disabled={isDisabled || item.isDisabledCheckbox}

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

In this case, I think we don't want to disable the checkbox in this case, we can add another field to the item like shouldPreventDisableCheckboxIfDisabled and only disable the checkbox if the item is disabled and shouldPreventDisableCheckboxIfDisabled is false

disabled={(isDisabled && !shouldPreventDisableCheckboxIfDisabled) || item.isDisabledCheckbox} 

App/src/components/SelectionList/TableListItem.tsx

Line 72 in 65fa00a

disabled={isDisabled || item.isDisabledCheckbox}

Then we can add shouldPreventDisableCheckboxIfDisabled here. It will be true if the member doesn't have any errors, isn't an admin, and isn't pending deletion.

shouldPreventDisableCheckboxIfDisabled: !(isPolicyAdmin && (policyEmployee.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || !isEmptyObject(policyEmployee.errors))

App/src/pages/workspace/WorkspaceMembersPage.tsx

Line 366 in 65fa00a

isDisabled:

What alternative solutions did you explore? (Optional)

Or we can remove isDisabled condition here and update isDisabledCheckbox accordingly to cover the case the member is deleting or has the error. But I still prefer the main solution, the reason is the alternative solution requires many changes in isDisabledCheckbox condition.

App/src/components/SelectionList/TableListItem.tsx

Line 72 in 65fa00a

disabled={isDisabled || item.isDisabledCheckbox}

[hoangzinh]

@davidcardoza can you help to add a "Bug" label to this GH issue? It will add a BZ member to this issue. Thank you

[FitseTLT]

Can't reproduce this

[hoangzinh]

hey @davidcardoza can I get confirmation that before you invited [email protected] to your workspace, you haven't chatted/messaged him in Expensify?

[hoangzinh]

@nkdengineer I got "Hmm..it's not here" page with your suggestion when was editing the role.

[nkdengineer]

@hoangzinh Can you share the full video of this bug above.

[hoangzinh]

I was wrong, @nkdengineer. Your solution allows clicking on the checkbox but not the item row. That makes sense. It would fix the current behavior

Observe that the checkbox to set admin permissions is grayed out and not selectable.

[hoangzinh]

Asking @davidcardoza on Slack https://expensify.slack.com/archives/C05LX9D6E07/p1730715576408849?thread_ts=1730139501.685939&cid=C05LX9D6E07

[davidcardoza]

hey @davidcardoza can I get confirmation that before you invited [email protected] to your workspace, you haven't chatted/messaged him in Expensify?

No we never chatted in Expensify prior to the invite.

[hoangzinh]

Thanks for confirming @davidcardoza. @nkdengineer's proposal looks good to me

Link to proposal #51677 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[MarioExpensify]

Hi @joekaufmanexpensify @anmurali shouldn't this have the bounty value in the issue title? Should we have this fixed before moving forward with the proposal validation?

[melvin-bot]

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

[melvin-bot]

Current assignee @hoangzinh is eligible for the External assigner, not assigning anyone new.

[joekaufmanexpensify]

Done!

[hoangzinh]

Quick updates: waiting @nkdengineer address this bug #52173 (comment).

Btw @MarioExpensify @jliexpensify can anyone add back "Weekly" label to this issue? the PR is in review so it should be "Weekly"

[jliexpensify]

I thought only Expensify employees could? We generally keep issues at Daily for urgency purposes/updates, but we can always adjust.

[hoangzinh]

@jliexpensify there is another scenario, when the PR is public and ready to review it will be applied Weekly label #51677 (comment)

[jliexpensify]

Ah yes, good point!

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

• fix enable user checkbox on workspace members page #52173

If no regressions arise, payment will be issued on 2024-12-19. 🎊

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

• @hoangzinh requires payment through NewDot Manual Requests
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

@hoangzinh @jliexpensify @hoangzinh The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[jliexpensify]

@kadiealexander the payment date for this falls for when I am OOO, but before I go, I'll prep the summary - can you pay it please?

[jliexpensify]

Payment Summary

• C: @nkdengineer $250 (to be paid via Upworks)
• C+: @hoangzinh $250 (to be paid via N.D)

New Upworks job - https://www.upwork.com/jobs/~021868493785995852561

[jliexpensify]

@nkdengineer I invited you to the new job here: https://www.upwork.com/jobs/~021868493785995852561

The old one closed automatically.

[hoangzinh]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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/44734/files#r1886550722

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

• Create a workspace if the logging account is not an owner/admin of any workspace

Test:

1. Sign in to the above account
2. Go to Profile > Workspaces > above workspace
3. Go to Members menu
4. Invite a new user that current login account hasn't chatted before
5. Verify that on the workspace members page, the checkbox of the invited user is enabled to be selectable

Do we agree 👍 or 👎

[hoangzinh]

Requested payment in ND

[nkdengineer]

@jliexpensify Thx! I accepted the job.

[jliexpensify]

FUUUUUUUU I accidentally paid this job early cc @kadiealexander 😓

Fingers crossed there are no regressions.

[hoangzinh]

it won't @jliexpensify. Challengeaccepted

[jliexpensify]

it won't

Famous last words

[garrettmknight]

$250 approved for @hoangzinh