From 72e0bb96908a0d0b8c9542f32cac33a364be7c2e Mon Sep 17 00:00:00 2001 From: Kevin Brian Bader Date: Fri, 3 Apr 2026 20:07:26 -0700 Subject: [PATCH 1/4] fix: skip auto-redistribution when adding splits with manual edits --- src/libs/actions/IOU/Split.ts | 4 +- tests/unit/SplitExpenseAutoAdjustmentTest.ts | 151 +++++++++++++++---- 2 files changed, 128 insertions(+), 27 deletions(-) diff --git a/src/libs/actions/IOU/Split.ts b/src/libs/actions/IOU/Split.ts index f6da266d7a9d..076e8fb82a01 100644 --- a/src/libs/actions/IOU/Split.ts +++ b/src/libs/actions/IOU/Split.ts @@ -2638,7 +2638,9 @@ function addSplitExpenseField( let redistributedSplitExpenses = updatedSplitExpenses; // Auto-redistribute amounts for all splits if this is not a distance request - if (!isDistanceRequest) { + // Only auto-redistribute if there are no manually edited splits - otherwise respect user's edits + const hasManuallyEditedSplits = existingSplits.some((split) => split.isManuallyEdited); + if (!isDistanceRequest && !hasManuallyEditedSplits) { redistributedSplitExpenses = redistributeSplitExpenseAmounts(updatedSplitExpenses, total, currency); } diff --git a/tests/unit/SplitExpenseAutoAdjustmentTest.ts b/tests/unit/SplitExpenseAutoAdjustmentTest.ts index c9d75521c2c9..a57e7ed4028b 100644 --- a/tests/unit/SplitExpenseAutoAdjustmentTest.ts +++ b/tests/unit/SplitExpenseAutoAdjustmentTest.ts @@ -91,46 +91,145 @@ describe('Split Expense Auto-Adjustment', () => { }); it('should preserve edited splits when adding a new split', async () => { - // Setup: 2 splits - one edited at $3, one unedited at $7 - const initialSplits = [ - createSplitExpense('split1', 300, true), // Edited/locked - createSplitExpense('split2', 700, false), // Unedited - ]; + // Setup: 2 splits - one edited at $3, one unedited at $7 + const initialSplits = [ + createSplitExpense('split1', 300, true), // Edited/locked + createSplitExpense('split2', 700, false), // Unedited + ]; + + const mockTransaction = createMockDraftTransaction(initialSplits); + + await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); + await waitForBatchedUpdates(); + + // Action: Add a new split when manual edits exist + addSplitExpenseField(mockTransaction, mockTransaction, undefined); + await waitForBatchedUpdates(); + + // Verify: New split starts at 0 when manual edits exist (no redistribution) + const draftTransaction = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); - const mockTransaction = createMockDraftTransaction(initialSplits); + const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; + expect(splitExpenses.length).toBe(3); - await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); - await waitForBatchedUpdates(); + // Verify split 1 is still 300 and marked as edited + const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); + expect(split1?.amount).toBe(300); + expect(split1?.isManuallyEdited).toBe(true); - // Action: Add a new split - addSplitExpenseField(mockTransaction, mockTransaction, undefined); - await waitForBatchedUpdates(); + // New split should start at 0 (not auto-redistributed when manual edits exist) + const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); + expect(newSplit?.amount).toBe(0); + expect(newSplit?.isManuallyEdited).toBe(false); - // Verify: 3 splits - // Split 1: locked at 300 - // New split + Split 2: share remaining 700 -> 350 each - const draftTransaction = await new Promise>((resolve) => { - const connection = Onyx.connect({ + // Split2 should remain unchanged at 700 + const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); + expect(split2?.amount).toBe(700); + + // Total will not match original (user needs to manually adjust) + const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); + expect(totalAmount).toBe(1000); // 300 + 700 + 0 + }); + + it('should handle negative amounts when adding split with manual edits', async () => { + // Setup: Original expense is -$10.00 + const negativeTotal = -1000; + const initialSplits = [ + createSplitExpense('split1', -2000, true), // Edited to -$20 + createSplitExpense('split2', 1000, false), // Unedited (remaining) + ]; + + const mockTransaction = createMockDraftTransaction(initialSplits, negativeTotal); + + await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); + await waitForBatchedUpdates(); + + // Action: Add a new split when manual edits exist + addSplitExpenseField(mockTransaction, mockTransaction, undefined); + await waitForBatchedUpdates(); + + // Verify: New split should start at 0, not incorrectly redistributed + const draftTransaction = await new Promise>((resolve) => { + const connection = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, callback: (value) => { Onyx.disconnect(connection); resolve(value); }, + }); }); + + const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; + expect(splitExpenses.length).toBe(3); + + // Verify split 1 is still -2000 and marked as edited + const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); + expect(split1?.amount).toBe(-2000); + expect(split1?.isManuallyEdited).toBe(true); + + // New split should start at 0 (not auto-redistributed) + const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); + expect(newSplit?.amount).toBe(0); + expect(newSplit?.isManuallyEdited).toBe(false); + + // Split2 should remain unchanged at 1000 + const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); + expect(split2?.amount).toBe(1000); + + // Total will not match original (user needs to manually adjust) + const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); + expect(totalAmount).toBe(-1000); // -2000 + 1000 + 0 }); - const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; - expect(splitExpenses.length).toBe(3); + it('should auto-redistribute when adding a split with no manual edits', async () => { + // Setup: 2 unedited splits at $5/$5 + const initialSplits = [ + createSplitExpense('split1', 500, false), + createSplitExpense('split2', 500, false), + ]; + + const mockTransaction = createMockDraftTransaction(initialSplits); + + await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); + await waitForBatchedUpdates(); + + // Action: Add a new split (no manual edits exist) + addSplitExpenseField(mockTransaction, mockTransaction, undefined); + await waitForBatchedUpdates(); + + // Verify: Should redistribute evenly to ~$3.33 each + const draftTransaction = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, + }); + }); - // Verify split 1 is still 300 and marked as edited - const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); - expect(split1?.amount).toBe(300); - expect(split1?.isManuallyEdited).toBe(true); + const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; + expect(splitExpenses.length).toBe(3); - // Verify total matches - const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(TOTAL_AMOUNT); - }); + // All splits should be redistributed evenly to ~$3.33 each + const amounts = splitExpenses.map((s) => s.amount).sort((a, b) => a - b); + expect(amounts).toEqual([333, 333, 334]); + + // All splits should be unedited + expect(splitExpenses.every((split) => !split.isManuallyEdited)).toBe(true); + + // Total should equal original amount + const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); + expect(totalAmount).toBe(TOTAL_AMOUNT); + }); }); describe('removeSplitExpenseField', () => { From 691cb5e39a142710ad82c28fe48b88e3bc1b7257 Mon Sep 17 00:00:00 2001 From: Kevin Brian Bader Date: Fri, 3 Apr 2026 20:09:23 -0700 Subject: [PATCH 2/4] chore: prettier --- tests/unit/SplitExpenseAutoAdjustmentTest.ts | 219 +++++++++---------- 1 file changed, 108 insertions(+), 111 deletions(-) diff --git a/tests/unit/SplitExpenseAutoAdjustmentTest.ts b/tests/unit/SplitExpenseAutoAdjustmentTest.ts index a57e7ed4028b..67f23a8674fe 100644 --- a/tests/unit/SplitExpenseAutoAdjustmentTest.ts +++ b/tests/unit/SplitExpenseAutoAdjustmentTest.ts @@ -91,145 +91,142 @@ describe('Split Expense Auto-Adjustment', () => { }); it('should preserve edited splits when adding a new split', async () => { - // Setup: 2 splits - one edited at $3, one unedited at $7 - const initialSplits = [ - createSplitExpense('split1', 300, true), // Edited/locked - createSplitExpense('split2', 700, false), // Unedited - ]; - - const mockTransaction = createMockDraftTransaction(initialSplits); - - await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); - await waitForBatchedUpdates(); - - // Action: Add a new split when manual edits exist - addSplitExpenseField(mockTransaction, mockTransaction, undefined); - await waitForBatchedUpdates(); - - // Verify: New split starts at 0 when manual edits exist (no redistribution) - const draftTransaction = await new Promise>((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); + // Setup: 2 splits - one edited at $3, one unedited at $7 + const initialSplits = [ + createSplitExpense('split1', 300, true), // Edited/locked + createSplitExpense('split2', 700, false), // Unedited + ]; + + const mockTransaction = createMockDraftTransaction(initialSplits); + + await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); + await waitForBatchedUpdates(); + + // Action: Add a new split when manual edits exist + addSplitExpenseField(mockTransaction, mockTransaction, undefined); + await waitForBatchedUpdates(); + + // Verify: New split starts at 0 when manual edits exist (no redistribution) + const draftTransaction = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, }); + }); - const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; - expect(splitExpenses.length).toBe(3); + const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; + expect(splitExpenses.length).toBe(3); - // Verify split 1 is still 300 and marked as edited - const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); - expect(split1?.amount).toBe(300); - expect(split1?.isManuallyEdited).toBe(true); + // Verify split 1 is still 300 and marked as edited + const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); + expect(split1?.amount).toBe(300); + expect(split1?.isManuallyEdited).toBe(true); - // New split should start at 0 (not auto-redistributed when manual edits exist) - const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); - expect(newSplit?.amount).toBe(0); - expect(newSplit?.isManuallyEdited).toBe(false); + // New split should start at 0 (not auto-redistributed when manual edits exist) + const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); + expect(newSplit?.amount).toBe(0); + expect(newSplit?.isManuallyEdited).toBe(false); - // Split2 should remain unchanged at 700 - const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); - expect(split2?.amount).toBe(700); + // Split2 should remain unchanged at 700 + const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); + expect(split2?.amount).toBe(700); - // Total will not match original (user needs to manually adjust) - const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(1000); // 300 + 700 + 0 - }); + // Total will not match original (user needs to manually adjust) + const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); + expect(totalAmount).toBe(1000); // 300 + 700 + 0 + }); - it('should handle negative amounts when adding split with manual edits', async () => { - // Setup: Original expense is -$10.00 - const negativeTotal = -1000; - const initialSplits = [ - createSplitExpense('split1', -2000, true), // Edited to -$20 - createSplitExpense('split2', 1000, false), // Unedited (remaining) - ]; + it('should handle negative amounts when adding split with manual edits', async () => { + // Setup: Original expense is -$10.00 + const negativeTotal = -1000; + const initialSplits = [ + createSplitExpense('split1', -2000, true), // Edited to -$20 + createSplitExpense('split2', 1000, false), // Unedited (remaining) + ]; - const mockTransaction = createMockDraftTransaction(initialSplits, negativeTotal); + const mockTransaction = createMockDraftTransaction(initialSplits, negativeTotal); - await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); - await waitForBatchedUpdates(); + await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); + await waitForBatchedUpdates(); - // Action: Add a new split when manual edits exist - addSplitExpenseField(mockTransaction, mockTransaction, undefined); - await waitForBatchedUpdates(); + // Action: Add a new split when manual edits exist + addSplitExpenseField(mockTransaction, mockTransaction, undefined); + await waitForBatchedUpdates(); - // Verify: New split should start at 0, not incorrectly redistributed - const draftTransaction = await new Promise>((resolve) => { - const connection = Onyx.connect({ + // Verify: New split should start at 0, not incorrectly redistributed + const draftTransaction = await new Promise>((resolve) => { + const connection = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, callback: (value) => { Onyx.disconnect(connection); resolve(value); }, - }); }); + }); + + const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; + expect(splitExpenses.length).toBe(3); - const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; - expect(splitExpenses.length).toBe(3); + // Verify split 1 is still -2000 and marked as edited + const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); + expect(split1?.amount).toBe(-2000); + expect(split1?.isManuallyEdited).toBe(true); - // Verify split 1 is still -2000 and marked as edited - const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); - expect(split1?.amount).toBe(-2000); - expect(split1?.isManuallyEdited).toBe(true); + // New split should start at 0 (not auto-redistributed) + const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); + expect(newSplit?.amount).toBe(0); + expect(newSplit?.isManuallyEdited).toBe(false); - // New split should start at 0 (not auto-redistributed) - const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); - expect(newSplit?.amount).toBe(0); - expect(newSplit?.isManuallyEdited).toBe(false); + // Split2 should remain unchanged at 1000 + const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); + expect(split2?.amount).toBe(1000); - // Split2 should remain unchanged at 1000 - const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); - expect(split2?.amount).toBe(1000); + // Total will not match original (user needs to manually adjust) + const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); + expect(totalAmount).toBe(-1000); // -2000 + 1000 + 0 + }); - // Total will not match original (user needs to manually adjust) - const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(-1000); // -2000 + 1000 + 0 - }); + it('should auto-redistribute when adding a split with no manual edits', async () => { + // Setup: 2 unedited splits at $5/$5 + const initialSplits = [createSplitExpense('split1', 500, false), createSplitExpense('split2', 500, false)]; - it('should auto-redistribute when adding a split with no manual edits', async () => { - // Setup: 2 unedited splits at $5/$5 - const initialSplits = [ - createSplitExpense('split1', 500, false), - createSplitExpense('split2', 500, false), - ]; - - const mockTransaction = createMockDraftTransaction(initialSplits); - - await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); - await waitForBatchedUpdates(); - - // Action: Add a new split (no manual edits exist) - addSplitExpenseField(mockTransaction, mockTransaction, undefined); - await waitForBatchedUpdates(); - - // Verify: Should redistribute evenly to ~$3.33 each - const draftTransaction = await new Promise>((resolve) => { - const connection = Onyx.connect({ - key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, - callback: (value) => { - Onyx.disconnect(connection); - resolve(value); - }, - }); + const mockTransaction = createMockDraftTransaction(initialSplits); + + await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); + await waitForBatchedUpdates(); + + // Action: Add a new split (no manual edits exist) + addSplitExpenseField(mockTransaction, mockTransaction, undefined); + await waitForBatchedUpdates(); + + // Verify: Should redistribute evenly to ~$3.33 each + const draftTransaction = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, + callback: (value) => { + Onyx.disconnect(connection); + resolve(value); + }, }); + }); - const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; - expect(splitExpenses.length).toBe(3); + const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; + expect(splitExpenses.length).toBe(3); - // All splits should be redistributed evenly to ~$3.33 each - const amounts = splitExpenses.map((s) => s.amount).sort((a, b) => a - b); - expect(amounts).toEqual([333, 333, 334]); + // All splits should be redistributed evenly to ~$3.33 each + const amounts = splitExpenses.map((s) => s.amount).sort((a, b) => a - b); + expect(amounts).toEqual([333, 333, 334]); - // All splits should be unedited - expect(splitExpenses.every((split) => !split.isManuallyEdited)).toBe(true); + // All splits should be unedited + expect(splitExpenses.every((split) => !split.isManuallyEdited)).toBe(true); - // Total should equal original amount - const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(TOTAL_AMOUNT); - }); + // Total should equal original amount + const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); + expect(totalAmount).toBe(TOTAL_AMOUNT); + }); }); describe('removeSplitExpenseField', () => { From 2e309483da394717e9849342a4494297cee7a1e9 Mon Sep 17 00:00:00 2001 From: Kevin Brian Bader Date: Sun, 5 Apr 2026 02:25:12 -0700 Subject: [PATCH 3/4] =?UTF-8?q?fix:=20rework=20logic=20=E2=99=BB=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/libs/actions/IOU/Split.ts | 9 ++- tests/unit/SplitExpenseAutoAdjustmentTest.ts | 84 +++++++++----------- 2 files changed, 42 insertions(+), 51 deletions(-) diff --git a/src/libs/actions/IOU/Split.ts b/src/libs/actions/IOU/Split.ts index 076e8fb82a01..a33e7ef0ae6f 100644 --- a/src/libs/actions/IOU/Split.ts +++ b/src/libs/actions/IOU/Split.ts @@ -2635,12 +2635,15 @@ function addSplitExpenseField( const currency = getCurrency(draftTransaction); const originalTransactionID = draftTransaction.comment?.originalTransactionID ?? transaction.transactionID; + // Check if existing splits already sum to the total + const existingSum = existingSplits.reduce((sum, s) => sum + s.amount, 0); + const splitsAlreadyMatchTotal = Math.abs(existingSum) === Math.abs(total); + let redistributedSplitExpenses = updatedSplitExpenses; // Auto-redistribute amounts for all splits if this is not a distance request - // Only auto-redistribute if there are no manually edited splits - otherwise respect user's edits - const hasManuallyEditedSplits = existingSplits.some((split) => split.isManuallyEdited); - if (!isDistanceRequest && !hasManuallyEditedSplits) { + // Skip redistribution if existing splits already match the total (new split stays at 0) + if (!isDistanceRequest && !splitsAlreadyMatchTotal) { redistributedSplitExpenses = redistributeSplitExpenseAmounts(updatedSplitExpenses, total, currency); } diff --git a/tests/unit/SplitExpenseAutoAdjustmentTest.ts b/tests/unit/SplitExpenseAutoAdjustmentTest.ts index 67f23a8674fe..c7ccfcf7ade0 100644 --- a/tests/unit/SplitExpenseAutoAdjustmentTest.ts +++ b/tests/unit/SplitExpenseAutoAdjustmentTest.ts @@ -102,11 +102,13 @@ describe('Split Expense Auto-Adjustment', () => { await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); await waitForBatchedUpdates(); - // Action: Add a new split when manual edits exist + // Action: Add a new split addSplitExpenseField(mockTransaction, mockTransaction, undefined); await waitForBatchedUpdates(); - // Verify: New split starts at 0 when manual edits exist (no redistribution) + // Verify: 3 splits + // Split 1: locked at 300 + // New split + Split 2: share remaining 700 -> 350 each const draftTransaction = await new Promise>((resolve) => { const connection = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, @@ -125,38 +127,29 @@ describe('Split Expense Auto-Adjustment', () => { expect(split1?.amount).toBe(300); expect(split1?.isManuallyEdited).toBe(true); - // New split should start at 0 (not auto-redistributed when manual edits exist) - const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); - expect(newSplit?.amount).toBe(0); - expect(newSplit?.isManuallyEdited).toBe(false); - - // Split2 should remain unchanged at 700 - const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); - expect(split2?.amount).toBe(700); - - // Total will not match original (user needs to manually adjust) + // Verify total matches const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(1000); // 300 + 700 + 0 + expect(totalAmount).toBe(TOTAL_AMOUNT); }); - it('should handle negative amounts when adding split with manual edits', async () => { - // Setup: Original expense is -$10.00 - const negativeTotal = -1000; + it('should keep new split at 0 when existing splits sum to total', async () => { + // Setup: Total -$10.00, splits already sum to -$10.00 + const total = -1000; const initialSplits = [ - createSplitExpense('split1', -2000, true), // Edited to -$20 - createSplitExpense('split2', 1000, false), // Unedited (remaining) + createSplitExpense('split1', -500), // -$5.00 + createSplitExpense('split2', -500), // -$5.00 = -$10.00 total ]; - const mockTransaction = createMockDraftTransaction(initialSplits, negativeTotal); + const mockTransaction = createMockDraftTransaction(initialSplits, total); await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); await waitForBatchedUpdates(); - // Action: Add a new split when manual edits exist + // Action: Add new split addSplitExpenseField(mockTransaction, mockTransaction, undefined); await waitForBatchedUpdates(); - // Verify: New split should start at 0, not incorrectly redistributed + // Verify: New split should stay at 0 (not redistributed) const draftTransaction = await new Promise>((resolve) => { const connection = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, @@ -170,39 +163,37 @@ describe('Split Expense Auto-Adjustment', () => { const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; expect(splitExpenses.length).toBe(3); - // Verify split 1 is still -2000 and marked as edited - const split1 = splitExpenses.find((s) => s.transactionID === 'split1'); - expect(split1?.amount).toBe(-2000); - expect(split1?.isManuallyEdited).toBe(true); + // Original splits unchanged + expect(splitExpenses.at(0)?.amount).toBe(-500); + expect(splitExpenses.at(1)?.amount).toBe(-500); - // New split should start at 0 (not auto-redistributed) - const newSplit = splitExpenses.find((s) => s.transactionID !== 'split1' && s.transactionID !== 'split2'); - expect(newSplit?.amount).toBe(0); - expect(newSplit?.isManuallyEdited).toBe(false); + // New split is 0, not redistributed + expect(splitExpenses.at(2)?.amount).toBe(0); + expect(splitExpenses.at(2)?.isManuallyEdited).toBe(false); - // Split2 should remain unchanged at 1000 - const split2 = splitExpenses.find((s) => s.transactionID === 'split2'); - expect(split2?.amount).toBe(1000); - - // Total will not match original (user needs to manually adjust) + // Total should be -$10.00 + $0 = -$10.00 (unchanged) const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(-1000); // -2000 + 1000 + 0 + expect(totalAmount).toBe(-1000); }); - it('should auto-redistribute when adding a split with no manual edits', async () => { - // Setup: 2 unedited splits at $5/$5 - const initialSplits = [createSplitExpense('split1', 500, false), createSplitExpense('split2', 500, false)]; + it('should redistribute when existing splits do not sum to total', async () => { + // Setup: Total -$10.00, splits sum to -$8.00 (don't match) + const total = -1000; // -$10.00 + const initialSplits = [ + createSplitExpense('split1', -300), // -$3.00 + createSplitExpense('split2', -500), // -$5.00 = -$8.00 total (mismatch!) + ]; - const mockTransaction = createMockDraftTransaction(initialSplits); + const mockTransaction = createMockDraftTransaction(initialSplits, total); await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction); await waitForBatchedUpdates(); - // Action: Add a new split (no manual edits exist) + // Action: Add new split addSplitExpenseField(mockTransaction, mockTransaction, undefined); await waitForBatchedUpdates(); - // Verify: Should redistribute evenly to ~$3.33 each + // Verify: All unedited splits should be redistributed const draftTransaction = await new Promise>((resolve) => { const connection = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, @@ -216,16 +207,13 @@ describe('Split Expense Auto-Adjustment', () => { const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; expect(splitExpenses.length).toBe(3); - // All splits should be redistributed evenly to ~$3.33 each + // All splits redistributed to sum to -$10.00 const amounts = splitExpenses.map((s) => s.amount).sort((a, b) => a - b); - expect(amounts).toEqual([333, 333, 334]); + expect(amounts).toEqual([-334, -333, -333]); // -$3.34, -$3.33, -$3.33 - // All splits should be unedited - expect(splitExpenses.every((split) => !split.isManuallyEdited)).toBe(true); - - // Total should equal original amount + // Total matches -$10.00 const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0); - expect(totalAmount).toBe(TOTAL_AMOUNT); + expect(totalAmount).toBe(-1000); }); }); From 58e67c50c2fbbdd4a4f4744cd6b4b5d5fdc4a308 Mon Sep 17 00:00:00 2001 From: Kevin Brian Bader Date: Sun, 5 Apr 2026 03:10:43 -0700 Subject: [PATCH 4/4] fix: limit redistribution skip to manually edited splits --- src/libs/actions/IOU/Split.ts | 9 +++++---- tests/unit/SplitExpenseAutoAdjustmentTest.ts | 14 +++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/libs/actions/IOU/Split.ts b/src/libs/actions/IOU/Split.ts index a33e7ef0ae6f..ba93852e54bb 100644 --- a/src/libs/actions/IOU/Split.ts +++ b/src/libs/actions/IOU/Split.ts @@ -2636,14 +2636,15 @@ function addSplitExpenseField( const originalTransactionID = draftTransaction.comment?.originalTransactionID ?? transaction.transactionID; // Check if existing splits already sum to the total - const existingSum = existingSplits.reduce((sum, s) => sum + s.amount, 0); + const existingSum = existingSplits.reduce((sum, split) => sum + split.amount, 0); + const hasManuallyEditedSplits = existingSplits.some((split) => split.isManuallyEdited); const splitsAlreadyMatchTotal = Math.abs(existingSum) === Math.abs(total); let redistributedSplitExpenses = updatedSplitExpenses; - // Auto-redistribute amounts for all splits if this is not a distance request - // Skip redistribution if existing splits already match the total (new split stays at 0) - if (!isDistanceRequest && !splitsAlreadyMatchTotal) { + // Skip redistribution only when manual edits exist AND splits sum to total + const shouldRedistribute = !splitsAlreadyMatchTotal || !hasManuallyEditedSplits; + if (!isDistanceRequest && shouldRedistribute) { redistributedSplitExpenses = redistributeSplitExpenseAmounts(updatedSplitExpenses, total, currency); } diff --git a/tests/unit/SplitExpenseAutoAdjustmentTest.ts b/tests/unit/SplitExpenseAutoAdjustmentTest.ts index c7ccfcf7ade0..db75f6b71b94 100644 --- a/tests/unit/SplitExpenseAutoAdjustmentTest.ts +++ b/tests/unit/SplitExpenseAutoAdjustmentTest.ts @@ -132,12 +132,12 @@ describe('Split Expense Auto-Adjustment', () => { expect(totalAmount).toBe(TOTAL_AMOUNT); }); - it('should keep new split at 0 when existing splits sum to total', async () => { + it('should keep new split at 0 when manual edits exist and sum matches total', async () => { // Setup: Total -$10.00, splits already sum to -$10.00 const total = -1000; const initialSplits = [ - createSplitExpense('split1', -500), // -$5.00 - createSplitExpense('split2', -500), // -$5.00 = -$10.00 total + createSplitExpense('split1', -500, true), // -$5.00 (edited) + createSplitExpense('split2', -500, true), // -$5.00 (edited) ]; const mockTransaction = createMockDraftTransaction(initialSplits, total); @@ -176,12 +176,12 @@ describe('Split Expense Auto-Adjustment', () => { expect(totalAmount).toBe(-1000); }); - it('should redistribute when existing splits do not sum to total', async () => { + it('should redistribute when no manual edits exist', async () => { // Setup: Total -$10.00, splits sum to -$8.00 (don't match) const total = -1000; // -$10.00 const initialSplits = [ - createSplitExpense('split1', -300), // -$3.00 - createSplitExpense('split2', -500), // -$5.00 = -$8.00 total (mismatch!) + createSplitExpense('split1', -500, false), // -$5.00 (unedited) + createSplitExpense('split2', -500, false), // -$5.00 (unedited) ]; const mockTransaction = createMockDraftTransaction(initialSplits, total); @@ -207,7 +207,7 @@ describe('Split Expense Auto-Adjustment', () => { const splitExpenses = draftTransaction?.comment?.splitExpenses ?? []; expect(splitExpenses.length).toBe(3); - // All splits redistributed to sum to -$10.00 + // Should redistribute evenly (splits sum to total, but NO manual edits) const amounts = splitExpenses.map((s) => s.amount).sort((a, b) => a - b); expect(amounts).toEqual([-334, -333, -333]); // -$3.34, -$3.33, -$3.33