Skip to content

Commit 3f3f7ea

Browse files
ValforteValforte
authored
Merge pull request Expensify#90716 from Expensify/claude-fixStaleSplitDraftTransaction
Add isDraft parameter to setSplitShares/resetSplitShares to prevent stale draft
2 parents 83aea42 + 637d1bd commit 3f3f7ea

3 files changed

Lines changed: 139 additions & 10 deletions

File tree

‎src/libs/actions/IOU/Split.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,15 +1077,17 @@ function completeSplitBill(
10771077
/**
10781078
* Sets the `splitShares` map that holds individual shares of a split bill
10791079
*/
1080-
function setSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, amount: number, currency: string, newAccountIDs: number[]) {
1080+
function setSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, amount: number, currency: string, newAccountIDs: number[], isDraft = true) {
10811081
if (!transaction) {
10821082
return;
10831083
}
10841084

1085+
const collectionKey = isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION;
1086+
10851087
// For pending split distance requests, we don't want to set split shares to zero amount
10861088
// instead we will reset it which would mean splitting the amount equally when the pending distance is resolved.
10871089
if (isDistanceRequestTransactionUtils(transaction) && !amount) {
1088-
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transaction.transactionID}`, {splitShares: null});
1090+
Onyx.merge(`${collectionKey}${transaction.transactionID}`, {splitShares: null});
10891091
return;
10901092
}
10911093

@@ -1116,18 +1118,18 @@ function setSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, amount: n
11161118
return acc;
11171119
}, {});
11181120

1119-
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transaction.transactionID}`, {splitShares});
1121+
Onyx.merge(`${collectionKey}${transaction.transactionID}`, {splitShares});
11201122
}
11211123

1122-
function resetSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, newAmount?: number, currency?: string) {
1124+
function resetSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, newAmount?: number, currency?: string, isDraft = true) {
11231125
if (!transaction) {
11241126
return;
11251127
}
11261128
const accountIDs = Object.keys(transaction.splitShares ?? {}).map((key) => Number(key));
11271129
if (!accountIDs) {
11281130
return;
11291131
}
1130-
setSplitShares(transaction, newAmount ?? transaction.amount, currency ?? transaction.currency, accountIDs);
1132+
setSplitShares(transaction, newAmount ?? transaction.amount, currency ?? transaction.currency, accountIDs, isDraft);
11311133
}
11321134

11331135
function setDraftSplitTransaction(

‎src/pages/iou/request/step/IOURequestStepAmount.tsx‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,12 +397,11 @@ function IOURequestStepAmount({
397397
const saveAmountAndCurrency = ({amount, paymentMethod}: AmountParams) => {
398398
const newAmount = convertToBackendAmount(Number.parseFloat(amount));
399399

400-
// Edits to the amount from the splits page should reset the split shares.
401-
if (transaction?.splitShares) {
402-
resetSplitShares(transaction, newAmount, selectedCurrency);
403-
}
404-
405400
if (!isEditing) {
401+
// Edits to the amount from the splits page should reset the split shares.
402+
if (transaction?.splitShares) {
403+
resetSplitShares(transaction, newAmount, selectedCurrency, true);
404+
}
406405
navigateToNextPage({amount, paymentMethod});
407406
return;
408407
}
@@ -427,6 +426,11 @@ function IOURequestStepAmount({
427426
return;
428427
}
429428

429+
// Reset split shares for non-split-bill edits (split-bill share recalculation is handled by the confirmation list).
430+
if (transaction?.splitShares) {
431+
resetSplitShares(transaction, newAmount, selectedCurrency, false);
432+
}
433+
430434
updateMoneyRequestAmountAndCurrency({
431435
transactionID,
432436
transactionThreadReport: report,

‎tests/actions/IOU/IOUSettersTest.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,16 @@ import {
1111
setMoneyRequestTaxAmount,
1212
setMoneyRequestTaxRate,
1313
} from '@libs/actions/IOU';
14+
import {resetSplitShares, setSplitShares} from '@libs/actions/IOU/Split';
1415
import ONYXKEYS from '@src/ONYXKEYS';
16+
import type Transaction from '@src/types/onyx/Transaction';
1517
import createRandomTransaction from '../../utils/collections/transaction';
1618
import getOnyxValue from '../../utils/getOnyxValue';
1719
import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates';
1820

1921
const TRANSACTION_ID = 'test-txn-1';
22+
const USER_ACCOUNT_ID = 1;
23+
const PARTICIPANT_ACCOUNT_ID = 2;
2024

2125
describe('IOU setter functions', () => {
2226
beforeAll(() => {
@@ -238,4 +242,123 @@ describe('IOU setter functions', () => {
238242
expect(draft?.reimbursable).toBe(false);
239243
});
240244
});
245+
246+
describe('setSplitShares', () => {
247+
function createTransactionWithSplitShares(): Transaction {
248+
const transaction = createRandomTransaction(1);
249+
transaction.transactionID = TRANSACTION_ID;
250+
transaction.amount = 10000;
251+
transaction.currency = 'USD';
252+
transaction.splitShares = {
253+
[USER_ACCOUNT_ID]: {amount: 5000, isModified: false},
254+
[PARTICIPANT_ACCOUNT_ID]: {amount: 5000, isModified: false},
255+
};
256+
return transaction;
257+
}
258+
259+
beforeEach(async () => {
260+
await Onyx.set(ONYXKEYS.SESSION, {accountID: USER_ACCOUNT_ID});
261+
await waitForBatchedUpdates();
262+
});
263+
264+
it('should write splitShares to TRANSACTION_DRAFT by default', async () => {
265+
const transaction = createTransactionWithSplitShares();
266+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`, transaction);
267+
await waitForBatchedUpdates();
268+
269+
setSplitShares(transaction, 10000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID]);
270+
await waitForBatchedUpdates();
271+
272+
const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
273+
expect(draft?.splitShares).toBeDefined();
274+
expect(draft?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
275+
expect(draft?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
276+
});
277+
278+
it('should write splitShares to TRANSACTION when isDraft is false', async () => {
279+
const transaction = createTransactionWithSplitShares();
280+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
281+
await waitForBatchedUpdates();
282+
283+
setSplitShares(transaction, 10000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID], false);
284+
await waitForBatchedUpdates();
285+
286+
const updated = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
287+
expect(updated?.splitShares).toBeDefined();
288+
expect(updated?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
289+
expect(updated?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
290+
});
291+
292+
it('should not create an orphaned TRANSACTION_DRAFT entry when isDraft is false', async () => {
293+
const transaction = createTransactionWithSplitShares();
294+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
295+
await waitForBatchedUpdates();
296+
297+
setSplitShares(transaction, 8000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID], false);
298+
await waitForBatchedUpdates();
299+
300+
const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
301+
expect(draft).toBeUndefined();
302+
});
303+
304+
it('should do nothing when transaction is null', async () => {
305+
setSplitShares(undefined, 10000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID]);
306+
await waitForBatchedUpdates();
307+
308+
const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
309+
expect(draft).toBeUndefined();
310+
});
311+
});
312+
313+
describe('resetSplitShares', () => {
314+
beforeEach(async () => {
315+
await Onyx.set(ONYXKEYS.SESSION, {accountID: USER_ACCOUNT_ID});
316+
await waitForBatchedUpdates();
317+
});
318+
319+
it('should reset splitShares on TRANSACTION_DRAFT by default', async () => {
320+
const transaction = createRandomTransaction(1);
321+
transaction.transactionID = TRANSACTION_ID;
322+
transaction.amount = 6000;
323+
transaction.currency = 'USD';
324+
transaction.splitShares = {
325+
[USER_ACCOUNT_ID]: {amount: 3000, isModified: false},
326+
[PARTICIPANT_ACCOUNT_ID]: {amount: 3000, isModified: false},
327+
};
328+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`, transaction);
329+
await waitForBatchedUpdates();
330+
331+
resetSplitShares(transaction, 8000, 'USD');
332+
await waitForBatchedUpdates();
333+
334+
const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
335+
expect(draft?.splitShares).toBeDefined();
336+
expect(draft?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
337+
expect(draft?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
338+
});
339+
340+
it('should reset splitShares on TRANSACTION when isDraft is false', async () => {
341+
const transaction = createRandomTransaction(1);
342+
transaction.transactionID = TRANSACTION_ID;
343+
transaction.amount = 6000;
344+
transaction.currency = 'USD';
345+
transaction.splitShares = {
346+
[USER_ACCOUNT_ID]: {amount: 3000, isModified: false},
347+
[PARTICIPANT_ACCOUNT_ID]: {amount: 3000, isModified: false},
348+
};
349+
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
350+
await waitForBatchedUpdates();
351+
352+
resetSplitShares(transaction, 8000, 'USD', false);
353+
await waitForBatchedUpdates();
354+
355+
const updated = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
356+
expect(updated?.splitShares).toBeDefined();
357+
expect(updated?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
358+
expect(updated?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
359+
360+
const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
361+
expect(draft).toBeUndefined();
362+
});
363+
});
241364
});

0 commit comments

Comments
 (0)