Skip to content

Conversation

@MobileMage
Copy link
Contributor

Explanation of Change

When reviewing duplicate expenses that span two workspaces with different feature configurations (e.g., Workspace A has taxes/categories/tags enabled, Workspace B does not), the review flow had multiple failures:

  1. Tax code page showed no optionsReviewTaxCode.tsx line 27 used reviewDuplicates?.reportID as a fallback, loading the kept transaction's report instead of the thread report. This caused all downstream data (transaction, violations, duplicate IDs) to be derived from the wrong source.
  2. Tax names resolved to NOT_FOUND — After fixing the report lookup, policy was still derived from the thread report's workspace (which may not have taxes enabled). Tax lookups (getTaxByID, getDefaultTaxCode, getTaxValue) need to use the kept transaction's workspace policy.
  3. Category/tag review steps skipped & "It's not here" on Confirmation — The kept transaction's violation data may list an incomplete subset of duplicates (cross-workspace duplicates aren't always in every transaction's violation data). TransactionPreview now also loads the thread transaction's duplicate list and merges both lists before calling getReviewNavigationRoute, ensuring all duplicates are represented.

Changes:

  • ReviewTaxCode.tsx: Use route.params.threadReportID directly (consistent with all other review pages). Move usePolicy to derive from reviewDuplicatesReport?.policyID (the kept transaction's workspace) instead of the thread report's workspace.
  • TransactionPreview/index.tsx: Load the thread transaction's duplicate list via its violations and merge with the kept transaction's duplicates in the navigation callback, ensuring complete cross-workspace coverage.

Fixed Issues

$ #80476
PROPOSAL: #80476 (comment)

Tests

Preconditions:

  • Workspace A with taxes, categories, and tags enabled
  • Workspace B with taxes, categories, and tags not enabled
  • Three duplicate expenses: A1 (WS-A, no category/tag), A2 (WS-A, with category/tag/tax), B (WS-B, no category/tag/tax)
  1. Open the duplicate transaction thread from WS-A
  2. Click "Keep this one" on A2 (the one with category, tag, and tax)
  3. Verify the review flow shows Category → Tag → Tax Code → Confirmation steps
  4. Verify tax code names resolve correctly (not NOT_FOUND or empty)
  5. Verify the Confirmation page renders correctly (no "It's not here" error)
  6. Go back and click "Keep this one" on B (the WS-B transaction)
  7. Verify the review flow skips category/tag/tax steps and goes straight to Confirmation
  8. Open the duplicate transaction thread from WS-B
  9. Click "Keep this one" on A2
  10. Verify the review flow shows Category → Tag → Tax Code → Confirmation steps
  11. Click "Keep this one" on B
  12. Verify the review flow skips to Confirmation (WS-B has no features enabled)
  • Verify that no errors appear in the JS console

Offline tests

The duplicate review flow requires online connectivity to load transaction violations and policy data. No offline-specific changes were made.

QA Steps

Same as tests above. Ensure two workspaces exist with different feature configurations (one with taxes/categories/tags enabled, one without) and three duplicate transactions spanning both workspaces.

  • 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
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • 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 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)

Screenshots/Videos

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

@MobileMage MobileMage requested review from a team as code owners January 31, 2026 00:57
@melvin-bot melvin-bot bot requested review from hungvu193 and trjExpensify and removed request for a team January 31, 2026 00:58
@melvin-bot
Copy link

melvin-bot bot commented Jan 31, 2026

@hungvu193 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 January 31, 2026 00:58
Use threadReportID directly in ReviewTaxCode (consistent with other review
pages) and derive the policy from the kept transaction's workspace for correct
tax lookups. Merge thread transaction's duplicate list in TransactionPreview
to ensure cross-workspace duplicates are fully represented in the review flow.
@MobileMage MobileMage force-pushed the fix/80476-review-tax-code-report-id branch from 6f1ece5 to f907dc9 Compare January 31, 2026 01:00
Copy link

@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: 6f1ece5e97

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

const navigateToReviewFields = useCallback(() => {
Navigation.navigate(getReviewNavigationRoute(Navigation.getActiveRoute(), route.params?.threadReportID, transaction, duplicates, policyCategories, transactionReport));
}, [route.params?.threadReportID, transaction, duplicates, policyCategories, transactionReport]);
const allDups = [...duplicates, ...(threadDupTxns ?? [])];

Choose a reason for hiding this comment

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

P2 Badge Filter thread duplicates before merging

duplicates has already been filtered with removeSettledAndApprovedTransactions, but threadDupTxns is appended without that filter. If the thread transaction’s violations include a duplicate that is settled/approved (e.g., one duplicate was already approved), allDups will reintroduce it and getReviewNavigationRoute will store it in reviewDuplicates.duplicates. That can cause the review flow to consider non‑reviewable transactions and pass settled/approved IDs into the merge params. Consider filtering/deduping threadDupTxns (or the merged list) the same way before navigation.

Useful? React with 👍 / 👎.

@codecov
Copy link

codecov bot commented Jan 31, 2026

Codecov Report

❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.

Files with missing lines Coverage Δ
...ents/ReportActionItem/TransactionPreview/index.tsx 81.81% <66.66%> (-1.52%) ⬇️
src/pages/TransactionDuplicate/ReviewTaxCode.tsx 0.00% <0.00%> (ø)
... and 120 files with indirect coverage changes

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.

1 participant