[$250] mWeb - Group - New users are not displayed in group name

[lanitochka17]

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.14
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): https://expensify.testrail.io/index.php?/tests/view/4784615
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Log in a new gmail account
3. Tap fab - start chat
4. Enter a new user with no expensify account and tap add to group
5. Tap group name.

Expected Result:

New users must be displayed in group name

Actual Result:

New users are not displayed in group name
Reproducible on these devises:
Samsung galaxy Ao2s/12 - Chrome.
Redmi note 10s/Android 13, Samsung Galaxy m2/Android 12

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

Bug6556868_1722299822512.newv.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01511ab0ad63b9289d
• Upwork Job ID: 1818320616796150714
• Last Price Increase: 2024-07-30
• Automatic offers:
• hoangzinh | Reviewer | 103362702
• nkdengineer | Contributor | 103362703

Issue Owner

Current Issue Owner: @hoangzinh

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@lschurr 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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[cretadn22]

Proposal

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

Group - New users are not displayed in group name

What is the root cause of that problem?

In this case, we use ReportUtils.getGroupChatName function to get group name

App/src/pages/NewChatConfirmPage.tsx

Line 64 in 5d26dbe

const groupName = newGroupDraft?.reportName ? newGroupDraft?.reportName : ReportUtils.getGroupChatName(participantAccountIDs ?? []);

And in ReportUtils.getGroupChatName function, we use getDisplayNameForParticipant function

App/src/libs/ReportUtils.ts

Line 2017 in 6bc174f

.map((participant) => getDisplayNameForParticipant(participant, isMultipleParticipantReport))

The getDisplayNameForParticipant function return empty string, because the App can't find accountID of new user in personalDetail

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

We need to add a fallback value when the App can't find accountID of new user in personalDetail. Keep in mind that the new user's email is stored in newGroupDraft.participants

In getGroupChatName function, we can replace participantAccountIDs param into participant param. This participant parameter will contain both the accountID and the login field, allowing us to use participant.login as a default value.

Additional, we maybe need to apply the same solution in here

App/src/pages/GroupChatNameEditPage.tsx

Line 50 in 5d26dbe

() => (report ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getGroupChatName(draftParticipantAccountIDs)),

Result:

What alternative solutions did you explore? (Optional)

We can introduce new param in getDisplayNameForParticipant function called participant. And then if the App can't find accountID of new user in personalDetail, we will use participant.login as a default value

[cretadn22]

I have implemented my solution locally and included the results as evidence in my proposal

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

New users are not displayed in group name
Reproducible on these devises:
Samsung galaxy Ao2s/12 - Chrome.
Redmi note 10s/Android 13, Samsung Galaxy m2/Android 12

What is the root cause of that problem?

If we select a new user on offline mode or select a new user before SearchForReports API is complete, the personal details of this user don't exist in Onyx. So getDisplayNameForParticipant returns an empty string here and then the new user doesn't display in the group chat name.

App/src/libs/ReportUtils.ts

Lines 2016 to 2018 in d2fdb27

	return participants
	.map((participant) => getDisplayNameForParticipant(participant, isMultipleParticipantReport))
	.sort((first, second) => localeCompare(first ?? '', second ?? ''))

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

getDisplayNameForParticipant function is used in many places of the app so we should handle to fallback the login in getGroupChatName function as a special case instead of handling it in getDisplayNameForParticipant function.

• We can pass newGroupDraft.partipants here instead of passing the array accountID of the group chat.

function getGroupChatName(draftParticipants?: SelectedParticipant[], shouldApplyLimit = false, report?: OnyxEntry<Report>): string | undefined {
let participants = draftParticipants?.map((participant) => participant.accountID) ?? Object.keys(report?.participants ?? {}).map(Number);

App/src/libs/ReportUtils.ts

Line 2002 in d2fdb27

function getGroupChatName(participantAccountIDs?: number[], shouldApplyLimit = false, report?: OnyxEntry<Report>): string | undefined {

• Then we will use the login in the participants data of the group to fallback here if the getDisplayNameForParticipant function returns an empty string.

getDisplayNameForParticipant(participant, isMultipleParticipantReport) || draftParticipants?.[index]?.login

App/src/libs/ReportUtils.ts

Line 2017 in d2fdb27

.map((participant) => getDisplayNameForParticipant(participant, isMultipleParticipantReport))

• After that, we need to update the place that we're using group draft data to get the group chat name

const groupName = newGroupDraft?.reportName ? newGroupDraft?.reportName : ReportUtils.getGroupChatName(newGroupDraft?.participants);

App/src/pages/NewChatConfirmPage.tsx

Line 64 in d2fdb27

const groupName = newGroupDraft?.reportName ? newGroupDraft?.reportName : ReportUtils.getGroupChatName(participantAccountIDs ?? []);

const existingReportName = useMemo(
    () => (report ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getGroupChatName(groupChatDraft?.participants)),
    [groupChatDraft?.participants, report],
);

App/src/pages/GroupChatNameEditPage.tsx

Lines 49 to 52 in d2fdb27

	const existingReportName = useMemo(
	() => (report ? ReportUtils.getGroupChatName(undefined, false, report) : ReportUtils.getGroupChatName(draftParticipantAccountIDs)),
	[draftParticipantAccountIDs, report],
	);

What alternative solutions did you explore? (Optional)

[nkdengineer]

Updated proposal

• Add an explanation

[hoangzinh]

Thanks for proposals, everyone. @nkdengineer's proposal looks good to me

• He has a deeper RCA.
• His solution is safer and similar with other places

Link to proposal #46502 (comment)

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[cretadn22]

His solution is safer and similar with other places

@hoangzinh What are the differences between my proposal and @nkdengineer's proposal? I notice that @nkdengineer's proposal is more detailed, but the main idea is the same.

[hoangzinh]

Ah @cretadn22 I didn't mean @nkdengineer's proposal has more details than yours. But:

• His RCA is deeper than yours. For example, in your RCA you mentioned to "The getDisplayNameForParticipant function return empty string, because the App can't find accountID of new user in personalDetail", but why the App can't find accountID for new user? @nkdengineer explained it well

If we select a new user on offline mode or select a new user before SearchForReports API is complete, the personal details of this user don't exist in Onyx.

• About solution, there is a difference between 2 proposals, you proposed a solution to change the value that passes in getDisplayNameForParticipant func, because this func is used in many other places, so compared with @nkdengineer's solution, it looks safer.

Btw, as a new contributor, your proposal looks good.

[cretadn22]

In getDisplayNameForParticipant function, we can replace participantAccountIDs param into participant param. This participant parameter will contain both the accountID and the login field, allowing us to use participant.login as a default value.

Ahhh, This is my mistake, I mean getGroupChatName function

[cretadn22]

The rest is correct to getGroupChatName, I only make mistake about function name

we can replace participantAccountIDs param into participant param. This participant parameter will contain both the accountID and the login field, allowing us to use participant.login as a default value.

[cretadn22]

This is my local implementation for the proposal. I have made code changed and thoroughly tested it before posting
proposal, and I’ve included an image of the result in my proposal.

[hoangzinh]

Yeah, but according to Contributing guides, I can only select 1 proposal that's earliest provided, best proposed solution + RCA. Let's wait for the internal engineer to review it.

[stitesExpensify]

This is a tough one, but I agree that @nkdengineer should get the job for the reasons Vinh mentioned here

[melvin-bot]

📣 @hoangzinh 🎉 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]

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

[melvin-bot]

This issue has not been updated in over 15 days. @hoangzinh, @lschurr, @stitesExpensify, @nkdengineer eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[hoangzinh]

Not overdue. The PR has been merged to Production since last week. Ready for payment. I will complete BZ checklist soon

[hoangzinh]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: Create Group Chats + Splits. #37458
• 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: https://github.com/Expensify/App/pull/37458/files#r1735956134
• 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: N/A
• Determine if we should create a regression test for this bug: 🚫 It's an edge case. It only happens when the user either quickly clicks "Add" button or adds a new user to a group when offline. It's just a group name, it won't cause any issue to user to continue create a group chat.

[nkdengineer]

@lschurr This is ready for payment for a while, could you help to apply the Daily label? TIA

[lschurr]

Hi folks! Sorry, I was OOO last week. Handling payments now.

[lschurr]

Payment summary:

• Contributor: $250 @nkdengineer (paid in Upwork)
• Reviewer: $250 @hoangzinh (paid in Upwork)