The user is redirected to the WS room, not to the report details#76827
The user is redirected to the WS room, not to the report details#76827stitesExpensify merged 5 commits intoExpensify:mainfrom
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@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] |
Reviewer Checklist
Screenshots/VideosAndroid: mWeb Chromeandroid-mweb.mp4iOS: mWeb Safariios-mweb.movMacOS: Chrome / Safariweb.mov |
|
|
||
| const policyIdReal = getIOURequestPolicyID(transaction, reportReal); | ||
| const transactionReport = getReportOrDraftReport(transaction?.reportID); | ||
| const report = reportReal ?? reportDraft ?? transactionReport; |
There was a problem hiding this comment.
@mkzie2 I noticed 2 potential issues:
- you memoized this below in
IOURequestStepConfirmation.tsxbut didn't memoize it here - you used
transactionReportas 3rd fallback here while not using it inIOURequestStepConfirmation.tsx
mind clarifying / updating (if needed) ?
There was a problem hiding this comment.
@ikevin127 In IOURequestStepConfirmation we used it in 2 places so I created reportWithDraftFallback, actually for report, we still use transactionReport as the fallback if reportWithDraftFallback is undefined
| */ | ||
| const transactionReport = getReportOrDraftReport(transaction?.reportID); | ||
| const reportWithDraftFallback = useMemo(() => reportReal ?? reportDraft, [reportDraft, reportReal]); | ||
| const shouldUseTransactionReport = |
There was a problem hiding this comment.
🟡 NAB: Complex Boolean Condition Readability (Minor)
The condition for shouldUseTransactionReport has become more complex with this change, it now has 3 levels of nesting with both && and || operators, making it difficult to understand at a glance. Per Expensify's STYLE.md, code should be readable and maintainable.
Suggested Fix: Extract conditions into descriptive named variables:
const isTransactionReportValidForUse =
!(isProcessingReport(transactionReport) && !policyReal?.harvesting?.enabled) &&
isReportOutstanding(transactionReport, policyReal?.id, undefined, false);
const hasNoFallbackReport = !reportWithDraftFallback;
const shouldUseTransactionReport =
transactionReport && (isTransactionReportValidForUse || hasNoFallbackReport);| * Also if the report was submitted and delayed submission is on, then we should use an initial report | ||
| */ |
There was a problem hiding this comment.
🟡 Issue: The existing comment at lines 171-174 no longer fully describes the new logic
Why it matters: The comment doesn't explain the new condition !reportWithDraftFallback. This is the key fix in the PR - using transactionReport when neither reportReal nor reportDraft exist.
Suggested Fix:
/*
* We want to use a report from the transaction if it exists
* Also if the report was submitted and delayed submission is on, then we should use an initial report
* Additionally, if neither reportReal nor reportDraft exist, we fallback to the transactionReport
* to ensure proper navigation after expense creation.
*/|
✅ Reviewer checklist completed. 🔄 Awaiting on comments to be addressed before approving. |
|
@ikevin127 Comments resolved. |
ikevin127
left a comment
There was a problem hiding this comment.
🟢 LGTM - Thanks for the updates!
|
✋ 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/stitesExpensify in version: 9.2.75-0 🚀
|
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 9.2.75-0 🚀
|
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 9.2.77-0 🚀
|
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.2.77-1 🚀
|
Explanation of Change
Fixed a bug where after creating the expense, the user is redirected to the WS room, not to the report details
Fixed Issues
$ #76598
PROPOSAL: #76598 (comment)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand 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
Screen.Recording.2025-12-08.at.19.02.24.mov
iOS: Native
iOS: mWeb Safari
Screen.Recording.2025-12-08.at.19.03.51.mov
MacOS: Chrome / Safari
Screen.Recording.2025-12-08.at.19.08.20.mov