[No QA] Harden TransitionTracker callback handling#95481
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
2781a49 to
7f08b9b
Compare
|
@codex review |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
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". |
TransitionTracker callback handlingTransitionTracker callback handling
|
@cretadn22 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] |
|
@MelvinBot review |
ReviewLGTM — clean, well-scoped hardening with good test coverage. No blocking issues. A couple of observations below. The Pusher Minor notes (non-blocking)
I couldn't run the unit tests in this environment, but Codecov reports 100% coverage on |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 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.29-0 🚀
|
|
🤖 No help site changes required. I reviewed the changes in this PR and confirmed no updates are needed to the help site files under WhyAll four changed files are internal engineering with no customer-facing behavior:
There are no new features, UI elements, tab/settings labels, buttons, workspace/billing/integration flows, or any product behavior a user would see or read about. The PR is marked Since no help site changes are required, I did not create a draft docs PR. @JakubKorytko, please confirm you agree no help site updates are needed here. If you believe a user-facing behavior change is involved that should be documented, let me know and I'll draft the help site PR. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.31-0 🚀
|
|
🤖 Help site review: no documentation changes required. I reviewed the changes in this PR and confirmed that no updates are needed to Expensify's help site files under Why: This PR is internal engineering hardening with no user-facing behavior:
None of this changes any product feature, UI, tab, setting, button, or user-facing flow — it's error-isolation plumbing behind navigation transitions and Pusher subscriptions. The help site documents customer-facing product behavior, which is unchanged here (the PR is even marked If you believe a specific help article is affected, let me know which flow and I'll take another look. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.31-0 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Hardened
TransitionTrackercallback execution by sharing guarded sync/async error handling across immediate and queued paths. Added an info log whenwaitForUpcomingTransitiontimes out without a transition start, plus unit coverage for these cases.Fixed Issues
$ #94673
PROPOSAL: N/A
Tests
N/A - this is internal
TransitionTrackerhardening with no direct user-facing flow. Covered by automated unit tests intests/unit/Navigation/TransitionTrackerTest.ts.Offline tests
N/A
QA Steps
N/A - no user-facing behavior change to validate manually on staging.
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: mWeb Safari
MacOS: Chrome / Safari