Add isDraft parameter to setSplitShares/resetSplitShares to prevent stale draft

src/libs/actions/IOU/Split.ts

@@ -1077,15 +1077,17 @@ function completeSplitBill(
 /**
  * Sets the `splitShares` map that holds individual shares of a split bill
  */
-function setSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, amount: number, currency: string, newAccountIDs: number[]) {
+function setSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, amount: number, currency: string, newAccountIDs: number[], isDraft = true) {
     if (!transaction) {
         return;
     }
 
+    const collectionKey = isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION;
+
     // For pending split distance requests, we don't want to set split shares to zero amount
     // instead we will reset it which would mean splitting the amount equally when the pending distance is resolved.
     if (isDistanceRequestTransactionUtils(transaction) && !amount) {
-        Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transaction.transactionID}`, {splitShares: null});
+        Onyx.merge(`${collectionKey}${transaction.transactionID}`, {splitShares: null});
         return;
     }
 
@@ -1116,18 +1118,18 @@ function setSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, amount: n
         return acc;
     }, {});
 
-    Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transaction.transactionID}`, {splitShares});
+    Onyx.merge(`${collectionKey}${transaction.transactionID}`, {splitShares});
 }
 
-function resetSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, newAmount?: number, currency?: string) {
+function resetSplitShares(transaction: OnyxEntry<OnyxTypes.Transaction>, newAmount?: number, currency?: string, isDraft = true) {
     if (!transaction) {
         return;
     }
     const accountIDs = Object.keys(transaction.splitShares ?? {}).map((key) => Number(key));
     if (!accountIDs) {
         return;
     }
-    setSplitShares(transaction, newAmount ?? transaction.amount, currency ?? transaction.currency, accountIDs);
+    setSplitShares(transaction, newAmount ?? transaction.amount, currency ?? transaction.currency, accountIDs, isDraft);
 }
 
 function setDraftSplitTransaction(

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

@@ -397,12 +397,11 @@ function IOURequestStepAmount({
     const saveAmountAndCurrency = ({amount, paymentMethod}: AmountParams) => {
         const newAmount = convertToBackendAmount(Number.parseFloat(amount));
 
-        // Edits to the amount from the splits page should reset the split shares.
-        if (transaction?.splitShares) {
-            resetSplitShares(transaction, newAmount, selectedCurrency);
-        }
-
         if (!isEditing) {
+            // Edits to the amount from the splits page should reset the split shares.
+            if (transaction?.splitShares) {
+                resetSplitShares(transaction, newAmount, selectedCurrency, true);
+            }
             navigateToNextPage({amount, paymentMethod});
             return;
         }
@@ -427,6 +426,11 @@ function IOURequestStepAmount({
             return;
         }
 
+        // Reset split shares for non-split-bill edits (split-bill share recalculation is handled by the confirmation list).
+        if (transaction?.splitShares) {
+            resetSplitShares(transaction, newAmount, selectedCurrency, false);
+        }
+
         updateMoneyRequestAmountAndCurrency({
             transactionID,
             transactionThreadReport: report,

tests/actions/IOU/IOUSettersTest.ts

@@ -11,12 +11,16 @@ import {
     setMoneyRequestTaxAmount,
     setMoneyRequestTaxRate,
 } from '@libs/actions/IOU';
+import {resetSplitShares, setSplitShares} from '@libs/actions/IOU/Split';
 import ONYXKEYS from '@src/ONYXKEYS';
+import type Transaction from '@src/types/onyx/Transaction';
 import createRandomTransaction from '../../utils/collections/transaction';
 import getOnyxValue from '../../utils/getOnyxValue';
 import waitForBatchedUpdates from '../../utils/waitForBatchedUpdates';
 
 const TRANSACTION_ID = 'test-txn-1';
+const USER_ACCOUNT_ID = 1;
+const PARTICIPANT_ACCOUNT_ID = 2;
 
 describe('IOU setter functions', () => {
     beforeAll(() => {
@@ -238,4 +242,123 @@ describe('IOU setter functions', () => {
             expect(draft?.reimbursable).toBe(false);
         });
     });
+
+    describe('setSplitShares', () => {
+        function createTransactionWithSplitShares(): Transaction {
+            const transaction = createRandomTransaction(1);
+            transaction.transactionID = TRANSACTION_ID;
+            transaction.amount = 10000;
+            transaction.currency = 'USD';
+            transaction.splitShares = {
+                [USER_ACCOUNT_ID]: {amount: 5000, isModified: false},
+                [PARTICIPANT_ACCOUNT_ID]: {amount: 5000, isModified: false},
+            };
+            return transaction;
+        }
+
+        beforeEach(async () => {
+            await Onyx.set(ONYXKEYS.SESSION, {accountID: USER_ACCOUNT_ID});
+            await waitForBatchedUpdates();
+        });
+
+        it('should write splitShares to TRANSACTION_DRAFT by default', async () => {
+            const transaction = createTransactionWithSplitShares();
+            await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`, transaction);
+            await waitForBatchedUpdates();
+
+            setSplitShares(transaction, 10000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID]);
+            await waitForBatchedUpdates();
+
+            const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
+            expect(draft?.splitShares).toBeDefined();
+            expect(draft?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
+            expect(draft?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
+        });
+
+        it('should write splitShares to TRANSACTION when isDraft is false', async () => {
+            const transaction = createTransactionWithSplitShares();
+            await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
+            await waitForBatchedUpdates();
+
+            setSplitShares(transaction, 10000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID], false);
+            await waitForBatchedUpdates();
+
+            const updated = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
+            expect(updated?.splitShares).toBeDefined();
+            expect(updated?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
+            expect(updated?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
+        });
+
+        it('should not create an orphaned TRANSACTION_DRAFT entry when isDraft is false', async () => {
+            const transaction = createTransactionWithSplitShares();
+            await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
+            await waitForBatchedUpdates();
+
+            setSplitShares(transaction, 8000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID], false);
+            await waitForBatchedUpdates();
+
+            const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
+            expect(draft).toBeUndefined();
+        });
+
+        it('should do nothing when transaction is null', async () => {
+            setSplitShares(undefined, 10000, 'USD', [USER_ACCOUNT_ID, PARTICIPANT_ACCOUNT_ID]);
+            await waitForBatchedUpdates();
+
+            const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
+            expect(draft).toBeUndefined();
+        });
+    });
+
+    describe('resetSplitShares', () => {
+        beforeEach(async () => {
+            await Onyx.set(ONYXKEYS.SESSION, {accountID: USER_ACCOUNT_ID});
+            await waitForBatchedUpdates();
+        });
+
+        it('should reset splitShares on TRANSACTION_DRAFT by default', async () => {
+            const transaction = createRandomTransaction(1);
+            transaction.transactionID = TRANSACTION_ID;
+            transaction.amount = 6000;
+            transaction.currency = 'USD';
+            transaction.splitShares = {
+                [USER_ACCOUNT_ID]: {amount: 3000, isModified: false},
+                [PARTICIPANT_ACCOUNT_ID]: {amount: 3000, isModified: false},
+            };
+            await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`, transaction);
+            await waitForBatchedUpdates();
+
+            resetSplitShares(transaction, 8000, 'USD');
+            await waitForBatchedUpdates();
+
+            const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
+            expect(draft?.splitShares).toBeDefined();
+            expect(draft?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
+            expect(draft?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
+        });
+
+        it('should reset splitShares on TRANSACTION when isDraft is false', async () => {
+            const transaction = createRandomTransaction(1);
+            transaction.transactionID = TRANSACTION_ID;
+            transaction.amount = 6000;
+            transaction.currency = 'USD';
+            transaction.splitShares = {
+                [USER_ACCOUNT_ID]: {amount: 3000, isModified: false},
+                [PARTICIPANT_ACCOUNT_ID]: {amount: 3000, isModified: false},
+            };
+            await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
+            await waitForBatchedUpdates();
+
+            resetSplitShares(transaction, 8000, 'USD', false);
+            await waitForBatchedUpdates();
+
+            const updated = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`);
+            expect(updated?.splitShares).toBeDefined();
+            expect(updated?.splitShares?.[USER_ACCOUNT_ID]).toBeDefined();
+            expect(updated?.splitShares?.[PARTICIPANT_ACCOUNT_ID]).toBeDefined();
+
+            const draft = await getOnyxValue(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${TRANSACTION_ID}`);
+            expect(draft).toBeUndefined();
+        });
+    });
 });