[HOLD for payment 2024-10-29] [HOLD for payment 2024-10-25] [$250] Receipt - Receipt 3dots menu doesn't show options to replace and delete receipt

[IuliiaHerets]

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: v9.0.50-0
Reproducible in staging?: Y
Reproducible in production?: N
Issue reported by: Applause Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home
2. Open 1:1 with an user (or workspace chat)
3. Submit an expense
4. Open the expense
5. Tap plus placeholder and using camera upload an image
6. Tap on receipt -- 3 dots
7. Note the receipt options

Expected Result:

Receipt 3dots menu must show options to replace and delete receipt.

Actual Result:

Receipt 3dots menu doesn't show options to replace and delete receipt.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6637231_1729136707540.Screenrecorder-2024-10-17-09-01-26-36.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021846827472906199686
• Upwork Job ID: 1846827472906199686
• Last Price Increase: 2024-10-17
• Automatic offers:
• mkzie2 | Contributor | 104459504

Issue Owner

Current Issue Owner: @RachCHopkins

[melvin-bot]

Triggered auto assignment to @Beamanator (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

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

[melvin-bot]

💬 A slack conversation has been started in #expensify-open-source

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

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

[melvin-bot]

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

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-10-17 08:46:35 UTC.

Proposal

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

Receipt 3dots menu doesn't show options to replace and delete receipt.

What is the root cause of that problem?

The readonly route param here is parsed as string:

App/src/pages/TransactionReceiptPage.tsx

Line 30 in 9f8e8c5

const readonly = route.params.readonly ?? false;

so when it is "false" (string), the canEditReceipt here is false (because !"false" is boolean false)

App/src/pages/TransactionReceiptPage.tsx

Line 75 in 9f8e8c5

allowDownload={!isEReceipt}

And eventually we cannot edit the receipt (neither replace nor delete)

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

Explicitly check if route.params.readonly === 'true'

What alternative solution did you explore? (Alternative)

Only show readonly if it is true, do not show readonly=false just like we did before:

App/src/ROUTES.ts

Line 1252 in eaef547

`r/${reportID}/transaction/${transactionID}/receipt?readonly=${readonly}${isFromReviewDuplicates ? '&isFromReviewDuplicates=true' : ''}` as const,

${readonly ? '?readonly=true' : ''}`

[Beamanator]

@mkzie2 thanks so much for your proposal, I will test your idea but can you also try to help figure out why this just now broke? Like what changed between prod & staging that caused this bug?

[Beamanator]

Ooh your change does look like it fixes the problem 👍 👍 🎉 though I'm still curious to try to find any regressed PR(s) if possible!

[Pujan92]

@mkzie2's RCA and proposal looks correct, let me also find the offended PR

[Pujan92]

It is from this PR #50546, earlier we were only passing the readonly param in the route when it is true but now we are passing default value too.

https://github.com/Expensify/App/pull/50546/files#diff-8c80af31c75a0f4fcb67272bd2df5b42c9611c040ecc66584fcd6bde05dbbd78L1179

[mkzie2]

Offending PR #50546. Explicitly this change. Before that we never have readonly=false, only readonly=true.

[Beamanator]

Wow good find both of y'all 😅

So it sounds like that PR only really uncovered a bug in the App/src/pages/TransactionReceiptPage.tsx file, right? If so, I think let's hire @mkzie2 to fix real quick

[mkzie2]

@Pujan92 I added an alternative solution, which one should we proceed?

[Pujan92]

Yes @Beamanator, I think so.

[Beamanator]

Amazing! Thanks!

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

• [CP Staging] fix: cannot edit receipt #50987

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

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

• @Pujan92 requires payment through NewDot Manual Requests
• @mkzie2 requires payment automatic offer (Contributor)

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

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

[Christinadobrzyn]

HI there, just checking to see if this might be a regression #51142

[Beamanator]

Hmm @Christinadobrzyn that seems unlikely b/c the fix for this issue is now in production 🤔

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.51-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [CP Staging] fix: cannot edit receipt #50987

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

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

• @Pujan92 requires payment through NewDot Manual Requests
• @mkzie2 requires payment automatic offer (Contributor)

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

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

[RachCHopkins]

@Pujan92 can you please complete the checklist so that I can issue payment?

[Pujan92]

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:

• [@Pujan92] The PR that introduced the bug has been identified. Link to the PR: fix: receipt in dupe detect is removable #47536
• [@Pujan92] 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/47536/files#r1816465370
• [@Pujan92] 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: NA
• [@Pujan92] Determine if we should create a regression test for this bug. Yes
• [@Pujan92] 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.

Regression Test Steps

1. Submit an expense and add a receipt to it
2. Goto that expense and open an image modal
3. Click on 3 dots menu
4. Verify Replace and Delete receipt options are shown

[melvin-bot]

@Beamanator, @Pujan92, @RachCHopkins, @mkzie2 Whoops! This issue is 2 days overdue. Let's get this updated quick!

[RachCHopkins]

Will pay this tomorrow!

[RachCHopkins]

Payment Summary:

Contributor: @mkzie2 paid $250 via Upwork
Contributor+: @Pujan92 to be paid $250 via Newdot manual request
Upwork job here

[RachCHopkins]

Contributor has been paid, the contract has been completed, and the Upwork post has been closed.