[HOLD for payment 2024-04-05] [$500] [MEDIUM] Expensify Cards: Wrong report header for the money spent

[m-natarajan]

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:
Reproducible in staging?: needs reproduction (card expense)
Reproducible in production?:
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: @coleaeason
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707427890816659

Action Performed:

1. User A spends using the card
2. Submits the bill to B
3. Check the report header as user A

Expected Result:

User A spent $x.xx should appear

Actual Result:

User B spent $x.xx appears

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016517b2a5bd47bbcb
• Upwork Job ID: 1762448288159342592
• Last Price Increase: 2024-02-27
• Automatic offers:
• aimane-chnaif | Reviewer | 0
• mollfpr | Contributor | 0

[melvin-bot]

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

[CortneyOfstad]

Clarifying which wave here

[grgia]

Wave 6 @CortneyOfstad !

[grgia]

maybe related to #35529 cc @mollfpr

[mollfpr]

Looking 👀

[CortneyOfstad]

Wahoo! Thanks @grgia!

[grgia]

cc @abzokhattab

[CortneyOfstad]

Not overdue 👍

[CortneyOfstad]

Not overdue

[mollfpr]

Sorry for the delay! I have checked and the PR #35529 doesn't make this issue. We don't have a check or utils function showing the spender's name on the money spent. It's default to use the manager name instead.

Here's are another place that using the display name wrong.

[CortneyOfstad]

Thanks for the update @mollfpr! Any thoughts on the above @grgia?

[melvin-bot]

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

[grgia]

Looks like we mistakenly only use the manager name, but it should be the spender in some cases. I think unless we make this change as an update from the linked PR, we can make this internal. It seems more like this is a forgotten case that we should have covered in that PR @mollfpr

[joekaufmanexpensify]

@CortneyOfstad I see you're back, so going to let you keep this one. LMK if there's anything else I can help with!

[CortneyOfstad]

Thanks @joekaufmanexpensify!

@mollfpr it looks like @aimane-chnaif is waiting for confirmation on the PR here. Thanks!

[mollfpr]

Checking 👀

[CortneyOfstad]

@grgia it shows that @mollfpr is waiting for confirmation on the PR here. Can you provide an update? Thanks!

[CortneyOfstad]

@grgia bump on the above ^^^

[mollfpr]

@CortneyOfstad The PR is approved #37773 (review) but needs more internal testing.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

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

• Fix the incorrect payer name on the non-reimbursable transaction #37773

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

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

• @mollfpr requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment automatic offer (Reviewer)

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

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

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[CortneyOfstad]

@mollfpr Can you complete the checklist before EOD tomorrow? Thanks!

[CortneyOfstad]

@mollfpr bump on the checklist 👍

[CortneyOfstad]

Payment Summary

@mollfpr to be paid $500 via NewDot
@aimane-chnaif paid $500 via Upwork

[mollfpr]

Sorry for the delay @CortneyOfstad

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] 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:

Not fully regression from #35529 but we do some improvement and cover another case to show correct verb.

[@mollfpr] 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:

The regression step should be enough.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] 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.

1. Log in to your account with the Expensify Card transaction ([email protected])
2. Go to the workspace chat with the transaction (https://staging.new.expensify.com/r/4612388815642526. If the transaction is settled, please create another Expensify Card transaction)
3. Verify the text on 3 places below correctly showing <requestor's name> spent
   • Alternate text on LHN
   • Report preview
   • Money report preview header
4. 👍 or 👎

[mollfpr]

@CortneyOfstad The PR I worked for this issue has a regression but I have fixed it #39208

[CortneyOfstad]

@mollfpr I don't see a regression listed. Only that it was merged without testing, which @Gonals handled. We should be good 👍

I think the regression test looks good — @grgia any changes you would like to see made (if any)?

[JmillsExpensify]

$500 approved for @mollfpr

[CortneyOfstad]

Regression test created here — https://github.com/Expensify/Expensify/issues/386030

Thanks!