Preserve openReport's shouldRetry:false opt-out for deep links - #98772
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
|
@mkhutornyi 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: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
Looks good. Doing some more regression tests |
|
Codex Review: Didn't find any major issues. Chef's kiss. 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". |
Review — LGTM ✅The fix is correct and minimal. Changing Scope check: I looked for other WRITE callers that pass Tests: Both new tests in Minor, non-blocking
|
Regression scan — no regressions found ✅The Blast-radius check. Four param types declare
Within
No other injection points. A full Live smoke test (web). Opened the Concierge report from the LHN — header, message body, and composer all rendered with no error banners or stuck spinners, confirming the high-frequency Residual risk (low, non-blocking)The only way this could bite is a future/other WRITE caller that passes EvidenceInbox LHN showing single Expensify Concierge chat next to opened reportConcierge report open with header, welcome message, and composer visible |
mountiny
left a comment
There was a problem hiding this comment.
Thanks, looks good to me
|
🚧 mountiny 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/mountiny in version: 9.4.57-0 🚀
|
|
🤖 No help site changes required. This PR is a purely internal fix to the API request layer. It changes
Since no changes are required, I did not create a draft docs PR. |
|
🚀 Deployed to production by https://github.com/Beamanator in version: 9.4.57-3 🚀
Bundle Size Analysis (Sentry): |


Explanation of Change
prepareRequestinmakeRequest.tsunconditionally setshouldRetry = trueon every write request, overwritingopenReport's explicitshouldRetry: falsefor the deep-link (unauthenticated public room preview) case. Changed to??=so an explicitfalseset by the caller is preserved. Added unit tests covering both the default-true and preserved-false cases.Fixed Issues
$ #98767
PROPOSAL: N/A
Tests
#announceroom OR create a public room in the workspace.https://dev.new.expensify.com:8082/r/<reportID>).OpenReport, and inspect the firstOpenReportrequest fired on page load.shouldRetryfield isfalse.mainorstaging(without this fix) to confirmshouldRetryshowstruethere instead.Offline tests
N/A
QA Steps
Same as tests, but using the staging environment (compare to prod)
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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: mWeb Safari
MacOS: Chrome / Safari
Web.mov