[HOLD for payment 2024-07-24] [$250] Submit expense - "Hold" option is not present in report details page

[lanitochka17]

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: 9.0.2-0
Reproducible in staging?: Y
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to a workspace chat with no prior expense submissions and click the plus sign
2. Submit an expense by adding the amount, merchant, and confirming it
3. Go to IOU, click the headers, and notice that the 'Hold' option is not available

Expected Result:

The "Hold" option should be present on the report details page

Actual Result:

After submitting a single expense in the workspace chat, the 'Hold' option appears in IOU when clicking the three dots. However, the "Hold" option is not present on the report details 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

Bug6525289_1719412383105.Screen_Recording_2024-06-26_at_5.13.22_AM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0166273ab3240af3ae
• Upwork Job ID: 1806101011812532362
• Last Price Increase: 2024-07-03
• Automatic offers:
• brunovjk | Contributor | 102893192
• nkdengineer | Contributor | 102991523

Issue Owner

Current Issue Owner: @sakluger

[melvin-bot]

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

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

[lanitochka17]

@aldo-expensify 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

[nyomanjyotisa]

Proposal

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

"Hold" option is not present in report details page

What is the root cause of that problem?

App/src/libs/ReportUtils.ts

Line 2749 in eb83809

function canHoldUnholdReportAction(reportAction: OnyxInputOrEntry<ReportAction>): {canHoldRequest: boolean; canUnholdRequest: boolean} {

On this code, if the current action is not MoneyRequestAction, it directly return return {canHoldRequest: false, canUnholdRequest: false};

But in this case, the current actionName is "REPORTPREVIEW", its childReportName is "IOU", and childMoneyRequestCount is 1

It need to show "Hold" in this case

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

https://github.com/Expensify/App/blob/eb83809486784265a4aba70a9ed0436dd3f29bbe/src/libs/ReportUtils.ts#L2749C10-L2760
update this canHoldUnholdReportAction function to not return return {canHoldRequest: false, canUnholdRequest: false}; if actionName is "REPORTPREVIEW", childReportName is "IOU", childMoneyRequestCount is 1. Then ensure it return either canHoldRequest/canUnholdRequest : true in this case

What alternative solutions did you explore? (Optional)

[melvin-bot]

Triggered auto assignment to @MitchExpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Triggered auto assignment to Design team member for new feature review - @dannymcclain (NewFeature)

[aldo-expensify]

From looking at the code:

App/src/pages/ReportDetailsPage.tsx

Lines 628 to 639 in 64c6624

	<ConfirmModal
	title={caseID === CASES.DEFAULT ? translate('task.deleteTask') : translate('iou.deleteExpense')}
	isVisible={isDeleteModalVisible}
	onConfirm={deleteTransaction}
	onCancel={() => setIsDeleteModalVisible(false)}
	onModalHide={() => ReportUtils.navigateBackAfterDeleteTransaction(navigateBackToAfterDelete.current)}
	prompt={caseID === CASES.DEFAULT ? translate('task.deleteConfirmation') : translate('iou.deleteConfirmation')}
	confirmText={translate('common.delete')}
	cancelText={translate('common.cancel')}
	danger
	shouldEnableNewFocusManagement
	/>

I don't think this was ever implemented and it doesn't seem to be part of the initial designs (original issue: #31300).

@robertjchen do you think we want to add this hold button in the report details?

Or should we just close this?

[aldo-expensify]

I tested this proposal and it doesn't seem to help... the HOLD button doesn't appear in the report details because the code is completely missing from there I think

[dannymcclain]

I think the Hold button should show up in the row of buttons near the top. This looks like another gotcha of the one-expense report. cc @JmillsExpensify

Here's a mock for the expense details showing the hold button:

[ShridharGoel]

Proposal

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

Submit expense - "Hold" option is not present in report details page.

What is the root cause of that problem?

This happens because shouldShowHoldAction in ReportDetailsPage becomes false because:

• There's a MONEY_REPORT check which becomes false.
• canHoldRequest and canUnholdRequest become false.

App/src/pages/ReportDetailsPage.tsx

Lines 447 to 448 in e24cf4d

	const shouldShowHoldAction =
	caseID !== CASES.MONEY_REPORT && (canHoldUnholdReportAction.canHoldRequest || canHoldUnholdReportAction.canUnholdRequest) && !ReportUtils.isArchivedRoom(parentReport);

Why do canHoldRequest and canUnholdRequest show as false?

This is because !ReportActionsUtils.isMoneyRequestAction(reportAction) becomes true which leads to an early return with {canHoldRequest: false, canUnholdRequest: false}

isMoneyRequestAction becomes true because this is a report preview action.

Also, moneyRequestReportID will always be 0 for report preview since it is using IOUReportID.

App/src/libs/ReportUtils.ts

Lines 2750 to 2759 in e24cf4d

	if (!ReportActionsUtils.isMoneyRequestAction(reportAction)) {
	return {canHoldRequest: false, canUnholdRequest: false};
	}
	const moneyRequestReportID = ReportActionsUtils.getOriginalMessage(reportAction)?.IOUReportID ?? 0;
	const moneyRequestReport = getReportOrDraftReport(String(moneyRequestReportID));
	if (!moneyRequestReportID || !moneyRequestReport) {
	return {canHoldRequest: false, canUnholdRequest: false};
	}

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

Add a report preview action check and update the moneyRequestReportID logic:

const isReportPreviewAction = ReportActionsUtils.isReportPreviewAction(reportAction)

if (!ReportActionsUtils.isMoneyRequestAction(reportAction) && !isReportPreviewAction) {
    return {canHoldRequest: false, canUnholdRequest: false};
}

const moneyRequestReportID = isReportPreviewAction ? getIOUReportIDFromReportActionPreview(reportAction) : (ReportActionsUtils.getOriginalMessage(reportAction)?.IOUReportID ?? 0);
const moneyRequestReport = getReportOrDraftReport(String(moneyRequestReportID));

if (!moneyRequestReportID || !moneyRequestReport) {
    return {canHoldRequest: false, canUnholdRequest: false};
}

Note that getIOUReportIDFromReportActionPreview is being used for report previews.

Then, reimbursable field is not available because of which canHoldRequest and canUnholdRequest will be false below:

App/src/libs/ReportUtils.ts

Lines 2784 to 2785 in e24cf4d

	const canHoldRequest = canHoldOrUnholdRequest && !isOnHold && (isRequestHoldCreator || (!isRequestIOU && canModifyStatus)) && !isScanning && !!transaction?.reimbursable;
	const canUnholdRequest = !!(canHoldOrUnholdRequest && isOnHold && (isRequestHoldCreator || (!isRequestIOU && canModifyStatus))) && !!transaction?.reimbursable;

We can either remove the reimbursable check or ensure that it is present in the backend data. Or, we can make it a conditional check only for non-preview actions.

Next, we need to remove the MONEY_REPORT check from shouldShowHoldAction in the details page (or skip it only for previews).

[melvin-bot]

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[nkdengineer]

@brunovjk The PR is here.

[melvin-bot]

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

[MitchExpensify]

Reassigning while I'm leave (PR is on staging) 🙇

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

• Add hold option to report detail page for combine report #44880

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

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

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

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

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: N/A
• [@brunovjk] 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
• [@brunovjk] 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
• [@brunovjk] Determine if we should create a regression test for this bug. Yes
• [@brunovjk] 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 Proposal

1. Navigate to a workspace chat with no prior expense submissions.
2. Click the plus sign to submit an expense, adding the amount and merchant, and confirm.
3. Go to IOU, click the headers, and observe the options.
4. Click on the report preview to open the combined report.
5. Click on the header to open the report detail page.
6. Verify that the "Hold" option is present on the report details page.

Do we agree 👍 or 👎

[sakluger]

@brunovjk you don't think that we should add a regression test for this one?

[brunovjk]

@sakluger I thought no, but what do you think?

What do you think of these tests @sakluger?

@nkdengineer @marcochavezf

[sakluger]

Yeah, I think we want to include this in our regression tests. Those steps look good!

[sakluger]

Summarizing payment on this issue:

Contributor: @nkdengineer $250, paid via Upwork
Contributor+: @brunovjk $250, paid via Upwork