[$500] [HOLD for payment 2024-01-18] Web - Double <Payer:> for IOU report preview in Search

[kbecciv]

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.19-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. [User A] Request money from User B.
2. [User B] Pay the request.
3. [User A] Click on the IOU preview to open IOU report.
4. [User A] Click Search icon.
5. Note that the IOU report shows double Payer: in the preview in Search list.

Expected Result:

Single Payer: for IOU report preview in Search

Actual Result:

Double Payer: for IOU report preview in Search

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

20231229_223646.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016165d0cee06c5ac9
• Upwork Job ID: 1748381394601844736
• Last Price Increase: 2024-01-19

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

[melvin-bot]

Triggered auto assignment to @cead22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[kbecciv]

Issue is not Reproduced on production

[paultsimura]

Proposal

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

What is the root cause of that problem?

We prepend the payer name twice:

App/src/libs/ReportUtils.ts

Lines 2029 to 2032 in 8432563

	const actualPayerName = report.managerID === currentUserAccountID ? '' : `${getDisplayNameForParticipant(report.managerID, true)}:`;
	const payerDisplayName = isPreviewMessageForParentChatReport ? payerName : actualPayerName;
	return Localize.translateLocal(translatePhraseKey, {amount: formattedAmount, payer: payerDisplayName ?? ''});

And:

App/src/libs/OptionsListUtils.js

Lines 512 to 516 in 9692e48

	const lastMessageTextFromReport = getLastMessageTextForReport(report);
	const lastActorDetails = personalDetailMap[report.lastActorAccountID] || null;
	const lastActorDisplayName =
	hasMultipleParticipants && lastActorDetails && lastActorDetails.accountID !== currentUserAccountID ? lastActorDetails.firstName || lastActorDetails.displayName : '';
	let lastMessageText = lastActorDisplayName ? `${lastActorDisplayName}: ${lastMessageTextFromReport}` : lastMessageTextFromReport;

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

Since the lastMessageTextFromReport has changed for the money reports to include the payer name, we should handle this case and set result.alternateText to only lastMessageTextFromReport, not lastMessageText for such options.

We currently do similar thing for tasks:

App/src/libs/OptionsListUtils.js

Lines 532 to 534 in 9692e48

	} else if (result.isTaskReport) {
	result.alternateText = showChatPreviewLine && lastMessageText ? lastMessageTextFromReport : Localize.translate(preferredLocale, 'report.noActivityYet');
	} else {

What alternative solutions did you explore? (Optional)

[mountiny]

I think this was known and wanted to address in a follow up @koko57

[koko57]

It was occuring long before the merge and I found it while I was working on it (it was spotted by me on staging) #30042 (comment)

it displayed [firstName lastName]: [firstName]: but after my changes I had to make for LHN it displays firstName doubled. We agreed that it will be fixed in a separate PR

[mountiny]

Yep its been listed in the issue already as a follow up, i think that this is not a blocker as it only appears in the search page and we will address this in New Year

[melvin-bot]

@koko57, @mountiny Whoops! This issue is 2 days overdue. Let's get this updated quick!

[koko57]

Will take care of it this week 🙂

[koko57]

@mountiny one question - should the previews in the SearchPage be 1:1 similar to LHN previews? If yes I'd use the same logic as in the LHN previews and maybe create a separate function out of it.

I've spotted another weird thing here - for some of the chats I have a strange fallback message

[mountiny]

Yeah i think we can make it 1:1 to the lhn if possible

[koko57]

@mountiny I started working on a refactor but it would require a lot of changes. I want to move the logic for creating alternative text out of the getOptionData from SidebarUtils and use the same logic in createOption OptionListUtils.
But as I wanted to avoid potential regressions and the problems we had with the first PR for LHN previews, maybe I could fix it more simply?

I copied the part of logic that displays the lastActorDisplayName only for certain types of the reportAction and pasted it in the createOption function. It's less invasive - so less regression-prone.
We could fix it this way now and then I could work on the refactor. WDYT? 🙂

[mountiny]

I think that ideally we do abstract away code to avoid duplication, but if you feel like this is not worth at this point, we could go this way too for now and refactor once more changes will be required

[koko57]

@mountiny I think it is not necessary to extract this logic now, I'd rather do this in this major refactor and extracting all the logic for getting alternateText later 🙂

[mountiny]

Ok then!

[mountiny]

PR merged

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

• Fix/33760 doubled display name on search page #33966

If no regressions arise, payment will be issued on 2024-01-18. 🎊

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

• @koko57 does not require payment (Contractor)
• @situchan requires payment (Needs manual offer from BZ)

[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:

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

[melvin-bot]

Payment Summary

Upwork Job

• Contributor: @koko57 is from an agency-contributor and not due payment
• ROLE: @situchan paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@michaelhaxhiu)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[mountiny]

@situchan Can you complete the checklist here? this was a simple follow up to previou PR, I feel like the reward should be $250 for this one

[situchan]

BZ Checklist:
Offending PR: #33346, though the root cause already existed - #33760 (comment)
As this was caught during staging regression testing, no need new regression test.
To prevent such bugs further, we decided to make LHN and search logic consistent as follow-up here

---

@situchan Can you complete the checklist here? this was a simple follow up to previou PR, I feel like the reward should be $250 for this one

I'm fine with any reward change but does that mean that now we have the right to request bounty increase for reviewing large PRs? For me, some PRs took several days to complete review.
I thought they were flatten no matter small or large PR, as they're balanced eventually.

PS: #33371 was just console log change PR and fully rewarded here

[mountiny]

I'm fine with any reward change but does that mean that now we have the right to request bounty increase for reviewing large PRs? For me, some PRs took several days to complete review. I thought they were flatten no matter small or large PR, as they're balanced eventually.

PS: #33371 was just console log change PR and fully rewarded #33478 (comment)

Yeah I see your point, lets keep it $500 then.

I think the root cause is that we should get the same C+ review the clean up PRs too as in this case it was a regression from PR reviewed by another C+ so then we end up paying more in total for one task

[michaelhaxhiu]

Yeah I see your point, lets keep it $500 then.

Sounds like we arrived at $500 standard rate for this PR review! I will make an upwork job now for it.

I thought they were flatten no matter small or large PR, as they're balanced eventually.

FWIW -- what you described here is indeed the core idea at play for the bounty methodology. It's a pay rate that is intended to be more than fair in the grand scheme of being a C+, unless a specific PR is a HUGE deviation from the typical workload and deserves a one-off consideration.

[melvin-bot]

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

[melvin-bot]

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

[situchan]

@michaelhaxhiu after making External, if you unassign me and reassign, I will get offer

[michaelhaxhiu]

I just sent you an offer actually. But that's a good reminder of how to use the automation

[michaelhaxhiu]

paid