[Performance] Resolve destinationReportID before confirmation mount in global-create - #94459
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
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.
|
|
@codex review |
destinationReportID before confirmation mount in global-createdestinationReportID before confirmation mount in global-create
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13dbc80987
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f7329220f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc96f1cdce
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15318823b4
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15318823b4
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safarichrome.mp4 |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.21-2 🚀
|
|
Is applause.expensifail.com domain is beta enabled?
|
|
You can easily check that, try to create pretty much any expense via FAB (but not QAB) - if you see amount as a separate full RHP page then the answer is no. If you fill the amount on the same page as other details (category, merchant) then yes, beta is enabled. |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.21-9 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Fix single-screen global-create flows (workspace expenses, self-DM tracking) where
transaction.reportIDwas undefined at Confirmation mount time, causing the pre-insert effect to miss its window. ExtractedresolveEarlyReportIDinuseResetIOUTypeto settransaction.reportIDimmediately afterinitMoneyRequest. Also resethasPreInsertFiredin the pre-insert cleanup so it can re-fire whendestinationReportIDtransitions from undefined to a valid value.Sentry Evidence
Note: This bug only affects the
NEW_MANUAL_EXPENSE_FLOWbeta (single-screen amount + confirmation). The old multi-step flow is unaffected.This is a regression introduced by the single-screen manual flow change (
IOURequestStartPagenow rendersIOURequestStepConfirmationdirectly on the Manual tab). The old multi-step flow usedAmountSubmission.tswhich calledsetTransactionReportbefore navigation to confirmation:With the single-screen flow, this call is bypassed entirely. The embedded confirmation screen resolves default participants via
useDefaultParticipantsbut never callssetTransactionReportfor workspace participants. This causesdestinationReportIDto be undefined at first render, the pre-insert effect fires with no destination, and subsequentsetTransactionReporttriggers a cleanup that removes the pre-insert without replacement (becausehasPreInsertFiredstays true).The impact contributes to the ~56% DEFAULT handler rate observed in Sentry (5.2k default vs 4k non-default in 24h).
Fixed Issues
$ #94462
PROPOSAL: N/A
Tests
Prerequisites: Enable the
NEW_MANUAL_EXPENSE_FLOWbeta. Use a narrow viewport (mobile device or resize browser to mobile width) - the pre-insert optimization this PR fixes only applies on narrow layout.[Sentry][ManualSubmitToDestinationVisible] Ending spanappears in the console with a sub-two-second duration andfast_path_handleris NOTdefaultOffline tests
N/A
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.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.mov
iOS: mWeb Safari
MacOS: Chrome / Safari
Web.mov