[HOLD for payment 2024-05-09] [$250] Group chat - Inconsistency in limitation when adding members via FAB and group details page

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.63-0
Reproducible in staging?: y
Reproducible in production?: n
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:

1. Go to staging.new.expensify.com
2. Go to FAB > Start chat.
3. Click Add to group until the max amount of members is reached.
4. Note that only 8 participants can be selected.
5. Create the group chat with 8 participants.
6. Click on the chat header.
7. Go to Members > Invite member.
8. Invite a few more members.

Expected Result:

There should be consistency whether more than 9 members (including the creator) is allowed in the group chat when adding members via FAB and group details page.

Actual Result:

In Step 3, when creating group chat via FAB, not more than 8 members can be selected.
In Step 8. when adding members from group details page, there is no limitation at all.

Workaround:

unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6453873_1713458029171.bandicam_2024-04-19_00-28-17-571.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01cde40901114ff34c
• Upwork Job ID: 1781091286528417792
• Last Price Increase: 2024-04-18
• Automatic offers:
• cubuspl42 | Reviewer | 0
• GandalfGwaihir | Contributor | 0

Issue Owner

Current Issue Owner: @zanyrenney

[melvin-bot]

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.

[melvin-bot]

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

[github-actions]

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

[kavimuru]

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

[jasperhuangg]

This appears to be a new feature that isn't on production yet (I think), so not worth blocking deploy on it. Plus it's a pretty minor bug. assigning @marcaaron since it looks like it was added in #39757

[melvin-bot]

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

[melvin-bot]

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

[jasperhuangg]

Gonna assign external label since this can definitely be fixed by a contributor, @marcaaron if you wanna fix it yourself go ahead.

[allgandalf]

Proposal

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

Inconsistency when adding group members on FAB, we allow any number of participants to be added via Report details members page, but we have a cap of maximum 8 participants when adding on New Chat page

What is the root cause of that problem?

We have set the limit of only 8 members when created the group maxParticipantsReached

App/src/pages/NewChatPage.tsx

Line 72 in 1db9087

const maxParticipantsReached = selectedOptions.length === CONST.REPORT.MAXIMUM_PARTICIPANTS;

Over here when we select 8 members maxParticipantsReached becomes true.

App/src/pages/NewChatPage.tsx

Lines 74 to 80 in 1db9087

	const headerMessage = OptionsListUtils.getHeaderMessage(
	filteredOptions.personalDetails.length + filteredOptions.recentReports.length !== 0,
	Boolean(filteredOptions.userToInvite),
	debouncedSearchTerm.trim(),
	maxParticipantsReached,
	selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())),
	);

This is passed on to OptionsListUtils.getHeaderMessage, which checks for the prop maxParticipantsReached:

App/src/libs/OptionsListUtils.ts

Lines 2134 to 2137 in 1db9087

	function getHeaderMessage(hasSelectableOptions: boolean, hasUserToInvite: boolean, searchValue: string, maxParticipantsReached = false, hasMatchedParticipant = false): string {
	if (maxParticipantsReached) {
	return Localize.translate(preferredLocale, 'common.maxParticipantsReached', {count: CONST.REPORT.MAXIMUM_PARTICIPANTS});
	}

If the prop it true then it returns early with a error message

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

we can set maxParticipantsReached to false,

If we want to add a new limit to number of participants to be added at once, then we should update the const value or create a new const below:

App/src/pages/NewChatPage.tsx

Line 72 in 1db9087

const maxParticipantsReached = selectedOptions.length === CONST.REPORT.MAXIMUM_PARTICIPANTS;

What alternative solutions did you explore? (Optional)

[allgandalf]

@jasperhuangg , what's the expected result here?

FYI, From a report details page of a group, we can add any amount of members we want, there is no limit to the number of members to be added

[gijoe0295]

Proposal

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

Inconsistency in limiting group members when creating chat and inviting member.

What is the root cause of that problem?

We limit the number to 8 when creating new chat but not when inviting member:

App/src/pages/NewChatPage.tsx

Line 72 in a58133b

const maxParticipantsReached = selectedOptions.length === CONST.REPORT.MAXIMUM_PARTICIPANTS;

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

1. If we don't want the limit, we should remove the above check and every other maxParticipantsReached usages including ones in getHeaderMessage.
2. If we want to apply limit, we should apply to invite member screen as well. In that case, disable the Invite member button when max participant is reached for group chat in both members page and invite page. Note that invite page, to calculate the total members, we should add the current number with the currently selected ones. We can optionally show a message when the max limit is reached. We should also update the optimistic data so that the remove operation will take effect immediately.

What alternative solutions did you explore? (Optional)

NA

[zanyrenney]

will you be able to review this today @ahmedGaber93 thanks!

[ahmedGaber93]

@zanyrenney sorry for the delay, reviewing today.

[zanyrenney]

Did you manage to review this? @ahmedGaber93

[ahmedGaber93]

@zanyrenney PR was reviewed 4 days ago.
and there is a new commits today that I will review in the next few hours.

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

• [Fix]: Inconsistency in limitation when adding members via FAB and group details page #40537

If no regressions arise, payment will be issued on 2024-05-09. 🎊

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

• @ahmedGaber93 requires payment (Needs manual offer from BZ)
• @GandalfGwaihir requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@ahmedGaber93] The PR that introduced the bug has been identified. Link to the PR:
• [@ahmedGaber93] 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:
• [@ahmedGaber93] A discussion in #expensify-bugs 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:
• [@ahmedGaber93] Determine if we should create a regression test for this bug.
• [@ahmedGaber93] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@zanyrenney] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[zanyrenney]

payout due tomorrow.

[zanyrenney]

[zanyrenney]

payment summary

@ahmedGaber93 requires payment (Needs manual offer from BZ) - owed $250 from ND.
@GandalfGwaihir requires payment automatic offer (Contributor) - paid $250 via upwork.

[ahmedGaber93]

@zanyrenney I get paid via upwork, not ND

[ahmedGaber93]

@zanyrenney bump for #40512 (comment)

[ahmedGaber93]

@zanyrenney friendly bump #40512 (comment)

[zanyrenney]

sure thing!

[zanyrenney]

paid @ahmedGaber93 $250 via upwork. closing!