From 737654579f0de7a3e7a0393203355ef5eaf87dd6 Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 6 Jun 2026 13:29:48 -0400 Subject: [PATCH] feat(balance): rewrite snapshot editor reducer for holdings + dispatch on account.kind (#213) Structural rewrite of useSnapshotEditor for N holdings per detailed account, and switch ALL simple/detailed dispatch from category_kind to the account's own kind. This is the state/plumbing layer; the full multi-security entry UI (SecurityPicker, rich sub-rows) lands in #214. Simple accounts behave identically. Reducer state shape: - values: Record (simple accounts, scalar) - holdings: Record (detailed accounts, one per title) HoldingDraft is a string-typed, editable mirror of SnapshotHoldingInput with a stable client-side rowId for React keys. Actions: ADD_HOLDING / REMOVE_HOLDING / SET_HOLDING_FIELD (plus existing SET_VALUE/PREFILL/RESET). The legacy priced scalar path (SET_PRICED_FIELD / pricedValues) is removed: after migration v16 (#211) every former-priced account is kind='detailed' with one holding, so those accounts flow through the holdings path. Dispatch: - LOADED hydrates detailed baskets via listHoldingsBySnapshotLine (edit) keeping the saved price, or getHoldingsForLatestSnapshot (new) dropping the price (qty-0 excluded server-side). Simple accounts keep the scalar value path. - SnapshotLineRow / SnapshotEditor / AccountForm now gate on account.kind, not category_kind. category.kind survives ONLY as the suggested seed default for a NEW account in AccountForm. Save: detailed accounts pass their holdings array into SnapshotLineInput.holdings (presence marks the line detailed; value = rounded-cent SUM); simple accounts pass a scalar value with no holdings. Blank holding rows are skipped; a partial row throws a typed error before any DB mutation. AccountForm: adds an entry-mode selector (defaulting to the category-mapped kind). New accounts persist as 'simple' (CreateBalanceAccountInput carries no kind, and the service is out of this issue's scope); converting a fresh account to detailed + pivot date is #215. Editing locks the selector for an already- detailed account (the detailed->simple downgrade is service-guarded). Tests: 19 new reducer/helper unit tests (pure exports; the project has no renderHook harness) covering ADD/REMOVE/SET_HOLDING_FIELD, LOADED-vs-PREFILL hydration (price drop, book_cost), qty-0 already excluded upstream, the build*Lines save builders, and the dispatch-on-account.kind regression (detailed account under a 'simple' category). Resolves #213. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/components/balance/AccountForm.tsx | 99 +++- src/components/balance/SnapshotEditor.tsx | 67 +-- src/components/balance/SnapshotLineRow.tsx | 342 ++++++++----- src/hooks/useSnapshotEditor.test.ts | 313 ++++++++++++ src/hooks/useSnapshotEditor.ts | 559 ++++++++++++++------- src/i18n/locales/en.json | 16 + src/i18n/locales/fr.json | 16 + src/pages/SnapshotEditPage.tsx | 34 +- 8 files changed, 1083 insertions(+), 363 deletions(-) create mode 100644 src/hooks/useSnapshotEditor.test.ts diff --git a/src/components/balance/AccountForm.tsx b/src/components/balance/AccountForm.tsx index 15a8bc0..d3f64d5 100644 --- a/src/components/balance/AccountForm.tsx +++ b/src/components/balance/AccountForm.tsx @@ -13,6 +13,7 @@ import { FormEvent, useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; import type { BalanceAccount, + BalanceAccountKind, BalanceAssetType, BalanceCategory, BalanceCategoryKind, @@ -37,6 +38,21 @@ export interface AccountFormValues { notes: string; /** Fiscal envelope; "" means "no envelope" (→ null in the payload). */ vehicle_type: BalanceVehicleType | ""; + /** + * Entry mode (Issue #213). 'simple' = one scalar value; 'detailed' = a basket + * of per-security holdings. Drives the symbol/price-fetch gating. For a NEW + * account the category kind only *suggests* the default (priced → detailed); + * the persisted mode is set on the account itself, and the detailed→simple + * downgrade is forbidden once holdings exist (service backstop). + */ + kind: BalanceAccountKind; +} + +/** Map a category's kind to the suggested default account entry mode (#213). */ +function defaultKindForCategory( + category: BalanceCategory | undefined +): BalanceAccountKind { + return category?.kind === "priced" ? "detailed" : "simple"; } interface AccountVariantProps { @@ -88,6 +104,8 @@ function defaultAccountValues( symbol: initial.symbol ?? "", notes: initial.notes ?? "", vehicle_type: initial.vehicle_type ?? "", + // Editing: the account's stored entry mode is authoritative. + kind: initial.kind, }; } // First active category as a sane default @@ -98,6 +116,8 @@ function defaultAccountValues( symbol: "", notes: "", vehicle_type: "", + // New account: suggest the category-mapped default entry mode. + kind: defaultKindForCategory(first), }; } @@ -132,10 +152,9 @@ function AccountVariant({ }, [initialAccount, categories]); const isEditing = !!initialAccount; - const selectedCategory = categories.find( - (c) => c.id === values.balance_category_id - ); - const isPriced = selectedCategory?.kind === "priced"; + // Symbol / price-fetch gating now keys on the ACCOUNT's entry mode (#213), + // not the category kind. `category.kind` only seeds the default below. + const isDetailed = values.kind === "detailed"; const trimmedName = values.name.trim(); const trimmedSymbol = values.symbol.trim(); const nameInvalid = touched && trimmedName.length === 0; @@ -168,8 +187,20 @@ function AccountVariant({ notes: payload.notes, vehicle_type: vehicleType, }; + // Forward the entry mode only when it actually changed (avoids a no-op + // downgrade attempt; the detailed → simple direction is service-guarded + // and the selector is locked for detailed accounts anyway). A simple → + // detailed upgrade here flags the account for per-title entry; the pivot + // date wizard (#215) will own setting `detailed_since`. + if (initialAccount && values.kind !== initialAccount.kind) { + updatePayload.kind = values.kind; + } await onSubmit(updatePayload); } else { + // NEW accounts persist as the DB default ('simple') — + // `CreateBalanceAccountInput` carries no `kind`. Converting a fresh + // account to detailed happens via the dedicated flow (#215); the + // selector above only previews the suggested mode + gating. await onSubmit(payload); } }; @@ -183,12 +214,17 @@ function AccountVariant({ + setValues({ + ...values, + kind: e.target.value as BalanceAccountKind, + }) + } + className="w-full px-3 py-2 rounded-lg border border-[var(--border)] bg-[var(--card)] text-sm focus:outline-none focus:ring-1 focus:ring-[var(--primary)] disabled:opacity-60" + > + + + +

+ {t("balance.account.form.kind.hint")} + {!isEditing && values.kind === "detailed" && ( + <> + {" "} + {t("balance.account.form.kind.detailedCreateHint")} + + )} +

+ +
- {catAccounts.map((acc) => { - const priced = pricedValues[acc.id]; - return ( - onValueChange(acc.id, next)} - onQuantityChange={(next) => onQuantityChange(acc.id, next)} - onUnitPriceChange={(next) => onUnitPriceChange(acc.id, next)} - disabled={disabled} - snapshotDate={snapshotDate} - /> - ); - })} + {catAccounts.map((acc) => ( + onValueChange(acc.id, next)} + onAddHolding={() => + onAddHolding(acc.id, acc.category_asset_type ?? "stock") + } + onRemoveHolding={(rowId) => onRemoveHolding(acc.id, rowId)} + onHoldingFieldChange={(rowId, field, value) => + onHoldingFieldChange(acc.id, rowId, field, value) + } + disabled={disabled} + snapshotDate={snapshotDate} + /> + ))}
))} diff --git a/src/components/balance/SnapshotLineRow.tsx b/src/components/balance/SnapshotLineRow.tsx index b0dcee0..3e609da 100644 --- a/src/components/balance/SnapshotLineRow.tsx +++ b/src/components/balance/SnapshotLineRow.tsx @@ -1,48 +1,58 @@ // SnapshotLineRow — single account line inside the snapshot editor. // -// Two variants are dispatched by `account.category_kind`: +// Two variants are dispatched by the account's OWN `account.kind` (#213), +// NOT by `category_kind`: // -// - `simple` (Issue #146): a single value input keyed by `account_id`. -// - `priced` (Issue #140): three inputs — `quantity`, `unit_price` (both -// required), and a read-only `value` field that -// renders `quantity * unit_price` live as the -// user types. An attribution tag `[Manuel]` -// appears next to the row; the `[via Maximus]` -// tag is rendered by PriceFetchControl (Issue #158). +// - `simple` (Issue #146): a single value input keyed by `account_id`. +// - `detailed` (Issue #213): N sub-rows, one per security held — each with +// `quantity`, `unit_price` (both required), a read-only live +// `value`, and the existing PriceFetchControl. The account's +// value is the sum across its holdings. // -// We keep this component dumb on purpose: it receives strings from the -// parent (the editor stores raw strings to preserve partial input) and -// emits new strings on every change. Numeric validation happens at save -// time in `useSnapshotEditor.save` against the service's -// `validateLineKindInvariants` helper. +// The OLD "priced scalar" variant (one security via account.symbol + scalar +// quantity/unit_price on the line) is SUPERSEDED: migration v16 (#211) +// converted every former-priced account into `kind='detailed'` with one +// holding, so those accounts now flow through the detailed (holdings) path. +// +// This PR (#213) keeps the detailed UI deliberately minimal — it must round- +// trip a converted 1-holding account and not crash; the full add/remove + +// SecurityPicker UX lands in #214. The symbol field here is a plain text input +// (the autocomplete picker is #214). +// +// We keep this component dumb on purpose: it receives strings from the parent +// (the editor stores raw strings to preserve partial input) and emits new +// strings on every change. Numeric validation happens at save time in +// `useSnapshotEditor.save`. import { ChangeEvent, useMemo } from "react"; import { useTranslation } from "react-i18next"; +import { Plus, Trash2 } from "lucide-react"; import type { BalanceAccountWithCategory } from "../../shared/types"; +import type { HoldingDraft } from "../../hooks/useSnapshotEditor"; import PriceFetchControl from "./PriceFetchControl"; -interface BaseProps { +interface Props { account: BalanceAccountWithCategory; disabled?: boolean; /** Snapshot date (YYYY-MM-DD) — passed through to PriceFetchControl. */ snapshotDate?: string; -} - -interface SimpleProps extends BaseProps { + /** Simple variant: the scalar value string + its change handler. */ value: string; onChange: (next: string) => void; - /** Optional priced handlers for callers that wire both at once. */ - quantityValue?: string; - unitPriceValue?: string; - onQuantityChange?: (next: string) => void; - onUnitPriceChange?: (next: string) => void; + /** Detailed variant: the holdings basket + mutators (#213). */ + holdings?: HoldingDraft[]; + onAddHolding?: () => void; + onRemoveHolding?: (rowId: string) => void; + onHoldingFieldChange?: ( + rowId: string, + field: keyof Omit, + value: string + ) => void; } -type Props = SimpleProps; - /** * Parse a string like "12.34" or "12,34" into a finite number, or null - * if invalid / empty. Used by the priced variant to compute the live + * if invalid / empty. Used by the detailed sub-rows to compute the live * `value` preview. */ function parseDecimal(raw: string): number | null { @@ -58,129 +68,84 @@ export default function SnapshotLineRow({ value, onChange, disabled, - quantityValue, - unitPriceValue, - onQuantityChange, - onUnitPriceChange, snapshotDate, + holdings, + onAddHolding, + onRemoveHolding, + onHoldingFieldChange, }: Props) { const { t } = useTranslation(); - const isPriced = account.category_kind === "priced"; + const isDetailed = account.kind === "detailed"; - // Compute the live value preview for priced rows. Returns null when - // either input cannot yet be parsed (so we display a placeholder). - const computedPricedValue = useMemo(() => { - if (!isPriced) return null; - const qty = parseDecimal(quantityValue ?? ""); - const price = parseDecimal(unitPriceValue ?? ""); - if (qty === null || price === null) return null; - return qty * price; - }, [isPriced, quantityValue, unitPriceValue]); - - if (isPriced) { - const handleQty = (e: ChangeEvent) => - onQuantityChange?.(e.target.value); - const handlePrice = (e: ChangeEvent) => - onUnitPriceChange?.(e.target.value); + // Account total across the basket (live as the user types). + const detailedTotal = useMemo(() => { + if (!isDetailed || !holdings) return null; + let total = 0; + for (const h of holdings) { + const qty = parseDecimal(h.quantity); + const price = parseDecimal(h.unit_price); + if (qty !== null && price !== null) total += qty * price; + } + return total; + }, [isDetailed, holdings]); + if (isDetailed) { + const rows = holdings ?? []; return ( -
-
-
+
+
+
{account.name} - {t("balance.snapshot.priced.attributionManual")} + {t("balance.snapshot.detailed.badge")}
- {account.symbol && ( -
- {account.symbol} -
+ {detailedTotal !== null && ( + + {detailedTotal.toLocaleString(undefined, { + minimumFractionDigits: 2, + maximumFractionDigits: 2, + })}{" "} + {account.currency} + )}
-
-
- - - {t("balance.snapshot.priced.quantity")} - + + {rows.length === 0 ? ( +

+ {t("balance.snapshot.detailed.empty")} +

+ ) : ( +
+ {rows.map((h) => ( + + onHoldingFieldChange?.(h.rowId, field, v) + } + onRemove={() => onRemoveHolding?.(h.rowId)} + /> + ))}
- - × - -
- - - {t("balance.snapshot.priced.unitPrice")} - -
- - = - -
- - - {t("balance.snapshot.priced.computedValue")} - -
- - {account.currency} - - {/* PriceFetchControl — wired next to the unit_price input. - Hidden when category_asset_type is null (legacy custom priced - rows pre-#169 migration; user must edit the category to set it). */} - {account.symbol && account.category_asset_type && ( - - onUnitPriceChange?.(String(price)) - } - /> - )} -
+ )} + +
); } @@ -220,3 +185,114 @@ export default function SnapshotLineRow({
); } + +// ----------------------------------------------------------------------------- +// Detailed sub-row — one security position (#213, minimal; full UX in #214). +// ----------------------------------------------------------------------------- + +function HoldingSubRow({ + holding, + accountName, + accountCurrency, + snapshotDate, + disabled, + onFieldChange, + onRemove, +}: { + holding: HoldingDraft; + accountName: string; + accountCurrency: string; + snapshotDate?: string; + disabled?: boolean; + onFieldChange: (field: keyof Omit, value: string) => void; + onRemove: () => void; +}) { + const { t } = useTranslation(); + const computedValue = useMemo(() => { + const qty = parseDecimal(holding.quantity); + const price = parseDecimal(holding.unit_price); + if (qty === null || price === null) return null; + return qty * price; + }, [holding.quantity, holding.unit_price]); + + const label = holding.symbol || accountName; + + return ( +
+ onFieldChange("symbol", e.target.value)} + disabled={disabled} + placeholder={t("balance.snapshot.detailed.symbolPlaceholder")} + className="w-28 px-2 py-1.5 rounded-lg border border-[var(--border)] bg-[var(--card)] text-sm focus:outline-none focus:ring-1 focus:ring-[var(--primary)] disabled:opacity-50" + aria-label={t("balance.snapshot.detailed.symbolLabel")} + autoComplete="off" + /> + onFieldChange("quantity", e.target.value)} + disabled={disabled} + placeholder={t("balance.snapshot.priced.quantityPlaceholder")} + className="w-20 px-2 py-1.5 rounded-lg border border-[var(--border)] bg-[var(--card)] text-sm text-right focus:outline-none focus:ring-1 focus:ring-[var(--primary)] disabled:opacity-50" + aria-label={t("balance.snapshot.priced.quantityLabel", { + account: label, + })} + /> + + × + + onFieldChange("unit_price", e.target.value)} + disabled={disabled} + placeholder={t("balance.snapshot.priced.unitPricePlaceholder")} + className="w-24 px-2 py-1.5 rounded-lg border border-[var(--border)] bg-[var(--card)] text-sm text-right focus:outline-none focus:ring-1 focus:ring-[var(--primary)] disabled:opacity-50" + aria-label={t("balance.snapshot.priced.unitPriceLabel", { + account: label, + })} + /> + + = + + + + {accountCurrency} + + {holding.symbol && ( + onFieldChange("unit_price", String(price))} + /> + )} + +
+ ); +} diff --git a/src/hooks/useSnapshotEditor.test.ts b/src/hooks/useSnapshotEditor.test.ts new file mode 100644 index 0000000..bc1b6f5 --- /dev/null +++ b/src/hooks/useSnapshotEditor.test.ts @@ -0,0 +1,313 @@ +// useSnapshotEditor — reducer + pure-helper unit tests (Issue #213). +// +// The project has no jsdom / @testing-library renderHook harness (see the note +// in StarterAccountsModal.test.tsx), so we test the EXTRACTED pure pieces of +// the editor's state machine directly: the `reducer`, the LOADED/prefill +// hydration mapper `holdingsFromServiceHoldings`, and the save builders +// `buildSimpleLines` / `buildDetailedLines`. This covers the holdings actions +// (ADD/REMOVE/SET_HOLDING_FIELD), prefill qty-0 exclusion + price drop, and the +// dispatch-on-account.kind invariant (a detailed account under a `simple` +// category still flows through the holdings path). + +import { describe, it, expect, vi } from "vitest"; + +// db is imported transitively (hook → balance.service → db → tauri plugins). +// Stub it so no module-load path tries a real connection. +vi.mock("../services/db", () => ({ + getDb: vi.fn(), + connectToProfile: vi.fn(), + initializeNewProfileDb: vi.fn(), +})); + +import { + reducer, + initialState, + makeEmptyHolding, + holdingsFromServiceHoldings, + buildSimpleLines, + buildDetailedLines, + type HoldingDraft, +} from "./useSnapshotEditor"; +import { BalanceServiceError } from "../services/balance.service"; +import type { BalanceSnapshotHoldingWithSecurity } from "../shared/types"; + +function holdingRow( + over: Partial +): BalanceSnapshotHoldingWithSecurity { + return { + id: 1, + snapshot_line_id: 10, + security_id: 100, + quantity: 5, + unit_price: 20, + value: 100, + book_cost: 80, + price_source: "manual", + price_fetched_at: null, + created_at: "2026-01-01", + updated_at: "2026-01-01", + security_symbol: "AAPL", + security_name: "Apple Inc.", + security_asset_type: "stock", + ...over, + }; +} + +const base = initialState("2026-06-06"); + +describe("reducer — holdings actions (#213)", () => { + it("ADD_HOLDING appends a draft to the account's basket", () => { + const h = makeEmptyHolding("stock"); + const next = reducer(base, { + type: "ADD_HOLDING", + payload: { accountId: 42, holding: h }, + }); + expect(next.holdings[42]).toHaveLength(1); + expect(next.holdings[42][0].rowId).toBe(h.rowId); + expect(next.isDirty).toBe(true); + }); + + it("ADD_HOLDING keeps other accounts' baskets untouched", () => { + const s1 = reducer(base, { + type: "ADD_HOLDING", + payload: { accountId: 1, holding: makeEmptyHolding() }, + }); + const s2 = reducer(s1, { + type: "ADD_HOLDING", + payload: { accountId: 2, holding: makeEmptyHolding() }, + }); + expect(s2.holdings[1]).toHaveLength(1); + expect(s2.holdings[2]).toHaveLength(1); + }); + + it("SET_HOLDING_FIELD updates only the targeted row + field", () => { + const a = makeEmptyHolding(); + const b = makeEmptyHolding(); + let s = reducer(base, { + type: "ADD_HOLDING", + payload: { accountId: 7, holding: a }, + }); + s = reducer(s, { type: "ADD_HOLDING", payload: { accountId: 7, holding: b } }); + s = reducer(s, { + type: "SET_HOLDING_FIELD", + payload: { accountId: 7, rowId: b.rowId, field: "quantity", value: "12" }, + }); + expect(s.holdings[7].find((h) => h.rowId === a.rowId)!.quantity).toBe(""); + expect(s.holdings[7].find((h) => h.rowId === b.rowId)!.quantity).toBe("12"); + }); + + it("REMOVE_HOLDING drops the targeted row, preserving the rest", () => { + const a = makeEmptyHolding(); + const b = makeEmptyHolding(); + let s = reducer(base, { + type: "ADD_HOLDING", + payload: { accountId: 7, holding: a }, + }); + s = reducer(s, { type: "ADD_HOLDING", payload: { accountId: 7, holding: b } }); + s = reducer(s, { + type: "REMOVE_HOLDING", + payload: { accountId: 7, rowId: a.rowId }, + }); + expect(s.holdings[7]).toHaveLength(1); + expect(s.holdings[7][0].rowId).toBe(b.rowId); + }); + + it("makeEmptyHolding gives each row a distinct rowId", () => { + expect(makeEmptyHolding().rowId).not.toBe(makeEmptyHolding().rowId); + }); + + it("RESET wipes both values and holdings", () => { + let s = reducer(base, { + type: "SET_VALUE", + payload: { accountId: 1, value: "100" }, + }); + s = reducer(s, { + type: "ADD_HOLDING", + payload: { accountId: 2, holding: makeEmptyHolding() }, + }); + s = reducer(s, { type: "RESET" }); + expect(s.values).toEqual({}); + expect(s.holdings).toEqual({}); + }); + + it("PREFILL merges simple values and detailed baskets", () => { + const draft = makeEmptyHolding(); + const s = reducer(base, { + type: "PREFILL", + payload: { values: { 1: "500" }, holdings: { 2: [draft] } }, + }); + expect(s.values[1]).toBe("500"); + expect(s.holdings[2][0].rowId).toBe(draft.rowId); + expect(s.isDirty).toBe(true); + }); +}); + +describe("holdingsFromServiceHoldings — LOADED vs PREFILL (#213)", () => { + it("LOADED (keepPrice) carries qty, price, book_cost and source", () => { + const rows = [holdingRow({ quantity: 5, unit_price: 20, book_cost: 80 })]; + const [d] = holdingsFromServiceHoldings(rows, { keepPrice: true }); + expect(d.symbol).toBe("AAPL"); + expect(d.asset_type).toBe("stock"); + expect(d.quantity).toBe("5"); + expect(d.unit_price).toBe("20"); + expect(d.book_cost).toBe("80"); + expect(d.price_source).toBe("manual"); + }); + + it("PREFILL (keepPrice=false) drops price + source, keeps qty + book_cost", () => { + const rows = [holdingRow({ quantity: 5, unit_price: 20, book_cost: 80 })]; + const [d] = holdingsFromServiceHoldings(rows, { keepPrice: false }); + expect(d.quantity).toBe("5"); + expect(d.book_cost).toBe("80"); + // Price is re-entered / re-fetched each snapshot, so it is intentionally blank. + expect(d.unit_price).toBe(""); + expect(d.price_source).toBeNull(); + expect(d.price_fetched_at).toBeNull(); + }); + + it("maps a NULL book_cost to an empty string", () => { + const [d] = holdingsFromServiceHoldings([holdingRow({ book_cost: null })], { + keepPrice: true, + }); + expect(d.book_cost).toBe(""); + }); + + it("preserves the per-security rows the service returns (qty-0 already excluded upstream)", () => { + // getHoldingsForLatestSnapshot excludes qty-0 server-side; the mapper is a + // faithful 1:1 projection — a sold-then-rebought title arrives as its + // latest non-zero holding and is mapped like any other. + const rows = [ + holdingRow({ security_symbol: "AAPL", quantity: 3 }), + holdingRow({ security_symbol: "MSFT", quantity: 7, security_id: 101 }), + ]; + const drafts = holdingsFromServiceHoldings(rows, { keepPrice: false }); + expect(drafts.map((d) => d.symbol)).toEqual(["AAPL", "MSFT"]); + expect(drafts.map((d) => d.quantity)).toEqual(["3", "7"]); + }); +}); + +describe("buildSimpleLines — scalar save path (#213)", () => { + it("emits only non-detailed accounts with a non-empty value", () => { + const lines = buildSimpleLines( + { 1: "100", 2: " ", 3: "250", 4: "999" }, + new Set([4]) // account 4 is detailed → must NOT appear here + ); + expect(lines).toEqual([ + { account_id: 1, value: 100, account_kind: "simple" }, + { account_id: 3, value: 250, account_kind: "simple" }, + ]); + }); + + it("accepts comma decimals and throws on a non-numeric value", () => { + expect(buildSimpleLines({ 1: "12,5" }, new Set())[0].value).toBe(12.5); + expect(() => buildSimpleLines({ 1: "abc" }, new Set())).toThrow( + BalanceServiceError + ); + }); +}); + +describe("buildDetailedLines — holdings save path (#213)", () => { + function draft(over: Partial): HoldingDraft { + return { + ...makeEmptyHolding("stock"), + ...over, + }; + } + + it("builds one line per detailed account, value = rounded-cent SUM of holdings", () => { + const holdings = { + 5: [ + draft({ symbol: "AAPL", quantity: "2", unit_price: "10.005", book_cost: "15" }), + draft({ symbol: "MSFT", quantity: "1", unit_price: "30.10" }), + ], + }; + const [line] = buildDetailedLines(holdings, new Set([5])); + expect(line.account_id).toBe(5); + expect(line.holdings).toHaveLength(2); + // AAPL 2 * 10.005 = 20.01 (rounded), MSFT 1 * 30.10 = 30.10 → 50.11. + expect(line.holdings![0].value).toBe(20.01); + expect(line.value).toBe(50.11); + // The aggregated line carries the holdings field (marks it detailed) and + // no scalar qty/price. + expect(line.holdings).toBeDefined(); + expect(line.quantity).toBeUndefined(); + expect(line.unit_price).toBeUndefined(); + }); + + it("emits an EMPTY holdings array for a detailed account with no positions (still detailed)", () => { + const [line] = buildDetailedLines({ 5: [] }, new Set([5])); + expect(line.holdings).toEqual([]); + expect(line.value).toBe(0); + }); + + it("includes a detailed account even when its basket key is absent", () => { + // Dispatch is on account.kind: account 9 is detailed but the user never + // touched it, so `holdings[9]` is undefined — it must still yield a line + // (with an empty holdings array) so the save records it as detailed. + const [line] = buildDetailedLines({}, new Set([9])); + expect(line.account_id).toBe(9); + expect(line.holdings).toEqual([]); + }); + + it("skips a fully-blank row but throws on a partially-filled one", () => { + const blank = draft({ symbol: "", quantity: "", unit_price: "" }); + const okRow = draft({ symbol: "BTC", quantity: "1", unit_price: "100", asset_type: "crypto" }); + const built = buildDetailedLines({ 5: [blank, okRow] }, new Set([5])); + expect(built[0].holdings).toHaveLength(1); + expect(built[0].holdings![0].symbol).toBe("BTC"); + // A symbol with no quantity is a partial row → typed error, no silent drop. + const partial = draft({ symbol: "ETH", quantity: "", unit_price: "" }); + expect(() => buildDetailedLines({ 5: [partial] }, new Set([5]))).toThrow( + BalanceServiceError + ); + }); + + it("carries asset_type + currency + book_cost through to the holding input", () => { + const [line] = buildDetailedLines( + { + 5: [ + draft({ + symbol: "btc", + asset_type: "crypto", + currency: "CAD", + quantity: "0.5", + unit_price: "1000", + book_cost: "400", + }), + ], + }, + new Set([5]) + ); + const h = line.holdings![0]; + expect(h.symbol).toBe("btc"); // trimmed only here; service normalizes case + expect(h.asset_type).toBe("crypto"); + expect(h.currency).toBe("CAD"); + expect(h.book_cost).toBe(400); + expect(h.value).toBe(500); + }); +}); + +describe("dispatch on account.kind — detailed under a 'simple' category (#213)", () => { + it("routes a detailed account through holdings even if its category is simple", () => { + // Regression target: the account's OWN kind decides the path. Account 5 is + // detailed; nothing about a 'simple' category should pull it into the + // scalar (buildSimpleLines) path. + const detailedIds = new Set([5]); + const values = { 5: "12345" }; // a stray scalar value must be ignored + const holdings = { + 5: [ + { + ...makeEmptyHolding("stock"), + symbol: "AAPL", + quantity: "2", + unit_price: "50", + }, + ], + }; + expect(buildSimpleLines(values, detailedIds)).toEqual([]); // not scalar + const [line] = buildDetailedLines(holdings, detailedIds); + expect(line.account_id).toBe(5); + expect(line.value).toBe(100); + expect(line.holdings![0].symbol).toBe("AAPL"); + }); +}); diff --git a/src/hooks/useSnapshotEditor.ts b/src/hooks/useSnapshotEditor.ts index a72d927..67b7135 100644 --- a/src/hooks/useSnapshotEditor.ts +++ b/src/hooks/useSnapshotEditor.ts @@ -1,16 +1,29 @@ // useSnapshotEditor — scoped useReducer hook backing SnapshotEditPage. // -// Lifecycle of a single snapshot (Issue #146 / Bilan #1b — simple kind only): +// Lifecycle of a single snapshot (Issue #146 / Bilan #1b; reworked for +// per-title detail in Issue #213 / Bilan détail par titre): // 1. mount in 'new' mode (no `?date=` query param) → user picks a date, -// types values, hits Save → service.createSnapshot + upsertLines; +// types values, hits Save → service.saveSnapshotAtomic; // 2. mount in 'edit' mode (`?date=YYYY-MM-DD`) → load snapshot + lines, -// user edits values, hits Save → upsertLines on the existing snapshot; +// user edits values, hits Save → upsert on the existing snapshot; // 3. delete → service.deleteSnapshot (the page wraps this in a // double-confirm modal that requires retyping the snapshot date). // -// Priced-kind UI lands in #140 (Bilan #2). Until then values are scalar -// numbers keyed by account_id and quantity/unit_price are forced to NULL by -// `upsertSnapshotLines` (the SQL CHECK guards the invariant too). +// ENTRY MODE DISPATCH (#213) — the editor classifies each account by its OWN +// `account.kind` (simple | detailed), NOT by `category_kind` (simple | priced). +// The category kind is only a *suggested default* for a brand-new account in +// AccountForm; once an account exists, its stored `kind` is authoritative. +// - simple : one scalar value per account, kept as a string in `values`. +// - detailed: a basket of per-security holdings in `holdings` — one +// `HoldingDraft` per title. On save the aggregated line carries +// NO scalar qty/price; the service recomputes value = SUM(holdings) +// and writes the holdings in the same transaction. +// +// The legacy "priced scalar" path (one security per account via account.symbol +// + scalar quantity/unit_price on the line) is SUPERSEDED: after migration v16 +// (#211) every former-priced account is now `kind='detailed'` with one holding, +// so those accounts flow through the holdings path. There is no scalar-priced +// editor branch anymore. import { useReducer, @@ -20,9 +33,11 @@ import { } from "react"; import type { BalanceAccountWithCategory, + BalanceAssetType, BalanceCategory, BalanceSnapshot, BalanceSnapshotLine, + BalanceSnapshotHoldingWithSecurity, } from "../shared/types"; import { listBalanceAccounts, @@ -32,15 +47,106 @@ import { listLinesBySnapshot, saveSnapshotAtomic, getPreviousSnapshot, + getHoldingsForLatestSnapshot, + listHoldingsBySnapshotLine, BalanceServiceError, + type SnapshotLineInput, + type SnapshotHoldingInput, } from "../services/balance.service"; export type SnapshotEditorMode = "new" | "edit"; -/** String-typed entry for a priced-kind line being edited. */ -export interface PricedEntry { +/** + * String-typed, editable mirror of one position inside a detailed account + * (Issue #213). All numeric fields are kept as strings to preserve empty / + * partial input; conversion to numbers happens at save time. `rowId` is a + * stable client-side identity so React can key the sub-rows even before the + * holding is persisted (a fresh holding has no DB id yet). + */ +export interface HoldingDraft { + /** Stable client-side row identity for React keys (NOT a DB id). */ + rowId: string; + /** Security symbol (normalized server-side). */ + symbol: string; + asset_type: BalanceAssetType; + /** ISO 4217; defaults to 'CAD'. */ + currency: string; + /** Optional human-readable security name. */ + security_name: string; quantity: string; unit_price: string; + /** Acquisition cost basis for the unrealized-gain column; optional. */ + book_cost: string; + /** Carried through from a fetched price so save can attribute the source. */ + price_source: string | null; + price_fetched_at: string | null; +} + +let holdingRowSeq = 0; +/** Monotonic client-side row id for holding drafts. */ +function nextRowId(): string { + holdingRowSeq += 1; + return `h${holdingRowSeq}`; +} + +/** + * Build an empty holding draft (used by ADD_HOLDING). `asset_type` defaults to + * the account's category asset_type when known, else 'stock'. + */ +export function makeEmptyHolding( + assetType: BalanceAssetType = "stock" +): HoldingDraft { + return { + rowId: nextRowId(), + symbol: "", + asset_type: assetType, + currency: "CAD", + security_name: "", + quantity: "", + unit_price: "", + book_cost: "", + price_source: null, + price_fetched_at: null, + }; +} + +/** + * Map server holdings (from `getHoldingsForLatestSnapshot` for prefill, or + * `listHoldingsBySnapshotLine` for an edited snapshot) into editable string + * drafts. `keepPrice` controls whether the unit_price is carried over: + * - LOADED (editing an existing snapshot) keeps the saved price. + * - PREFILL of the NEXT snapshot drops the price (the user re-enters or + * re-fetches it) but keeps quantity + book_cost. Titles with quantity 0 are + * already excluded by `getHoldingsForLatestSnapshot`; a title sold then + * re-bought reappears because its latest non-zero holding wins server-side. + */ +export function holdingsFromServiceHoldings( + rows: BalanceSnapshotHoldingWithSecurity[], + opts: { keepPrice: boolean } +): HoldingDraft[] { + return rows.map((h) => ({ + rowId: nextRowId(), + symbol: h.security_symbol, + asset_type: h.security_asset_type, + // The joined holdings view doesn't carry the security currency; CAD is the + // MVP currency and the save path defaults it server-side anyway. + currency: "CAD", + security_name: h.security_name ?? "", + quantity: + h.quantity !== null && h.quantity !== undefined ? String(h.quantity) : "", + unit_price: opts.keepPrice + ? h.unit_price !== null && h.unit_price !== undefined + ? String(h.unit_price) + : "" + : "", + book_cost: + h.book_cost !== null && h.book_cost !== undefined + ? String(h.book_cost) + : "", + // Prefill drops the source (the carried price is stale); LOADED keeps it. + price_source: opts.keepPrice ? h.price_source : null, + price_fetched_at: opts.keepPrice ? h.price_fetched_at : null, + })); } interface State { @@ -54,16 +160,17 @@ interface State { /** Used to group lines by category in the editor view. */ categories: BalanceCategory[]; /** - * Map of account_id → string-typed value (simple kind only). We keep - * strings to preserve empty / partial input; conversion to number - * happens at save time. + * Map of account_id → string-typed value (simple accounts only). We keep + * strings to preserve empty / partial input; conversion to number happens + * at save time. */ values: Record; /** - * Map of account_id → string-typed `{quantity, unit_price}` (priced - * kind only). Same partial-input guarantee as `values`. + * Map of account_id → array of `HoldingDraft` (detailed accounts only — + * dispatched on `account.kind === 'detailed'`). One entry per security held. + * Same partial-input guarantee as `values`. */ - pricedValues: Record; + holdings: Record; /** Snapshot whose values would prefill if the user clicks "Prefill". */ previousSnapshot: BalanceSnapshot | null; /** Lines from `previousSnapshot` (loaded lazily when needed). */ @@ -88,7 +195,7 @@ type Action = accounts: BalanceAccountWithCategory[]; categories: BalanceCategory[]; values: Record; - pricedValues: Record; + holdings: Record; previousSnapshot: BalanceSnapshot | null; previousLines: BalanceSnapshotLine[] | null; }; @@ -96,10 +203,16 @@ type Action = | { type: "SET_DATE"; payload: string } | { type: "SET_VALUE"; payload: { accountId: number; value: string } } | { - type: "SET_PRICED_FIELD"; + type: "ADD_HOLDING"; + payload: { accountId: number; holding: HoldingDraft }; + } + | { type: "REMOVE_HOLDING"; payload: { accountId: number; rowId: string } } + | { + type: "SET_HOLDING_FIELD"; payload: { accountId: number; - field: "quantity" | "unit_price"; + rowId: string; + field: keyof Omit; value: string; }; } @@ -107,13 +220,13 @@ type Action = type: "PREFILL"; payload: { values: Record; - pricedValues: Record; + holdings: Record; }; } | { type: "RESET" } | { type: "CLEAR_DIRTY" }; -function initialState(initialDate: string): State { +export function initialState(initialDate: string): State { return { mode: "new", snapshotDate: initialDate, @@ -121,7 +234,7 @@ function initialState(initialDate: string): State { accounts: [], categories: [], values: {}, - pricedValues: {}, + holdings: {}, previousSnapshot: null, previousLines: null, isLoading: false, @@ -132,7 +245,11 @@ function initialState(initialDate: string): State { }; } -function reducer(state: State, action: Action): State { +/** + * Pure reducer — exported so the editor's state machine can be unit-tested + * without rendering the hook (the project has no jsdom/renderHook harness). + */ +export function reducer(state: State, action: Action): State { switch (action.type) { case "SET_LOADING": return { ...state, isLoading: action.payload }; @@ -155,7 +272,7 @@ function reducer(state: State, action: Action): State { accounts: action.payload.accounts, categories: action.payload.categories, values: action.payload.values, - pricedValues: action.payload.pricedValues, + holdings: action.payload.holdings, previousSnapshot: action.payload.previousSnapshot, previousLines: action.payload.previousLines, isLoading: false, @@ -176,20 +293,41 @@ function reducer(state: State, action: Action): State { }, isDirty: true, }; - case "SET_PRICED_FIELD": { - const existing = - state.pricedValues[action.payload.accountId] ?? { - quantity: "", - unit_price: "", - }; - const next: PricedEntry = - action.payload.field === "quantity" - ? { ...existing, quantity: action.payload.value } - : { ...existing, unit_price: action.payload.value }; + case "ADD_HOLDING": { + const existing = state.holdings[action.payload.accountId] ?? []; return { ...state, - pricedValues: { - ...state.pricedValues, + holdings: { + ...state.holdings, + [action.payload.accountId]: [...existing, action.payload.holding], + }, + isDirty: true, + }; + } + case "REMOVE_HOLDING": { + const existing = state.holdings[action.payload.accountId] ?? []; + return { + ...state, + holdings: { + ...state.holdings, + [action.payload.accountId]: existing.filter( + (h) => h.rowId !== action.payload.rowId + ), + }, + isDirty: true, + }; + } + case "SET_HOLDING_FIELD": { + const existing = state.holdings[action.payload.accountId] ?? []; + const next = existing.map((h) => + h.rowId === action.payload.rowId + ? { ...h, [action.payload.field]: action.payload.value } + : h + ); + return { + ...state, + holdings: { + ...state.holdings, [action.payload.accountId]: next, }, isDirty: true, @@ -199,19 +337,16 @@ function reducer(state: State, action: Action): State { return { ...state, values: { ...state.values, ...action.payload.values }, - pricedValues: { - ...state.pricedValues, - ...action.payload.pricedValues, - }, + holdings: { ...state.holdings, ...action.payload.holdings }, isDirty: true, }; case "RESET": return { ...state, // Keep the loaded structure (accounts, categories, snapshot) but wipe - // user input back to a clean slate sourced from the saved lines. + // user input back to a clean slate. values: {}, - pricedValues: {}, + holdings: {}, isDirty: true, }; case "CLEAR_DIRTY": @@ -240,6 +375,122 @@ function todayISO(): string { return `${yyyy}-${mm}-${dd}`; } +/** Parse "12.34" / "12,34" → finite number, or null when empty/invalid. */ +function parseDecimal(raw: string | null | undefined): number | null { + if (raw === null || raw === undefined) return null; + const trimmed = String(raw).trim().replace(",", "."); + if (!trimmed) return null; + const n = Number(trimmed); + return Number.isFinite(n) ? n : null; +} + +/** + * Build the simple-account `SnapshotLineInput[]` from the editor's `values` + * map. Only accounts whose own kind is NOT detailed contribute here; detailed + * accounts go through `buildDetailedLines`. THROWS a typed BalanceServiceError + * on the first invalid value so no DB mutation happens on bad input (#176). + * Exported for unit tests. + */ +export function buildSimpleLines( + values: Record, + detailedAccountIds: ReadonlySet +): SnapshotLineInput[] { + return Object.entries(values) + .filter( + ([accountIdStr, v]) => + !detailedAccountIds.has(Number(accountIdStr)) && + v !== undefined && + String(v).trim().length > 0 + ) + .map(([accountIdStr, raw]) => { + const accountId = Number(accountIdStr); + const num = parseDecimal(raw); + if (num === null) { + throw new BalanceServiceError( + "snapshot_value_invalid", + `Invalid value for account ${accountId}: "${raw}"` + ); + } + return { + account_id: accountId, + value: num, + account_kind: "simple" as const, + }; + }); +} + +/** + * Build the detailed-account `SnapshotLineInput[]` (one per account, each + * carrying its `holdings` array) from the editor's `holdings` map. The presence + * of the `holdings` field — even an empty array — marks the line detailed for + * the service. Empty / blank holding rows (no symbol AND no qty AND no price) + * are dropped before save so a half-typed row doesn't fail validation. THROWS a + * typed error on a partially-filled row. The aggregated `value` is the SUM of + * the rounded-cent holding values; the service re-rounds and re-validates it. + * Exported for unit tests. + */ +export function buildDetailedLines( + holdings: Record, + detailedAccountIds: ReadonlySet +): SnapshotLineInput[] { + const lines: SnapshotLineInput[] = []; + for (const accountId of detailedAccountIds) { + const drafts = holdings[accountId] ?? []; + const built: SnapshotHoldingInput[] = []; + for (const d of drafts) { + const symbol = d.symbol.trim(); + const qtyRaw = String(d.quantity ?? "").trim(); + const priceRaw = String(d.unit_price ?? "").trim(); + const isBlank = symbol.length === 0 && qtyRaw.length === 0 && priceRaw.length === 0; + if (isBlank) continue; // skip an untouched / freshly-added empty row + if (symbol.length === 0) { + throw new BalanceServiceError( + "snapshot_holding_invalid", + `A holding for account ${accountId} is missing its symbol` + ); + } + const qty = parseDecimal(d.quantity); + const price = parseDecimal(d.unit_price); + if (qty === null) { + throw new BalanceServiceError( + "snapshot_priced_quantity_required", + `Invalid quantity for ${symbol} (account ${accountId}): "${d.quantity}"` + ); + } + if (price === null) { + throw new BalanceServiceError( + "snapshot_priced_unit_price_required", + `Invalid unit price for ${symbol} (account ${accountId}): "${d.unit_price}"` + ); + } + const bookCost = parseDecimal(d.book_cost); + const value = Math.round(qty * price * 100) / 100; + built.push({ + symbol, + asset_type: d.asset_type, + currency: d.currency || "CAD", + security_name: d.security_name.trim() || null, + quantity: qty, + unit_price: price, + value, + book_cost: bookCost, + price_source: d.price_source, + price_fetched_at: d.price_fetched_at, + }); + } + // Aggregated value = rounded-cent SUM of the holdings' rounded-cent values. + const total = + Math.round(built.reduce((s, h) => s + h.value, 0) * 100) / 100; + lines.push({ + account_id: accountId, + value: total, + // `holdings` present (even empty) ⇒ detailed save path; qty/price omitted. + holdings: built, + }); + } + return lines; +} + interface Options { /** ISO date from the route query string. `undefined` means 'new' mode. */ dateParam?: string | null; @@ -271,42 +522,50 @@ export function useSnapshotEditor(options: Options = {}) { listBalanceAccounts(), listBalanceCategories(), ]); + const values: Record = {}; + const holdings: Record = {}; + let previousLines: BalanceSnapshotLine[] | null = null; + // Index each account's OWN kind (simple|detailed) — this, not the + // category kind, decides which input map a line belongs to (#213). + const accountById = new Map(); + for (const acc of accounts) accountById.set(acc.id, acc); + const existing = await getSnapshotByDate(targetDate); const isEdit = !!existing; - let values: Record = {}; - let pricedValues: Record = {}; - let previousLines: BalanceSnapshotLine[] | null = null; - // Index account kinds for quick line classification. - const kindByAccountId = new Map(); - for (const acc of accounts) { - kindByAccountId.set(acc.id, acc.category_kind); - } if (existing) { const lines = await listLinesBySnapshot(existing.id); for (const line of lines) { - // The line itself carries quantity / unit_price for priced kinds; - // we still cross-check against the account kind to decide which - // input map this row belongs to (it dictates what the user sees). - const kind = kindByAccountId.get(line.account_id); - if ( - kind === "priced" || - (line.quantity !== null && line.unit_price !== null) - ) { - pricedValues[line.account_id] = { - quantity: - line.quantity !== null && line.quantity !== undefined - ? String(line.quantity) - : "", - unit_price: - line.unit_price !== null && line.unit_price !== undefined - ? String(line.unit_price) - : "", - }; + const acc = accountById.get(line.account_id); + if (acc?.kind === "detailed") { + // Hydrate the basket from this line's persisted holdings. + const rows = await listHoldingsBySnapshotLine(line.id); + holdings[line.account_id] = holdingsFromServiceHoldings(rows, { + keepPrice: true, + }); } else { values[line.account_id] = String(line.value); } } + // Detailed accounts with NO line yet at this snapshot still get an + // (empty) basket so the editor renders the detailed variant for them. + for (const acc of accounts) { + if (acc.kind === "detailed" && holdings[acc.id] === undefined) { + holdings[acc.id] = []; + } + } + } else { + // 'new' mode: prefill detailed baskets from each account's latest + // snapshot holdings (qty-0 excluded server-side), price dropped. + for (const acc of accounts) { + if (acc.kind === "detailed") { + const rows = await getHoldingsForLatestSnapshot(acc.id); + holdings[acc.id] = holdingsFromServiceHoldings(rows, { + keepPrice: false, + }); + } + } } + const previous = await getPreviousSnapshot(targetDate); if (previous) { previousLines = await listLinesBySnapshot(previous.id); @@ -321,7 +580,7 @@ export function useSnapshotEditor(options: Options = {}) { accounts, categories, values, - pricedValues, + holdings, previousSnapshot: previous, previousLines, }, @@ -348,21 +607,30 @@ export function useSnapshotEditor(options: Options = {}) { }); }, []); - const setLineQuantity = useCallback( - (accountId: number, value: string) => { + const addHolding = useCallback( + (accountId: number, assetType: BalanceAssetType = "stock") => { dispatch({ - type: "SET_PRICED_FIELD", - payload: { accountId, field: "quantity", value }, + type: "ADD_HOLDING", + payload: { accountId, holding: makeEmptyHolding(assetType) }, }); }, [] ); - const setLineUnitPrice = useCallback( - (accountId: number, value: string) => { + const removeHolding = useCallback((accountId: number, rowId: string) => { + dispatch({ type: "REMOVE_HOLDING", payload: { accountId, rowId } }); + }, []); + + const setHoldingField = useCallback( + ( + accountId: number, + rowId: string, + field: keyof Omit, + value: string + ) => { dispatch({ - type: "SET_PRICED_FIELD", - payload: { accountId, field: "unit_price", value }, + type: "SET_HOLDING_FIELD", + payload: { accountId, rowId, field, value }, }); }, [] @@ -373,127 +641,68 @@ export function useSnapshotEditor(options: Options = {}) { }, []); /** - * Build the prefill map from the previous snapshot. Per spec-decisions - * row "Bouton Pré-remplir": - * - simple kind → copy value - * - priced kind → copy quantity, leave unit_price blank (the user - * must enter or fetch a fresh price each time). + * Build the prefill map from the previous snapshot (simple accounts only — + * detailed accounts are prefilled from their latest holdings at LOAD time in + * 'new' mode, which is more accurate than copying the previous *line*). Per + * spec-decisions row "Bouton Pré-remplir": simple → copy value. */ const prefillFromPrevious = useCallback(() => { const lines = state.previousLines; if (!lines || lines.length === 0) return; - const accountKindById = new Map(); - for (const acc of state.accounts) { - accountKindById.set(acc.id, acc.category_kind); - } + const accountById = new Map(); + for (const acc of state.accounts) accountById.set(acc.id, acc); const nextSimple: Record = {}; - const nextPriced: Record = {}; for (const line of lines) { - const kind = accountKindById.get(line.account_id); - if (!kind) continue; // archived account — skip - if (kind === "simple") { + const acc = accountById.get(line.account_id); + if (!acc) continue; // archived account — skip + if (acc.kind === "simple") { nextSimple[line.account_id] = String(line.value); - } else { - // Priced: copy quantity, leave unit_price blank — quantities don't - // change unless the user buys / sells, prices always change. - nextPriced[line.account_id] = { - quantity: - line.quantity !== null && line.quantity !== undefined - ? String(line.quantity) - : "", - unit_price: "", - }; } + // Detailed accounts: intentionally NOT prefilled from the previous line + // here — their basket was already hydrated from the latest holdings. } dispatch({ type: "PREFILL", - payload: { values: nextSimple, pricedValues: nextPriced }, + payload: { values: nextSimple, holdings: {} }, }); }, [state.previousLines, state.accounts]); /** - * Persist the editor state to the database (#176 — atomic). + * Persist the editor state to the database (#176 — atomic; #213 — detailed). * * Order of operations: - * 1. Build & validate `simpleLines` and `pricedLines` arrays from - * editor state. Any input parsing error throws BEFORE any DB - * mutation happens, so an invalid form never produces an orphan - * snapshot row. - * 2. Call `saveSnapshotAtomic` which wraps `INSERT INTO - * balance_snapshots` (new mode) and the line rewrite in a single - * `BEGIN/COMMIT/ROLLBACK` transaction. + * 1. Build & validate `simpleLines` (scalar) and `detailedLines` (holdings) + * from editor state. Any input parsing error throws BEFORE any DB + * mutation, so an invalid form never produces an orphan snapshot row. + * 2. Call `saveSnapshotAtomic` which wraps the snapshot INSERT (new mode), + * the line rewrite AND the holdings rewrite in a single BEGIN/COMMIT/ + * ROLLBACK transaction. * * Modes: - * - 'new' mode: atomic helper inserts the snapshot row and its lines. - * - 'edit' mode: only the lines get rewritten on the existing snapshot. - * - * Only accounts with a non-empty value (after trim) are persisted; empty - * fields mean "no entry for this account at this date" — they're cleared - * by the rewrite-all strategy in `saveSnapshotAtomic`. + * - 'new' mode: atomic helper inserts the snapshot row + its lines/holdings. + * - 'edit' mode: only the lines/holdings get rewritten on the existing row. */ const save = useCallback(async (): Promise<{ snapshotId: number }> => { dispatch({ type: "SET_SAVING", payload: true }); dispatch({ type: "SET_ERROR", payload: { message: null, code: null } }); try { - // Step 1 — build & validate every line in memory. THROW HERE means - // no DB mutation has happened yet, so no orphan snapshot can be - // left behind by a validation failure (#176). - const simpleLines = Object.entries(state.values) - .filter(([, v]) => v !== undefined && String(v).trim().length > 0) - .map(([accountIdStr, raw]) => { - const accountId = Number(accountIdStr); - const trimmed = String(raw).trim().replace(",", "."); - const num = Number(trimmed); - if (!Number.isFinite(num)) { - throw new BalanceServiceError( - "snapshot_value_invalid", - `Invalid value for account ${accountId}: "${raw}"` - ); - } - return { - account_id: accountId, - value: num, - account_kind: "simple" as const, - }; - }); - const pricedLines = Object.entries(state.pricedValues) - .filter( - ([, entry]) => - entry && - String(entry.quantity ?? "").trim().length > 0 && - String(entry.unit_price ?? "").trim().length > 0 - ) - .map(([accountIdStr, entry]) => { - const accountId = Number(accountIdStr); - const qtyTrim = String(entry.quantity).trim().replace(",", "."); - const priceTrim = String(entry.unit_price).trim().replace(",", "."); - const qty = Number(qtyTrim); - const price = Number(priceTrim); - if (!Number.isFinite(qty)) { - throw new BalanceServiceError( - "snapshot_priced_quantity_required", - `Invalid quantity for account ${accountId}: "${entry.quantity}"` - ); - } - if (!Number.isFinite(price)) { - throw new BalanceServiceError( - "snapshot_priced_unit_price_required", - `Invalid unit_price for account ${accountId}: "${entry.unit_price}"` - ); - } - return { - account_id: accountId, - account_kind: "priced" as const, - quantity: qty, - unit_price: price, - // value = qty * price; the service re-validates the relation - // within PRICED_VALUE_TOLERANCE before persisting. - value: qty * price, - }; - }); + // Set of detailed account ids — dispatched on each account's OWN kind. + const detailedAccountIds = new Set(); + for (const acc of state.accounts) { + if (acc.kind === "detailed") detailedAccountIds.add(acc.id); + } + + // Step 1 — build & validate every line in memory. THROW HERE means no DB + // mutation has happened yet, so no orphan snapshot can be left behind by + // a validation failure (#176). + const simpleLines = buildSimpleLines(state.values, detailedAccountIds); + const detailedLines = buildDetailedLines( + state.holdings, + detailedAccountIds + ); // Step 2 — atomic write. BEGIN / INSERT snapshot (if 'new') / - // INSERT lines / COMMIT, with ROLLBACK on any failure. + // INSERT lines + holdings / COMMIT, with ROLLBACK on any failure. const existingSnapshotId = state.mode === "edit" && state.snapshot ? state.snapshot.id : null; // Edit-mode date move (#200): when the user changed the date of an @@ -509,7 +718,7 @@ export function useSnapshotEditor(options: Options = {}) { const { snapshotId } = await saveSnapshotAtomic({ existingSnapshotId, snapshot_date: state.snapshotDate, - lines: [...simpleLines, ...pricedLines], + lines: [...simpleLines, ...detailedLines], moveToDate, }); dispatch({ type: "CLEAR_DIRTY" }); @@ -527,7 +736,8 @@ export function useSnapshotEditor(options: Options = {}) { state.snapshot, state.snapshotDate, state.values, - state.pricedValues, + state.holdings, + state.accounts, loadForDate, ]); @@ -549,8 +759,9 @@ export function useSnapshotEditor(options: Options = {}) { state, setDate, setLineValue, - setLineQuantity, - setLineUnitPrice, + addHolding, + removeHolding, + setHoldingField, reset, prefillFromPrevious, save, diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index f36ad8c..7bdfe6a 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -1661,6 +1661,13 @@ "fhsa": "FHSA", "resp": "RESP", "hint": "Tax shelter for this account (TFSA, RRSP…). Optional — the asset class stays the type." + }, + "kind": { + "label": "Entry mode", + "simple": "Single amount", + "detailed": "By title (securities)", + "hint": "Single amount = one total per snapshot. By title = a breakdown into individual securities (quantity × price).", + "detailedCreateHint": "New accounts start as a single amount; convert one to by-title from the accounts list." } } }, @@ -1756,6 +1763,15 @@ "attributionManual": "Manual", "attributionManualHint": "Value entered manually. Automatic price fetching will land in a later release." }, + "detailed": { + "badge": "By title", + "badgeHint": "This account is broken down into individual securities. Its value is the sum of its positions.", + "empty": "No position yet. Add a title to record this account's holdings.", + "addTitle": "Add a title", + "removeTitle": "Remove this title", + "symbolLabel": "Security symbol", + "symbolPlaceholder": "Symbol" + }, "delete": { "title": "Delete this snapshot?", "body": "This permanently deletes the snapshot dated {{date}} and all its lines. To confirm, retype the date below.", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 4f33e2a..b40381c 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -1661,6 +1661,13 @@ "fhsa": "CELIAPP", "resp": "REEE", "hint": "Enveloppe fiscale du compte (CELI, REER…). Optionnel — la classe d'actif reste le type." + }, + "kind": { + "label": "Mode de saisie", + "simple": "Montant unique", + "detailed": "Par titre (valeurs mobilières)", + "hint": "Montant unique = un total par snapshot. Par titre = un détail en valeurs mobilières individuelles (quantité × cours).", + "detailedCreateHint": "Les nouveaux comptes démarrent en montant unique ; convertissez-en un en « par titre » depuis la liste des comptes." } } }, @@ -1756,6 +1763,15 @@ "attributionManual": "Manuel", "attributionManualHint": "Valeur saisie manuellement. La récupération automatique des prix arrivera dans une prochaine version." }, + "detailed": { + "badge": "Par titre", + "badgeHint": "Ce compte est détaillé en titres individuels. Sa valeur est la somme de ses positions.", + "empty": "Aucune position. Ajoutez un titre pour saisir le contenu de ce compte.", + "addTitle": "Ajouter un titre", + "removeTitle": "Retirer ce titre", + "symbolLabel": "Symbole du titre", + "symbolPlaceholder": "Symbole" + }, "delete": { "title": "Supprimer ce snapshot ?", "body": "Cette action supprime définitivement le snapshot du {{date}} et toutes ses lignes. Pour confirmer, retapez la date ci-dessous.", diff --git a/src/pages/SnapshotEditPage.tsx b/src/pages/SnapshotEditPage.tsx index abae416..15b9ce8 100644 --- a/src/pages/SnapshotEditPage.tsx +++ b/src/pages/SnapshotEditPage.tsx @@ -49,8 +49,9 @@ export default function SnapshotEditPage() { const isEditMode = state.mode === "edit"; const canPrefill = !!state.previousSnapshot; - // Aggregate value across simple + priced lines (computed live as the - // user types). Priced contribution = quantity × unit_price. + // Aggregate value across simple + detailed accounts (computed live as the + // user types). A detailed account contributes the sum of its holdings' + // quantity × unit_price (#213). const totalValue = useMemo(() => { let total = 0; let hasAny = false; @@ -63,19 +64,21 @@ export default function SnapshotEditPage() { hasAny = true; } } - for (const entry of Object.values(state.pricedValues)) { - if (!entry) continue; - const qty = Number(String(entry.quantity ?? "").trim().replace(",", ".")); - const price = Number( - String(entry.unit_price ?? "").trim().replace(",", ".") - ); - if (Number.isFinite(qty) && Number.isFinite(price)) { - total += qty * price; - hasAny = true; + for (const basket of Object.values(state.holdings)) { + if (!basket) continue; + for (const h of basket) { + const qty = Number(String(h.quantity ?? "").trim().replace(",", ".")); + const price = Number( + String(h.unit_price ?? "").trim().replace(",", ".") + ); + if (Number.isFinite(qty) && Number.isFinite(price)) { + total += qty * price; + hasAny = true; + } } } return hasAny ? total : null; - }, [state.values, state.pricedValues]); + }, [state.values, state.holdings]); const handleSave = async () => { try { @@ -202,10 +205,11 @@ export default function SnapshotEditPage() { accounts={state.accounts} categories={state.categories} values={state.values} - pricedValues={state.pricedValues} + holdings={state.holdings} onValueChange={editor.setLineValue} - onQuantityChange={editor.setLineQuantity} - onUnitPriceChange={editor.setLineUnitPrice} + onAddHolding={editor.addHolding} + onRemoveHolding={editor.removeHolding} + onHoldingFieldChange={editor.setHoldingField} disabled={state.isSaving} snapshotDate={state.snapshotDate} />