Skip to content

Show the auto-approval limit notice on the expense preview - #99796

Draft
KioCoan wants to merge 4 commits into
Expensify:mainfrom
KioCoan:showOverLimitNoticeOnExpensePreview
Draft

Show the auto-approval limit notice on the expense preview#99796
KioCoan wants to merge 4 commits into
Expensify:mainfrom
KioCoan:showOverLimitNoticeOnExpensePreview

Conversation

@KioCoan

@KioCoan KioCoan commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Follow-up to Show the auto-approval limit notice when the submitter is also the approver, as requested in review.

That PR let a submitter who is also the report's approver see the overAutoApprovalLimit notice, so they finally learn why their report was held for manual approval. It landed on the expense detail Amount row and in Search, but not on the expense preview card, which is what this issue reports.

The preview is the only surface that gates on the showInReview flag that comes down with the violation. createTransactionPreviewConditionals asks hasNoticeTypeViolation(..., showInReview = true), which counts a notice only when the payload literally carries showInReview: true. The backend never sets it for this viewer: it decides that flag from whether the requester owns the report, and the violations query does not look at managerID, so it cannot tell that this submitter is also the approver. I confirmed the payload on a live held report, and it arrives with no showInReview key at all:

{"name": "overAutoApprovalLimit", "type": "notice", "data": {"amount": 10000, "currency": "USD", "formattedLimit": "$100"}}

The expense detail (useViolations) and the Search transaction row apply no showInReview filter, which is why they show it today and the preview does not.

Rather than change that flag in the backend, this adds a narrow exception on the client, next to the distance MODIFIED_AMOUNT exception that already sits in the same two functions. shouldShowViolation is the single rule for who should see this notice, so the new helper asks it directly. It re-applies that rule rather than trusting the incoming array, because TransactionPreview passes violations already filtered by useTransactionViolations while MoneyRequestReportTransactionList passes raw Onyx violations, where a bare name check would light up the row for a plain submitter who must not see it.

The helper is called from both createTransactionPreviewConditionals and transactionHasRBR, which the file documents as mirrors of each other, so the report transaction list keeps sorting RBR rows the same way the preview flags them.

The preview shows the dot and "Review required" rather than the full sentence. "Expense exceeds auto-approval limit of $100.00" is 46 characters against the 40 character RBR_MESSAGE_MAX_CHARACTERS_FOR_PREVIEW cap, so it takes the same reviewRequired fallback every other long violation message takes on a preview card. The full sentence still shows on the expense detail and in Search.

Fixed Issues

$#99613

Tests

Unit tests

  1. Added four cases to tests/unit/TransactionPreviewUtils.test.ts, two for createTransactionPreviewConditionals and two for transactionHasRBR, using an overAutoApprovalLimit notice with no showInReview key, which is the shape the backend actually sends. Every violation fixture already in that file stamps showInReview: true, which is why this gap was never caught.
  2. Ran them against main first and confirmed the two positive cases fail, so the tests prove the fix rather than the fixture.
  3. Ran npx jest tests/unit/TransactionPreviewUtils.test.ts tests/unit/TransactionUtilsTest.ts tests/unit/useTransactionViolationsTest.ts tests/ui/TransactionPreviewContentTest.tsx tests/ui/MoneyRequestReportPreview.test.tsx and all pass. The TransactionUtilsTest and useTransactionViolationsTest runs guard the shouldShowViolation behavior from PR 99016.

Browser, against a local backend

Set up a workspace where I am the admin and submit to myself, with auto-approval enabled and a limit of $100, then created four reports on it: $175 held, $101 held, $150 approved and $99 auto-approved.

  1. With the change stashed, opened the workspace chat and confirmed the baseline: the held $175 card shows no dot and no message.
  2. Restored the change and confirmed exactly two cards now show the dot and "Review required", the two held over-limit reports, while both approved reports stay clean. That is the isProcessingReport half of the rule holding.
  3. Opened the $175 expense and confirmed the Amount row still shows the full "Expense exceeds auto-approval limit of $100.00" notice, so the detail view is unchanged.
  4. Ran an A/B/A control from the console: Onyx.merge('report_<reportID>', {managerID: <some other accountID>}) makes the dot and message disappear and flips the card button from Approve to View, and merging the real approver back restores both. That is the isReportManager half of the rule holding, without needing a second account.
  • Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

  1. Create a new workspace and make sure it is on the Control plan, since these approval rules only exist there.
  2. Go to Workspace settings > More features and enable both Workflows and Rules.
  3. Go to Workspace settings > Workflows and turn on Add approvals. Leave yourself as the approver, so you are both the submitter and the approver.
  4. Go to Workspace settings > Rules > Advanced. Leave Prevent self-approvals off, turn on Auto-approve compliant reports, and set Auto-approve reports with all expenses under to $100. If Random report audit is present, set it to 0%.

Every expense below needs a merchant and a category, otherwise a missing field violation blocks auto-approval on its own and hides the behavior being tested.

  1. Open the workspace chat, create an expense of $175 with a merchant and a category, and submit it.
  2. Verify the report is held for manual approval: the preview card shows the Outstanding badge and an Approve button.
  3. Verify the preview card now shows a red dot and "Review required" on the line under the merchant. This is the fix. The message reads "Review required" rather than the full sentence because the full sentence is longer than a preview card line allows, which is how every long violation message behaves on these cards.
  4. Open the expense from that card and verify the Amount row shows the full notice, "Expense exceeds auto-approval limit of $100.00".
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

Before: the $175 expense is correctly held for manual approval and the card offers an Approve button, but nothing on it says why it was not auto-approved. The approved $150 report above it is the control.
99613-before-full

After: the same screen, same scroll position. The held report now explains itself with the dot and "Review required", and the approved report above is untouched, so the change does not leak outside reports awaiting approval.

99613-after-full

Control, a different approver: the same held report with the approver set to someone else. No dot, no message, and the button reads View rather than Approve. This is the behavior we are deliberately preserving, since a submitter who cannot approve the report has nothing to act on.

99613-control-other-approver-full

@KioCoan

KioCoan commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@arosiclair do we need a C+ to review this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant