Repository navigation
[$250] Expenses - RBR is missing in expense preview for a preview with a held and not held expenses #55263
Description
Activity
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.⚠️ Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format.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
showInReviewfield in transaction violations
App/src/libs/TransactionUtils/index.ts
Lines 934 to 936 in 16ad2bc
So
hasViolationreturned false and the RBR isn't displayed on the report previewWhat changes do you think we should make in order to solve the problem?
Line 8434 in 16ad2bc
When holding a request, in optimistic data we need to add
showInReviewfield is undefined (or true) to the transaction violationWhat specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
In the UTs for
putOnHoldfunction, we need to a case to verify that after we callputOnHold,ReportUtils.hasViolationsfunction will return true with new transactionViolations from OnyxWhat alternative solutions did you explore? (Optional)
🚨 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
Line 8424 in 0617354
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 trueWhat alternative solutions did you explore? (Optional)
None
@anmurali Eep! 4 days overdue now. Issues have feelings too...
- changed the title
[-]Expenses - RBR is missing in expense preview for a preview with a held and not held expenses[/-][+][$250] Expenses - RBR is missing in expense preview for a preview with a held and not held expenses[/+]on Jan 22, 2025 Job added to Upwork: https://www.upwork.com/jobs/~021881910594973100808
Triggered auto assignment to Contributor-plus team member for initial proposal review - @brunovjk (
External)74 remaining items
Update: Not overdue, still holding for #55844 (comment)
#55844 is almost done. Waiting to get the backend PRs deployed
Reacted by isabelastisserReacted by Bruno RochaSummary:
We originally fixed this same issue with this PR, but after resolving it we noticed another issue, where the same one was resolved by changes in the backend by @cristipaval.
Next steps
- Confirm that both issues are no longer reproducible.
- Create regression tests (I will do this)
- Follow up with payment regarding PR in the frontend
- Close both issues
@cristipaval, does that make sense to you? Thanks.
@brunovjk I don't think we need
Confirm that both issues are no longer reproducible.
since you're creating regression tests. This behavior will be tested on each App version from now on.
Other than this, great summary, thank you!
Reacted by Bruno Rocha- changed the title
[-][HOLD #55844] [$250] Expenses - RBR is missing in expense preview for a preview with a held and not held expenses[/-][+][$250] Expenses - RBR is missing in expense preview for a preview with a held and not held expenses[/+]on Apr 22, 2025 Not overdue.
BugZero Checklist:
- [Contributor] Classify the bug:
Bug classification
Source of bug:
- 1a. Result of the original design (eg. a case wasn't considered)
- 1b. Mistake during implementation
- 1c. Backend bug
- 1z. Other:
Where bug was reported:
- 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
- 2b. Reported on staging (eg. found during regression or PR testing)
- 2d. Reported on a PR
- 2z. Other:
Who reported the bug:
- 3a. Expensify user
- 3b. Expensify employee
- 3c. Contributor
- 3d. QA
- 3z. Other:
-
[Contributor] 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: No PR directly introduced this bug.
-
[Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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:
-
[Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
Regression Test Proposal
Test:
- Sign in to ND.
- Navigate to a conversation.
- Send 2 or more expenses in the conversation.
- Navigate to the expenses sent in step 3.
- From the expenses sent in step 3, hold one of the expenses submitted.
- Navigate to the expense preview.
- Verify: RBR is displayed in expense preview with a held expense in it.
- Click on the expense preview.
- Go back to the main chat.
- RBR on the expense preview in the main chat should persist after visiting expense preview and returning to main chat.
Do we agree 👍 or 👎
@isabelastisser after submitting the regression test above, we should proceed with payment to me (@brunovjk) and @thelullabyy for the implementation of this PR. Then we can close this issue :D Thanks.
Reacted by isabelastisserThe payments were made in Upwork. All set!
Reacted by Bruno Rocha- moved this from Bugs and Follow Up Issues to Done in #expensify-bugs
on Apr 23, 2025
Metadata
Metadata
Labels
Type
Projects
- Status
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:
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:
Screenshots/Videos
Bug6714568_1736930629853.bandicam_2025-01-15_11-28-56-495.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @brunovjk