[HOLD for payment 2024-07-17] [$250] Actionable whisper - No option to add receipt when submitting to someone

[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: 1.4.85-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/4646136
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to self DM.
3. Track a manual expense
4. Click Submit it to someone from the actionable whisper
5. Select a user

Expected Result:

There will be an option to add receipt on the confirmation page

Actual Result:

There is no option to add receipt on the confirmation 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

Bug6517560_1718732559665.bandicam_2024-06-19_01-40-29-806.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015c134d920ca4197c
• Upwork Job ID: 1803928874102505326
• Last Price Increase: 2024-06-20
• Automatic offers:
• hungvu193 | Reviewer | 102918536
• nkdengineer | Contributor | 102918540

Issue Owner

Current Issue Owner: @adelekennedy

[melvin-bot]

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

We think that this bug might be related to #vip-vsp

[lanitochka17]

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

[adelekennedy]

yep - I can reproduce, and in this case I agree this is a bug as we allow you to add a receipt after selecting the receiver from the global create

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

There is no option to add receipt on the confirmation page

What is the root cause of that problem?

This is happening because we have the !isMovingTransactionFromTrackExpense here

App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

Line 554 in 633e708

requestType === CONST.IOU.REQUEST_TYPE.MANUAL && (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.TRACK) && !isMovingTransactionFromTrackExpense

There's no reason to exclude the Add receipt from the moving tracked expense use case.

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

Remove !isMovingTransactionFromTrackExpense in

App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

Line 554 in 633e708

requestType === CONST.IOU.REQUEST_TYPE.MANUAL && (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.TRACK) && !isMovingTransactionFromTrackExpense

I tested any it works well, no issue.

Or we can allow adding receipt only if the tracked expense doesn't already have a receipt, so !isMovingTransactionFromTrackExpense should be updated to

(!isMovingTransactionFromTrackExpense || !hasReceipt)

Where hasReceipt is the receipt check based on existence of receiptPath or transaction?.receipt, like

const hasReceipt = !!receiptPath;

What alternative solutions did you explore? (Optional)

If we go with

Remove !isMovingTransactionFromTrackExpense in

App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

Line 554 in 633e708

requestType === CONST.IOU.REQUEST_TYPE.MANUAL && (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.TRACK) && !isMovingTransactionFromTrackExpense

I'd suggest an improvement that the text (and maybe icon) we show here will be based on hasReceipt. So if hasReceipt, that will be Replace receipt, not Add receipt

[nkdengineer]

Proposal updated

[hungvu193]

I'm quite busy today, I'll take a look this weekend

[hungvu193]

shouldShowThreeDotsButton condition was added in this PR
More context in this conversation. I'll investigate and test a little bit more before making a final decision 😄

[hungvu193]

We're also hiding Three dot menu (Add receipt button) if isMovingTransactionFromTrackExpense is true, because we're having a BE bug, the receipt image is reverted after we added it.

Screen.Recording.2024-06-24.at.16.28.39.mov

IMO, we should add Add receipt button back even when isMovingTransactionFromTrackExpense is true, and create another BE issue for the issue I mentioned above.
Let's me know what's your thought 🎀 👀 🎀

[melvin-bot]

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

[hungvu193]

@yuwenmemon Please take a look at my comment here. Ty 😄

[hungvu193]

little bump @yuwenmemon 🙇

[yuwenmemon]

That sounds good to me! The BE issue is just that the receipt image disappears, correct?

[hungvu193]

That sounds good to me! The BE issue is just that the receipt image disappears, correct?

Yes correct.
In that case, we can go with @nkdengineer 's proposal

[nkdengineer]

In that case, we can go with @nkdengineer 's proposal

@yuwenmemon Could you assign me here so I can start with the PR 🙇

[yuwenmemon]

Yep!

[nkdengineer]

@hungvu193 this PR is ready for review

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

• fix: No option to add receipt when submitting to someone #44670

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @hungvu193 requires payment automatic offer (Reviewer)
• @nkdengineer 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:

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

[melvin-bot]

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

• fix: No option to add receipt when submitting to someone #44670

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

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

• @hungvu193 requires payment automatic offer (Reviewer)
• @nkdengineer 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:

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

[melvin-bot]

@yuwenmemon, @hungvu193, @adelekennedy, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick!

[adelekennedy]

due to a bug a bunch of issue git hit with a double payment day but this should have been paid on the 17th - handling payment now!

[adelekennedy]

Payouts due:

• Contributor: $250 @hungvu193 (NewDot)
• Contributor+: $250 @nkdengineer (Upwork)

Upwork job is here.

bump also @nkdengineer complete the checklist above

[hungvu193]

• The PR that introduced the bug has been identified. Link to the PR: Fix scan in progress shown for manual request with receipt moved from tracked expense #42984
• 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
• 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
• Determine if we should create a regression test for this bug.

I don't think we need regression test for this one. Actually, this PR was revert becauseConvertTrackedExpenseToRequest command doesn't accept a receipt as a parameter, more details in this comment

[hungvu193]

I'll request on ND later.

[hungvu193]

Requested on ND

[JmillsExpensify]

$250 approved for @hungvu193