diff --git a/docs/testing-strategy.md b/docs/testing-strategy.md index 90d4d7852..8df9fb0c2 100644 --- a/docs/testing-strategy.md +++ b/docs/testing-strategy.md @@ -72,7 +72,8 @@ removes disposable volumes; native teardown stops the server and preserves state are no shared authentication files or setup dependency. Cookies follow the configured host. The transport test checks BigInt/Date serialization through the actual Next.js API endpoint. -`pnpm test` selects `src/**/*.{test,spec}.{ts,tsx}` except `src/tests/integration/`. +`pnpm test` selects `src/**/*.{test,spec}.{ts,tsx}` and `tests/**/*.{test,spec}.{ts,tsx}`, +except `src/tests/integration/` and `tests/e2e/`. `pnpm test:integration` selects that integration directory only. Playwright selects `tests/e2e/`. Failures retain traces, screenshots, videos, and the HTML report even when local retries are disabled. CI uploads `test-results/e2e` and `playwright-report` on failure and always stops its diff --git a/jest.config.ts b/jest.config.ts index 2b482a51a..70f428282 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -4,6 +4,9 @@ export default () => configureJest({ testEnvironment: 'node', setupFilesAfterEnv: ['/src/tests/setup/unit.ts'], - testMatch: ['/src/**/*.{test,spec}.{ts,tsx}'], - testPathIgnorePatterns: ['/src/tests/integration/'], + testMatch: [ + '/src/**/*.{test,spec}.{ts,tsx}', + '/tests/**/*.{test,spec}.{ts,tsx}', + ], + testPathIgnorePatterns: ['/src/tests/integration/', '/tests/e2e/'], }); diff --git a/src/lib/shareSplit.ts b/src/lib/shareSplit.ts new file mode 100644 index 000000000..92325f4b8 --- /dev/null +++ b/src/lib/shareSplit.ts @@ -0,0 +1,145 @@ +import { BigMath } from '~/utils/numbers'; + +const MAX_SHARE_OPERATIONS = 20000; + +interface WeightBounds { + min: bigint; + max: bigint; +} + +const getRoundedWeightBounds = ( + cost: bigint, + totalCost: bigint, + weightCount: bigint, +): WeightBounds => ({ + min: BigMath.max(1n, ((cost - 1n) * weightCount + totalCost - 1n) / totalCost), + max: ((cost + 1n) * weightCount - 1n) / totalCost, +}); + +/** Round proportional weights to a fixed total, respecting any required rounding intervals. */ +const apportionWeights = ( + costs: bigint[], + totalCost: bigint, + weightCount: bigint, + bounds = costs.map(() => ({ min: 0n, max: weightCount })), +): bigint[] | undefined => { + if ( + bounds.some(({ min, max }) => min > max) || + weightCount < bounds.reduce((sum, { min }) => sum + min, 0n) || + weightCount > bounds.reduce((sum, { max }) => sum + max, 0n) + ) { + return undefined; + } + + const weights = costs.map((cost, index) => + BigMath.min( + bounds[index]!.max, + BigMath.max(bounds[index]!.min, (cost * weightCount) / totalCost), + ), + ); + const remaining = weightCount - weights.reduce((sum, weight) => sum + weight, 0n); + if (0n === remaining) { + return weights; + } + if (0n > remaining) { + return undefined; + } + + const candidates = costs + .map((cost, index) => ({ index, deficit: cost * weightCount - weights[index]! * totalCost })) + .filter(({ index }) => weights[index]! < bounds[index]!.max) + .toSorted((a, b) => + a.deficit === b.deficit ? a.index - b.index : a.deficit > b.deficit ? -1 : 1, + ); + + // Rounding down leaves fewer than one extra unit per participant to allocate. + if (remaining > BigInt(candidates.length)) { + return undefined; + } + candidates.slice(0, Number(remaining)).forEach(({ index }) => { + weights[index]! += 1n; + }); + return weights; +}; + +/** Prefer whole percentages, then tenths, then hundredths, always summing to 100%. */ +export const recoverPercentageWeights = (costs: bigint[]): bigint[] => { + const totalCost = costs.reduce((sum, cost) => sum + cost, 0n); + if (0n === totalCost) { + return costs.map(() => 0n); + } + + // Exactly three precision levels: the number of candidates never grows with the amount. + for (const weightCount of [100n, 1000n, 10000n]) { + const bounds = costs.map((cost) => + 0n === cost ? { min: 0n, max: 0n } : getRoundedWeightBounds(cost, totalCost, weightCount), + ); + const weights = apportionWeights(costs, totalCost, weightCount, bounds); + if (weights) { + return weights.map((weight) => (weight * 10000n) / weightCount); + } + } + + // Some tiny portions cannot be represented at 0.01% precision. Use the nearest + // Balanced percentages rather than returning an invalid total such as 99.99%. + return apportionWeights(costs, totalCost, 10000n)!; +}; + +/** Find the smallest integer ratio consistent with truncation and leftover-penny allocation. */ +export const recoverShareWeights = (costs: bigint[]): bigint[] => { + const positiveCosts = costs.filter((cost) => 0n < cost); + const exactGcd = positiveCosts.reduce((gcd, cost) => BigMath.gcd(gcd, cost), 0n); + const exactWeights = costs.map((cost) => (0n === exactGcd ? 0n : (cost * 100n) / exactGcd)); + + if (2 > positiveCosts.length) { + return exactWeights; + } + + const totalCost = positiveCosts.reduce((sum, cost) => sum + cost, 0n); + const exactShareCount = totalCost / exactGcd; + let operations = 0; + + // Search total share counts in ascending order, checking each candidate in linear time. + // Allowed weight intervals avoid enumerating combinations of pennies. + for (let shareCount = BigInt(positiveCosts.length); shareCount < exactShareCount; shareCount++) { + const bounds: { min: bigint; max: bigint }[] = []; + let minSum = 0n; + let maxSum = 0n; + + for (const cost of positiveCosts) { + if (MAX_SHARE_OPERATIONS <= operations) { + return exactWeights; + } + operations++; + + // A cost equals floor(totalCost * weight / shareCount), with at most one extra penny. + // Thus: cost - 1 <= totalCost * weight / shareCount < cost + 1. + const { min, max } = getRoundedWeightBounds(cost, totalCost, shareCount); + if (min > max || minSum + min > shareCount) { + break; + } + + bounds.push({ min, max }); + minSum += min; + maxSum += max; + } + + if (bounds.length === positiveCosts.length && shareCount <= maxSum) { + let remaining = shareCount - minSum; + let positiveIndex = 0; + + return costs.map((cost) => { + if (0n === cost) { + return 0n; + } + + const { min, max } = bounds[positiveIndex++]!; + const extra = BigMath.min(remaining, max - min); + remaining -= extra; + return (min + extra) * 100n; + }); + } + } + + return exactWeights; +}; diff --git a/src/store/addStore.ts b/src/store/addStore.ts index 0dbf1d40b..39b2d0359 100644 --- a/src/store/addStore.ts +++ b/src/store/addStore.ts @@ -4,6 +4,7 @@ import { create } from 'zustand'; import { DEFAULT_CATEGORY } from '~/lib/category'; import { type CurrencyCode } from '~/lib/currency'; +import { recoverPercentageWeights, recoverShareWeights } from '~/lib/shareSplit'; import type { TransactionAddInputModel } from '~/types'; import { shuffleArray } from '~/utils/array'; import { BigMath } from '~/utils/numbers'; @@ -304,7 +305,7 @@ export function calculateParticipantSplit( amount: ((getSplitShare(p) ?? 0n) * amount) / 10000n, })); canSplitScreenClosed = - 0 === 100 - participants.reduce((acc, p) => acc + Number(getSplitShare(p) ?? 0n) / 100, 0); + 10000n === participants.reduce((acc, p) => acc + (getSplitShare(p) ?? 0n), 0n); break; case SplitType.SHARE: const totalShare = participants.reduce((acc, p) => acc + Number(getSplitShare(p) ?? 0n), 0); @@ -406,34 +407,19 @@ export function calculateSplitShareBasedOnAmount( break; case SplitType.PERCENTAGE: - participants.forEach((p) => { - splitShares[p.id]![splitType] = - 0n === amount - ? 0n - : paidBy?.id !== p.id - ? (BigMath.abs(p.amount ?? 0n) * 10000n) / amount - : (BigMath.abs(amount - (p.amount ?? 0n)) * 10000n) / amount; - }); - - break; - case SplitType.SHARE: - const amounts = participants - .filter(({ amount }) => Boolean(amount)) - .map((p) => - p.id === paidBy?.id ? BigMath.abs(amount - p.amount!) : BigMath.abs(p.amount!), - ) - .filter((s) => s !== 0n); - - const gcdValue = amounts.length > 1 ? amounts.reduce((a, b) => BigMath.gcd(a, b)) : 1n; - - participants.forEach((p) => { - splitShares[p.id]![splitType] = - 0n === amount - ? 0n - : ((p.id === paidBy?.id ? BigMath.abs(amount - p.amount!) : BigMath.abs(p.amount!)) * - 100n) / - gcdValue; + const costs = participants.map((p) => + p.id === paidBy?.id ? BigMath.abs(amount - (p.amount ?? 0n)) : BigMath.abs(p.amount ?? 0n), + ); + const shares = + 0n === amount + ? costs.map(() => 0n) + : SplitType.PERCENTAGE === splitType + ? recoverPercentageWeights(costs) + : recoverShareWeights(costs); + + participants.forEach((p, index) => { + splitShares[p.id]![splitType] = shares[index]; }); break; diff --git a/src/tests/addStore.test.ts b/src/tests/addStore.test.ts index 026f654d0..51319ccd3 100644 --- a/src/tests/addStore.test.ts +++ b/src/tests/addStore.test.ts @@ -664,6 +664,86 @@ describe('calculateSplitShareBasedOnAmount', () => { }); describe('SplitType.SHARE', () => { + it.each([ + { + scenario: 'the payer received the penny', + amount: 7793n, + balances: [4675n, -4675n], + shares: [200n, 300n], + }, + { + scenario: 'the other participant received the penny', + amount: 7793n, + balances: [4676n, -4676n], + shares: [200n, 300n], + }, + { + scenario: 'two participants received pennies', + amount: 12002n, + balances: [6001n, -3001n, -3000n], + shares: [200n, 100n, 100n], + }, + { + scenario: 'a participant has no share', + amount: 7793n, + balances: [4675n, -4675n, 0n], + shares: [200n, 300n, 0n], + }, + { + scenario: 'the payer has no share', + amount: 7793n, + balances: [7793n, -3118n, -4675n], + shares: [0n, 200n, 300n], + }, + { + scenario: 'the payer has a zero net balance', + amount: 7793n, + balances: [0n, 0n], + shares: [100n, 0n], + }, + { + scenario: 'penny-sized costs must remain positive', + amount: 2n, + balances: [1n, -1n], + shares: [100n, 100n], + }, + ])('recovers a simple ratio when $scenario (#727)', ({ amount, balances, shares }) => { + const participants = createParticipants( + [user1, user2, user3].slice(0, balances.length), + balances, + ); + const splitShares = createSplitShares(participants, SplitType.SHARE, []); + + calculateSplitShareBasedOnAmount(amount, participants, SplitType.SHARE, splitShares, user1); + + expect( + participants.map((participant) => splitShares[participant.id]?.[SplitType.SHARE]), + ).toEqual(shares); + expect(participants.map((participant) => participant.amount)).toEqual(balances); + }); + + it('falls back to the exact ratio for a large search (#727)', () => { + const costs = Array.from({ length: 1000 }, (_, index) => 1000000000000n + BigInt(index)); + const participants = createParticipants( + costs.map((_, index) => + createMockUser(index + 1, `User ${index}`, `user${index}@example.com`), + ), + costs.map((cost) => -cost), + ); + const splitShares = createSplitShares(participants, SplitType.SHARE, []); + + calculateSplitShareBasedOnAmount( + costs.reduce((sum, cost) => sum + cost, 0n), + participants, + SplitType.SHARE, + splitShares, + ); + + expect( + participants.map((participant) => splitShares[participant.id]?.[SplitType.SHARE]), + ).toEqual(costs.map((cost) => cost * 100n)); + }); + it('should calculate share values based on participant amounts', () => { const participants = createParticipants([user1, user2, user3], [-6000n, -3000n, -3000n]); const splitShares: Record> = {}; @@ -1141,6 +1221,93 @@ describe('Function Reversibility Tests', () => { }); }); +describe('useAddExpenseStore share preservation on reopen (#727)', () => { + const { actions } = useAddExpenseStore.getState(); + + beforeEach(() => { + actions.resetState(); + }); + + it.each([ + { + scenario: 'an evenly divisible 2:3 split', + amount: 10000n, + shares: [200n, 300n], + expectedShares: [200n, 300n], + }, + { + scenario: 'the reported 77.93 expense split 2:3', + amount: 7793n, + shares: [200n, 300n], + expectedShares: [200n, 300n], + }, + { + scenario: 'a 2:3 split requiring penny rounding', + amount: 10001n, + shares: [200n, 300n], + expectedShares: [200n, 300n], + }, + { + scenario: 'a non-reduced 4:6 split', + amount: 10000n, + shares: [400n, 600n], + expectedShares: [200n, 300n], + }, + ])('recovers $scenario on reopen', ({ amount, shares, expectedShares }) => { + actions.setCurrentUser(user1); + actions.setParticipants(createParticipants([user1, user2])); + actions.setExpenseDate(new Date('2024-01-01')); + actions.setAmount(amount); + actions.setSplitType(SplitType.SHARE); + [user1, user2].forEach((user, index) => { + actions.setSplitShare(SplitType.SHARE, user.id, shares[index] ?? 0n); + }); + + // Saving sends only monetary balances, not the entered share counts. + const state = useAddExpenseStore.getState(); + const savedExpense = { + amount: state.amount, + splitType: state.splitType, + expenseDate: state.expenseDate, + participants: state.participants.map((participant) => ({ + userId: participant.id, + amount: participant.amount!, + })), + }; + expect( + savedExpense.participants.reduce((sum, participant) => sum + participant.amount, 0n), + ).toBe(0n); + + // Leaving the page clears the draft. Reload in the same order as pages/add.tsx. + actions.resetState(); + actions.setPaidBy(user1); + actions.setAmount(savedExpense.amount); + actions.setParticipants( + createParticipants( + savedExpense.participants.map( + (participant) => [user1, user2].find((user) => user.id === participant.userId)!, + ), + savedExpense.participants.map((participant) => participant.amount), + ), + savedExpense.splitType, + ); + actions.setExpenseDate(savedExpense.expenseDate); + + // Saving and reopening again should not cause the recovered ratio to drift. + Array.from({ length: 3 }).forEach(() => { + const reopened = useAddExpenseStore.getState(); + expect( + reopened.participants.reduce((sum, participant) => sum + participant.amount!, 0n), + ).toBe(0n); + // SHARE inputs use hundredths: 200n and 300n display as 2 and 3 shares. + expect( + [user1, user2].map((user) => reopened.splitShares[user.id]?.[SplitType.SHARE]), + ).toEqual(expectedShares); + actions.setParticipants(reopened.participants, reopened.splitType); + }); + }); +}); + // Regression test for #658: editing a negative expense must preserve the sign describe('useAddExpenseStore sign preservation on edit (#658)', () => { const { toUIString } = getCurrencyHelpers({ locale: 'en-US', currency: 'USD' }); diff --git a/tests/percentageSplit.test.ts b/tests/percentageSplit.test.ts new file mode 100644 index 000000000..7ca51ae87 --- /dev/null +++ b/tests/percentageSplit.test.ts @@ -0,0 +1,133 @@ +import { SplitType } from '@prisma/client'; + +import { + calculateSplitShareBasedOnAmount, + initSplitShares, + useAddExpenseStore, +} from '~/store/addStore'; +import { createTestUser } from '~/tests/helpers/user'; + +const users = [1, 2, 3].map((id) => createTestUser({ id })); +const payer = users[0]!; + +describe('percentage split recovery on reopen (#727)', () => { + const { actions } = useAddExpenseStore.getState(); + + beforeEach(() => { + actions.resetState(); + }); + + it.each([ + { + scenario: 'the payer received the penny for 77.93 split 40%/60%', + amount: 7793n, + balances: [4675n, -4675n], + percentages: [4000n, 6000n], + }, + { + scenario: 'the other participant received the penny for 77.93 split 40%/60%', + amount: 7793n, + balances: [4676n, -4676n], + percentages: [4000n, 6000n], + }, + { + scenario: 'whole percentages take priority over simpler fractional ratios', + amount: 703n, + balances: [401n, -401n], + percentages: [4300n, 5700n], + }, + { + scenario: 'tenths of a percent are needed', + amount: 7793n, + balances: [4870n, -4870n], + percentages: [3750n, 6250n], + }, + { + scenario: 'hundredths of a percent are needed', + amount: 30001n, + balances: [20002n, -20002n], + percentages: [3333n, 6667n], + }, + { + scenario: 'three rounded percentages must still sum to 100%', + amount: 30001n, + balances: [20001n, -9999n, -10002n], + percentages: [3333n, 3333n, 3334n], + }, + { + scenario: 'a participant has no percentage', + amount: 7793n, + balances: [4675n, -4675n, 0n], + percentages: [4000n, 6000n, 0n], + }, + { + scenario: 'the payer has no percentage', + amount: 7793n, + balances: [7793n, -3118n, -4675n], + percentages: [0n, 4000n, 6000n], + }, + { + scenario: 'the payer has zero net balance', + amount: 7793n, + balances: [0n, 0n], + percentages: [10000n, 0n], + }, + ])('recovers percentages when $scenario', ({ amount, balances, percentages }) => { + const participants = balances.map((balance, index) => ({ ...users[index]!, amount: balance })); + const splitShares = Object.fromEntries( + participants.map((participant) => [participant.id, initSplitShares()]), + ); + + calculateSplitShareBasedOnAmount( + amount, + participants, + SplitType.PERCENTAGE, + splitShares, + payer, + ); + + expect( + participants.map((participant) => splitShares[participant.id]?.[SplitType.PERCENTAGE]), + ).toEqual(percentages); + expect(participants.map((participant) => participant.amount)).toEqual(balances); + }); + + it.each([ + { amount: 7793n, percentages: [4000n, 6000n] }, + { amount: 7793n, percentages: [3750n, 6250n] }, + { amount: 30001n, percentages: [3333n, 6667n] }, + { amount: 30001n, percentages: [3333n, 3333n, 3334n] }, + { amount: 1000000n, percentages: [1n, 6568n, 3431n] }, + ])('keeps $percentages on repeated saves of $amount', ({ amount, percentages }) => { + const participants = users.slice(0, percentages.length); + actions.setCurrentUser(payer); + actions.setParticipants(participants); + actions.setAmount(amount); + actions.setSplitType(SplitType.PERCENTAGE); + participants.forEach((participant, index) => { + actions.setSplitShare(SplitType.PERCENTAGE, participant.id, percentages[index]!); + }); + + Array.from({ length: 3 }).forEach(() => { + // Save monetary balances and clear the editor, then load as pages/add.tsx does. + const saved = useAddExpenseStore.getState(); + actions.resetState(); + actions.setPaidBy(payer); + actions.setAmount(saved.amount); + actions.setParticipants(saved.participants, saved.splitType); + + const reopened = useAddExpenseStore.getState(); + const recovered = reopened.participants.map( + (participant) => reopened.splitShares[participant.id]?.[SplitType.PERCENTAGE], + ); + expect(recovered).toEqual(percentages); + expect(recovered.reduce((sum, percentage) => sum + (percentage ?? 0n), 0n)).toBe( + 10000n, + ); + expect(reopened.canSplitScreenClosed).toBe(true); + expect( + reopened.participants.reduce((sum, participant) => sum + participant.amount!, 0n), + ).toBe(0n); + }); + }); +}); diff --git a/tests/shareSplit.test.ts b/tests/shareSplit.test.ts new file mode 100644 index 000000000..870a1c240 --- /dev/null +++ b/tests/shareSplit.test.ts @@ -0,0 +1,58 @@ +import { recoverPercentageWeights, recoverShareWeights } from '~/lib/shareSplit'; + +describe('recoverShareWeights', () => { + it('finds a rounded ratio even when no penny-subtracted variant has that exact ratio', () => { + expect(recoverShareWeights([3118n, 4675n])).toEqual([200n, 300n]); + }); + + it('uses the smallest total share count when rounding allows several ratios', () => { + expect(recoverShareWeights([1n, 2n])).toEqual([100n, 100n]); + }); + + it('recovers costs beyond the safe integer range without losing precision', () => { + const amount = 90071992547409933n; + const payerCost = (amount * 2n) / 5n + 1n; + const otherCost = (amount * 3n) / 5n; + + expect(payerCost + otherCost).toBe(amount); + expect(recoverShareWeights([payerCost, otherCost])).toEqual([200n, 300n]); + }); + + it('preserves zero costs and normalizes a single positive cost', () => { + expect(recoverShareWeights([0n, 7793n, 0n])).toEqual([0n, 100n, 0n]); + expect(recoverShareWeights([0n, 0n])).toEqual([0n, 0n]); + expect(recoverShareWeights([])).toEqual([]); + }); +}); + +describe('recoverPercentageWeights', () => { + it('balances ambiguous whole percentages using the nearest proportional allocation', () => { + expect(recoverPercentageWeights([1n, 1n, 1n])).toEqual([3400n, 3300n, 3300n]); + }); + + it('falls back to a valid 100% total when a positive portion cannot fit 0.01% precision', () => { + expect(recoverPercentageWeights([1n, 20000n])).toEqual([0n, 10000n]); + }); + + it('recovers amounts beyond the safe integer range without losing precision', () => { + const amount = 90071992547409933n; + expect(recoverPercentageWeights([(amount * 2n) / 5n + 1n, (amount * 3n) / 5n])).toEqual([ + 4000n, + 6000n, + ]); + }); + + it('preserves zero costs and handles a single participant', () => { + expect(recoverPercentageWeights([0n, 7793n, 0n])).toEqual([0n, 10000n, 0n]); + expect(recoverPercentageWeights([0n, 0n])).toEqual([0n, 0n]); + expect(recoverPercentageWeights([])).toEqual([]); + }); + + it('keeps a valid total for more participants than available percentage units', () => { + const percentages = recoverPercentageWeights(Array(10001).fill(1n)); + + expect(percentages.reduce((sum, percentage) => sum + percentage, 0n)).toBe(10000n); + expect(percentages.filter((percentage) => 1n === percentage)).toHaveLength(10000); + expect(percentages[10000]).toBe(0n); + }); +});