Repository navigation
fix: Duplicate per diem expenses created in Expensify Classic - #106564
neil-marcellini wants to merge 2 commits into
Conversation
|
@ZhenjaHorbach 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] |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56fb86ffb1
ℹ️ 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".
| : sourceCustomUnit.attributes, | ||
| subRates: sourceCustomUnit.subRates?.map((subRate) => ({ | ||
| ...subRate, | ||
| rate: policySubRates.find((policySubRate) => policySubRate.id === subRate.id)?.rate ?? subRate.rate, |
There was a problem hiding this comment.
When a workspace's sub-rate price has changed since the original expense was created, duplicating its report now creates the optimistic transaction at the new price but leaves the report totals at the old price. duplicateReport() calculates its running totals from the source transactions through buildDuplicateReportCurrencyAmounts(), and getPerDiemExpenseInformation() applies those new*Total overrides instead of accumulating the newly computed per diem amount. For example, duplicating a report containing one 50.00 per diem after its rate becomes 60.00 shows a 60.00 expense in a 50.00 report, persisting while offline. Update the report-total calculation to account for refreshed prices, and avoid reusing stored converted amounts when the underlying amount changes. This also preserves the offline optimistic-update behavior documented in AGENTS.md.
AGENTS.md reference: AGENTS.md:L97-L100
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
(Neil's AI agent)
Agreed, this was new in this PR. duplicateReport sets the report total from the source expenses' amounts, so re-pricing a sub-rate that already had a price would make the copied expense and the report total disagree after a price change.
Fixed in 2a76dc2: a sub-rate keeps its own price, and only one without a price (Expensify Classic data) takes it from the workspace. Added the test per diem expense whose workspace price changed since it was created, which fails on the previous commit.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
garrettmknight
left a comment
garrettmknight
left a comment
There was a problem hiding this comment.
Looks good from a product perspective.
| : sourceCustomUnit.attributes, | ||
| subRates: sourceCustomUnit.subRates?.map((subRate) => ({ | ||
| ...subRate, | ||
| rate: subRate.rate ?? policySubRates.find((policySubRate) => policySubRate.id === subRate.id)?.rate, |
There was a problem hiding this comment.
Actually, based on the ts, subRate.rate can't be null or undefined
But this is just a small remark
|
Looks like BE issue is not fixed yet when I try to dublicate report which was created in expensify classic Screen.Recording.2026-11-13.at.09.08.42.mov |
Explanation of Change
(Neil's AI agent)
Duplicating a per diem expense that was created in Expensify Classic fails with "An unexpected error occurred".
Classic saves per diem dates with a time zone offset (
2026-07-31T05:30:00.000+02:00) and saves sub-rates without their price. Duplicate copied those fields intoCreatePerDiemRequestunchanged. The API only accepts dates like2026-07-31 05:30:00, so it dropped both dates and Auth rejected the request with402 Missing startTime.When a per diem expense is duplicated, this PR now:
yyyy-MM-dd HH:mm:ss, keeping the time the user entered (2026-07-31T05:30:00.000+02:00becomes2026-07-31 05:30:00). Dates already in that format are unchanged.This covers duplicating a single expense and duplicating a whole report, since both go through
createExpenseByType.Fixed Issues
(Neil's AI agent)
$ https://github.com/Expensify/Expensify/issues/700865
PROPOSAL: N/A, internal issue
Tests
(Neil's AI agent)
Automated, in
tests/actions/IOUTest/DuplicateTest.ts:per diem expense created in Expensify Classicfails without this change, with the server receivingstartDateTime: '2026-07-31T05:30:00.000+02:00'and a sub-rate with norate.per diem expense whose workspace price changed since it was createdchecks that a sub-rate with a price keeps it.Offline tests
(Neil's AI agent)
QA Steps
(Neil's AI agent)
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari