[$250] Workspace - Admin is unable to click on the member to view member profile in workspace chat

IuliiaHerets commented 1 month ago

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:

Go to staging.new.expensify.com
[Admin] Invite employee to workspace.
[Admin & employee] Go to workspace chat.
[Admin & employee] Click on the room header > Members.
[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:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/e86ec869-2c68-4db7-aae6-d13fc557af18

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: @eVoloshchak

melvin-bot[bot] commented 1 month ago

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 commented 1 month ago

@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

Nodebrute commented 1 month ago

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 are disabling when admin clicks on members https://github.com/Expensify/App/blob/f375d59dbf1f4ac944f93c4bf96372524252a02f/src/pages/RoomMembersPage.tsx#L219

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

We can remove this check isAdmin https://github.com/Expensify/App/blob/f375d59dbf1f4ac944f93c4bf96372524252a02f/src/pages/RoomMembersPage.tsx#L219

What alternative solutions did you explore? (Optional)

Or we can remove (isPolicyExpenseChat && isAdmin)

nyomanjyotisa commented 1 month ago

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 https://github.com/Expensify/App/blob/5224d9132d0fbbcaf79e8715377f6c04191176e7/src/pages/RoomMembersPage.tsx#L217-L222

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 commented 1 month ago

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 commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

zanyrenney commented 3 weeks ago

have you managed to review thiss yet @eVoloshchak

eVoloshchak commented 2 weeks ago

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[bot] commented 2 weeks ago

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

dominictb commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 2 weeks ago

@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 commented 2 weeks ago

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

zanyrenney commented 2 weeks ago

Hey @eVoloshchak please review the thoughts above?

Just an FYI I'll be OOO until Tuesday 1st, please reapply the bug label if this needs urgent action in my absence. Otherwise I will get to it when I return. Thanks!

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 1 week ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 1 week ago

@tgolen, @eVoloshchak, @zanyrenney Still overdue 6 days?! Let's take care of this!

zanyrenney commented 1 week ago

Bump @eVoloshchak please can you prioritise the review of the proposals?

We need to keep up progress here. If you don't think this is a real bug or are concerned about the value, feel free to chime in and add your thoughts, otherwise I will assume we are progressing here.

eVoloshchak commented 1 week ago

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

Just verified this is correct, the proposed solution allows you to select (and remove) the admin of the workspace, which is incorrect behavior. Thanks for catching that @dominictb Let's proceed with @dominictb's solution, apologies for the confusion

🎀👀🎀 C+ reviewed!

melvin-bot[bot] commented 1 week ago

Current assignee @tgolen is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

melvin-bot[bot] commented 1 week ago

📣 @dominictb 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

mvtglobally commented 1 week ago

Issue not reproducible during KI retests. (First week)

Expensify / App