Skip to content

83847 merge transaction v3 - #96163

Closed
huult wants to merge 10 commits into
Expensify:mainfrom
huult:83847-merge-transaction-v3
Closed

83847 merge transaction v3#96163
huult wants to merge 10 commits into
Expensify:mainfrom
huult:83847-merge-transaction-v3

Conversation

@huult

@huult huult commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Fixed Issues

$ #83847
PROPOSAL:

Tests

Same QA step

  • Verify that no errors appear in the JS console

Offline tests

QA Steps

  1. Open the Expensify app.
  2. Go to the workspace chat and create 2 receipt expenses.
  3. Open one of the expenses and tap More.
  4. Select Merge.
  5. Verify that the merge list page opens and you can navigate back normally.
  6. Select another receipt expense and tap Continue.
  7. Verify that the merge receipt page opens and you can navigate back normally.
  8. Complete the merge options and tap Continue.
  9. Verify that the merge confirmation page opens and you can navigate back normally.
  • 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 verified there are no new alerts related to the canBeMissing param for useOnyx
  • 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

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-07-15.at.13.10.47.mov

@huult
huult requested review from a team as code owners July 15, 2026 06:11
@melvin-bot
melvin-bot Bot requested review from JmillsExpensify and suneox and removed request for a team July 15, 2026 06:11
@melvin-bot

melvin-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

@suneox 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 a team and mjasikowski and removed request for a team July 15, 2026 06:11
@melvin-bot

melvin-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

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

Comment thread src/libs/Navigation/types.ts Outdated

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

Copy link
Copy Markdown

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: 1a87da7fb1

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread config/eslint/eslint.seatbelt.tsv Outdated
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Coverage Δ
src/SCREENS.ts 100.00% <ø> (ø)
.../components/MoneyRequestHeaderSecondaryActions.tsx 0.50% <ø> (+<0.01%) ⬆️
src/hooks/useSelectedTransactionsActions.ts 77.10% <ø> (ø)
src/libs/Navigation/linkingConfig/config.ts 66.66% <ø> (ø)
src/hooks/useSearchBulkActions.ts 53.90% <0.00%> (ø)
.../TransactionMerge/MergeTransactionsListContent.tsx 0.00% <0.00%> (ø)
...ages/TransactionMerge/DynamicDetailsReviewPage.tsx 0.00% <0.00%> (ø)
...nsactionMerge/DynamicMergeTransactionsListPage.tsx 0.00% <0.00%> (ø)
...ages/TransactionMerge/DynamicReceiptReviewPage.tsx 0.00% <0.00%> (ø)
src/ROUTES.ts 21.71% <0.00%> (+0.15%) ⬆️
... and 3 more
... and 10 files with indirect coverage changes

@JmillsExpensify JmillsExpensify left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@suneox

suneox commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Manual test cases based on code change


DYNAMIC_ROUTES.MERGE_TRANSACTION_LIST — Merge list (pick source expense)

Case 1.1 — Entry: Expense report → More → Merge (1 expense) → list

Target: src/hooks/useExpenseActions.ts:338src/libs/actions/MergeTransaction.ts:79 (PR: createDynamicRoute(LIST.getRoute)).
Precondition: Logged in; ≥1 receipt expense in a workspace chat; Merge visible in More.

// Do NOT inject via Onyx: merge eligibility is DERIVED from transaction/policy/report.
// Use a test account that already has a real receipt expense in a workspace chat.

Steps:
1. Open the app and log in with the test account.
2. From the LHN, open a workspace chat that has a receipt expense.
3. Open one expense (expense thread/report).
4. Tap MoreMerge.
5. Verify the Merge list opens (select-expense header).
6. Verify URL is {base}/merge/<transactionID> (e.g. /r/<reportID>/merge/<id>), with no ?backTo= / isOnSearch.
7. Tap Back → verify return to the expense/report; URL no longer has /merge/….
Expected: List opens; sibling dynamic URL; Back returns to entry.

Case 1.2 — Entry: Transaction header → Merge

Target: src/components/MoneyRequestHeaderSecondaryActions.tsx:385.
Precondition: Same as 1.1; viewing an expense thread.
Steps:
1. Open an expense thread from the workspace chat.
2. Tap Merge in the header secondary actions.
3. Verify list + URL {base}/merge/<id>.
4. Tap Back → return to the expense thread.
Expected: Header entry behaves like the More menu.

Case 1.3 — Entry: Search → bulk Merge 2 expenses (skip list)

Target: src/hooks/useSearchBulkActions.ts:2024.
Precondition: Search has ≥2 receipt expenses eligible to merge.

// isOnSearch query param is removed in this PR — post-merge uses isSearchTopmostFullScreenRoute().
// Exercise via real Search UI; no Onyx inject for isOnSearch.

Steps:
1. Open Search → find 2 eligible receipt expenses.
2. Select both → bulk Merge.
3. Verify you do not land on the list — go straight to receipt / details / confirmation.
4. Verify URL is {base}/merge/<id>/… with a Search-related base.
5. (Full post-merge → Case 4.6.) Here only verify you land on the right page (receipt/details/confirmation) with a Search base.
Expected: Skip list for 2 txns; sibling URL on Search base; no ?isOnSearch.

Case 1.4 — Entry: Search selected transactions → Merge

Target: src/hooks/useSelectedTransactionsActions.ts:466.
Precondition: Search with selected eligible transactions.
Steps:
1. In Search, select 1–2 expenses.
2. Choose Merge from the selected-transactions menu.
3. Verify nav: list if 1 txn; skip list if 2.
Expected: Selected-transactions entry works; policies arg stays in the correct slot after the signature change.

Case 1.5 — Forward: pick source from list → receipt/details

Target: src/pages/TransactionMerge/MergeTransactionsListContent.tsx:166 (PR: listBackPath).
Precondition: On list (1.1); ≥1 eligible source.
Steps:
1. From the list, pick a source expense → Continue.
2. Verify URL is {base}/merge/<id>/receipt (or details/confirmation) — not nested …/merge/<id>/merge/<id>/….
3. ⚠️ Tap Back from the next page → expect return to the list. If it jumps straight to the report → matches P1 in the review (do not PASS until fixed).
Expected: Sibling forward on the same base; step-by-step Back (after P1 fix).

Case 1.6 — Refresh / deeplink new list URL

Target: src/libs/Navigation/linkingConfig/config.ts — PR maps DYNAMIC_LIST_PAGEDYNAMIC_ROUTES.MERGE_TRANSACTION_LIST.path.
Precondition: On list; copy the full URL.
Steps:
1. Reload / paste the URL in a new tab.
2. Verify list restores (not home / 404).
3. Back → return to base report/search.
Expected: Refresh/deeplink {base}/merge/<id> works.

Case 1.7 — ⚠️ Backward-compat: old URL /merge/<transactionID>

Precondition: Logged in; valid transactionID from a prior merge flow.
Steps:
1. Paste https://dev.new.expensify.com:8082/merge/<transactionID> (optional ?backTo=/r/<reportID>).
2. Expected (after fix): opens merge list with a valid base (if ?backTo= present → {backTo}/merge/<id>).
3. ⚠️ Caveat (current PR): may Not Found / wrong stack — capture as bug evidence; do not PASS until redirect lands or breakage is explicitly accepted.
Expected: After backward-compat fix → old URL resolves.

Case 1.8 — Entry: Search RHP expense → header Merge → list

Target: src/components/MoneyRequestHeaderSecondaryActions.tsx:385 (PR: drops isOnSearch derived from route.name.startsWith('search')).
Precondition: Logged in; Search has ≥1 receipt expense eligible to merge; can open it in Search RHP.

// PR removes isOnSearch on header Merge — previously derived from route.name.
// Must exercise from Search RHP (unlike Case 1.2 workspace). No Onyx inject.

Steps:
1. Open the app, log in → LHN Search.
2. Find 1 eligible receipt expense → open it in Search RHP.
3. Tap Merge on the header secondary actions.
4. Verify Merge list opens; URL is {searchBase}/merge/<transactionID>.
5. Verify URL has no ?isOnSearch= / ?backTo=.
6. Tap Back → return to Search expense RHP (Search context preserved).
Expected: Header Merge from Search RHP opens list with sibling URL; does not depend on isOnSearch param.

Case 1.9 — Forward: list opened from Search → Continue keeps Search listBackPath

Target: src/pages/TransactionMerge/MergeTransactionsListContent.tsx:166 (PR: listBackPath).
Precondition: On merge list opened from Search (Case 1.8 or Search select 1 → Merge); ≥1 eligible source.
Steps:
1. From Case 1.8 (or Search → select 1 expense → Merge), stay on the list.
2. Pick a source expense → Continue.
3. Verify URL is {searchBase}/merge/<id>/receipt (or details/confirmation) — not nested …/merge/<id>/merge/<id>/….
4. Verify base path stays in Search context (does not jump to a bare workspace /r/… if entry was Search).
5. ⚠️ Tap Back from the next page → expect return to the list. If it jumps straight to Search (skipping list) → matches P1 in the review (do not PASS until fixed).
Expected: listBackPath keeps Search base on forward; sibling URL; step-by-step Back (after P1 fix).


DYNAMIC_ROUTES.MERGE_TRANSACTION_RECEIPT — Receipt review

Case 2.1 — Entry: 2 receipts → bulk Merge → receipt page

Target: src/libs/actions/MergeTransaction.ts:102.
Precondition: 2 receipt expenses (both have receipt images).
Steps:
1. From Search/report, merge 2 receipt expenses (1.3 or list → Continue).
2. Verify Receipt review (two receipts).
3. Verify URL {base}/merge/<id>/receipt.
4. ⚠️ Back → expect list (if entered from list) or search/report (if list was skipped). If Back always jumps to entry and skips list → P1.
Expected: Receipt opens for 2 receipts; back-nav matches stack.

Case 2.2 — Forward: receipt → confirmation (no conflicts)

Target: PR DynamicReceiptReviewPagecreateDynamicRoute(CONFIRMATION, backTo).
Precondition: 2 receipts; fields identical after receipt selection.
Steps:
1. On receipt review, pick a receipt → Continue.
2. Verify Confirmation; URL {base}/merge/<id>/confirmation (sibling, not nested under /receipt/).
3. ⚠️ Back → expect receipt (after P1 fix).
Expected: Forward OK; no /not-found (#94940 regression addressed).

Case 2.3 — Forward: receipt → details (has conflicts)

Target: PR DynamicReceiptReviewPage → DETAILS.
Precondition: 2 receipts + conflicting fields.
Steps:
1. Receipt → Continue.
2. Verify Details; URL {base}/merge/<id>/details.
Expected: Conflict path goes through details.

Case 2.4 — Refresh / deeplink new receipt URL

Steps:
1. On receipt → copy URL → reload / new tab.
2. Verify restore; not 404.
Expected: Deeplink {base}/merge/<id>/receipt works.

Case 2.5 — ⚠️ Backward-compat: /merge/<id>/receipt

Steps:
1. Paste https://dev.new.expensify.com:8082/merge/<transactionID>/receipt.
2. Expected (after fix): redirect to {base}/merge/<id>/receipt.
3. ⚠️ Caveat: no OldRoutes yet — record actual behavior.


DYNAMIC_ROUTES.MERGE_TRANSACTION_DETAILS — Details review

Case 3.1 — Entry: conflicts → details

Target: src/libs/actions/MergeTransaction.ts:126.
Precondition: 2 expenses with ≥1 conflict; may arrive via receipt 2.3.
Steps:
1. Trigger a merge with conflicts.
2. Verify Details + URL {base}/merge/<id>/details.
3. ⚠️ Back → expect receipt or list/search (per stack).
Expected: Conflict fields shown; Back correct.

Case 3.2 — Forward: details → confirmation (resolved) — core regression check

Target: PR DynamicDetailsReviewPagecreateDynamicRoute(CONFIRMATION, backTo).
Precondition: On details; all conflicts resolved.
Steps:
1. Resolve every conflict field.
2. Tap Continue.
3. Verify Confirmation (not /not-found).
4. Verify URL {base}/merge/<id>/confirmation.
5. ⚠️ Back → expect details (P1 if it jumps to the report).
Expected: Forward details→confirmation PASSES (fixes #94940). Step-by-step Back after P1 fix.

Case 3.3 — Refresh / deeplink details

Steps:
1. Reload a URL containing /merge/<id>/details.
2. Verify restore + Onyx merge state.
Expected: Refresh OK.

Case 3.4 — ⚠️ Backward-compat: /merge/<id>/details

Steps: Paste old standalone URL → Expected redirect after fix; ⚠️ may fail on current PR.


DYNAMIC_ROUTES.MERGE_TRANSACTION_CONFIRMATION — Confirmation + complete

Case 4.1 — Entry: identical → confirmation (skip)

Target: src/libs/actions/MergeTransaction.ts:122.
Precondition: 2 identical expenses / no conflicts.
Steps:
1. Merge 2 identical expenses.
2. Land on Confirmation; URL {base}/merge/<id>/confirmation.
3. Back → verify stack (entry or prior step).
Expected: Skip path OK.

Case 4.2 — Complete merge: Merge button → success (report entry)

Target: PR DynamicConfirmationPagemergeTransactionRequest.
Precondition: Confirmation with valid merge data; entry from workspace/report (not Search). For Search post-merge → Case 4.6.
Steps:
1. Review summary → tap Merge.
2. Verify success; modal closes → report / expense thread.
3. Verify UI shows one merged expense.
Expected: E2E complete from report entry.

Case 4.3 — Full happy path (PR QA mirror) + Back-through-wizard

Precondition: Workspace chat; 2 receipt expenses.
Steps:
1. Create 2 receipt expenses in the workspace chat.
2. Expense → MoreMerge → list (/merge/<id>).
3. Pick expense 2 → Continue → receipt.
4. Pick receipt → Continue → details or confirmation.
5. If details: resolve → Continue → confirmation.
6. Before Merge: from confirmation Back → details (or receipt) → list → report — one Back per step.
7. Walk forward again → Merge complete.
Expected: Forward/back wizard correct; ⚠️ if Back skips steps = reproduces P1.

Case 4.4 — Refresh / deeplink confirmation

Steps: Reload confirmation URL → restore; Back stack OK.
Expected: Deeplink OK.

Case 4.5 — ⚠️ Backward-compat: /merge/<id>/confirmation

Steps: Paste old URL → Expected redirect after fix; ⚠️ may fail on current PR.

Case 4.6 — Complete merge from Search → dismiss + open expense in Search RHP

Target: PR DynamicConfirmationPage — post-merge uses only isSearchTopmostFullScreenRoute() (drops isOnSearch || …).
Precondition: Search fullscreen is topmost; ≥2 eligible expenses; reach Confirmation via Search bulk/select (Case 1.3 / 1.4) or 1.8→1.9→forward; do not leave Search context before Merge.

// PR removes isOnSearch param — Confirmation only checks isSearchTopmostFullScreenRoute().
// Keep Search as topmost fullscreen for the whole flow. No Onyx inject.

Steps:
1. Open Search → bulk or select 2 eligible expenses → reach Confirmation (keep Search fullscreen topmost).
2. Verify confirmation URL has a Search base ({searchBase}/merge/<id>/confirmation).
3. Tap Merge.
4. Verify modal dismisses → expense opens in Search RHP (Search context kept; not stuck on confirmation).
5. Verify no navigation-related console errors.
Expected: Search post-merge path works without isOnSearch query/param; relies on isSearchTopmostFullScreenRoute().

@suneox

suneox commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

@huult The old route redirection is failing on these test cases

  • 1.7 /merge/3093367965053198935
  • 2.5 /merge/3093367965053198935/receipt
  • 3.4 /merge/3093367965053198935/details
  • 4.5 /merge/3093367965053198935/confirmation
CleanShot.2026-07-19.at.09.47.27.1.mp4
  • Open expense detail from search show page not found
CleanShot.2026-07-19.at.09.54.53.1.mp4

@huult

huult commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Checking...

@huult

huult commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

I'm still investigating the old routes issue and will provide an update tmr.

@huult

huult commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

I'll close this PR for now and investigate the OldRoutes issue later.

@huult huult closed this Jul 21, 2026
@suneox suneox mentioned this pull request Jul 27, 2026
53 tasks
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.

3 participants