Add Explain support for UNREPORTEDTRANSACTION report actions#86565
Add Explain support for UNREPORTEDTRANSACTION report actions#86565
Conversation
Add reasoning field to OriginalMessageUnreportedTransaction type and update UnreportedTransactionAction component to show the inline Explain link when reasoning is present, following the existing pattern from MovedTransactionAction. Co-authored-by: Alex Beaman <Beamanator@users.noreply.github.com>
|
The failing check ( Failing test:
Why it's unrelated:
Recommendation: Re-run the failing job. If it persists, it may warrant a separate investigation. |
|
The PR checklist item "I added unit tests for any new feature or bug fix" is checked, but no tests were actually added.
|
|
@MelvinBot address my comment ^ |
Add two test cases mirroring the existing MOVED_TRANSACTION tests: - Verify no Explain link renders when reasoning is absent - Verify Explain link renders when reasoning is present Co-authored-by: Alex Beaman <Beamanator@users.noreply.github.com>
|
Added unit tests for
All 25 tests in |
|
The failing Check for unused styles check is unrelated to this PR's code changes. It failed during the Analysis: The error occurred 3 times during Please re-run the failed job to resolve this. |
|
Minor test gap: the |
|
@ikevin127 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b226e6ebd6
ℹ️ 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".
| if (hasReasoning(action)) { | ||
| return ( | ||
| <ReportActionItemMessageWithExplain | ||
| message={unreportedTransactionMessage} | ||
| action={action} | ||
| childReport={childReport} | ||
| originalReport={originalReport} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
Preserve pending-delete formatting for explained unreports
When fromReport.pendingFields.preview is DELETE and the action includes reasoning, this early return skips the <del> rendering path entirely. That means the same UNREPORTED_TRANSACTION action is no longer shown as deleted (and keeps normal interactive styling) solely because reasoning is present, which regresses the previous pending-delete behavior for this action type. This is observable for optimistic/unsettled delete states where the source report is pending removal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is known, and is how other report actions with Explain act too.
|
@ikevin127 would you please review and test and let me know if there's anything unclear here? 🙏 |
Reviewer Checklist
Screenshots/VideosTBD |
|
Code-wise this LGTM 🟢 📊 Summary
Overall Recommendation: ✅ APPROVE The PR is ready for merge. It's a clean implementation that follows established patterns, has proper test coverage, and introduces minimal risk. @Beamanator The only issue is I'm not able to actually manually test that BE-triggered Should I be able to test this manually but I'm not doing it right ? If so could you please provide some steps that you used to verify this ? |
Explanation of Change
This PR adds Explain (Concierge reasoning) support for
UNREPORTEDTRANSACTIONreport actions, mirroring the existing pattern fromMovedTransactionAction.When an RTER rejection unreports a transaction, the backend now sends a
reasoningfield on the UNREPORTEDTRANSACTION report action (via Web-Expensify #51341 + Auth #20363). This PR wires up the App frontend to display the inline Explain link when reasoning is present.Changes:
OriginalMessage.ts— Addedreasoning?: stringtoOriginalMessageUnreportedTransaction(matchingOriginalMessageMovedTransaction)UnreportedTransactionAction.tsx— AddedchildReport/originalReportprops and ahasReasoning()check that rendersReportActionItemMessageWithExplainwhen reasoning exists (same pattern asMovedTransactionAction.tsx)PureReportActionItem.tsx— PassedchildReportandoriginalReporttoUnreportedTransactionAction(matching theMOVED_TRANSACTIONcase)Fixed Issues
$ https://github.com/Expensify/Expensify/issues/608207
Tests
Offline tests
No offline impact — the Explain feature only appears when
reasoningis present in the report action data which comes from the server. Offline behavior is unchanged.QA Steps
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
N/A — No visual UI changes; this enables an existing component pattern for a new action type.
Android: mWeb Chrome
N/A
iOS: Native
N/A
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
N/A