Skip to content

Commit 4e6f872

Browse files
Fix Your spend offline total sign and empty Search page after submit
Use signed getAmount instead of Math.abs when patching Your spend snapshot totals so the section reflects spend as negative (and credit lines reduce the total) instead of a positive, inflated value. Also inject a moving report's reimbursable transactions into the snapshot data when it enters a section so the linked Search page is not empty offline, and remove them on leave/rollback. Update the related unit tests to the corrected signed convention and data writes.
1 parent a5d9074 commit 4e6f872

3 files changed

Lines changed: 131 additions & 35 deletions

File tree

src/libs/actions/IOU/YourSpendSnapshotUpdate.ts

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type {OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
1+
import type {NullishDeep, OnyxCollection, OnyxEntry, OnyxUpdate} from 'react-native-onyx';
22
import Onyx from 'react-native-onyx';
33
// eslint-disable-next-line no-restricted-imports -- Your spend "Awaiting approval"/"Repaid" totals are a billing/paid-only feature (Collect/Control), so paid-group scoping is intentional here.
44
import {isPaidGroupPolicy} from '@libs/PolicyUtils';
@@ -9,6 +9,7 @@ import {buildAwaitingApprovalQuery, buildRepaidLast30DaysQuery, get30DaysAgoDate
99
import CONST from '@src/CONST';
1010
import ONYXKEYS from '@src/ONYXKEYS';
1111
import type {Policy, Report, SearchResults, Transaction} from '@src/types/onyx';
12+
import type {SearchResultDataType} from '@src/types/onyx/SearchResults';
1213

1314
type YourSpendSnapshotOnyxData = {
1415
optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.SNAPSHOT>>;
@@ -172,8 +173,10 @@ function calculateYourSpendTotalDiff(iouReport: OnyxEntry<Report>, updatedTransa
172173
return null;
173174
}
174175

175-
const currentTotal = Math.abs(getAmount(transaction, isExpenseReportLocal));
176-
const updatedTotal = Math.abs(getAmount(updatedTransaction, isExpenseReportLocal));
176+
// Signed to match the snapshot `total` convention (spend negative, credits positive), so the delta moves the
177+
// snapshot total in the correct direction (e.g. raising a spend amount makes the negative total more negative).
178+
const currentTotal = getAmount(transaction, isExpenseReportLocal);
179+
const updatedTotal = getAmount(updatedTransaction, isExpenseReportLocal);
177180

178181
if (currentTotal === updatedTotal) {
179182
return 0;
@@ -219,19 +222,23 @@ function getSnapshotSearchResults(snapshotHash: number | undefined) {
219222
return allSnapshots?.[snapshotKey]?.search;
220223
}
221224

222-
/** Returns a transaction's reimbursable amount in the snapshot currency, or null when conversion is unavailable offline. */
225+
/**
226+
* Returns a transaction's reimbursable amount in the snapshot currency, or null when conversion is unavailable offline.
227+
* The value is signed to match the search `total` convention (spend is negative, credits positive), i.e. it mirrors
228+
* `getAmount(transaction, isFromExpenseReport)` rather than its magnitude.
229+
*/
223230
function getReimbursableTransactionAmountInCurrency(transaction: Transaction, iouReport: OnyxEntry<Report>, targetCurrency: string): number | null {
224231
const isExpenseReportLocal = isExpenseReport(iouReport) || isInvoiceReportReportUtils(iouReport);
225232
const transactionCurrency = getCurrency(transaction);
226233

227234
if (transactionCurrency === targetCurrency) {
228-
return Math.abs(getAmount(transaction, isExpenseReportLocal));
235+
return getAmount(transaction, isExpenseReportLocal);
229236
}
230237
// `convertedAmount` is denominated in the report's policy output currency, not necessarily the snapshot
231238
// currency. Only trust it when those match; otherwise we'd add a value in the wrong currency to the total.
232239
const policyOutputCurrency = iouReport?.policyID ? allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${iouReport.policyID}`]?.outputCurrency : undefined;
233240
if (transaction.convertedAmount != null && policyOutputCurrency === targetCurrency) {
234-
return Math.abs(getConvertedAmount(transaction, isExpenseReportLocal));
241+
return getConvertedAmount(transaction, isExpenseReportLocal);
235242
}
236243

237244
return null;
@@ -242,6 +249,10 @@ type ReportReimbursableAggregate = {
242249
// Number of reimbursable transactions contributing to `total`. Kept alongside `total` so the snapshot's
243250
// result `count` can be patched in lockstep when the report enters or leaves a Your spend section.
244251
count: number;
252+
// The reimbursable transactions that contributed to `total`. The Home row reads its amount from
253+
// `snapshot.search.total`, but the Search page renders its list from `snapshot.data`. These are injected into
254+
// `data` so an offline-submitted report's expenses are visible when the user opens the section offline.
255+
transactions: Transaction[];
245256
};
246257

247258
/** Sums reimbursable transactions (and counts them) in the snapshot currency, optionally restricted to the last 30 days. */
@@ -253,6 +264,7 @@ function getReportReimbursableTotal(
253264
): ReportReimbursableAggregate | null {
254265
let total = 0;
255266
let count = 0;
267+
const transactions: Transaction[] = [];
256268

257269
for (const reportTransaction of reportTransactions) {
258270
if (!reportTransaction || reportTransaction.reimbursable === false) {
@@ -271,9 +283,46 @@ function getReportReimbursableTotal(
271283
}
272284
total += amount;
273285
count += 1;
286+
transactions.push(reportTransaction);
287+
}
288+
289+
return {total, count, transactions};
290+
}
291+
292+
/**
293+
* Builds the snapshot `data` writes that add (or remove) a report's reimbursable transactions when it enters (or
294+
* leaves) a Your spend section. The Home row only needs `search.total`/`count`, but the Search page the row links to
295+
* renders its list from `snapshot.data`; without these writes an offline-submitted report's section opens empty.
296+
*/
297+
function buildSnapshotDataUpdatesForHash(snapshotHash: number | undefined, transactions: Transaction[], enters: boolean, leaves: boolean): YourSpendSnapshotOnyxData {
298+
// Only a section crossing (enter XOR leave) changes membership. Staying put, or no transactions, is a no-op.
299+
if (!snapshotHash || transactions.length === 0 || enters === leaves) {
300+
return {optimisticData: [], successData: [], failureData: []};
274301
}
275302

276-
return {total, count};
303+
const snapshotKey = `${ONYXKEYS.COLLECTION.SNAPSHOT}${snapshotHash}` as const;
304+
// Only patch a snapshot that's actually been loaded; writing into a missing one would create a partial entry.
305+
if (!allSnapshots?.[snapshotKey]?.search) {
306+
return {optimisticData: [], successData: [], failureData: []};
307+
}
308+
309+
const presentData: SearchResultDataType = {};
310+
const absentData: NullishDeep<SearchResultDataType> = {};
311+
for (const transaction of transactions) {
312+
const transactionKey = `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}` as const;
313+
presentData[transactionKey] = transaction;
314+
absentData[transactionKey] = null;
315+
}
316+
317+
// Entering adds the transactions (rollback removes them); leaving removes them (rollback restores them).
318+
const optimisticDataValue = enters ? presentData : absentData;
319+
const failureDataValue = enters ? absentData : presentData;
320+
321+
return {
322+
optimisticData: [{onyxMethod: Onyx.METHOD.MERGE, key: snapshotKey, value: {data: optimisticDataValue}}],
323+
successData: [],
324+
failureData: [{onyxMethod: Onyx.METHOD.MERGE, key: snapshotKey, value: {data: failureDataValue}}],
325+
};
277326
}
278327

279328
/**
@@ -311,6 +360,7 @@ function getYourSpendSnapshotReportMoveUpdates({
311360
const countDiff = (enters ? aggregate.count : 0) - (leaves ? aggregate.count : 0);
312361
if (diff !== 0 || countDiff !== 0) {
313362
mergeYourSpendSnapshotOnyxData(result, buildSnapshotTotalUpdatesForHash(approvalQueryJSON?.hash, diff, approvalTargetCurrency, countDiff));
363+
mergeYourSpendSnapshotOnyxData(result, buildSnapshotDataUpdatesForHash(approvalQueryJSON?.hash, aggregate.transactions, enters, leaves));
314364
}
315365
}
316366
}
@@ -328,6 +378,7 @@ function getYourSpendSnapshotReportMoveUpdates({
328378
const countDiff = (enters ? aggregate.count : 0) - (leaves ? aggregate.count : 0);
329379
if (diff !== 0 || countDiff !== 0) {
330380
mergeYourSpendSnapshotOnyxData(result, buildSnapshotTotalUpdatesForHash(paymentQueryJSON?.hash, diff, paymentTargetCurrency, countDiff));
381+
mergeYourSpendSnapshotOnyxData(result, buildSnapshotDataUpdatesForHash(paymentQueryJSON?.hash, aggregate.transactions, enters, leaves));
331382
}
332383
}
333384
}

tests/actions/IOUTest/GetYourSpendSnapshotReportMoveUpdatesTest.ts

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,13 @@ function buildExpenseReport(overrides: Partial<Report> = {}): Report {
7070
} as Report;
7171
}
7272

73+
// Expense-report transactions are stored with the opposite sign, so a positive stored `amount` (e.g. 10000) is a
74+
// spend that `getAmount`/the snapshot total represent as negative (-10000). Mirrors real Onyx data.
7375
function buildTransaction(overrides: Partial<Transaction> = {}): Transaction {
7476
return {
7577
transactionID: 'reportMoveTxn',
7678
reportID: EXPENSE_REPORT_ID,
77-
amount: -10000,
79+
amount: 10000,
7880
currency: CONST.CURRENCY.USD,
7981
reimbursable: true,
8082
created: '2026-01-15',
@@ -83,6 +85,8 @@ function buildTransaction(overrides: Partial<Transaction> = {}): Transaction {
8385
} as Transaction;
8486
}
8587

88+
const TRANSACTION_KEY = `${ONYXKEYS.COLLECTION.TRANSACTION}reportMoveTxn` as const;
89+
8690
beforeAll(() => {
8791
Onyx.init({
8892
keys: ONYXKEYS,
@@ -120,7 +124,7 @@ async function seedRepaidSnapshot(total: number, currency: string = CONST.CURREN
120124

121125
describe('getYourSpendSnapshotReportMoveUpdates', () => {
122126
it('adds the report total to awaiting approval when a report is submitted (OPEN -> SUBMITTED)', async () => {
123-
const snapshotKey = await seedAwaitingApprovalSnapshot(10000);
127+
const snapshotKey = await seedAwaitingApprovalSnapshot(-10000);
124128

125129
const {optimisticData, failureData} = getYourSpendSnapshotReportMoveUpdates({
126130
iouReport: buildExpenseReport(SUBMITTED_STATUS),
@@ -130,22 +134,32 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
130134
currentUserAccountID: ACCOUNT_ID,
131135
});
132136

137+
// Submitting adds the report's (negative) spend to the section total and injects the transaction into
138+
// `data` so the linked Search page is not empty offline.
133139
expect(optimisticData).toEqual([
134140
expect.objectContaining({
135141
key: snapshotKey,
136-
value: {search: {total: 20000, count: 2, currency: CONST.CURRENCY.USD}},
142+
value: {search: {total: -20000, count: 2, currency: CONST.CURRENCY.USD}},
143+
}),
144+
expect.objectContaining({
145+
key: snapshotKey,
146+
value: {data: {[TRANSACTION_KEY]: buildTransaction()}},
137147
}),
138148
]);
139149
expect(failureData).toEqual([
140150
expect.objectContaining({
141151
key: snapshotKey,
142-
value: {search: {total: 10000, count: 1, currency: CONST.CURRENCY.USD}},
152+
value: {search: {total: -10000, count: 1, currency: CONST.CURRENCY.USD}},
153+
}),
154+
expect.objectContaining({
155+
key: snapshotKey,
156+
value: {data: {[TRANSACTION_KEY]: null}},
143157
}),
144158
]);
145159
});
146160

147161
it('subtracts the report total from awaiting approval when a report is retracted (SUBMITTED -> OPEN)', async () => {
148-
const snapshotKey = await seedAwaitingApprovalSnapshot(30000);
162+
const snapshotKey = await seedAwaitingApprovalSnapshot(-30000);
149163

150164
const {optimisticData} = getYourSpendSnapshotReportMoveUpdates({
151165
iouReport: buildExpenseReport(OPEN_STATUS),
@@ -155,16 +169,21 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
155169
currentUserAccountID: ACCOUNT_ID,
156170
});
157171

172+
// Leaving the section removes the (negative) spend from the total and removes the transaction from `data`.
158173
expect(optimisticData).toEqual([
159174
expect.objectContaining({
160175
key: snapshotKey,
161-
value: {search: {total: 20000, count: 0, currency: CONST.CURRENCY.USD}},
176+
value: {search: {total: -20000, count: 0, currency: CONST.CURRENCY.USD}},
177+
}),
178+
expect.objectContaining({
179+
key: snapshotKey,
180+
value: {data: {[TRANSACTION_KEY]: null}},
162181
}),
163182
]);
164183
});
165184

166185
it('subtracts the report total from awaiting approval when a report is rejected (SUBMITTED -> OPEN)', async () => {
167-
const snapshotKey = await seedAwaitingApprovalSnapshot(30000);
186+
const snapshotKey = await seedAwaitingApprovalSnapshot(-30000);
168187

169188
const {optimisticData} = getYourSpendSnapshotReportMoveUpdates({
170189
iouReport: buildExpenseReport(OPEN_STATUS),
@@ -177,13 +196,17 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
177196
expect(optimisticData).toEqual([
178197
expect.objectContaining({
179198
key: snapshotKey,
180-
value: {search: {total: 20000, count: 0, currency: CONST.CURRENCY.USD}},
199+
value: {search: {total: -20000, count: 0, currency: CONST.CURRENCY.USD}},
200+
}),
201+
expect.objectContaining({
202+
key: snapshotKey,
203+
value: {data: {[TRANSACTION_KEY]: null}},
181204
}),
182205
]);
183206
});
184207

185208
it('adds the report total back to awaiting approval when a report is unapproved (APPROVED -> SUBMITTED)', async () => {
186-
const snapshotKey = await seedAwaitingApprovalSnapshot(10000);
209+
const snapshotKey = await seedAwaitingApprovalSnapshot(-10000);
187210

188211
const {optimisticData} = getYourSpendSnapshotReportMoveUpdates({
189212
iouReport: buildExpenseReport(SUBMITTED_STATUS),
@@ -196,17 +219,22 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
196219
expect(optimisticData).toEqual([
197220
expect.objectContaining({
198221
key: snapshotKey,
199-
value: {search: {total: 20000, count: 2, currency: CONST.CURRENCY.USD}},
222+
value: {search: {total: -20000, count: 2, currency: CONST.CURRENCY.USD}},
223+
}),
224+
expect.objectContaining({
225+
key: snapshotKey,
226+
value: {data: {[TRANSACTION_KEY]: buildTransaction()}},
200227
}),
201228
]);
202229
});
203230

204231
it('subtracts the report total from repaid when a payment is cancelled (REIMBURSED -> APPROVED)', async () => {
205-
const snapshotKey = await seedRepaidSnapshot(30000);
232+
const snapshotKey = await seedRepaidSnapshot(-30000);
233+
const recentTransaction = buildTransaction({created: getRecentCreatedDate()});
206234

207235
const {optimisticData} = getYourSpendSnapshotReportMoveUpdates({
208236
iouReport: buildExpenseReport(APPROVED_STATUS),
209-
reportTransactions: [buildTransaction({created: getRecentCreatedDate()})],
237+
reportTransactions: [recentTransaction],
210238
fromStatus: REIMBURSED_STATUS,
211239
toStatus: APPROVED_STATUS,
212240
currentUserAccountID: ACCOUNT_ID,
@@ -215,7 +243,11 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
215243
expect(optimisticData).toEqual([
216244
expect.objectContaining({
217245
key: snapshotKey,
218-
value: {search: {total: 20000, count: 0, currency: CONST.CURRENCY.USD}},
246+
value: {search: {total: -20000, count: 0, currency: CONST.CURRENCY.USD}},
247+
}),
248+
expect.objectContaining({
249+
key: snapshotKey,
250+
value: {data: {[TRANSACTION_KEY]: null}},
219251
}),
220252
]);
221253
});
@@ -280,11 +312,12 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
280312
});
281313

282314
it('patches using convertedAmount when the snapshot currency differs from the transaction currency', async () => {
283-
const snapshotKey = await seedAwaitingApprovalSnapshot(10000);
315+
const snapshotKey = await seedAwaitingApprovalSnapshot(-10000);
316+
const convertedTransaction = buildTransaction({currency: CONST.CURRENCY.EUR, amount: 10000, convertedAmount: 5000});
284317

285318
const {optimisticData} = getYourSpendSnapshotReportMoveUpdates({
286319
iouReport: buildExpenseReport({...SUBMITTED_STATUS, currency: CONST.CURRENCY.EUR}),
287-
reportTransactions: [buildTransaction({currency: CONST.CURRENCY.EUR, amount: -10000, convertedAmount: -5000})],
320+
reportTransactions: [convertedTransaction],
288321
fromStatus: OPEN_STATUS,
289322
toStatus: SUBMITTED_STATUS,
290323
currentUserAccountID: ACCOUNT_ID,
@@ -293,7 +326,11 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
293326
expect(optimisticData).toEqual([
294327
expect.objectContaining({
295328
key: snapshotKey,
296-
value: {search: {total: 15000, count: 2, currency: CONST.CURRENCY.USD}},
329+
value: {search: {total: -15000, count: 2, currency: CONST.CURRENCY.USD}},
330+
}),
331+
expect.objectContaining({
332+
key: snapshotKey,
333+
value: {data: {[TRANSACTION_KEY]: convertedTransaction}},
297334
}),
298335
]);
299336
});
@@ -307,7 +344,7 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
307344

308345
const {optimisticData} = getYourSpendSnapshotReportMoveUpdates({
309346
iouReport: buildExpenseReport({...SUBMITTED_STATUS, currency: CONST.CURRENCY.GBP}),
310-
reportTransactions: [buildTransaction({currency: CONST.CURRENCY.GBP, amount: -10000, convertedAmount: -5000})],
347+
reportTransactions: [buildTransaction({currency: CONST.CURRENCY.GBP, amount: 10000, convertedAmount: 5000})],
311348
fromStatus: OPEN_STATUS,
312349
toStatus: SUBMITTED_STATUS,
313350
currentUserAccountID: ACCOUNT_ID,
@@ -334,7 +371,11 @@ describe('getYourSpendSnapshotReportMoveUpdates', () => {
334371
expect(optimisticData).toEqual([
335372
expect.objectContaining({
336373
key: snapshotKey,
337-
value: {search: {total: 10000, count: 1, currency: CONST.CURRENCY.USD}},
374+
value: {search: {total: -10000, count: 1, currency: CONST.CURRENCY.USD}},
375+
}),
376+
expect.objectContaining({
377+
key: snapshotKey,
378+
value: {data: {[TRANSACTION_KEY]: buildTransaction()}},
338379
}),
339380
]);
340381
});

0 commit comments

Comments
 (0)