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

[$250] Group - ‘Make member’ option is displayed when selecting a member #40422

Closed
5 of 6 tasks
izarutskaya opened this issue Apr 18, 2024 · 33 comments
Closed
5 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production 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

Comments

@izarutskaya
Copy link

izarutskaya commented Apr 18, 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: v1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): natnael.expensify+3@gmail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Click on ‘FAB’ > Start chat
  2. Select multiple users > Click on ‘Next’ button > Click on ‘Start group’ button
  3. Click on chat header > Members > select one of the members > click selected dropdown menu

Expected Result:

There shouldn’t be ‘Make member’ option on the list

Actual Result:

There is ‘Make member’ option on the list

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

Bug6453298_1713425313263.Screen_Recording_2024-04-18_at_10.22.46_in_the_morning.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0136b9de7bfa3b057f
  • Upwork Job ID: 1780995867481071616
  • Last Price Increase: 2024-04-25
  • Automatic offers:
    • shubham1206agra | Contributor | 0
    • ShridharGoel | Contributor | 0
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Apr 18, 2024
Copy link

melvin-bot bot commented Apr 18, 2024

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

Copy link

melvin-bot bot commented Apr 18, 2024

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

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

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

@izarutskaya
Copy link
Author

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

@izarutskaya
Copy link
Author

We think this issue might be related to the #vip-vsb.

@ShridharGoel
Copy link
Contributor

ShridharGoel commented Apr 18, 2024

Proposal

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

‘Make member’ option is displayed in groups even when user is already a member, and vice-versa.

What is the root cause of that problem?

Make member option is added directly in options without any check. Admin option also doesn't have any check, so same issue will happen there as well ("Make admin" will show even though user is already an admin).

{
text: translate('workspace.people.makeMember'),
value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_MEMBER,
icon: Expensicons.MakeAdmin,
onSelected: () => changeUserRole(CONST.REPORT.ROLE.MEMBER),
},

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

"Make member" option should only be shown when user is admin.
"Make admin" option should only be shown when user is member.

if (selectedMembers.find((accountId) => report.participants?.[accountId]?.role === CONST.REPORT.ROLE.ADMIN)) {
  options.push({
      text: translate('workspace.people.makeMember'),
      value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_MEMBER,
      icon: Expensicons.MakeAdmin,
      onSelected: () => changeUserRole(CONST.REPORT.ROLE.MEMBER),
  })
}

if (selectedMembers.find((accountId) => report.participants?.[accountId]?.role === CONST.REPORT.ROLE.MEMBER)) {

 {
  options.push({
      text: translate('workspace.people.makeAdmin'),
          value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_ADMIN,
      icon: Expensicons.MakeAdmin,
      onSelected: () => changeUserRole(CONST.REPORT.ROLE.ADMIN),
  })
}

Also, add selectedMembers as a dependency in useMemo.

And remove both of them from the existing place where they are added directly in options.

Note: We need to discuss if we want to handle the case when both types are selected (atleast one admin and one member). As of now, both options would show in such a case.

Full code
const bulkActionsButtonOptions = useMemo(() => {
    const options: Array<DropdownOption<WorkspaceMemberBulkActionType>> = [
        {
            text: translate('workspace.people.removeMembersTitle'),
            value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.REMOVE,
            icon: Expensicons.RemoveMembers,
            onSelected: () => setRemoveMembersConfirmModalVisible(true),
        },
    ];

    const isAtleastOneAdminSelected = selectedMembers.find((accountId) => report.participants?.[accountId]?.role === CONST.REPORT.ROLE.ADMIN);

    if (isAtleastOneAdminSelected) {
        options.push({
            text: translate('workspace.people.makeMember'),
            value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_MEMBER,
            icon: Expensicons.MakeAdmin,
            onSelected: () => changeUserRole(CONST.REPORT.ROLE.MEMBER),
        })
    }

    const isAtleastOneMemberSelected = selectedMembers.find((accountId) => report.participants?.[accountId]?.role === CONST.REPORT.ROLE.MEMBER);

    if (isAtleastOneMemberSelected) {
        options.push({
            text: translate('workspace.people.makeAdmin'),
                value: CONST.POLICY.MEMBERS_BULK_ACTION_TYPES.MAKE_ADMIN,
            icon: Expensicons.MakeAdmin,
            onSelected: () => changeUserRole(CONST.REPORT.ROLE.ADMIN),
        })
    }

    return options;
}, [changeUserRole, translate, setRemoveMembersConfirmModalVisible, selectedMembers]);

@ShridharGoel
Copy link
Contributor

#39757

@marcaaron Can I fix this? I've posted a proposal above.

@adelekennedy
Copy link

@rlinoz is this actually a deploy blocker? Or should we deprioritize this to daily instead of hourly?

@rlinoz
Copy link
Contributor

rlinoz commented Apr 18, 2024

Nothing weird happens if we click on make member, so I agree this is not a blocker, thanks @adelekennedy !

@rlinoz rlinoz removed the DeployBlockerCash This issue or pull request should block deployment label Apr 18, 2024
@adelekennedy adelekennedy added Daily KSv2 and removed Hourly KSv2 labels Apr 18, 2024
@adelekennedy
Copy link

anotha one for you @rlinoz I think this can be worked on externally - do you need to investigate this first before I do that, or can I move forward with adding the external label

@rlinoz
Copy link
Contributor

rlinoz commented Apr 18, 2024

Yep, we can make it external

@rlinoz rlinoz added Help Wanted Apply this label when an issue is open to proposals by contributors External Added to denote the issue can be worked on by a contributor labels Apr 18, 2024
@melvin-bot melvin-bot bot changed the title Group - ‘Make member’ option is displayed when selecting a member [$250] Group - ‘Make member’ option is displayed when selecting a member Apr 18, 2024
Copy link

melvin-bot bot commented Apr 18, 2024

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

Copy link

melvin-bot bot commented Apr 18, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 26, 2024
Copy link

melvin-bot bot commented Apr 26, 2024

📣 @shubham1206agra 🎉 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 📖

@rlinoz rlinoz added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 26, 2024
@shubham1206agra
Copy link
Contributor

@ShridharGoel's proposal looks good.

🎀 👀 🎀 C+ reviewed

Side Note - Use .some instead of .find

Copy link

melvin-bot bot commented Apr 26, 2024

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

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 26, 2024
Copy link

melvin-bot bot commented Apr 26, 2024

📣 @ShridharGoel 🎉 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 📖

@melvin-bot melvin-bot bot added the Overdue label Apr 29, 2024
@rlinoz
Copy link
Contributor

rlinoz commented Apr 29, 2024

Hey @ShridharGoel can you give us an ETA on the PR?

@melvin-bot melvin-bot bot removed the Overdue label Apr 29, 2024
@ShridharGoel
Copy link
Contributor

Will try to open PR within 24 hours.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Apr 30, 2024
@ShridharGoel
Copy link
Contributor

PR is up #41319

@rlinoz
Copy link
Contributor

rlinoz commented May 10, 2024

Automation failed, this has been deployed to prod #41319 (comment)

@rlinoz rlinoz added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels May 10, 2024
@adelekennedy
Copy link

Payment due in 3 days

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels May 16, 2024
Copy link

melvin-bot bot commented May 16, 2024

@rlinoz @ShridharGoel @shubham1206agra @adelekennedy this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@adelekennedy
Copy link

@ShridharGoel and @shubham1206agra it looks like the offers in Upwork are still pending but I believe payment is due (the automation failed) will you accept and then I can pay you out?

@melvin-bot melvin-bot bot removed the Overdue label May 16, 2024
@shubham1206agra
Copy link
Contributor

@adelekennedy Accepted

@adelekennedy
Copy link

adelekennedy commented May 17, 2024

Payouts due:

Upwork job is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production 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
Projects
None yet
Development

No branches or pull requests

7 participants