[HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [HOLD for payment 2024-07-10] [$250] [One expense reports] Update the UI for Held transactions in Single Transaction Report Preview / MoneyRequestAction

[NikkiWines]

Slack discussion here

Goal

Expand upon the new UI pattern introduced as part of the Wave 5/6: Update how we display / report expenses doc to include reports where all transactions that are on HOLD (including reports with only one transaction where we show only one expense) using the copy "All requests were put on hold. Review the comments for next steps."

Note 1: that a report could be in two states simultaneously: the receipt could be scanning and the transaction(s) on the report could be on hold

Note 2: One-transaction reports currently do not show an option to hold an expense, but this is easily rectified by including the logic from here in src/components/MoneyReportHeader.tsx

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017ed767635e9fa640
• Upwork Job ID: 1777965895318855680
• Last Price Increase: 2024-05-10
• Automatic offers:
• dukenv0307 | Reviewer | 0
• tienifr | Contributor | 0

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @JmillsExpensify

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @dukenv0307 (Internal)

[NikkiWines]

Adding design so we can get some mocks of how this should ideally look 🙇

[melvin-bot]

Triggered auto assignment to @shawnborton (Design), see these Stack Overflow questions for more details.

[shawnborton]

Note 1: that a report could be in two states simultaneously: the receipt could be scanning and the transaction(s) on the report could be on hold

My understanding was that this isn't possible for a one-expense report: you can't hold a scanning receipt. Is that right @trjExpensify @JmillsExpensify ?

[trjExpensify]

Doesn't look like we block the Hold request action on a scanning receipt today, so we double up:

.. but I think it makes sense to not allow it, because we're going to apply the "pending" pattern to a scanning receipt anyway and it can't be approved/paid. It'll be moved to another report if that action is taken.

[shawnborton]

I believe @dragnoir is in the process of fixing that. @dragnoir can you provide an update?

[shawnborton]

In terms of the banner style, I posted to Slack but I think it might look something like this:

[JmillsExpensify]

Can't think of a reason why we'd block holding a scanning receipt, though I'm not super passionate.

[shawnborton]

Ah, you said the opposite over here:

[dragnoir]

I believe @dragnoir is in the process of fixing that. @dragnoir can you provide an update?

Yes, we are working on an update where you can't HOLD a scanning request. The option to HOLD on the 3 dots menu will no be available.

[shawnborton]

What's the latest here?

[dragnoir]

Once this PR is merged #40101 then I will finish the PR of this issue #38471

[shawnborton]

We just need to get the PR here reviewed and merged it seems.

[trjExpensify]

Did this part of the OP not get worked on?

Note 2: One-transaction reports currently do not show an option to hold an expense, but this is easily rectified by including the logic from here in src/components/MoneyReportHeader.tsx

[trjExpensify]

Bump @dragnoir ^^

[trjExpensify]

I'm going to move this into [One expense reports] to track for August!

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

⚠️ 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]

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

• Feature: Hold expense in one transaction view v2 #43618

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

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

• @tienifr requires payment automatic offer (Contributor)
• @dukenv0307 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:

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

[dukenv0307]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: N/A
• 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. Yes
• 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:

1. Submit an expense (make sure it's the only expense in the expense report)
2. Press the report preview to open one transaction view
3. Put the expense on hold
4. Verify the hold education interstitial appears
5. Verify the hold message and reason appear in the expense report
6. Verify Hold banner appears in report header
7. Unhold the expense
8. Verify the unhold message appears
9. Verify Hold banner disappears in report header
10. Verify Submit button appears in both report preview and report header

Do we 👍 or 👎

[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: extra padding below money report header #44545

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

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

• @tienifr requires payment automatic offer (Contributor)
• @dukenv0307 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:

• [@dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
• [@dukenv0307] 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:
• [@dukenv0307] 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:
• [@dukenv0307] Determine if we should create a regression test for this bug.
• [@dukenv0307] 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.
• [@JmillsExpensify] 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: extra padding below money report header #44545

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

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

• @tienifr requires payment automatic offer (Contributor)
• @dukenv0307 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:

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

[JmillsExpensify]

@alexpensify are we paying this PR via your issue or this one?

[alexpensify]

I started the payment process in my GH. We can combine the two since it was one PR.

[JmillsExpensify]

Ok cool, so then I'll just close this one out. Someone please comment if you disagree.