[HOLD for payment 2024-12-19] [Search v1.2] - Different approval behavior when approving one held expense vs multiple held expenses

[IuliiaHerets]

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.66-0
Reproducible in staging?: Y
Reproducible in production?: N/A - new feature, doesn't exist in prod
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

Precondition:

• Add approvals feature is enabled.
• Admin is the approver.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Submit an expense and hold the expense.
4. Go to Search.
5. Click Approve button on the expense.
6. Note that it opens expense RHP and the approval can only be done from the expense RHP.
7. Go back to workspace chat.
8. Submit another expense (so there are two expenses in the same report, with one held and the other unheld).
9. Go to Search > Outstanding.
10. Click Approve.

Expected Result:

App should open expense report RHP so that approval can be done by approving the pending amount or all amount.

Actual Result:

Expense report with held expense can be approved without the confirmation modal in Search.

There is inconsistency when approving only one held expense and approving two expenses with different held status.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6674042_1732360026672.20241123_190310.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @trjExpensify

[melvin-bot]

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

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

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

[gijoe0295]

Proposal

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

Expense report with held expense can be approved without the confirmation modal in Search.

What is the root cause of that problem?

We're checking if the report has any held transactions to open the report RHP:

App/src/libs/actions/Search.ts

Lines 60 to 64 in a266c4c

	const hasHeldExpense = ReportUtils.hasHeldExpenses('', allReportTransactions);
	if (hasHeldExpense) {
	goToItem();
	return;
	}

However, allReportTransactions are not computed correctly:

App/src/libs/actions/Search.ts

Lines 52 to 56 in a266c4c

	const allReportTransactions = (
	isReportListItemType(item)
	? Object.entries(data)
	.filter(([itemKey, value]) => itemKey.startsWith(ONYXKEYS.COLLECTION.REPORT) && (value as SearchTransaction)?.reportID === item.reportID)
	.map((report) => report[1])

The above logic does not get the transactions from the report item but just get the report data itself.

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

For item of type ReportListItem, we already include all its transactions inside item.transactions.

const allReportTransactions = isReportListItemType(item) ? item.transactions : [data[`${ONYXKEYS.COLLECTION.TRANSACTION}${item.transactionID}`]];

[allgandalf]

@gijoe0295 were you able to find the offending PR ?

[mountiny]

cc @luacmartins can you have a look please

I dont think this is a blocker as the feature works its just a different UX to discuss

[luacmartins]

I agree it doesn't need to be a blocker. I'll take a look at it since my PR introduced this feature

[trjExpensify]

Just so I'm following along:

• If you click Approve on a single-expense report when the expense is held, it opens the RHP.
• If you click Approve on a multi-expense report when one of the expenses is held, it approves the report not opens the RHP.

So the second one is the bug, right?

[luacmartins]

Yea, that second one should also open the RHP

[melvin-bot]

@trjExpensify, @luacmartins Huh... This is 4 days overdue. Who can take care of this?

[luacmartins]

Working on it this week

[luacmartins]

PR in review

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

• Correctly filter report transactions #53613

If no regressions arise, payment will be issued on 2024-12-19. 🎊

[melvin-bot]

@luacmartins @trjExpensify @luacmartins The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]