Align receipt view scan failure violation with transaction row#89277
Align receipt view scan failure violation with transaction row#89277
Conversation
…nsistency Replace canEditMoneyRequest with wasActionTakenByCurrentUser when determining whether to show "Enter details manually" in violation messages on the receipt view. This aligns the receipt view behavior with the transaction row, ensuring non-submitters see a consistent message without the misleading edit prompt. Co-authored-by: dmkt9 <dmkt9@users.noreply.github.com>
|
@MelvinBot update your checklist:
|
|
Updated the PR checklist:
|
|
@MelvinBot update your checklist: |
|
Updated the Tests and Offline Tests sections to: "Same as the QA steps." |
|
@MelvinBot mark this PR as ready |
|
Marked this PR as ready for review. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari2026-04-30.22-17-36.mp4 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5d2347dc7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| violation, | ||
| translate, | ||
| canEdit, | ||
| canEdit: isActionTakenByCurrentUser, |
There was a problem hiding this comment.
Respect readonly mode in SmartScan failure message
Passing canEdit: isActionTakenByCurrentUser drops the prior editability checks (readonly/write permission), so the SmartScan violation can now tell submitters to “Enter details manually” even when this view is intentionally non-editable. This is reproducible in readonly render paths (e.g., duplicate/merge confirmation screens) where MoneyRequestReceiptView is shown with edits disabled, making the message inaccurate and misleading.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is a false positive. The canEdit parameter in getViolationTranslation doesn't control actual editability — it only determines the violation message text (whether to append "Enter details manually.").
The same wasActionTakenByCurrentUser-only pattern is already used everywhere else this parameter is set:
TransactionItemRowRBR.tsx:70SimpleMessageContent.tsx:99OptionsListUtils/index.ts:823ReportNameUtils.ts:481
The whole point of this PR is to align the receipt view with these existing surfaces so the message is consistent.
| violation, | ||
| translate, | ||
| canEdit, | ||
| canEdit: isActionTakenByCurrentUser, |
There was a problem hiding this comment.
| canEdit: isActionTakenByCurrentUser, | |
| canEdit: isActionTakenByCurrentUser && isEditable, |
There was a problem hiding this comment.
Applied — now also gating on isEditable so the "Enter details manually" prompt won't show in readonly render paths.
Co-authored-by: dmkt9 <dmkt9@users.noreply.github.com>
Co-authored-by: dmkt9 <dmkt9@users.noreply.github.com>
|
Fixed the failing React Compiler Compliance check: added missing |
|
@MelvinBot use the correct PR checklist from https://raw.githubusercontent.com/Expensify/App/main/.github/PULL_REQUEST_TEMPLATE.md to fix the PR author checklist failure |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
Updated the PR Author Checklist to match the current PR template. Added all missing checklist items (Spanish translation, new code patterns, DRY check, constants, deeplinks, Storybook, CSS styles, assets, etc.) and the Screenshots/Videos section. All checkboxes are checked. |
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Fix makes sense 👍
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.67-0 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR and found one help site article that needs updating: Affected article: The article previously stated the scan failure message is always "Receipt scanning failed. Enter details manually." This PR changes the behavior so that only the expense submitter sees "Enter details manually" — non-submitters (admins, approvers) now see just "Receipt scanning failed." Draft PR created: #89643 The |
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.3.67-13 🚀
|
Explanation of Change
Follow-up to #86896. The receipt view in
MoneyRequestReceiptViewwas usingcanEditMoneyRequestto determine thecanEditparameter passed togetViolationTranslation, while the transaction row useswasActionTakenByCurrentUser. This caused an inconsistency where the "Receipt scanning failed" violation message could differ between the transaction row and the receipt view on the admin side.This PR replaces the
canEditMoneyRequestcheck withwasActionTakenByCurrentUserin the receipt view, aligning both surfaces so non-submitters consistently see "Receipt scanning failed." without the misleading "Enter details manually." prompt.Fixed Issues
$ #86574
Tests
Same as the QA steps.
Offline tests
Same as the QA steps.
QA Steps
Prerequisite: User A is a member of a workspace where User B has an admin role.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
AI Tests
npm run typecheck-tsgonpm run lint-changednpx prettier --check