[$250] Plural Remove Members on the workspace members page even when we only have one member selected

[m-natarajan]

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.41-6 Reproducible in staging?: Y Reproducible in production?: Y 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: @allgandalf Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1727729303045069

Action Performed:

1. Go to staging.new.expensify.com
2. Select any workspace from settings which have members
3. Click Members > select any added member
4. Click the dropdown in the 1 selected pill

   Expected Result:

   Remove member displayed as only one member selected

   Actual Result:

   Remove members displayed even if only one member selected

   Workaround:

   Unknown

   Platforms:

   Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021841413637223772836
• Upwork Job ID: 1841413637223772836
• Last Price Increase: 2024-10-02
• Automatic offers:
• allgandalf | Reviewer | 104321898
• nkdengineer | Contributor | 104321900

Issue Owner

Current Issue Owner: @

[melvin-bot[bot]]

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

[github-actions[bot]]

adrian-brc Your proposal will be dismissed because you did not follow the proposal template.

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-09-30 23:16:29 UTC.

Proposal

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

Plural Remove Members on the workspace members page even when we only have one member selected

What is the root cause of that problem?

We are using always using workspace.people.removeMembersTitle. https://github.com/Expensify/App/blob/98ac9accc3adf7fbec14ac0e8c75a88086689f62/src/pages/workspace/WorkspaceMembersPage.tsx#L480 https://github.com/Expensify/App/blob/98ac9accc3adf7fbec14ac0e8c75a88086689f62/src/pages/workspace/WorkspaceMembersPage.tsx#L640 https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/RoomMembersPage.tsx#L276 https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/RoomMembersPage.tsx#L371 https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/ReportParticipantsPage.tsx#L269 https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/ReportParticipantsPage.tsx#L393

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

• We should check selectedEmployees.length and use workspace.people.removeMemberTitle if it is 1 otherwise workspace.people.removeMembersTitle.

• The same need to be done in ReportParticipantsPage & RoomMembersPage. In these components we need to use selectedMembers.length

• We also need to update confirmModalPrompt. https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/workspace/WorkspaceMembersPage.tsx#L102-L111 TO:

  const confirmModalPrompt = useMemo(() => {
      const approverAccountID = selectedEmployees.find((selectedEmployee) => Member.isApprover(policy, selectedEmployee));
      if (!approverAccountID) {
          return selectedEmployees.length === 1
              ? translate('workspace.people.removeMemberPrompt', {memberName: getDisplayNameForParticipant(selectedEmployees[0])})
              : translate('workspace.people.removeMembersPrompt');
      }
      return translate('workspace.people.removeMembersWarningPrompt', {
          memberName: getDisplayNameForParticipant(approverAccountID),
          ownerName: getDisplayNameForParticipant(policy?.ownerAccountID),
      });
  }, [selectedEmployees, policy, translate]);

• Same should be done in ReportParticipantsPage & RoomMembersPage. https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/ReportParticipantsPage.tsx#L397 https://github.com/Expensify/App/blob/8f5f60988fa630e866734a550b070fabe8875b15/src/pages/RoomMembersPage.tsx#L375

What alternative solutions did you explore? (Optional)

[Krishna2323]

PROPOSAL UPDATED

• Updated main solution to include the changes for the prompt.

[allgandalf]

@trjExpensify Can i be the C+ for this issue? I reported this bug on slack

[trjExpensify]

Sure!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[nkdengineer]

Edited by proposal-police: This proposal was edited at 2024-10-02 10:44:25 UTC.

Proposal

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

Remove members displayed even if only one member selected

What is the root cause of that problem?

We're always using workspace.people.removeMembersTitle without checking whether the selected employee is one or many

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/workspace/WorkspaceMembersPage.tsx#L480

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

Following the same pattern of this translation we can update removeMembersTitle translation like this

removeMembersTitle: () => ({
    one: 'Remove member',
    other: 'Remove members',
}),
removeMembersTitle: () => ({
    one: 'Eliminar miembro',
    other: 'Eliminar miembros',
}),

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/languages/en.ts#L883

Then for each place below, we only need to pass the count param as the length of the selected member to this translation to display the translation accordingly with the length of the selected member.

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/ReportParticipantsPage.tsx#L269

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/ReportParticipantsPage.tsx#L393

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/workspace/WorkspaceMembersPage.tsx#L480

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/workspace/WorkspaceMembersPage.tsx#L640

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/ReportParticipantsPage.tsx#L269

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/RoomMembersPage.tsx#L276

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/RoomMembersPage.tsx#L371

When we implement this, we should wrap this logic and other place that we use the new translation in InteractionsManager.runAfterInteraction or use Modal.close to ensure the modal is displayed correctly when we confirm delete.

https://github.com/nkdengineer/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/pages/workspace/WorkspaceMembersPage.tsx#L211

OPTIONAL: We can make the same approach for removeMembersPrompt translation. And update all places that we're using removeMembersPrompt without checking the length of the selected member

removeMembersPrompt: ({memberName}: {memberName: string}) => ({
    one: `Are you sure you want to remove ${memberName}?`,
    other: 'Are you sure you want to remove these members?',
}),

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/languages/en.ts#L3352

With this approach we can remove removeMemberTitle and removeMemberPrompt translation and use removeMembersTitle/removeMembersPrompt with count param as 1

What alternative solutions did you explore? (Optional)

[nkdengineer]

Updated proposal

• Fix another case.

[allgandalf]

I will review this on monday not overdue

[trjExpensify]

@allgandalf did you review?

[allgandalf]

Sorry I was occupied with a deploy blocker yesterday, Reviewing now ♻️

[allgandalf]

♻️ both @Krishna2323 's and @nkdengineer 's proposals solve this bug.

🟢 Both have the correct RCA which addresses why the bug occurs

💭 In particular I feel @nkdengineer 's proposal is a bit of over-engineering and @Krishna2323 's proposal is a simplified approach towards this bug. So I would leave it upto the internal engineer for this case which one to select.

I would prefer @Krishna2323 's proposal though.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[nkdengineer]

In particular I feel @nkdengineer 's https://github.com/Expensify/App/issues/49961#issuecomment-2388303991 is a bit of over-engineering

@allgandalf I don't think so.

1. We already have a translation like this here

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/languages/en.ts#L883-L885

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/components/Search/SearchPageHeader.tsx#L284

https://github.com/Expensify/App/blob/487816184680744a2ad53a03460cbbdec363c432/src/components/Search/SearchPageHeader.tsx#L146-L148

2. With this approach, we can get the translation cleaner everywhere we use this. We only need to pass to count param to this translation instead of has an if else in all places to get the title.

cc @rlinoz

[allgandalf]

Lets see what @rlinoz opinion is here 😄

[Krishna2323]

We also have translation, as I suggested in my proposal, so I don't think that matters much. My proposal does solve the issue in a way that is commonly used in many places.

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/reportFields/WorkspaceReportFieldsPage.tsx#L168

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/reportFields/WorkspaceReportFieldsPage.tsx#L274

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L186

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L206

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L228

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx#L377-L378

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx#L186

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx#L208

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx#L240

https://github.com/Expensify/App/blob/41c0b9ddd5576cf216db63cfc0d40ef74995fadc/src/pages/workspace/reportFields/ReportFieldsListValuesPage.tsx#L343-L344

cc: @rlinoz

[rlinoz]

From the readme of the project it looks like we prefer to implement plurals with the count option, also checked internally and that is the general feel as well, so let's go with @nkdengineer's proposal

[melvin-bot[bot]]

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

Offer link Upwork job

[melvin-bot[bot]]

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