Skip to content

Fix iOS freeze when picking a brand-new P2P recipient on the distance confirmation step#95771

Open
MelvinBot wants to merge 2 commits into
mainfrom
claude-fixDistanceP2PFreezeReportIdMismatch
Open

Fix iOS freeze when picking a brand-new P2P recipient on the distance confirmation step#95771
MelvinBot wants to merge 2 commits into
mainfrom
claude-fixDistanceP2PFreezeReportIdMismatch

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

On iOS, starting a Track distance expense and then picking a brand‑new recipient (an email you have no existing chat with) on the confirmation step froze the app instead of landing in the new chat.

This is a regression from #94755, which fixed the older "There is a previously existing chat between these users." error on the same flow. To avoid binding the expense to the auto‑assigned workspace chat, handleParticipantsAdded now writes a freshly generated optimistic reportID (call it A) onto the draft transaction for a brand‑new P2P recipient. The problem is that this ID was never handed to the submit action, so on submit createDistanceRequestgetMoneyRequestInformation built the optimistic chat report at a different generated ID (B). The confirmation screen was left subscribed to report A, which never materialized (the real chat was created at B), so the screen hangs waiting on a report that never loads.

The fix keeps the generated reportID coherent end‑to‑end — the same contract the legacy participants‑step flow (useParticipantSubmission) already honors. When the confirmation page has committed the draft transaction to a fresh optimistic reportID for a brand‑new P2P recipient, that same ID is now threaded into the distance‑create submission as the optimistic chat report ID, so the optimistic chat report is actually created at the ID the page is subscribed to. That collapses the A‑vs‑B mismatch: the screen's report resolves and navigation lands on the real new‑user chat.

Changes:

  • src/pages/iou/request/step/confirmation/useExpenseSubmission.ts — in createDistanceRequest, for the narrow brand‑new P2P case (no resolvable report, participant is not a policy expense chat, participant has no existing reportID, and transaction.reportID is a real generated ID rather than UNREPORTED_REPORT_ID), pass transaction.reportID as optimisticChatReportID.
  • src/libs/actions/IOU/Split.ts — extend CreateDistanceRequestInformation / createDistanceRequest to accept an optional optimisticChatReportID and thread it into getMoneyRequestInformation, which already honors it when building a new optimistic chat report. Existing chats, policy expense chats, self‑DM/track fallback, and split flows are unaffected (they carry a real reportID, so the new value is undefined).
  • tests/actions/IOUTest/SplitTest.ts — regression test asserting createDistanceRequest builds the optimistic chat at the provided optimisticChatReportID.

This implements the approved proposal.

Note: This draft was generated by MelvinBot from the approved proposal. The freeze is a runtime symptom that could not be fully reproduced from static analysis alone; the human co‑author should confirm the fix resolves the freeze on an iOS build before marking this PR ready for review.

Fixed Issues

$ #95468
PROPOSAL: #95468 (comment)

Tests

// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review".
// Please describe what tests you performed that validates your changes worked.

Suggested manual test:

  1. On iOS, tap Global + (FAB) → Track distance → Map tab → set Start and Stop addresses → Next.
  2. Enter a brand‑new user email (a domain you have no existing chat with) → Create x.xx expense.
  3. Verify the app does not freeze, navigation goes to the chat with the new user, and the expense is created there with no "There is a previously existing chat between these users." error.
  4. Regression: repeat with an existing‑chat recipient and with a workspace (policy expense chat) recipient — both should still land on the correct report.
  • Verify that no errors appear in the JS console

Offline tests

Same as Tests — the optimistic chat report is created at the same ID before the API round‑trip, so the expense should appear in the new chat while offline.

QA Steps

// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".

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 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 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)
  • 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)
  • 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.
  • 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

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

MelvinBot and others added 2 commits July 9, 2026 23:51
…ipient

Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
…ChatReportID

Co-authored-by: thelullabyy <thelullabyy@users.noreply.github.com>
@MelvinBot MelvinBot requested a review from a team July 9, 2026 23:58
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I created this PR from an upstream branch since I don't have push access to your fork.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-fixDistanceP2PFreezeReportIdMismatch
git checkout -b claude-fixDistanceP2PFreezeReportIdMismatch upstream/claude-fixDistanceP2PFreezeReportIdMismatch
git push -u origin claude-fixDistanceP2PFreezeReportIdMismatch

Then you can close this PR and open a new one from your fork.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 I looked into the failing checks. No code fix is needed — the only failing check is PR Author Checklist / checklist, which fails because the checklist boxes and the Tests/QA Steps sections in the PR body aren't filled out yet.

Every substantive CI check passed:

  • typecheck, ESLint check, Oxfmt check, React Compiler Compliance
  • ✅ All 8 test (job N) shards, Bun tests, Storybook tests
  • Compare knip issues against main, Check for unused styles, spellcheck, dryRun

The checklist check can only be cleared by you (the human co-author): tick the remaining boxes in the PR Author Checklist and fill out the Tests and QA Steps sections. As the PR body notes, the freeze is a runtime symptom, so please confirm the fix on an iOS build before checking the testing boxes and marking the PR ready for review. That's a human task by design — MelvinBot can't check those boxes for you.

Note on the remaining pending checks

At the time of this analysis, branch-perf-tests and baseline-perf-tests were still running. These are performance benchmarks (Reassure), not correctness gates — a regression there is surfaced as a comment for review, not something addressed by this change, which only threads an optional optimisticChatReportID through the distance-create submission. Nothing in the diff should affect them.

@thelullabyy

Copy link
Copy Markdown
Contributor

@MelvinBot Please replace all [ ] characters in PR description by [x], then mark this PR as ready for review

@MelvinBot MelvinBot marked this pull request as ready for review July 10, 2026 02:05
@MelvinBot MelvinBot requested a review from a team as a code owner July 10, 2026 02:05
@melvin-bot melvin-bot Bot requested a review from DylanDylann July 10, 2026 02:05
@melvin-bot

melvin-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

@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 requested review from heyjennahay and removed request for a team July 10, 2026 02:05
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done, thelullabyy:

  • Replaced every [ ] with [x] in the PR description (all checklist, Tests, and QA boxes are now checked).
  • Marked this PR as ready for review.

⚠️ Heads up: this checked off the human-only Tests/QA sections and the "tested on all platforms" boxes, which still contain the // TODO placeholders and no screenshots/videos. Since you're the assignee you directed this, but please make sure the testing evidence reflects what was actually run before reviewers rely on it.

@thelullabyy

Copy link
Copy Markdown
Contributor

@codex review

@thelullabyy

Copy link
Copy Markdown
Contributor

@DylanDylann Please ignore this PR. This is fixed PR for my PR's regression. I will review it

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 1cea8fcd63

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants