[CP Staging] Revert "Use freshly computed report totals from backend instead of potentially stale stored total"#91730
Conversation
…tentially stale stored total"
melvin-bot
Bot
requested review from
joekaufmanexpensify and
neil-marcellini
and removed request for
a team
|
@neil-marcellini 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] |
allgandalf
changed the title
![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: 2847828f2c
ℹ️ 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".
| }) => ({ | ||
| reportID, | ||
| action, | ||
| total, |
There was a problem hiding this comment.
deriveSelectedReports() now copies total directly from the grouped report row, but bulk pay later forwards selectedReports[].total as paymentData.amount (see useSearchBulkActions mapping to amount: report.total). For expense reports with non-reimbursable transactions, total includes amounts that should not be reimbursed, so bulk pay can submit an inflated payment amount. This value should remain the reimbursable amount (total - nonReimbursableTotal or the backend-computed reimbursable field).
Useful? React with 👍 / 👎.
| policyID={report?.policyID} | ||
| hash={transactionItem?.hash} | ||
| amount={getReimbursableTotal(report)} | ||
| amount={report?.total} |
There was a problem hiding this comment.
The row pay action now sends report?.total into PayActionCell, and that value is used as the payment amount in payMoneyRequestOnSearch. On expense reports that contain non-reimbursable items (or temporarily stale totals), this can overpay/underpay because total is not guaranteed to match the payable reimbursable balance. The amount passed here should use reimbursable-only math rather than raw report total.
Useful? React with 👍 / 👎.
| if (aTotal == null || bTotal == null || aNonReimbursableTotal == null || bNonReimbursableTotal == null) { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
The reimbursable-total sorter now returns 0 whenever either row is missing total or nonReimbursableTotal. In mixed datasets this makes many pairwise comparisons no-ops, so selecting the Reimbursable Total column can leave rows effectively unsorted. The previous behavior treated missing values as 0; keeping that fallback avoids unstable ordering for partially populated search rows.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
…rt-totals [CP Staging] Revert "Use freshly computed report totals from backend instead of potentially stale stored total" (cherry picked from commit e608451) (cherry-picked to staging by mountiny)
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.
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.82-3 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed all changes in this PR. This is a revert of #89047, which only changed internal logic for how report totals are computed (reverting from freshly computed totals back to stored No help site changes are required. @allgandalf, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.82-3 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.3.83-0 🚀
Bundle Size Analysis (Sentry): |
|
I reviewed the changes in this PR against the help site articles under No help site changes are required. This PR reverts #89047, rolling back internal code changes to how report totals are computed (e.g., switching from The existing help site articles that reference report totals, held expenses, and approval workflows (e.g., @allgandalf, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.3.83-3 🚀
|
4 participants
Explanation of Change
Reverts #89047
Fixed Issues
$ #91729
PROPOSAL: N/A
Tests
Verify that, Pay button will show the unheld amount because the report now contains only pending amount.
Offline tests
Same as Tests
QA Steps
Same as Tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.