Skip to content

[Odometer] App stuck on E screen after refresh on confirm odometer page#88702

Merged
Julesssss merged 4 commits intoExpensify:mainfrom
software-mansion-labs:jakubkalinski0/App_stuck_on_E_screen_after_refresh_on_confirm_odometer_page
Apr 27, 2026
Merged

[Odometer] App stuck on E screen after refresh on confirm odometer page#88702
Julesssss merged 4 commits intoExpensify:mainfrom
software-mansion-labs:jakubkalinski0/App_stuck_on_E_screen_after_refresh_on_confirm_odometer_page

Conversation

@jakubkalinski0
Copy link
Copy Markdown
Contributor

@jakubkalinski0 jakubkalinski0 commented Apr 24, 2026

Explanation of Change

This PR fixes a freeze on the odometer confirmation screen after a browser refresh. Two mechanisms were racing to detect expired blob URLs - useRestartOnOdometerImagesFailure and a pre-flight check inside OdometerReceiptStitcher - producing double clearOdometerDraftTransactionState + navigation. The hook now owns recovery and exposes hasVerifiedBlobs and the stitcher waits on that flag. Duplicate pre-flight/redirect was removed.

Fixed Issues

$ #88678
PROPOSAL: N/A

Tests

  1. Open the NewDot app on web
  2. Click the FAB -> Track Distance -> switch to the Odometer tab
  3. Enter a start odometer reading and capture a start odometer image
  4. Enter an end odometer reading and capture an end odometer image
  5. Tap Next to proceed to the confirmation screen and verify the stitched odometer receipt appears
  6. Hard-refresh the browser
  7. Verify that the app redirects you back to the odometer tab with empty readings and no image thumbnails with the page being fully responsive. Verify that the confirmation screen does not stay stuck on the Expensify ("E") loading screen

Regression check - happy path:

  1. Repeat steps 1–5 above without refreshing
  2. Tap Confirm/Create expense and verify the expense submits successfully with the stitched odometer image as the receipt
  • Verify that no errors appear in the JS console

Offline tests

Same as Tests

QA Steps

Same as Tests

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

MacOS: Chrome / Safari
Screen.Recording.2026-04-24.at.13.13.01.mov

@jakubkalinski0
Copy link
Copy Markdown
Contributor Author

@codex review

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
.../useRestartOnOdometerImagesFailure/index.native.ts 100.00% <100.00%> (ø)
...es/iou/request/step/IOURequestStepConfirmation.tsx 67.94% <100.00%> (+0.15%) ⬆️
...uest/step/confirmation/OdometerReceiptStitcher.tsx 30.76% <40.00%> (+8.54%) ⬆️
...c/hooks/useRestartOnOdometerImagesFailure/index.ts 0.00% <0.00%> (ø)
... and 26 files with indirect coverage changes

@jakubkalinski0 jakubkalinski0 marked this pull request as ready for review April 24, 2026 11:15
@jakubkalinski0 jakubkalinski0 requested review from a team as code owners April 24, 2026 11:15
@melvin-bot melvin-bot Bot requested review from DylanDylann and trjExpensify and removed request for a team and trjExpensify April 24, 2026 11:15
@melvin-bot
Copy link
Copy Markdown

melvin-bot Bot commented Apr 24, 2026

@DylanDylann 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]

@melvin-bot melvin-bot Bot removed the request for review from a team April 24, 2026 11:15
Comment thread src/hooks/useRestartOnOdometerImagesFailure/index.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 872259d9df

ℹ️ 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".

Comment thread src/pages/iou/request/step/confirmation/OdometerReceiptStitcher.tsx
@jakubkalinski0
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ 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".

@jakubkalinski0
Copy link
Copy Markdown
Contributor Author

@DylanDylann Fight with codex was won, so it's all yours now

@jakubkalinski0
Copy link
Copy Markdown
Contributor Author

jakubkalinski0 commented Apr 26, 2026

This message is a result of my discussion on Slack with @DylanDylann. Writing it up here so anyone picking this up later has the full arc - a lot of work went into analyzing the situation here as it was pretty confusing. Every time I thought I had figured out the possible root cause, new possible culprits seemed to jump out at me from every corner. I tried ti explain everything as clearly as possible below.

Note

TLDR. Three causes had to align for the freeze to happen:

  1. A race I introduced - duplicate blob-expiration detection paths (stitcher pre-flight + hook) firing on mount.
  2. A timing change from PR #86958 - the decomposition refactor spread previously-batched setter writes across more microtask boundaries.
  3. A latent weakness in react-native-onyx - under the resulting stress pattern (merge -> ... -> set -> merge -> ... on one key), Onyx.set can emit a semantically-incoherent phantom snapshot.

The hasVerifiedBlobs gate proposed in this PR eliminates cause 1, which is the part I directly own. It's empirically verified to resolve the freeze. Recommendation: merge this PR as-is and file an upstream-Onyx issue for cause 3. Cause 2 is not separately actionable - I tried the obvious React-side mitigations and none of them break the loop (there was no issue when I reverted the whole PR though, but I wouldn't recommend doing that).


Part 1 - Initial thought process, and the recommended fix for this deploy blocker

What I initially thought

Two pieces of code on the confirmation screen detect expired blob: URLs after a browser refresh and act on them independently. Both fire on mount, and both write the same cleanup to Onyx.

The two detection paths (pre-fix)
  • useRestartOnOdometerImagesFailure - a hook called from IOURequestStepConfirmation. On mount it runs Promise.all(checkIfLocalFileIsAccessible(...)) against the start image, end image, and stitched receipt. If any of them is expired, it calls clearOdometerDraftTransactionState (4 Onyx.merges + 1 Onyx.set on the backup key) and then navigateToStartMoneyRequestStep to bounce the user back to the start of the odometer flow.
  • OdometerReceiptStitcher's pre-flight block - sitting inside the stitcher's own useEffect, runs the same Promise.all(checkIfLocalFileIsAccessible(...)) over the same blob URLs. On expiration it does the exact same recovery - clearOdometerDraftTransactionState + navigateToStartMoneyRequestStep.

My initial theory was simple: two detectors racing each other on mount, both writing the same cleanup, leaves the confirmation screen stuck re-rendering on an inconsistent Onyx state. Made sense to me initially + the fix worked. But when I sat down to write the detailed explanation, I realized that something "seems off".

What broke the theory

I added a direct Onyx.connect({key: 'transactionsDraft_1'}) probe - a subscriber, with no useOnyx/React layers between it and Onyx's raw cache. During the freeze it emits two mutually incompatible snapshots alternating every ~80 ms:

The two alternating snapshots
State A: amount=N,  merchant="...distance merchant", no images
State B: amount=0,  merchant="Pending...",           images=...<pre-refresh blob>

State B seems off - initMoneyRequest's Onyx.set on the same key should have erased those pre-refresh images before DistanceRequestController's writes land. They can't coexist in a single cache entry under normal behavior. But Onyx was holding both and alternating emissions, which leads to "stuck on E screen / unresponsive page".

That observation alone ruled out the race story - the bug is observable below those layers. But it didn't yet explain why Onyx ended up in that state.

What's actually happening - three things stacking

Three causes had to align. Removing any one of them prevents the freeze.

Cause 1 - The race I introduced (duplicate detection paths)

Two unsynchronized async detectors firing on mount and writing the same cleanup with no coordination.

Why this qualifies as a textbook race

The two detectors above (useRestartOnOdometerImagesFailure and OdometerReceiptStitcher's pre-flight) are both async, both fire on mount, both detect the same expired blobs, and both write the same cleanup. Neither path was idempotent against the other, neither guarded against the other already having run, and neither could cancel the other's in-flight work. Textbook race condition: two concurrent async operations producing mutating side effects on shared state with no coordination.

Cause 2 - Timing change from PR #86958

PR #86958 extracted side-effect components (DistanceRequestController, TaxController, etc.) out of MoneyRequestConfirmationList into separate child components. The antipattern - useEffects that have transaction in their dependency array and write back to the same transaction Onyx key inside the effect body, creating a self-feeding loop - existed before the refactor. But the timing changed, spreading previously-batched setter writes across more microtask boundaries.

How the decomposition changed the batching

The useEffect deps and bodies were carried over essentially verbatim - the transaction-in-deps + write-to-transaction antipattern existed before the refactor. What changed is when the writes flush.

In the inlined version, all those effects ran inside a single component's effect phase, in the same microtask, so the setter writes batched into one Onyx emission cycle and the parent re-rendered once with all updates applied. After the refactor, the effects run in a child component (DistanceRequestController), which means each write triggers a parent useOnyx re-render -> child re-render -> effects re-run round-trip with new microtask boundaries. The decomposition spread previously-batched writes across multiple ticks.

Important

I empirically confirmed this by reverting PR #86958 locally on top of current main (kept everything else, including the duplicate-detection race and the unfixed Onyx). The freeze stopped reproducing. So the refactor is what tipped the balance: the same merge/set/merge sequence has been sitting in the code for a while, but before #86958 the writes ran in one tight batch and Onyx handled it fine. After the refactor those same writes are spread across more ticks, which means more separate merge cycles in flight at the same time. With more pending merges around when initMoneyRequest's set fires, one of them is more likely to be mid-broadcast, and that's the overlap that produces the bad snapshot.

Cause 3 - A latent weakness in react-native-onyx

Even granting that (1) and (2) together produce a stress pattern on a single Onyx key (merge -> ... -> set -> merge -> ... inside ~200 ms), the library still ends up emitting that semantically-incoherent State B to subscribers. The merge-set cancellation logic in setWithRetry is incomplete. Details and the proposed library-side hardening are in Part 2.

What that race + timing produces in the trace
  • One of the two detectors wins and fires its 4-merge clear + the navigation.
  • The navigation triggers useResetIOUType's focus effect on the next screen, which calls initMoneyRequest -> another Onyx.set on the same key.
  • The confirmation screen stays alive under the RHP stack, so DistanceRequestController's mount-time useEffects re-fire against the not-yet-fully-unmounted tree, adding more merges, now spread across multiple ticks because they're in a child component.

Net result: a stack of merge -> merge -> merge -> merge -> set -> merge -> merge -> merge against a single Onyx key inside ~200 ms, distributed across enough microtasks to hit the library weakness. That's the shape that produces the alternating cache emissions I observed.

Why the fix in this PR is the recommended fix at the moment

The hasVerifiedBlobs gate makes the stitcher's effect early-return until the hook has finished its blob verification. That eliminates cause 1 directly: now there's exactly one detector (the hook), exactly one clear path, and the stitcher only ever runs after the hook has finished and confirmed the blobs are healthy. The duplicate-detection race is gone, the merge burst is replaced by a single ordered sequence, and the freeze stops reproducing.

This is the fix in the right place - it addresses the part I'm directly responsible for (two unsynchronized detectors mutating shared state) without reaching into a refactor decision (#86958) or library internals (Onyx). Empirical verification on this PR's branch confirms it: the repro doesn't reproduce.

Other things I tried and why they didn't work

Including these here so anyone picking this up later doesn't waste time on the same dead ends:

  • DistanceRequestController dep-array cleanup. The visible write loop in the trace was setMoneyRequestAmount / setMoneyRequestPendingFields / setMoneyRequestMerchant re-firing every ~100 ms. I tried removing/narrowing the transaction dep on the two useEffects. Didn't stop the freeze - distanceRequestAmount, hasRoute, distance, rate are computed from transaction in the parent, and those derived props still oscillate when Onyx's cache oscillates. Controller-level dep narrowing can't break a feedback loop whose inputs are themselves computed from the oscillating value.
  • Minimal Onyx patch - adding delete OnyxUtils.getMergeQueuePromise()[key] alongside the existing delete OnyxUtils.getMergeQueue()[key] in setWithRetry / multiSetWithRetry. Theory: the in-flight merge promise's .then() was firing broadcastUpdate with a stale snapshot after the set had already broadcast, so explicitly dropping the promise reference would prevent that. Didn't work - the queue-deletion check at the top of the merge's .then() already prevents stale broadcasts, so dropping the promise reference was redundant. The actual library fragility is timing-based, not stale-reference-based (more on that in Part 2).

Recommendation

Important

Merge this PR as-is (hasVerifiedBlobs gate in useRestartOnOdometerImagesFailure + stitcher effect guard) as the deploy-blocker resolution. It correctly addresses the race I introduced and is empirically verified to resolve the freeze. Reverting the parent PR to investigate a library-level fix isn't worth the cost.

One follow-up worth doing on its own timeline (does not block this PR):

  • File a separate issue on react-native-onyx for the library-side observation in Part 2.

Cause 2 (the timing change from PR #86958) is not separately actionable. The decomposition isn't a regression worth reverting, and I empirically tested the two obvious React-side mitigations - wrapping DistanceRequestController with React.memo, and narrowing transaction out of the effect dep arrays - and neither broke the freeze. The reason is structural: the values driving the effect re-runs (transaction itself, distanceRequestAmount, hasRoute, rate) are all derived from the oscillating Onyx cache, so any local React-side guard sees real value changes coming from below and re-fires. Memoization on the child can't help when the parent passes fresh transaction/policy object references on every Onyx emission, and dep-narrowing can't help when the narrowed scalars are themselves recomputed from the oscillating value. The only fix that addresses the loop driver is the Onyx-level hardening in Part 2.


Part 2 - A separate note on Onyx behavior worth checking out

This is a separate observation, not a blocker for this PR, but it might be worth further investigation because I found it while investigating and it could affect other flows.

What I observed

Under the stress pattern from Part 1, Onyx.set emits a snapshot that doesn't correspond to any single point in the write history - a phantom blend of pre-set and post-set state.

Why the emission is semantically incoherent

Even granting that I introduced a race condition and that PR #86958's refactor spread writes across more microtask boundaries, the Onyx.set semantics under the resulting stress feel off. Strictly speaking, Onyx.set(key, X) should mean "the next subscriber emission is X" - not "the next emission may be a phantom blend of pre-set state and post-set partial writes from queued merges that haven't drained yet". The direct Onyx.connect probe shows Onyx emitting a snapshot that combines initMoneyRequest's post-set values (amount=0, merchant=Pending...) with pre-set field values (the dead odometer images that the set replaced), which doesn't correspond to any single point in the write history.

Where the cancellation logic falls short

The library does try to handle merge-set interleaving - setWithRetry has an explicit if (hasPendingMergeForKey(key)) delete mergeQueue[key] branch that exists specifically to cancel pending merges when a set fires against the same key. So the case is anticipated. But the cancellation is structurally incomplete. Deleting mergeQueue[key] removes the queued partials, but it does not cancel the merge's outstanding Promise - mergeQueuePromise[key] is still alive and its .then() callback is still scheduled to run in a later microtask. The set proceeds to call broadcastUpdate(key, X) synchronously, then later the merge's .then() fires, sees the empty queue, and is supposed to bail.

Empirically, the bail isn't clean: the direct Onyx.connect probe shows two rapid emissions to the subscriber - first the expected post-set value, then a phantom blend of post-set fields mixed with pre-set fields. I haven't pinned the exact internal sequence with breakpoints, but the fix that resolves it is informative. Deferring the set's broadcastUpdate behind the pending merge promise (so the set only broadcasts after the merge's .then() has had its chance to run and clean up) makes the phantom emission disappear. Whatever the precise mechanism, it depends on the set and the merge being "in flight" at the same time - forcing them onto different microtask turns closes the window.

Proposed library-side hardening

Defer the set's broadcastUpdate behind any pending merge promise for the same key.

Pseudocode for setWithRetry
function setWithRetry({ key, value, options }, retryAttempt) {
    // Capture the pending merge promise BEFORE deleting it.
    const pendingMergePromise = OnyxUtils.getMergeQueuePromise()[key];

    if (OnyxUtils.hasPendingMergeForKey(key)) {
        delete OnyxUtils.getMergeQueue()[key];
        delete OnyxUtils.getMergeQueuePromise()[key];
    }

    // ... preconditions (skippableCollectionMemberIDs, undefined / null value handling) ...

    // Wrap everything the set used to do inline (compatibility check, broadcastUpdate, storage.setItem, retry wiring)
    // into a closure that can be chained behind any in-flight merge.
    const doSet = () => {
        // existing set body unchanged
    };

    // If there was a pending merge promise, wait for it to resolve before broadcasting.
    // The merge's own `.then()` still runs and still bails on the null queue check (which is fine - it was
    // going to do nothing anyway). The broadcast fires in the next microtask turn, on a settled cache.
    return pendingMergePromise ? pendingMergePromise.then(doSet, doSet) : doSet();
}

Same shape applied to multiSetWithRetry.

Why I tested this

To confirm whether the library-side weakness was independently fixable. Result: with only the library patch (and the hasVerifiedBlobs gate reverted), the alternating snapshot emissions disappeared and the freeze did not occur.

Verification setup

I applied the patch directly to node_modules/react-native-onyx/dist/OnyxUtils.js (no patch-package - just enough to test), reverted the hasVerifiedBlobs gate to isolate the library patch, and reran the repro. Every emission to the direct Onyx.connect probe corresponded to a real writer. So the patch closes the library-side weakness independently of any app-side change.

Why we should investigate further and NOT apply it here

Warning

react-native-onyx is used across every chat, every workspace, every money request in the app. Changing the async semantics of Onyx.set (it now chains through any pending merge before broadcasting, instead of broadcasting synchronously) is a behavior change with broad blast radius. Some code somewhere may rely on the synchronous broadcast and break in subtle ways I wouldn't catch in odometer-only smoke testing.

What should be done next?

The issue should be investigated further before framing this as a library bug. The empirical signature (phantom blended snapshot during merge-set overlap) and the empirical fix (defer the broadcast) are clear, but the library's current behavior could be deliberate - preserving some invariant on the subscriber pipeline, optimizing for throughput, or anticipating callers won't fire merge -> set -> merge on the same key in tight succession. Worth a closer reading of the merge/set logic with someone who maintains Onyx before deciding whether to open a hardening PR or document this as an app-side pattern to avoid.

Tip

Happy to share full trace logs if useful.

Video of the emitted phantom state:

output.mp4

@github-actions
Copy link
Copy Markdown
Contributor

🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here.

Copy link
Copy Markdown
Contributor

@Julesssss Julesssss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing well for me, I'm going to get this merged to unblock deploy. Would appreciate a better review from @DylanDylann later on though, we can follow up with changes if necessary

Screenshot 2026-04-27 at 12 59 03

@Julesssss
Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Android: mWeb Chrome
iOS: HybridApp
iOS: mWeb Safari
MacOS: Chrome / Safari

@Julesssss Julesssss merged commit a29af33 into Expensify:main Apr 27, 2026
35 of 37 checks passed
OSBotify pushed a commit that referenced this pull request Apr 27, 2026
…pp_stuck_on_E_screen_after_refresh_on_confirm_odometer_page

[Odometer] App stuck on E screen after refresh on confirm odometer page

(cherry picked from commit a29af33)

(cherry-picked to staging by Julesssss)
@OSBotify OSBotify added the CP Staging marks PRs that have been CP'd to staging label Apr 27, 2026
@mitarachim
Copy link
Copy Markdown

Deploy Blocker #89008 was identified to be related to this PR.

@OSBotify
Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.3.62-13 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

@MelvinBot
Copy link
Copy Markdown
Contributor

No help site changes are required for this PR.

This is an internal bug fix (race condition between blob URL verification and odometer image stitching) — no user-facing features, workflows, UI labels, or settings were added or changed. The existing Distance Expenses article already accurately describes the odometer flow.

@OSBotify
Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.3.63-1 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 cancelled 🔪
🍎 iOS 🍎 cancelled 🔪

Bundle Size Analysis (Sentry):

@OSBotify
Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/Julesssss in version: 9.3.64-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

@DylanDylann
Copy link
Copy Markdown
Contributor

LGTM 💯

@arosiclair arosiclair mentioned this pull request Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CP Staging marks PRs that have been CP'd to staging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants