[WAITING ON CHECKLIST] [$250] Expense Details - The header of the expense details displays "[amount] for [Description]"

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

Action Performed:

1. Navigate to https://staging.new.expensify.com/
2. Create a workspace
3. Go to the workspace chat
4. Click the plus button > Submit Expense > enter amount, description, and merchant, and complete the submission
5. Open the expense details

Expected Result:

The conversation title in the LHN should display "[amount] for [Merchant]"

Actual Result:

The conversation title in the LHN displays "[amount] for [Description]"

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

Bug6620960_1727776903763.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021841274388644598293
• Upwork Job ID: 1841274388644598293
• Last Price Increase: 2024-10-02

Issue Owner

Current Issue Owner: @

[melvin-bot]

Triggered auto assignment to @jliexpensify (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]

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

[nkdengineer]

Proposal

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

What is the root cause of that problem?

We pass the comment as description here then LHN displays amount for description

App/src/libs/ReportUtils.ts

Line 3471 in d46f218

let comment = !isEmptyObject(linkedTransaction) ? TransactionUtils.getDescription(linkedTransaction) : undefined;

App/src/libs/ReportUtils.ts

Line 3482 in d46f218

return `${requestorName ? `${requestorName}: ` : ''}${Localize.translateLocal('iou.submittedAmount', {formattedAmount: amountToDisplay, comment})}`;

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

Here we should get the merchant first then fallback to description if the merchant is empty

let comment = !isEmptyObject(linkedTransaction) ? TransactionUtils.getMerchant(linkedTransaction) || TransactionUtils.getDescription(linkedTransaction) : undefined; 

App/src/libs/ReportUtils.ts

Line 3471 in d46f218

let comment = !isEmptyObject(linkedTransaction) ? TransactionUtils.getDescription(linkedTransaction) : undefined;

OPTIONAL: we can also do the same here

What alternative solutions did you explore? (Optional)

[bernhardoj]

Proposal

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

The last message when we submit an expense with a merchant and description will show the description instead of the merchant.

What is the root cause of that problem?

We always use the description here.

App/src/libs/ReportUtils.ts

Line 3471 in d46f218

let comment = !isEmptyObject(linkedTransaction) ? TransactionUtils.getDescription(linkedTransaction) : undefined;

App/src/libs/ReportUtils.ts

Line 3482 in d46f218

return `${requestorName ? `${requestorName}: ` : ''}${Localize.translateLocal('iou.submittedAmount', {formattedAmount: amountToDisplay, comment})}`;

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

We need to follow the same logic from getTransactionReportName which checks whether the merchant is empty or not before using it because (none) merchant is also empty.

App/src/libs/ReportUtils.ts

Line 3322 in d46f218

const comment = (!TransactionUtils.isMerchantMissing(transaction) ? TransactionUtils.getMerchant(transaction) : TransactionUtils.getDescription(transaction)) ?? '';

let comment = !isEmptyObject(linkedTransaction) ? (!TransactionUtils.isMerchantMissing(linkedTransaction) ? TransactionUtils.getMerchant(linkedTransaction) : TransactionUtils.getDescription(linkedTransaction)) : undefined;

We can apply the same fix here

App/src/libs/ReportUtils.ts

Line 3373 in d46f218

return Localize.translateLocal('iou.didSplitAmount', {formattedAmount, comment: TransactionUtils.getDescription(linkedTransaction) ?? ''});

App/src/libs/ReportUtils.ts

Line 3395 in d46f218

return Localize.translateLocal('iou.trackedAmount', {formattedAmount, comment: TransactionUtils.getDescription(linkedTransaction) ?? ''});

App/src/libs/ReportUtils.ts

Line 7193 in d46f218

comment: TransactionUtils.getDescription(transaction) ?? '',

NOTE: I think we can create a getMerchantOrDescription so the logic is reusable.

[jliexpensify]

I can repro but not sure if we made this change intentionally, so asking here: https://expensify.slack.com/archives/C036QM0SLJK/p1727826242838209

[melvin-bot]

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

[melvin-bot]

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

[jliexpensify]

Confirmed that this seems to be a bug:

merchant should take priority over description like the preview component

[jliexpensify]

@alitoshmatov I'm OOO from 3rd to 14th, but I don't think anything is needed from me during this period. Feel free to reassign to another B0 if urgent payment is needed, otherwise I'll catch up on this issue when I get back!

[alitoshmatov]

@nkdengineer @bernhardoj Thank you both for proposals. Your RCA is correct and you provide with almost the same solution.

It is hard choice to make since both proposals are very similar solution, even though @bernhardoj was the second one to submit he did have small additional changes.

I think we should go with @bernhardoj's proposal since it has additional changes that we might have missed it

C+ reviewed 🎀 👀 🎀

[melvin-bot]

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

[melvin-bot]

❌ There was an error making the offer to @alitoshmatov for the Reviewer role. The BZ member will need to manually hire the contributor.

[bernhardoj]

PR is ready

cc: @alitoshmatov

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

• Fix the money request preview text shows description instead of merchant #50372

If no regressions arise, payment will be issued on 2024-10-22. 🎊

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

• @bernhardoj requires payment through NewDot Manual Requests
• @alitoshmatov 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:

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

[jliexpensify]

Payment Summary

• C: @bernhardoj $250 (owed via New.Dot payment)
• C+: @alitoshmatov $250 (owed vis Upworks)

[bernhardoj]

Requested in ND.

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @bernhardoj owed $250 via NewDot
• ROLE: @alitoshmatov paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@jliexpensify)

• 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/1841274388644598293/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[melvin-bot]

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

[jliexpensify]

Sorry, upworks is being really weird and won't let me access the job so I closed it and am trying to see if another Job is created.

[jliexpensify]

@alitoshmatov waiting on checklist, and invited you to a new job here:

https://www.upwork.com/jobs/~021848861002130198557

[jliexpensify]

Paid @alitoshmatov via Upworks, just waiting on checklist

[alitoshmatov]

• The PR that introduced the bug has been identified. Link to the PR: Track Expense flow #37282
• 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/37282/files#r1812400386
• 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: No need
• Determine if we should create a regression test for this bug. No need

[JmillsExpensify]

$250 approved for @bernhardoj based on this summary.