Skip to content

Fix wrong duplicate report total for foreign-currency transactions in a non-USD workspace#97041

Draft
MelvinBot wants to merge 1 commit into
mainfrom
claude-duplicateReportForeignCurrencyTotal
Draft

Fix wrong duplicate report total for foreign-currency transactions in a non-USD workspace#97041
MelvinBot wants to merge 1 commit into
mainfrom
claude-duplicateReportForeignCurrencyTotal

Conversation

@MelvinBot

Copy link
Copy Markdown
Contributor

Explanation of Change

When the workspace default currency is non-USD, duplicating a report ("more" → "Duplicate report") produced a duplicate whose total was wrong (often 0) in both the report preview and the details page.

Root cause: duplicateReport pre-creates the new report in the policy's output currency, then copies each source transaction into it via the money-request flow. The optimistic total in getMoneyRequestInformation is only accumulated inside the iouReport?.currency === currency guard, which has no conversion branch. When a copied transaction's currency differs from the (non-USD) report currency — e.g. the CSV-imported USD transactions from the repro — it is silently dropped from the running total, so the duplicate's total (and the reimbursableTotal / nonReimbursableTotal buckets the preview reads via getMoneyRequestSpendBreakdown) stay at 0.

Fix:

  • In duplicateReport, seed an optimistic snapshot of the report totals (total, reimbursableTotal, nonReimbursableTotal, and the matching unheld* buckets) computed from the eligible transactions expressed in the report currency — using each transaction's own amount when its currency already matches the report, otherwise its stored convertedAmount. Signs follow the expense-report convention (stored negative). This is gated to same-workspace duplicates whose source report currency matches the new report currency, which guarantees convertedAmount is valid; cross-currency duplicates are left for the server response to reconcile.
  • In getMoneyRequestInformation (MoneyRequestBuilder.ts), apply this optional reportTotalsOverride snapshot directly, outside the currency-equality guard, so mismatched-currency copies are no longer skipped. Only defined fields are applied, so a valid zero total is honored. The snapshot is threaded through requestMoney; all other flows are unaffected (they never pass it).

These are optimistic values only — the server response remains the source of truth.

⚠️ Draft raised from MelvinBot's proposal for ChavdaSachin to review. Scope is intentionally the same-workspace/same-currency repro. Two follow-up decisions worth confirming during review: (1) whether a cross-workspace duplicate with a different target currency should show a pending/loading total until the server responds, and (2) whether any real duplicate flow ever hits the "create a fresh money-request report" branch (where the first copied transaction dictates the report currency) rather than reusing the pre-created report — the fix targets the reuse branch, which the repro and the existing analysis both exercise.

Fixed Issues

$ #94009
PROPOSAL: #94009 (comment)

Tests

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

Suggested manual test for the reviewer/co-author:

  1. Set a workspace (and personal preference) default currency to a non-USD currency (e.g. GBP).
  2. Import the transactions from the linked issue's CSV (they are in USD) into that workspace and add them to a report.
  3. Open the expense report → more → Duplicate report.
  4. Verify the duplicate report's total in both the preview and the details page matches the source report's total (and is not 0/incorrect).
  • Verify that no errors appear in the JS console

Offline tests

QA Steps

// TODO: The human co-author must fill out the QA steps before marking this PR as "ready for review". These must be filled out, or the issue title must include "[No QA]."

  • 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

…nsactions

When the workspace default currency is non-USD, duplicating a report created the duplicate in the policy currency while its copied transactions kept their own (e.g. USD) currency. The per-transaction total accumulation in getMoneyRequestInformation only runs when the transaction currency matches the report currency, so mismatched transactions were dropped and the duplicate's optimistic total (and reimbursable/non-reimbursable buckets read by the preview) stayed at 0, showing the wrong amount.

Seed the optimistic report totals from the eligible transactions expressed in the report currency and apply them outside the currency-equality guard, for same-workspace duplicates whose source report currency matches the new report currency.

Co-authored-by: Chavda Sachin <ChavdaSachin@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team July 25, 2026 22:18
@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-duplicateReportForeignCurrencyTotal
git checkout -b claude-duplicateReportForeignCurrencyTotal upstream/claude-duplicateReportForeignCurrencyTotal
git push -u origin claude-duplicateReportForeignCurrencyTotal

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

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Hey ChavdaSachin — I looked into the failing checks. There's nothing for me to fix in the code.

The only red check is PR Author Checklist / checklist, and it's failing for the expected reason on a MelvinBot-raised draft: authorChecklist.ts reports "PR Author Checklist is not completely filled out. Please check every box" — the author-checklist boxes are still unchecked and the Tests/QA Steps sections still have the // TODO placeholders.

Those are human-only sections that I'm not allowed to fill out or tick on your behalf, so I can't clear this check by pushing a commit. As the human co-author, you'll need to:

  1. Fill in the Tests section (the suggested repro steps are already stubbed in the PR body).
  2. Fill in the QA Steps section (or add [No QA] to the title if not applicable).
  3. Tick every box in the PR Author Checklist.
  4. Mark the PR Ready for review.

Every functional check is green:

  • typecheck
  • ✅ All 8 test shards (incl. the new DuplicateTest.ts)
  • Storybook tests
  • ESLint, Oxfmt, React Compiler Compliance, knip, unused-styles, spellcheck
  • ✅ All build jobs, CLA, verifySignedCommits, validate
  • ✅ Reassure performance tests (baseline / branch / validate)

So no code fix is needed here — the checklist just needs you to complete the human sections before this PR can go green.

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