From 1f0d527ed1cd5b76355d3cef921a4d31b1815f06 Mon Sep 17 00:00:00 2001 From: krokosik Date: Sun, 21 Jun 2026 17:04:08 +0200 Subject: [PATCH 1/5] fix: preserve negative sign when editing an expense The edit form loaded the amount with toUIString(amount, false, true), which strips the minus, while setAmount(amount) correctly set isNegative=true. The store was left inconsistent: the input showed '200' but the model said negative. Any digit edit re-derived isNegative from the now-positive typed BigInt, flipping the sign on save. If the user didn't touch the field, isNegative stayed true and the value was unchanged - matching the reporter's repro exactly. Fix the sign flow through all layers: - numbers.ts: thread 'signed' through normalizeToMaxLength and the string branch of parseToCleanString so the minus survives onFocus of CurrencyInput (allowNegative). Remove unused toUIStringSigned. - add.tsx: load the edit form with toUIString(amount, true, true). - currency-input.tsx: pass allowNegative to parseToCleanString on focus so the minus is not re-stripped. - AddExpensePage.tsx, CurrencyConversion.tsx: keep the sign when converting currencies on a negative amount. - Export.tsx: keep the minus for negative expenses in CSV export (same root cause, parseToCleanString(expense.amount, true)). Closes #658 --- src/components/AddExpense/AddExpensePage.tsx | 2 +- src/components/Friend/CurrencyConversion.tsx | 4 +- src/components/Friend/Export.tsx | 2 +- src/components/ui/currency-input.tsx | 2 +- src/pages/add.tsx | 2 +- src/tests/addStore.test.ts | 50 ++++++++++++++++++++ src/tests/number.test.ts | 41 ++++++++++++++++ src/utils/numbers.ts | 7 ++- 8 files changed, 100 insertions(+), 10 deletions(-) diff --git a/src/components/AddExpense/AddExpensePage.tsx b/src/components/AddExpense/AddExpensePage.tsx index 90a206e4..3652d68e 100644 --- a/src/components/AddExpense/AddExpensePage.tsx +++ b/src/components/AddExpense/AddExpensePage.tsx @@ -251,7 +251,7 @@ export const AddOrEditExpensePage: React.FC<{ to: currency, }); setAmount(targetAmount); - setAmountStr(getCurrencyHelpersCached(currency).toUIString(targetAmount, false, true)); + setAmountStr(getCurrencyHelpersCached(currency).toUIString(targetAmount, true, true)); previousCurrencyRef.current = null; }, [setAmount, setAmountStr, currency, getCurrencyHelpersCached], diff --git a/src/components/Friend/CurrencyConversion.tsx b/src/components/Friend/CurrencyConversion.tsx index b58974a3..48860d66 100644 --- a/src/components/Friend/CurrencyConversion.tsx +++ b/src/components/Friend/CurrencyConversion.tsx @@ -53,7 +53,7 @@ export const CurrencyConversion: React.FC<{ }, [getCurrencyRate.isPending]); useEffect(() => { - setAmountStr(toUIString(amount, false, true)); + setAmountStr(toUIString(amount, true, true)); if (editingRate) { const precision = getRatePrecision(editingRate); setRate(editingRate.toFixed(precision)); @@ -136,7 +136,7 @@ export const CurrencyConversion: React.FC<{ from: targetCurrency, to: currency, }); - setAmountStr(toUIString(amount, false, true)); + setAmountStr(toUIString(amount, true, true)); } }, [rate, toUIString, targetCurrency, currency], diff --git a/src/components/Friend/Export.tsx b/src/components/Friend/Export.tsx index 4b93450c..2a562b9b 100644 --- a/src/components/Friend/Export.tsx +++ b/src/components/Friend/Export.tsx @@ -53,7 +53,7 @@ export const Export: React.FC = ({ expense.paidBy === currentUserId ? 'You' : friendName, expense.name, expense.category, - parseToCleanString(expense?.amount), + parseToCleanString(expense?.amount, true), expense.splitType, format(new Date(expense.expenseDate), 'yyyy-MM-dd HH:mm:ss'), expense.currency, diff --git a/src/components/ui/currency-input.tsx b/src/components/ui/currency-input.tsx index c5c94178..4e0ac4fc 100644 --- a/src/components/ui/currency-input.tsx +++ b/src/components/ui/currency-input.tsx @@ -21,7 +21,7 @@ const CurrencyInput: React.FC< className={cn('text-lg placeholder:text-sm', className)} inputMode="decimal" value={strValue} - onFocus={() => onValueChange({ strValue: parseToCleanString(strValue) })} + onFocus={() => onValueChange({ strValue: parseToCleanString(strValue, allowNegative) })} onBlur={() => { const formattedValue = format(strValue, { signed: allowNegative, hideSymbol }); return onValueChange({ strValue: formattedValue }); diff --git a/src/pages/add.tsx b/src/pages/add.tsx index ec37bd4f..bf1cf9cc 100644 --- a/src/pages/add.tsx +++ b/src/pages/add.tsx @@ -195,7 +195,7 @@ const AddPage: NextPageWithUser<{ setAmountStr( getCurrencyHelpersCached(expenseQuery.data.currency).toUIString( expenseQuery.data.amount, - false, + true, true, ), ); diff --git a/src/tests/addStore.test.ts b/src/tests/addStore.test.ts index 84a86b36..376a6eca 100644 --- a/src/tests/addStore.test.ts +++ b/src/tests/addStore.test.ts @@ -6,7 +6,9 @@ import { calculateParticipantSplit, calculateSplitShareBasedOnAmount, initSplitShares, + useAddExpenseStore, } from '~/store/addStore'; +import { getCurrencyHelpers } from '~/utils/numbers'; // Mock dependencies jest.mock('~/utils/array', () => ({ @@ -1079,3 +1081,51 @@ describe('Function Reversibility Tests', () => { }); }); }); + +// 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' }); + const { actions } = useAddExpenseStore.getState(); + + beforeEach(() => { + actions.resetState(); + }); + + it('loads a negative expense with the minus sign visible in amountStr', () => { + actions.setAmount(-20000n); + actions.setAmountStr(toUIString(-20000n, true, true)); + + const state = useAddExpenseStore.getState(); + expect(state.amountStr).toBe('-200'); + expect(state.isNegative).toBe(true); + expect(state.amount).toBe(20000n); + }); + + it('preserves the sign when the user edits digits without removing the minus', () => { + actions.setAmount(-20000n); + actions.setAmountStr(toUIString(-20000n, true, true)); + + // Simulate the user changing -200 to -200.01 via the input + actions.setAmountStr('-200.01'); + actions.setAmount(-20001n); + + const state = useAddExpenseStore.getState(); + expect(state.amountStr).toBe('-200.01'); + expect(state.isNegative).toBe(true); + expect(state.amount).toBe(20001n); + }); + + it('flips the sign when the user deletes the minus and edits the value', () => { + actions.setAmount(-20000n); + actions.setAmountStr(toUIString(-20000n, true, true)); + + // Simulate the user deleting the minus and changing the value to 200.01 + actions.setAmountStr('200.01'); + actions.setAmount(20001n); + + const state = useAddExpenseStore.getState(); + expect(state.amountStr).toBe('200.01'); + expect(state.isNegative).toBe(false); + expect(state.amount).toBe(20001n); + }); +}); diff --git a/src/tests/number.test.ts b/src/tests/number.test.ts index 3d191032..0cec4f20 100644 --- a/src/tests/number.test.ts +++ b/src/tests/number.test.ts @@ -46,6 +46,15 @@ describe('getCurrencyHelpers', () => { ])('should format %p as %p with signed flag', (value, expected) => { expect(toUIString(value, true)).toBe(expected); }); + + it.each([ + [12345n, '123.45'], + [-12345n, '-123.45'], + [-50n, '-0.5'], + [-0n, '0'], + ])('should format %p as %p with signed flag and hideSymbol', (value, expected) => { + expect(toUIString(value, true, true)).toBe(expected); + }); }); describe('JPY (no decimals)', () => { const currency = 'JPY'; @@ -173,6 +182,38 @@ describe('getCurrencyHelpers', () => { expect(sanitizeInput(input, true)).toBe(expected); }); }); + + describe('parseToCleanString', () => { + const { parseToCleanString } = getCurrencyHelpers({ + locale: 'en-US', + currency: 'USD', + }); + + it.each([ + ['-200', '-200'], + ['-200.01', '-200.01'], + ['--200', '-200'], + ['-$200.00', '-200.00'], + ])('should keep the minus sign for %p with signed flag', (input, expected) => { + expect(parseToCleanString(input, true)).toBe(expected); + }); + + it.each([ + [-20000n, '-200.00'], + [-12345n, '-123.45'], + [-50n, '-0.50'], + [-0n, '0.00'], + ])('should keep the minus sign for bigint %p with signed flag', (value, expected) => { + expect(parseToCleanString(value, true)).toBe(expected); + }); + + it.each([ + ['-200', '200'], + ['-123.45', '123.45'], + ])('should drop the minus sign for %p without signed flag', (input, expected) => { + expect(parseToCleanString(input)).toBe(expected); + }); + }); }); describe('currencyConversion', () => { diff --git a/src/utils/numbers.ts b/src/utils/numbers.ts index ea7ef7de..3bed072a 100644 --- a/src/utils/numbers.ts +++ b/src/utils/numbers.ts @@ -130,8 +130,8 @@ export const getCurrencyHelpers = ({ return cleaned; }; - const normalizeToMaxLength = (inputString: string) => { - const sanitized = sanitizeInput(inputString); + const normalizeToMaxLength = (inputString: string, signed = false) => { + const sanitized = sanitizeInput(inputString, signed); const trimmedExceedingDecimals = trimExceedingDecimals(sanitized); return trimmedExceedingDecimals.endsWith(decimalSeparator) ? trimmedExceedingDecimals.slice(0, -1) @@ -173,7 +173,7 @@ export const getCurrencyHelpers = ({ } if (typeof value === 'string') { - return normalizeToMaxLength(value); + return normalizeToMaxLength(value, signed); } return ''; @@ -233,7 +233,6 @@ export const getCurrencyHelpers = ({ return { parseToCleanString, toUIString, - toUIStringSigned: (value: unknown) => toUIString(value, true), format, formatter, sanitizeInput, From 5c69a0235a7e3ee1abb50e733e2be87b299682b0 Mon Sep 17 00:00:00 2001 From: krokosik Date: Sun, 21 Jun 2026 17:51:37 +0200 Subject: [PATCH 2/5] Remove stray console log --- src/components/Expense/ConvertibleBalance.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/Expense/ConvertibleBalance.tsx b/src/components/Expense/ConvertibleBalance.tsx index c9f6e951..6446affb 100644 --- a/src/components/Expense/ConvertibleBalance.tsx +++ b/src/components/Expense/ConvertibleBalance.tsx @@ -160,8 +160,6 @@ export const ConvertibleBalance: React.FC = ({ return total; }, [shouldShowAll, balances, ratesQuery, selectedCurrency, t, setSelectedCurrency]); - console.log(selectedCurrency, groupDefaultCurrency); - if (0 === balances.length) { return ; } From 3ee4e4e1f0d64744770e6f8d7c6c524cc415eb75 Mon Sep 17 00:00:00 2001 From: krokosik Date: Sun, 21 Jun 2026 17:53:36 +0200 Subject: [PATCH 3/5] fix: clear init refs on unmount so edit form repopulates The initializedExpenseIdRef/groupId/friendId refs introduced in 83268ac (v2.1.0, #608) persist across React StrictMode's simulated unmount in dev, while the resetState() cleanup clears the store. On remount, the edit effect sees ref === expenseId and early-returns, leaving the store empty and the form showing the add flow even though the URL and cached query data are correct. A hard reload fixes it because it clears the React Query cache so the effect doesn't run until data arrives, by which time StrictMode is done. Clear the refs alongside resetState in the unmount cleanup so refs and store are always reset together. Safe in production (refs are destroyed on real unmount anyway). Also fix: preserve negative sign when editing an expense The edit form loaded the amount with toUIString(amount, false, true), which strips the minus, while setAmount(amount) correctly set isNegative=true. The store was left inconsistent: the input showed '200' but the model said negative. Any digit edit re-derived isNegative from the now-positive typed BigInt, flipping the sign on save. Fix the sign flow through all layers: - numbers.ts: thread 'signed' through normalizeToMaxLength and the string branch of parseToCleanString so the minus survives onFocus of CurrencyInput (allowNegative). Remove unused toUIStringSigned. - add.tsx: load the edit form with toUIString(amount, true, true). - currency-input.tsx: pass allowNegative to parseToCleanString on focus so the minus is not re-stripped. - AddExpensePage.tsx, CurrencyConversion.tsx: keep the sign when converting currencies on a negative amount. - Export.tsx: keep the minus for negative expenses in CSV export (same root cause, parseToCleanString(expense.amount, true)). Closes #658 --- src/pages/add.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/pages/add.tsx b/src/pages/add.tsx index bf1cf9cc..d26d049e 100644 --- a/src/pages/add.tsx +++ b/src/pages/add.tsx @@ -44,7 +44,15 @@ const AddPage: NextPageWithUser<{ const initializedFriendIdRef = useRef(null); const initializedExpenseIdRef = useRef(null); - useEffect(() => () => resetState(), [resetState]); + useEffect( + () => () => { + resetState(); + initializedExpenseIdRef.current = null; + initializedGroupIdRef.current = null; + initializedFriendIdRef.current = null; + }, + [resetState], + ); // TODO: Set this globally from env var with app router later const { setMaxUploadFileSizeMB } = useAppStore((s) => s.actions); From 99a19d6867349c2ae0d6fb3d7233a2e31d5ab910 Mon Sep 17 00:00:00 2001 From: krokosik Date: Sun, 21 Jun 2026 18:11:50 +0200 Subject: [PATCH 4/5] fix: navigate immediately on save to prevent add-page flash The save handler awaited update(session) before navigating. Between mutation success (loading indicator disappears) and navPromise firing, the form was visible without a loading state, then briefly re-rendered with cleared store state during the route transition - showing the empty add flow for a moment. Navigate immediately and fire update(session) in the background. The session update is a fast client-side operation and remains valid after AddOrEditExpensePage unmounts because SessionProvider lives in _app. The currency value is captured in the closure before resetState clears the store, and onCurrencyPick already persists currency to the DB via updateProfile.mutate, so the server has the correct value regardless. --- src/components/AddExpense/AddExpensePage.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/components/AddExpense/AddExpensePage.tsx b/src/components/AddExpense/AddExpensePage.tsx index 3652d68e..febfb54a 100644 --- a/src/components/AddExpense/AddExpensePage.tsx +++ b/src/components/AddExpense/AddExpensePage.tsx @@ -172,16 +172,14 @@ export const AddOrEditExpensePage: React.FC<{ navPromise = async () => router.back(); } + navPromise().catch(console.error); update((session: any) => ({ ...session, user: { ...(session?.user ?? {}), currency, }, - })) - .then(() => navPromise()) - .then(() => resetState()) - .catch(console.error); + })).catch(console.error); } } }, @@ -206,7 +204,6 @@ export const AddOrEditExpensePage: React.FC<{ expenseDate, expenseId, router, - resetState, addExpenseMutation, group, paidBy, From f286665f98e360761a44760fe70eb751d6ff43fa Mon Sep 17 00:00:00 2001 From: krokosik Date: Sun, 21 Jun 2026 18:20:34 +0200 Subject: [PATCH 5/5] revert: restore original signed=false behavior for currency conversions The signed=true changes to currency conversion call sites and CSV export caused incorrect behavior. Revert to the original signed=false usage in: - AddExpensePage.tsx onConvertAmount - CurrencyConversion.tsx (both call sites) - Export.tsx CSV export The core #658 fix (add.tsx loading with signed=true and currency-input.tsx onFocus with allowNegative) is kept. --- src/components/AddExpense/AddExpensePage.tsx | 2 +- src/components/Friend/CurrencyConversion.tsx | 4 ++-- src/components/Friend/Export.tsx | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/AddExpense/AddExpensePage.tsx b/src/components/AddExpense/AddExpensePage.tsx index febfb54a..e61c988e 100644 --- a/src/components/AddExpense/AddExpensePage.tsx +++ b/src/components/AddExpense/AddExpensePage.tsx @@ -248,7 +248,7 @@ export const AddOrEditExpensePage: React.FC<{ to: currency, }); setAmount(targetAmount); - setAmountStr(getCurrencyHelpersCached(currency).toUIString(targetAmount, true, true)); + setAmountStr(getCurrencyHelpersCached(currency).toUIString(targetAmount, false, true)); previousCurrencyRef.current = null; }, [setAmount, setAmountStr, currency, getCurrencyHelpersCached], diff --git a/src/components/Friend/CurrencyConversion.tsx b/src/components/Friend/CurrencyConversion.tsx index 48860d66..b58974a3 100644 --- a/src/components/Friend/CurrencyConversion.tsx +++ b/src/components/Friend/CurrencyConversion.tsx @@ -53,7 +53,7 @@ export const CurrencyConversion: React.FC<{ }, [getCurrencyRate.isPending]); useEffect(() => { - setAmountStr(toUIString(amount, true, true)); + setAmountStr(toUIString(amount, false, true)); if (editingRate) { const precision = getRatePrecision(editingRate); setRate(editingRate.toFixed(precision)); @@ -136,7 +136,7 @@ export const CurrencyConversion: React.FC<{ from: targetCurrency, to: currency, }); - setAmountStr(toUIString(amount, true, true)); + setAmountStr(toUIString(amount, false, true)); } }, [rate, toUIString, targetCurrency, currency], diff --git a/src/components/Friend/Export.tsx b/src/components/Friend/Export.tsx index 2a562b9b..4b93450c 100644 --- a/src/components/Friend/Export.tsx +++ b/src/components/Friend/Export.tsx @@ -53,7 +53,7 @@ export const Export: React.FC = ({ expense.paidBy === currentUserId ? 'You' : friendName, expense.name, expense.category, - parseToCleanString(expense?.amount, true), + parseToCleanString(expense?.amount), expense.splitType, format(new Date(expense.expenseDate), 'yyyy-MM-dd HH:mm:ss'), expense.currency,