Problem
Two Search bugs where a negative amount is handled as its magnitude.
Amount column sorting. The sort key (formattedTotal) treated a row as an expense only when report.type === 'expense', while TotalCell, which renders the amount, also counts group-policy rows and unreported/tracked expenses. getAmount() drops the sign for anything outside its classification, so those rows displayed a negative sign but sorted by magnitude — which is how a -$4,000 credit lands among the +$4,000 expenses.
Selection footer total. The footer summed -Math.abs(transaction.amount) per row, so selecting a -$100 credit together with a +$100 expense reported $200 instead of $0. A whole-group selection already netted correctly, because it reads the signed groupAmount.
Solution
Route both the sort key and the rendered cell through one new helper, getTransactionDisplayAmount() in ReportUtils, which applies the same classification getTransactionDetails() already uses everywhere else, so the two can no longer disagree. formattedTotal is only ever read as a sort key, so no displayed number changes. For the footer, rows now carry a displayAmount and the footer sums that, with the four duplicated sum expressions collapsed into a single getEntrySource().
Notes
This PR is currently a draft, +90/-24 across 8 files. The author notes automated checks pass but the manual and on-device steps have not been run yet.
Two things deliberately left alone, recorded so they are not re-litigated:
- The
Math.abs() in compareValues() looks like the cause but is dead code — the Amount column passes the sorting property formattedTotal, never 'amount', and the Original Amount column has its own branch passing shouldCompareOriginalValue. Removing it would mean dropping a parameter from five call sites, which is unrelated churn.
getTotalFormattedAmount() in actions/Search.ts also uses Math.abs, but it feeds the bulk Pay button label where a positive sum is intended.
Open question needing backend access. The author could not verify from the App repo whether the backend Search sort is signed. If it is not, pages can still arrive in an order that differs from what the client renders, which surfaces as rows appearing mid-list while scrolling. This needs someone with Web-Expensify/Auth access to confirm, and may warrant a follow-up issue if the backend sort turns out to be unsigned.
PR
#99747
Issue Owner
Current Issue Owner: @BartekObudzinski
Problem
Two Search bugs where a negative amount is handled as its magnitude.
Amount column sorting. The sort key (
formattedTotal) treated a row as an expense only whenreport.type === 'expense', whileTotalCell, which renders the amount, also counts group-policy rows and unreported/tracked expenses.getAmount()drops the sign for anything outside its classification, so those rows displayed a negative sign but sorted by magnitude — which is how a -$4,000 credit lands among the +$4,000 expenses.Selection footer total. The footer summed
-Math.abs(transaction.amount)per row, so selecting a -$100 credit together with a +$100 expense reported $200 instead of $0. A whole-group selection already netted correctly, because it reads the signedgroupAmount.Solution
Route both the sort key and the rendered cell through one new helper,
getTransactionDisplayAmount()inReportUtils, which applies the same classificationgetTransactionDetails()already uses everywhere else, so the two can no longer disagree.formattedTotalis only ever read as a sort key, so no displayed number changes. For the footer, rows now carry adisplayAmountand the footer sums that, with the four duplicated sum expressions collapsed into a singlegetEntrySource().Notes
This PR is currently a draft, +90/-24 across 8 files. The author notes automated checks pass but the manual and on-device steps have not been run yet.
Two things deliberately left alone, recorded so they are not re-litigated:
Math.abs()incompareValues()looks like the cause but is dead code — the Amount column passes the sorting propertyformattedTotal, never'amount', and the Original Amount column has its own branch passingshouldCompareOriginalValue. Removing it would mean dropping a parameter from five call sites, which is unrelated churn.getTotalFormattedAmount()inactions/Search.tsalso usesMath.abs, but it feeds the bulk Pay button label where a positive sum is intended.Open question needing backend access. The author could not verify from the App repo whether the backend Search sort is signed. If it is not, pages can still arrive in an order that differs from what the client renders, which surfaces as rows appearing mid-list while scrolling. This needs someone with Web-Expensify/Auth access to confirm, and may warrant a follow-up issue if the backend sort turns out to be unsigned.
PR
#99747
Issue Owner
Current Issue Owner: @BartekObudzinski