[HOLD #55844] [$250] Expenses - RBR is missing in expense preview for a preview with a held and not held expenses

[izarutskaya]

Held on #55844 (comment)

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.85-4
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
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): N/A
Issue reported by: Applause Internal Team
Device used: Windows 11/ Chrome, Android 13/ Chrome
App Component: Money Requests

Action Performed:

1. Sign in to staging.new.expensify.com
2. Navigate to a conversation
3. Send 2 or more expenses in the conversation
4. Navigate to the expenses sent in step 3
5. From the expenses sent in step 3, hold one of the expenses submitted
6. Navigate to the expense preview

Expected Result:

RBR should be displayed in expense preview with a held expense in it.

Actual Result:

RBR is missing from expense preview with a held expense in it.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6714568_1736930629853.bandicam_2025-01-15_11-28-56-495.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021881910594973100808
• Upwork Job ID: 1881910594973100808
• Last Price Increase: 2025-01-22
• Automatic offers:
• brunovjk | Reviewer | 105808991
• thelullabyy | Contributor | 105808995

Issue Owner

Current Issue Owner: @cristipaval

[melvin-bot]

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

[github-actions]

⚠️ Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format.

[thelullabyy]

Proposal

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

RBR is missing from the expense preview with a held expense in it.

What is the root cause of that problem?

When users hold a money request, we don't set a value for showInReview field in transaction violations

App/src/libs/TransactionUtils/index.ts

Lines 934 to 936 in 16ad2bc

	function hasViolation(transactionID: string | undefined, transactionViolations: OnyxCollection<TransactionViolations>, showInReview?: boolean): boolean {
	return !!transactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + transactionID]?.some(
	(violation: TransactionViolation) => violation.type === CONST.VIOLATION_TYPES.VIOLATION && (showInReview === undefined || showInReview === (violation.showInReview ?? false)),

So hasViolation returned false and the RBR isn't displayed on the report preview

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

App/src/libs/actions/IOU.ts

Line 8434 in 16ad2bc

const newViolation = {name: CONST.VIOLATIONS.HOLD, type: CONST.VIOLATION_TYPES.VIOLATION};

When holding a request, in optimistic data we need to add showInReview field is undefined (or true) to the transaction violation

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

In the UTs for putOnHold function, we need to a case to verify that after we call putOnHold, ReportUtils.hasViolations function will return true with new transactionViolations from Onyx

What alternative solutions did you explore? (Optional)

[Kalydosos]

🚨 Edited by proposal-police: This proposal was edited at 2025-01-15 19:35:33 UTC.

Proposal

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

RBR is missing in expense preview for a preview with a held and not held expenses

What is the root cause of that problem?

When putting an expense on hold, the FE creates a transaction violation but does not set the attribute showInReview to true

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

As in ReportPreview.tsx, we need transactions violations which attribute showInReview are setted to true, we must add showInReview : true to the transaction violation created when putting an expense on hold. That will change the following line from

App/src/libs/actions/IOU.ts

Line 8424 in 0617354

const newViolation = {name: CONST.VIOLATIONS.HOLD, type: CONST.VIOLATION_TYPES.VIOLATION};

to

const newViolation = {name: CONST.VIOLATIONS.HOLD, type: CONST.VIOLATION_TYPES.VIOLATION, showInReview: true};

RESULT

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

Given a held expense transaction
When TransactionUtils.hasViolation is called for such transaction with the parameter showInReview set to true
Then the result must always be true

What alternative solutions did you explore? (Optional)

None

[melvin-bot]

@anmurali Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[melvin-bot]

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

[brunovjk]

Thank you all for the proposal, both seem correct to me and similar, I believe we can go with the first proposal from @thelullabyy. Thanks.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

Offer link
Upwork job

[melvin-bot]

📣 @thelullabyy 🎉 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 📖

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

[mallenexpensify]

We haven't covered this #55844 yet. Maybe we should wait for payment here? Thanks.

I removed 'hold for payment' here. Please comment when payment's due, thx

[melvin-bot]

@anmurali, @cristipaval, @brunovjk, @thelullabyy 12 days overdue. Walking. Toward. The. Light...

[brunovjk]

Update: Still holding for #55844 (comment)

[cristipaval]

Update:

I just took over the internal issue that we're holding on

[melvin-bot]

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

[cristipaval]

Making it Weekly as the issue that this one is held on

[brunovjk]

Not overdue, still holding for #55844

[cristipaval]

still on hold