Skip to content

fix: skip auto-redistribution when adding splits with manual edits #87124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
blimpich merged 4 commits into from
Apr 6, 2026
Merged

fix: skip auto-redistribution when adding splits with manual edits #87124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
72e0bb9
fix: skip auto-redistribution when adding splits with manual edits
ikevin127 Apr 4, 2026
691cb5e
chore: prettier
ikevin127 Apr 4, 2026
2e30948
fix: rework logic ♻️
ikevin127 Apr 5, 2026
58e67c5
fix: limit redistribution skip to manually edited splits
ikevin127 Apr 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/libs/actions/IOU/Split.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2635,10 +2635,16 @@ 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, split) => sum + split.amount, 0);
const hasManuallyEditedSplits = existingSplits.some((split) => split.isManuallyEdited);
const splitsAlreadyMatchTotal = Math.abs(existingSum) === Math.abs(total);
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Compare split totals with sign before skipping redistribution

Using Math.abs(existingSum) === Math.abs(total) treats opposite-signed totals as a match, so a negative transaction can incorrectly skip redistribution when manually edited splits sum to the positive magnitude (for example, +1000 vs -1000). In that case shouldRedistribute becomes false, the added split stays at 0, and the draft remains unbalanced, which then blocks saving with validation errors; this regression is introduced by the new skip condition and can be avoided by checking signed equality instead of absolute equality.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@ikevin127 ikevin127 Apr 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intended, as using absolute values to check equality between splits sum amount and total transaction amount is the only reliable way that works for both positive and negative amounts ✅


let redistributedSplitExpenses = updatedSplitExpenses;

// Auto-redistribute amounts for all splits if this is not a distance request
if (!isDistanceRequest) {
// Skip redistribution only when manual edits exist AND splits sum to total
const shouldRedistribute = !splitsAlreadyMatchTotal || !hasManuallyEditedSplits;
if (!isDistanceRequest && shouldRedistribute) {
redistributedSplitExpenses = redistributeSplitExpenseAmounts(updatedSplitExpenses, total, currency);
}

Expand Down
84 changes: 84 additions & 0 deletions tests/unit/SplitExpenseAutoAdjustmentTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,90 @@ describe('Split Expense Auto-Adjustment', () => {
const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0);
expect(totalAmount).toBe(TOTAL_AMOUNT);
});

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, true), // -$5.00 (edited)
createSplitExpense('split2', -500, true), // -$5.00 (edited)
];

const mockTransaction = createMockDraftTransaction(initialSplits, total);

await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction);
await waitForBatchedUpdates();

// Action: Add new split
addSplitExpenseField(mockTransaction, mockTransaction, undefined);
await waitForBatchedUpdates();

// Verify: New split should stay at 0 (not redistributed)
const draftTransaction = await new Promise<OnyxEntry<Transaction>>((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);

// Original splits unchanged
expect(splitExpenses.at(0)?.amount).toBe(-500);
expect(splitExpenses.at(1)?.amount).toBe(-500);

// New split is 0, not redistributed
expect(splitExpenses.at(2)?.amount).toBe(0);
expect(splitExpenses.at(2)?.isManuallyEdited).toBe(false);

// Total should be -$10.00 + $0 = -$10.00 (unchanged)
const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0);
expect(totalAmount).toBe(-1000);
});

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', -500, false), // -$5.00 (unedited)
createSplitExpense('split2', -500, false), // -$5.00 (unedited)
];

const mockTransaction = createMockDraftTransaction(initialSplits, total);

await Onyx.set(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${ORIGINAL_TRANSACTION_ID}`, mockTransaction);
await waitForBatchedUpdates();

// Action: Add new split
addSplitExpenseField(mockTransaction, mockTransaction, undefined);
await waitForBatchedUpdates();

// Verify: All unedited splits should be redistributed
const draftTransaction = await new Promise<OnyxEntry<Transaction>>((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);

// 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

// Total matches -$10.00
const totalAmount = splitExpenses.reduce((sum, split) => sum + split.amount, 0);
expect(totalAmount).toBe(-1000);
});
});

describe('removeSplitExpenseField', () => {
Expand Down
Loading