From e5dd9d4063814cdf0265fa3b61fe44050c2b55db Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:08:52 -0700 Subject: [PATCH 1/5] fix: clearer unequal-split language and prevent silent save failure Fixes #645 --- public/locales/en/common.json | 9 ++ src/components/AddExpense/AddExpensePage.tsx | 82 +++++++++++++++++-- .../AddExpense/SplitTypeSection.tsx | 5 ++ src/store/addStore.ts | 17 +++- src/tests/addStore.test.ts | 60 ++++++++++++++ 5 files changed, 160 insertions(+), 13 deletions(-) diff --git a/public/locales/en/common.json b/public/locales/en/common.json index 590275bc..ec9c5f7d 100644 --- a/public/locales/en/common.json +++ b/public/locales/en/common.json @@ -189,6 +189,12 @@ "warning": "Warning: Don't use send invite if it's invalid email. use add to Split Pro instead. Your account will be blocked if this feature is misused" }, "split_type_section": { + "direction": { + "no_money_flow": "No one else owes anything", + "owes_payer": "{{debtor}} owes {{payer}}", + "owes_you": "{{debtor}} owes you", + "you_owe": "You owe {{payer}}" + }, "split_equally": "split equally", "split_unequally": "split unequally", "types": { @@ -213,6 +219,9 @@ "title": "Share", "total_shares": "Total shares" } + }, + "validation": { + "invalid_split": "Adjust who owes this expense before saving." } }, "sponsor_us": "Sponsor us", diff --git a/src/components/AddExpense/AddExpensePage.tsx b/src/components/AddExpense/AddExpensePage.tsx index 90a206e4..68d002b4 100644 --- a/src/components/AddExpense/AddExpensePage.tsx +++ b/src/components/AddExpense/AddExpensePage.tsx @@ -1,3 +1,4 @@ +import { SplitType } from '@prisma/client'; import { HeartHandshakeIcon, Landmark, RefreshCcwDot, X } from 'lucide-react'; import { useTranslation } from 'next-i18next'; import Link from 'next/link'; @@ -55,8 +56,70 @@ export const AddOrEditExpensePage: React.FC<{ const cronExpression = useAddExpenseStore((s) => s.cronExpression); const multipleTransactions = useAddExpenseStore((s) => s.multipleTransactions); - const { t, displayName, generateSplitDescription, getCurrencyHelpersCached } = - useTranslationWithUtils(); + const { t, displayName, getCurrencyHelpersCached } = useTranslationWithUtils(); + const splitValidationMessage = t( + 'expense_details.add_expense_details.split_type_section.validation.invalid_split', + ); + const splitDescription = React.useMemo(() => { + const splitEquallyText = t( + 'expense_details.add_expense_details.split_type_section.split_equally', + ); + + if (SplitType.EQUAL !== splitType) { + return t('expense_details.add_expense_details.split_type_section.split_unequally'); + } + + if (!paidBy || !currentUser) { + return splitEquallyText; + } + + const selectedParticipants = participants.filter((participant) => { + const share = splitShares[participant.id]?.[SplitType.EQUAL]; + return share === undefined || 0n !== share; + }); + + if (0 === selectedParticipants.length) { + return splitValidationMessage; + } + + const debtor = selectedParticipants[0]; + if (1 === selectedParticipants.length && debtor) { + if (debtor.id === paidBy.id) { + return t('expense_details.add_expense_details.split_type_section.direction.no_money_flow'); + } + + const debtorName = displayName(debtor, currentUser.id); + const payerName = displayName(paidBy, currentUser.id); + + if (paidBy.id === currentUser.id) { + return t('expense_details.add_expense_details.split_type_section.direction.owes_you', { + debtor: debtorName, + }); + } + + if (debtor.id === currentUser.id) { + return t('expense_details.add_expense_details.split_type_section.direction.you_owe', { + payer: payerName, + }); + } + + return t('expense_details.add_expense_details.split_type_section.direction.owes_payer', { + debtor: debtorName, + payer: payerName, + }); + } + + return `${splitEquallyText} (${selectedParticipants.length})`; + }, [ + currentUser, + displayName, + paidBy, + participants, + splitShares, + splitType, + splitValidationMessage, + t, + ]); const { setCurrency, @@ -111,6 +174,7 @@ export const AddOrEditExpensePage: React.FC<{ } if (!isExpenseSettled) { + toast.error(splitValidationMessage); setSplitScreenOpen(true); return; } @@ -220,6 +284,7 @@ export const AddOrEditExpensePage: React.FC<{ multipleTransactions, setSingleTransaction, update, + splitValidationMessage, ]); const handleDescriptionChange = useCallback( @@ -345,16 +410,15 @@ export const AddOrEditExpensePage: React.FC<{

{t('ui.and')}

+ {!isExpenseSettled ? ( +

+ {splitValidationMessage} +

+ ) : null}
= (props) => { > {fmtSummartyText(amount, totalShares, toUIString)}

+ {!canSplitScreenClosed ? ( +

+ {t('expense_details.add_expense_details.split_type_section.validation.invalid_split')} +

+ ) : null} {isBoolean && (
{!isExpenseSettled ? ( -

- {splitValidationMessage} -

+

{splitValidationMessage}

) : null}
diff --git a/src/store/addStore.ts b/src/store/addStore.ts index b17f2907..68d943ff 100644 --- a/src/store/addStore.ts +++ b/src/store/addStore.ts @@ -351,7 +351,14 @@ export function calculateParticipantSplit( if (canSplitScreenClosed) { let penniesLeft = updatedParticipants.reduce((acc, p) => acc + (p.amount ?? 0n), 0n); - const participantsToPick = updatedParticipants.filter((p) => p.amount); + const roundedToZeroParticipants = + SplitType.EQUAL === splitType + ? updatedParticipants.filter((p) => 0n === (p.amount ?? 0n) && 0n !== getSplitShare(p)) + : []; + const participantsToPick = + 0 < roundedToZeroParticipants.length + ? roundedToZeroParticipants + : updatedParticipants.filter((p) => p.amount); const seed = cyrb128( `${participantsToPick diff --git a/src/tests/addStore.test.ts b/src/tests/addStore.test.ts index 6c7ff907..c44b28e8 100644 --- a/src/tests/addStore.test.ts +++ b/src/tests/addStore.test.ts @@ -172,6 +172,26 @@ describe('calculateParticipantSplit', () => { expect(result.canSplitScreenClosed).toBe(false); }); + it('should keep a penny-only equal split valid when rounding zeroes every share', () => { + const participants = createParticipants([user1, user2]); + const splitShares = createSplitShares(participants, SplitType.EQUAL, [1n, 1n]); + + const state: Partial = { + amount: 1n, + participants, + splitType: SplitType.EQUAL, + splitShares, + paidBy: user1, + expenseDate: new Date('2024-01-01'), + }; + + const result = calculateParticipantSplit(state as AddExpenseState); + + expect(result.participants[0]?.amount).toBe(1n); + expect(result.participants[1]?.amount).toBe(-1n); + expect(result.canSplitScreenClosed).toBe(true); + }); + it('should mark equal split incomplete when every share is disabled', () => { const participants = createParticipants([user1, user2]); const splitShares = createSplitShares(participants, SplitType.EQUAL, [0n, 0n]); From 137b779b4a8f9e966682a0b59cc3200d4f7017fa Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:38:24 -0700 Subject: [PATCH 4/5] chore: drop unneeded jest.config env tweak (CI sets SKIP_ENV_VALIDATION) --- jest.config.ts | 102 ++++++++++++++++++++++++------------------------- 1 file changed, 50 insertions(+), 52 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index 972f0b53..f45658bc 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -6,8 +6,6 @@ import type { Config } from 'jest'; import nextJest from 'next/jest.js'; -process.env.SKIP_ENV_VALIDATION ??= 'true'; - // @ts-expect-error we are extending BigInt prototype for JSON serialization // oxlint-disable-next-line no-extend-native BigInt.prototype.toJSON = function toJSON() { @@ -22,28 +20,28 @@ const createJestConfig = nextJest({ const config: Config = { // All imported modules in your tests should be mocked automatically - // Automock: false, + // automock: false, // Stop running tests after `n` failures - // Bail: 0, + // bail: 0, // The directory where Jest should store its cached dependency information - // CacheDirectory: "C:\\Users\\Wiktor\\AppData\\Local\\Temp\\jest", + // cacheDirectory: "C:\\Users\\Wiktor\\AppData\\Local\\Temp\\jest", // Automatically clear mock calls, instances, contexts and results before every test - // ClearMocks: false, + // clearMocks: false, // Indicates whether the coverage information should be collected while executing the test - // CollectCoverage: false, + // collectCoverage: false, // An array of glob patterns indicating a set of files for which coverage information should be collected - // CollectCoverageFrom: undefined, + // collectCoverageFrom: undefined, // The directory where Jest should output its coverage files - // CoverageDirectory: undefined, + // coverageDirectory: undefined, // An array of regexp pattern strings used to skip coverage collection - // CoveragePathIgnorePatterns: [ + // coveragePathIgnorePatterns: [ // "\\\\node_modules\\\\" // ], @@ -51,7 +49,7 @@ const config: Config = { coverageProvider: 'v8', // A list of reporter names that Jest uses when writing coverage reports - // CoverageReporters: [ + // coverageReporters: [ // "json", // "text", // "lcov", @@ -59,41 +57,41 @@ const config: Config = { // ], // An object that configures minimum threshold enforcement for coverage results - // CoverageThreshold: undefined, + // coverageThreshold: undefined, // A path to a custom dependency extractor - // DependencyExtractor: undefined, + // dependencyExtractor: undefined, // Make calling deprecated APIs throw helpful error messages - // ErrorOnDeprecated: false, + // errorOnDeprecated: false, // The default configuration for fake timers - // FakeTimers: { + // fakeTimers: { // "enableGlobally": false // }, // Force coverage collection from ignored files using an array of glob patterns - // ForceCoverageMatch: [], + // forceCoverageMatch: [], // A path to a module which exports an async function that is triggered once before all test suites - // GlobalSetup: undefined, + // globalSetup: undefined, // A path to a module which exports an async function that is triggered once after all test suites - // GlobalTeardown: undefined, + // globalTeardown: undefined, // A set of global variables that need to be available in all test environments - // Globals: {}, + // globals: {}, // The maximum amount of workers used to run your tests. Can be specified as % or a number. E.g. maxWorkers: 10% will use 10% of your CPU amount + 1 as the maximum worker number. maxWorkers: 2 will use a maximum of 2 workers. - // MaxWorkers: "50%", + // maxWorkers: "50%", // An array of directory names to be searched recursively up from the requiring module's location - // ModuleDirectories: [ + // moduleDirectories: [ // "node_modules" // ], // An array of file extensions your modules use - // ModuleFileExtensions: [ + // moduleFileExtensions: [ // "js", // "mjs", // "cjs", @@ -110,107 +108,107 @@ const config: Config = { }, // An array of regexp pattern strings, matched against all module paths before considered 'visible' to the module loader - // ModulePathIgnorePatterns: [], + // modulePathIgnorePatterns: [], // Activates notifications for test results - // Notify: false, + // notify: false, // An enum that specifies notification mode. Requires { notify: true } - // NotifyMode: "failure-change", + // notifyMode: "failure-change", // A preset that is used as a base for Jest's configuration - // Preset: undefined, + // preset: undefined, // Run tests from one or more projects - // Projects: undefined, + // projects: undefined, // Use this configuration option to add custom reporters to Jest - // Reporters: undefined, + // reporters: undefined, // Automatically reset mock state before every test - // ResetMocks: false, + // resetMocks: false, // Reset the module registry before running each individual test - // ResetModules: false, + // resetModules: false, // A path to a custom resolver - // Resolver: undefined, + // resolver: undefined, // Automatically restore mock state and implementation before every test - // RestoreMocks: false, + // restoreMocks: false, // The root directory that Jest should scan for tests and modules within - // RootDir: undefined, + // rootDir: undefined, // A list of paths to directories that Jest should use to search for files in - // Roots: [ + // roots: [ // "" // ], // Allows you to use a custom runner instead of Jest's default test runner - // Runner: "jest-runner", + // runner: "jest-runner", // The paths to modules that run some code to configure or set up the testing environment before each test - // SetupFiles: [], + // setupFiles: [], // A list of paths to modules that run some code to configure or set up the testing framework before each test - // SetupFilesAfterEnv: [], + // setupFilesAfterEnv: [], // The number of seconds after which a test is considered as slow and reported as such in the results. - // SlowTestThreshold: 5, + // slowTestThreshold: 5, // A list of paths to snapshot serializer modules Jest should use for snapshot testing - // SnapshotSerializers: [], + // snapshotSerializers: [], // The test environment that will be used for testing testEnvironment: 'jsdom', // Options that will be passed to the testEnvironment - // TestEnvironmentOptions: {}, + // testEnvironmentOptions: {}, // Adds a location field to test results - // TestLocationInResults: false, + // testLocationInResults: false, // The glob patterns Jest uses to detect test files - // TestMatch: [ + // testMatch: [ // "**/__tests__/**/*.[jt]s?(x)", // "**/?(*.)+(spec|test).[tj]s?(x)" // ], // An array of regexp pattern strings that are matched against all test paths, matched tests are skipped - // TestPathIgnorePatterns: [ + // testPathIgnorePatterns: [ // "\\\\node_modules\\\\" // ], // The regexp pattern or array of patterns that Jest uses to detect test files - // TestRegex: [], + // testRegex: [], // This option allows the use of a custom results processor - // TestResultsProcessor: undefined, + // testResultsProcessor: undefined, // This option allows use of a custom test runner - // TestRunner: "jest-circus/runner", + // testRunner: "jest-circus/runner", // A map from regular expressions to paths to transformers - // Transform: undefined, + // transform: undefined, // An array of regexp pattern strings that are matched against all source file paths, matched files will skip transformation - // TransformIgnorePatterns: [ + // transformIgnorePatterns: [ // "\\\\node_modules\\\\", // "\\.pnp\\.[^\\\\]+$" // ], // An array of regexp pattern strings that are matched against all modules before the module loader will automatically return a mock for them - // UnmockedModulePathPatterns: undefined, + // unmockedModulePathPatterns: undefined, // Indicates whether each individual test should be reported during the run - // Verbose: undefined, + // verbose: undefined, // An array of regexp patterns that are matched against all source file paths before re-running tests in watch mode - // WatchPathIgnorePatterns: [], + // watchPathIgnorePatterns: [], // Whether to use watchman for file crawling - // Watchman: true, + // watchman: true, }; export default createJestConfig(config); From caa4f29ca0523afeb17dc110ee176063ab442caa Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:31:06 -0700 Subject: [PATCH 5/5] fix: address review feedback on unequal-split messaging Reworks the unequal-split wording and guard per the inline review. Lint, type-check, 141 tests, and a production build verified. --- src/components/AddExpense/AddExpensePage.tsx | 92 +++----------------- src/store/addStore.ts | 14 +-- src/tests/addStore.test.ts | 20 ----- src/utils/strings.ts | 54 ++++++++---- 4 files changed, 47 insertions(+), 133 deletions(-) diff --git a/src/components/AddExpense/AddExpensePage.tsx b/src/components/AddExpense/AddExpensePage.tsx index 778972ff..4fa7203c 100644 --- a/src/components/AddExpense/AddExpensePage.tsx +++ b/src/components/AddExpense/AddExpensePage.tsx @@ -1,4 +1,3 @@ -import { SplitType } from '@prisma/client'; import { HeartHandshakeIcon, Landmark, RefreshCcwDot, X } from 'lucide-react'; import { useTranslation } from 'next-i18next'; import Link from 'next/link'; @@ -56,73 +55,8 @@ export const AddOrEditExpensePage: React.FC<{ const cronExpression = useAddExpenseStore((s) => s.cronExpression); const multipleTransactions = useAddExpenseStore((s) => s.multipleTransactions); - const { t, displayName, getCurrencyHelpersCached } = useTranslationWithUtils(); - const splitValidationMessage = t( - 'expense_details.add_expense_details.split_type_section.validation.invalid_split', - ); - const splitDescription = React.useMemo(() => { - const splitEquallyText = t( - 'expense_details.add_expense_details.split_type_section.split_equally', - ); - - if (SplitType.EQUAL !== splitType) { - return t('expense_details.add_expense_details.split_type_section.split_unequally'); - } - - if (!paidBy || !currentUser) { - return splitEquallyText; - } - - const selectedParticipants = participants.filter((participant) => { - const share = splitShares[participant.id]?.[SplitType.EQUAL]; - return share === undefined || 0n !== share; - }); - - if (0 === selectedParticipants.length) { - return splitValidationMessage; - } - - const splitParticipant = selectedParticipants[0]; - if (1 === selectedParticipants.length && splitParticipant) { - if (splitParticipant.id === paidBy.id) { - return t('expense_details.add_expense_details.split_type_section.direction.no_money_flow'); - } - - const debtor = isNegative ? paidBy : splitParticipant; - const payer = isNegative ? splitParticipant : paidBy; - const debtorName = displayName(debtor, currentUser.id); - const payerName = displayName(payer, currentUser.id); - - if (payer.id === currentUser.id) { - return t('expense_details.add_expense_details.split_type_section.direction.owes_you', { - debtor: debtorName, - }); - } - - if (debtor.id === currentUser.id) { - return t('expense_details.add_expense_details.split_type_section.direction.you_owe', { - payer: payerName, - }); - } - - return t('expense_details.add_expense_details.split_type_section.direction.owes_payer', { - debtor: debtorName, - payer: payerName, - }); - } - - return `${splitEquallyText} (${selectedParticipants.length})`; - }, [ - currentUser, - displayName, - isNegative, - paidBy, - participants, - splitShares, - splitType, - splitValidationMessage, - t, - ]); + const { t, displayName, generateSplitDescription, getCurrencyHelpersCached } = + useTranslationWithUtils(); const { setCurrency, @@ -131,7 +65,6 @@ export const AddOrEditExpensePage: React.FC<{ setAmount, setAmountStr, resetState, - setSplitScreenOpen, setExpenseDate, setMultipleTransactions, setIsTransactionLoading, @@ -176,12 +109,6 @@ export const AddOrEditExpensePage: React.FC<{ return; } - if (!isExpenseSettled) { - toast.error(splitValidationMessage); - setSplitScreenOpen(true); - return; - } - setMultipleTransactions([]); setIsTransactionLoading(false); @@ -263,7 +190,6 @@ export const AddOrEditExpensePage: React.FC<{ } } }, [ - setSplitScreenOpen, description, currency, isNegative, @@ -279,7 +205,6 @@ export const AddOrEditExpensePage: React.FC<{ paidBy, splitType, fileKey, - isExpenseSettled, setMultipleTransactions, transactionId, setIsTransactionLoading, @@ -287,7 +212,6 @@ export const AddOrEditExpensePage: React.FC<{ multipleTransactions, setSingleTransaction, update, - splitValidationMessage, ]); const handleDescriptionChange = useCallback( @@ -413,13 +337,17 @@ export const AddOrEditExpensePage: React.FC<{

{t('ui.and')}

- {!isExpenseSettled ? ( -

{splitValidationMessage}

- ) : null}
0n !== getSplitShare(p)).length; updatedParticipants = participants.map((p) => ({ ...p, - amount: - 0 === totalParticipants || 0n === getSplitShare(p) - ? 0n - : amount / BigInt(totalParticipants), + amount: 0n === getSplitShare(p) ? 0n : amount / BigInt(totalParticipants), })); canSplitScreenClosed = 0 < totalParticipants; break; @@ -351,14 +348,7 @@ export function calculateParticipantSplit( if (canSplitScreenClosed) { let penniesLeft = updatedParticipants.reduce((acc, p) => acc + (p.amount ?? 0n), 0n); - const roundedToZeroParticipants = - SplitType.EQUAL === splitType - ? updatedParticipants.filter((p) => 0n === (p.amount ?? 0n) && 0n !== getSplitShare(p)) - : []; - const participantsToPick = - 0 < roundedToZeroParticipants.length - ? roundedToZeroParticipants - : updatedParticipants.filter((p) => p.amount); + const participantsToPick = updatedParticipants.filter((p) => p.amount); const seed = cyrb128( `${participantsToPick diff --git a/src/tests/addStore.test.ts b/src/tests/addStore.test.ts index c44b28e8..6c7ff907 100644 --- a/src/tests/addStore.test.ts +++ b/src/tests/addStore.test.ts @@ -172,26 +172,6 @@ describe('calculateParticipantSplit', () => { expect(result.canSplitScreenClosed).toBe(false); }); - it('should keep a penny-only equal split valid when rounding zeroes every share', () => { - const participants = createParticipants([user1, user2]); - const splitShares = createSplitShares(participants, SplitType.EQUAL, [1n, 1n]); - - const state: Partial = { - amount: 1n, - participants, - splitType: SplitType.EQUAL, - splitShares, - paidBy: user1, - expenseDate: new Date('2024-01-01'), - }; - - const result = calculateParticipantSplit(state as AddExpenseState); - - expect(result.participants[0]?.amount).toBe(1n); - expect(result.participants[1]?.amount).toBe(-1n); - expect(result.canSplitScreenClosed).toBe(true); - }); - it('should mark equal split incomplete when every share is disabled', () => { const participants = createParticipants([user1, user2]); const splitShares = createSplitShares(participants, SplitType.EQUAL, [0n, 0n]); diff --git a/src/utils/strings.ts b/src/utils/strings.ts index 65dbaf66..bd6adcc5 100644 --- a/src/utils/strings.ts +++ b/src/utils/strings.ts @@ -52,42 +52,58 @@ export function generateSplitDescription( splitShares: AddExpenseState['splitShares'], paidBy?: Participant, currentUser?: Participant, + isNegative = false, ): string { - // Only enhance the description for EQUAL split type - if (splitType !== SplitType.EQUAL) { + if (SplitType.EQUAL !== splitType) { return t('expense_details.add_expense_details.split_type_section.split_unequally'); } + const splitEquallyText = t( + 'expense_details.add_expense_details.split_type_section.split_equally', + ); if (!paidBy || !currentUser) { - return t('expense_details.add_expense_details.split_type_section.split_equally'); + return splitEquallyText; } - // Get participants who are actually selected for the split (have non-zero shares) - // If split shares are not initialized yet (undefined), include all participants const selectedParticipants = participants.filter((p) => { const share = splitShares[p.id]?.[SplitType.EQUAL]; - return share === undefined || share !== 0n; + return share === undefined || 0n !== share; }); - // If no one is selected, fall back to default - if (selectedParticipants.length === 0) { - return t('expense_details.add_expense_details.split_type_section.split_equally'); + const splitParticipant = selectedParticipants[0]; + if (!splitParticipant) { + return splitEquallyText; } - // Case 1: Paying for exactly one person - if (selectedParticipants.length === 1) { - const beneficiary = selectedParticipants[0]; - const beneficiaryName = displayName(t, beneficiary as User, currentUser.id) || 'someone'; - return `${t('ui.expense.user.paid')} ${t('ui.expense.for')} ${beneficiaryName}`; + if (1 !== selectedParticipants.length) { + return `${splitEquallyText} (${selectedParticipants.length})`; } - // Case 2: Splitting with multiple people - if (selectedParticipants.length > 1) { - return `${t('expense_details.add_expense_details.split_type_section.split_equally')} (${selectedParticipants.length})`; + if (splitParticipant.id === paidBy.id) { + return t('expense_details.add_expense_details.split_type_section.direction.no_money_flow'); } - // Fallback to default for all other cases - return t('expense_details.add_expense_details.split_type_section.split_equally'); + const debtor = isNegative ? paidBy : splitParticipant; + const payer = isNegative ? splitParticipant : paidBy; + const debtorName = displayName(t, debtor, currentUser.id); + const payerName = displayName(t, payer, currentUser.id); + + if (payer.id === currentUser.id) { + return t('expense_details.add_expense_details.split_type_section.direction.owes_you', { + debtor: debtorName, + }); + } + + if (debtor.id === currentUser.id) { + return t('expense_details.add_expense_details.split_type_section.direction.you_owe', { + payer: payerName, + }); + } + + return t('expense_details.add_expense_details.split_type_section.direction.owes_payer', { + debtor: debtorName, + payer: payerName, + }); } export function getCurrencyName(t: TFunction, code: CurrencyCode, plural = false): string {