fix: preserve negative sign when editing an expense#689
Merged
Conversation
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
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
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes two issues with the edit-expense flow.
1. Edit form shows empty/add flow instead of the expense (regression)
The
initializedExpenseIdRef/initializedGroupIdRef/initializedFriendIdRefrefs were introduced in 83268ac (v2.1.0, #608) to prevent re-initialization on PWA reconnect. They persist across React StrictMode's simulated unmount/remount in dev mode, while the existingresetState()cleanup atadd.tsx:47clears the store. On StrictMode remount, the edit effect seesref.current === expenseIdand early-returns, leaving the store empty and the form rendering the add flow ("select members", all fields empty) even though the URL has the correctexpenseIdand the cachedexpenseQuery.datais populated. A hard reload works around it because it clears the React Query cache so the effect doesn't run until data arrives, by which time StrictMode is done.Fix: clear the three 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).2. Negative expenses shown as positive when editing (#658)
The edit form loaded the amount string with
toUIString(amount, false, true)(signed=false), which strips the minus, whilesetAmount(amount)correctly setisNegative=true. The store was left inconsistent: the input showed200but the model said "negative 200". Any digit edit re-derivedisNegativefrom the now-positive typed BigInt, flipping the sign on save. If the user didn't touch the field,isNegativestayedtrueand the value was unchanged — matching the reporter's repro exactly.The sign was lost at multiple layers, so the fix threads
signedthrough all of them:src/utils/numbers.ts—normalizeToMaxLengthnow accepts and forwardssignedtosanitizeInput; the string branch ofparseToCleanStringforwardssigned. The bigint branch keeps its externalsignprefix (no double-minus). Removed the unusedtoUIStringSignedhelper.src/pages/add.tsx— load the edit form withtoUIString(amount, true, true)soamountStr = '-200'matchesisNegative = true.src/components/ui/currency-input.tsx— passallowNegativetoparseToCleanStringon focus so the minus is not re-stripped when the field gains focus.src/components/AddExpense/AddExpensePage.tsx— keep the sign when converting currencies on a negative expense (onConvertAmount).src/components/Friend/CurrencyConversion.tsx— same fix in two places.src/components/Friend/Export.tsx—parseToCleanString(expense.amount, true)so CSV export of negative expenses keeps the minus (same root cause, surfaced during investigation).After the fix, editing a
-200expense shows-200in the field. Changing digits to-200.01preserves the sign; deleting the minus and saving flips it to+200.01(explicit user intent).Tests
src/tests/number.test.ts— addedtoUIStringsigned +hideSymbolcases, and a newparseToCleanStringdescribe block covering signed string and bigint inputs (plus the defaultsigned=falsebehavior).src/tests/addStore.test.ts— regression tests driving the real Zustand store: loading a negative expense keeps the minus inamountStr+isNegative=true; editing digits without removing the minus preserves the sign; deleting the minus flips it.Verification
pnpm prettier --check .— clean (only a pre-existing.opencode/package.jsonwarning, unrelated)pnpm lint— 0 errors, 284 pre-existing warningspnpm tsgo --noEmit— cleanpnpm test— 155/155 passpnpm build --no-lint— successCloses #658
Checklist
CONTRIBUTING.mdin its entirety