Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[$500] Workspace - Member role is not highlighted for newly-invited member #37940

Closed
6 tasks done
kavimuru opened this issue Mar 8, 2024 · 19 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

kavimuru commented Mar 8, 2024

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.49-0
Reproducible in staging?: y
Reproducible in production?: no, new feature
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause internal team
Slack conversation:

Action Performed:

Precondition:

  • User is owner of Collect workspace.
  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Collect workspace.
  3. Go to Member.
  4. Click Invite member.
  5. Invite a member.
  6. Click on the newly-invited member.
  7. Click Role.

Expected Result:

Member role should be highlighted.

Actual Result:

Member role is not highlighted.

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

Bug6406295_1709857118224.bandicam_2024-03-08_08-14-42-914.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01dbe90339ab40577e
  • Upwork Job ID: 1766054279724601344
  • Last Price Increase: 2024-03-08
@kavimuru kavimuru added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 8, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Mar 8, 2024
Copy link
Contributor

github-actions bot commented Mar 8, 2024

👋 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.

Copy link

melvin-bot bot commented Mar 8, 2024

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

@kavimuru
Copy link
Author

kavimuru commented Mar 8, 2024

@robertjchen 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.

@Tony-MK
Copy link
Contributor

Tony-MK commented Mar 8, 2024

Proposal

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

Workspace - Member role is not highlighted for newly-invited member

What is the root cause of that problem?

When inviting a member, the inviteMemberToWorkspace and addMembersToWorkspace function does not set the member?.role attribute optimistically.

const optimisticMembersState = {policyID, inviterEmail};
const failureMembersState = {policyID, inviterEmail};

Therefore, just like the WorkspaceMemberDetailsPage, If the role of the member, member?.role, is not present or equal not to CONST.POLICY.ROLE.ADMIN then the role is CONST.POLICY.ROLE.USER.

However, the WorkspaceMemberDetailsRoleSelectionPage uses the condition below for regular users.

isSelected: member?.role === CONST.POLICY.ROLE.USER,

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

Add a default role, role: CONST.POLICY.ROLE.USER, to the optimisticMembersState.

const optimisticMembersState = {policyID, inviterEmail, role: CONST.POLICY.ROLE.USER};

Then handle the successMembersState and failureMembersState.

Optional: We should probably set pendingAction as CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD to adhere to the offline patterns.

What alternative solutions did you explore? (Optional)

isSelected: member?.role === CONST.POLICY.ROLE.USER,

Change the above condition to something a condition like the one below.

isSelected: member?.role !== CONST.POLICY.ROLE.ADMIN,

@robertjchen robertjchen added Weekly KSv2 Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 Weekly KSv2 labels Mar 8, 2024
@jliexpensify
Copy link
Contributor

@robertjchen should this be External?

@robertjchen robertjchen added the External Added to denote the issue can be worked on by a contributor label Mar 8, 2024
@melvin-bot melvin-bot bot changed the title Workspace - Member role is not highlighted for newly-invited member [$500] Workspace - Member role is not highlighted for newly-invited member Mar 8, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 8, 2024
Copy link

melvin-bot bot commented Mar 8, 2024

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

@robertjchen
Copy link
Contributor

Yep, let's field some proposals. I was curious if it was related to backend policy-related Onyx updates.

@jeremic-dev
Copy link

jeremic-dev commented Mar 8, 2024

PROPOSAL:

The issue actually seems to be with policy related Onyx updates. Namely, Policy method addMembersToWorkspace:

function addMembersToWorkspace(invitedEmailsToAccountIDs: InvitedEmailsToAccountIDs, welcomeNote: string, policyID: string) {
does not set any role on policy member data (it does not do it on any kind of data -> optimistic or successful) therefore policyMembers Onyx data does not have any role value when reading it in UI component:
isSelected: member?.role === CONST.POLICY.ROLE.USER,

Also, there is one more issue -> Policy method openWorkspaceMembersPage:

function openWorkspaceMembersPage(policyID: string, clientMemberEmails: string[]) {
gets triggered when WorkspaceMembersPage (page where all members are listed) is mounted ONLY, and that method actually updates policyMembers in Onyx DB with default role value. That is actually the reason for having roles displayed for any other member which is not newly created. Having this method called when policyMembers change, will fix the issue.

So fixing any of these two issues will do the job, I would personally suggest fixing addMembersToWorkspace method in order to avoid additional re-render (in comparison to fixing second issue with calling openWorkspaceMembersPage method when update happens).

Copy link

melvin-bot bot commented Mar 8, 2024

📣 @jeremic-dev! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

Copy link

melvin-bot bot commented Mar 8, 2024

⚠️ Invalid email. Please make sure to create an Expensify account with this email first here.

@jeremic-dev
Copy link

Contributor details
Your Expensify account email: nikola.jeremic.dev@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01cc4903b759f2e9bd

Copy link

melvin-bot bot commented Mar 8, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@Victor-Nyagudi
Copy link
Contributor

Victor-Nyagudi commented Mar 8, 2024

Proposal

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

Workspace - Member role is not highlighted for newly-invited member(s).

What is the root cause of that problem?

A user is given the 'Member' role by default when added to a workspace if they're not an admin.

title={member?.role === CONST.POLICY.ROLE.ADMIN ? translate('common.admin') : translate('common.member')}

However, when we add a new member to a workspace, the addMembersToWorkspace() method is fired, but we do not set the default role for these newly added member(s).

Since the member's role isn't specified i.e. is undefined, isSelected ends up being false, and the member option remains unchecked.

isSelected: member?.role === CONST.POLICY.ROLE.USER,

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

I propose giving new member(s) added to the workspace a default role of 'user' inside addMembersToWorkspace() method.

- optimisticMembersState[accountID] = {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD};
+ optimisticMembersState[accountID] = {pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, role: CONST.POLICY.ROLE.USER};
Short demo after changes
expensify-newly-added-member-not-highlighted-solution-demo.mov
expensify-newly-added-member-not-highlighted-multiple-people-solution-demo.mov

What alternative solutions did you explore? (Optional)

None so far.

@luacmartins
Copy link
Contributor

luacmartins commented Mar 8, 2024

cc @burczu this came from this PR. We have a followup issue and draft PR to fix a few of these issues, let's add this one to it

@robertjchen
Copy link
Contributor

Got it, in that case we should close this out and consolidate to the fixes that are ongoing there 👍

@melvin-bot melvin-bot bot removed the Overdue label Mar 11, 2024
@luacmartins
Copy link
Contributor

Added to the tracking issue

@jliexpensify
Copy link
Contributor

Upworks job removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

8 participants