[Ready for C+ checklist and payment][$250] Split - Group chat does not show bill creator name in LHN preview after refreshing the page

[lanitochka17]

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.60-5
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
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Request money > Manual
3. Create a split bill with two users
4. Once landed in group chat, note that group chat in LHN shows the bill creator name in the preview
5. Refresh the page

Expected Result:

The group chat will continue to show the bill creator name in LHN preview after refreshing the page

Actual Result:

The group chat does not show the bill creator name in LHN preview after refreshing the page

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

Bug6438312_1712255589440.20240405_023013.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0126a3e7c5e2856078
• Upwork Job ID: 1777107502133911552
• Last Price Increase: 2024-04-28
• Automatic offers:
• eh2077 | Reviewer | 0
• bernhardoj | Contributor | 0

[melvin-bot]

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

[lanitochka17]

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

[bernhardoj]

Proposal

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

The LHN subtitle shows the sender name when creating the split bill, but then it disappears.

What is the root cause of that problem?

When we refresh/wait for a moment, we will get the correct response from the BE and the sender name disappears is already correct because we don't want to show the sender name if it's coming from ourself.

App/src/libs/OptionsListUtils.ts

Lines 529 to 533 in 2995f2c

	function getLastActorDisplayName(lastActorDetails: Partial<PersonalDetails> | null, hasMultipleParticipants: boolean) {
	return hasMultipleParticipants && lastActorDetails && lastActorDetails.accountID !== currentUserAccountID
	? // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
	lastActorDetails.firstName || PersonalDetailsUtils.getDisplayNameOrDefault(lastActorDetails)
	: '';

The logic to show the sender name is, by taking the report lastActorAccountID and getting the user data from the ID.

App/src/libs/SidebarUtils.ts

Line 297 in 2995f2c

let lastActorDetails: Partial<PersonalDetails> | null = report.lastActorAccountID && personalDetails?.[report.lastActorAccountID] ? personalDetails[report.lastActorAccountID] : null;

However, when we create a split bill, the lastActorAccountID is 0, so the user detail is empty.

App/src/libs/ReportUtils.ts

Line 3781 in 2995f2c

lastActorAccountID: 0,

If it's empty, we will get it from the last report action person data which contains the user display name.

App/src/libs/SidebarUtils.ts

Lines 299 to 307 in 2995f2c

	if (!lastActorDetails && visibleReportActionItems[report.reportID]) {
	const lastActorDisplayName = visibleReportActionItems[report.reportID]?.person?.[0]?.text;
	lastActorDetails = lastActorDisplayName
	? {
	displayName: lastActorDisplayName,
	accountID: report.lastActorAccountID,
	}
	: null;
	}

Because 0 is not the same as the current user ID, the current user display name is shown as the sender.

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

Set the lastActorAccountID optimistically to currentUserAccountID when creating a split bill (and revert it when fails). We did this when adding a message/task too.

We might need to do this for other money request too.

What alternative solutions did you explore? (Optional)

1. Don't show the display name when the lastActorAccountID is 0.

OR

2. If we always want to show the sender name, we need to remove the accountID check (lastActorDetails.accountID !== currentUserAccountID) here.
   

   App/src/libs/OptionsListUtils.ts

   Lines 536 to 537 in 9a6cafc

   	function getLastActorDisplayName(lastActorDetails: Partial<PersonalDetails> | null, hasMultipleParticipants: boolean) {
   	return hasMultipleParticipants && lastActorDetails && lastActorDetails.accountID !== currentUserAccountID

[melvin-bot]

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

[melvin-bot]

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

[eh2077]

Reviewing proposals

[eh2077]

@bernhardoj 's proposal looks good to me. I agree with their RCA and their solution. I also agree we should fix other money request flows together.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@aldo-expensify bump on this!

[aldo-expensify]

I have some doubts about the "Expected results" being correct here. If we say that the frontend is right, and the split creator name shouldn't appear in the report's last message in the LHN from the perspective of the creator, this means that the last message of the report is different for each participant, and I think we may want to avoid that.

@marcaaron can I get your eyes here since I think you know about groups. 🙏

[marcaaron]

I agree 1000%.

With Group Chats, we did not really change much about how this works on the frontend side of things though - just replaced the Group DM with a Group Chat so I'm not sure where we went wrong or if this was just always awkward.

Could probably take this to #vip-split room and loop in whoever is working on Splits now. cc @arielgreen @dubielzyk-expensify and @youssef-lr? Can we see about rolling this in as a polish item for this project?

[dubielzyk-expensify]

Seems like a minor bug. I agree that it'd be nice to fix it though, but fine with this as a polish item

[eh2077]

@kadiealexander Based on the discussions above, can you help to update the Expected results section of this issue?

[melvin-bot]

@kadiealexander @aldo-expensify @eh2077 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[eh2077]

@kadiealexander Based on the discussions above, can you help to update the Expected results section of this issue?

@kadiealexander Friendly bump!

[aldo-expensify]

It is not clear to me from the conversation which option we want, so I asked here https://expensify.slack.com/archives/C05RECHFBEW/p1713833726038629 to confirm

[aldo-expensify]

Sorry @eh2077 @bernhardoj for the back and forward. The conclusion from the slack conversation is:

The bill creator name should appear in the last message, and the front end should take care of that. The backend won't send the last message with the bill creator in it.

I think this was what @bernhardoj original solution was trying to achieve, correct?

This code seems related to that:

App/src/libs/OptionsListUtils.ts

Lines 711 to 713 in 42d4ed7

	if (shouldDisplayLastActorName && lastActorDisplayName && lastMessageTextFromReport) {
	lastMessageText = `${lastActorDisplayName}: ${lastMessageTextFromReport}`;
	}

[eh2077]

@aldo-expensify Thanks for driving clarity.

@bernhardoj Can you take some time to update proposal based on the expected result^ ?

[aldo-expensify]

I just noticed a mistake in the expected results I typed above. The backend WON'T send the bill creator as part of the last message:

[bernhardoj]

My proposal has both behaviors, showing or not showing the sender name if it's the current user. If I understand correctly from @aldo-expensify comment, we always want to show the sender's name, so it should be the 2nd alternative.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[eh2077]

@bernhardoj Thanks for the update. Yeah, the 2nd alternative makes sense to me.

@aldo-expensify All yours.

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @aldo-expensify is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

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

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

[bernhardoj]

PR is ready

cc: @eh2077

[eh2077]

The fix has hit production #41483 (comment)

[kadiealexander]

Sorry guys, this one slipped through the cracks!

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:

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR:
• [@eh2077] 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:
• [@eh2077] 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:
• [@eh2077] Determine if we should create a regression test for this bug.
• [@eh2077] 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.
• [@kadiealexander] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Payouts due:

• Contributor: $250 @bernhardoj
• Contributor+: $250 @eh2077

Upwork job is here.

[eh2077]

BugZero Checklis

• [@eh2077] The PR that introduced the bug has been identified. Link to the PR: I can't find the specific PR caused this issue. I think this case is not handled when we added the feature.
• [@eh2077] 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: N/A
• [@eh2077] 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
• [@eh2077] Determine if we should create a regression test for this bug. This bug was reported by QA team and it's an edge case, so I think we don't need a regression test for it.