From dafdd4ce17cb0a9108e2f5d68ec9c6983f165a0c Mon Sep 17 00:00:00 2001 From: le king fu Date: Sat, 25 Apr 2026 16:27:16 -0400 Subject: [PATCH] feat(balance): add returns + transfers section to balance.service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue #142 / Bilan #4 — TS bridge for the Modified Dietz command + plain CRUD for transfer linking. Types (`src/shared/types/index.ts`): - `BalanceTransferDirection` ('in' | 'out') - `BalanceAccountTransfer` (raw row) + `BalanceAccountTransferWithTransaction` (joined view) - `AccountReturn` (mirrors the Rust struct, ready to receive the invoke payload as-is) Service (`src/services/balance.service.ts`): - `computeAccountReturn(accountId, periodStart, periodEnd)`: resolves the active profile's `db_filename` from `loadProfiles()` and calls the `compute_account_return` Tauri command. - `linkTransfer(accountId, transactionId, direction, notes?)`: INSERT with duplicate guard (typed `transfer_already_linked` error instead of raw SQL UNIQUE failure). - `unlinkTransfer(accountId, transactionId)`: DELETE with `transfer_not_linked` guard for stale-UI calls. - `listAccountTransfers(accountId, dateRange?)`: joined SELECT for modal/list rendering. - `listLinkedTransactionIds()`: returns a `Set` for the transaction icon (one query, in-memory `.has()` lookups thereafter). - `listAllLinkedTransfersForTooltip()`: returns `Map` for tooltip rendering. - `suggestTransferDirection(amount)`: pure helper for the modal — maps negative bank amounts to 'in', positive to 'out'. - `isLinkedTransactionFkError(error)`: detects the canonical SQLite "FK constraint failed" text so `transactionService.deleteTransaction` can surface a clear i18n message. - 5 new error codes added to `BalanceErrorCode`. Tests (`balance.service.test.ts`): 22 new vitest cases bringing the file to 85 passed. Mocks `@tauri-apps/api/core` `invoke` and `./profileService` `loadProfiles`. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/services/balance.service.test.ts | 230 +++++++++++++++++++++++ src/services/balance.service.ts | 262 ++++++++++++++++++++++++++- src/shared/types/index.ts | 54 ++++++ 3 files changed, 545 insertions(+), 1 deletion(-) diff --git a/src/services/balance.service.test.ts b/src/services/balance.service.test.ts index 1510e75..cb4877f 100644 --- a/src/services/balance.service.test.ts +++ b/src/services/balance.service.test.ts @@ -4,7 +4,17 @@ vi.mock("./db", () => ({ getDb: vi.fn(), })); +vi.mock("@tauri-apps/api/core", () => ({ + invoke: vi.fn(), +})); + +vi.mock("./profileService", () => ({ + loadProfiles: vi.fn(), +})); + import { getDb } from "./db"; +import { invoke } from "@tauri-apps/api/core"; +import { loadProfiles } from "./profileService"; import { listBalanceCategories, createBalanceCategory, @@ -30,6 +40,14 @@ import { getSnapshotTotalsByCategoryAndDate, getAccountsLatestSnapshot, getAccountsPeriodAnchor, + computeAccountReturn, + linkTransfer, + unlinkTransfer, + listAccountTransfers, + listLinkedTransactionIds, + listAllLinkedTransfersForTooltip, + isLinkedTransactionFkError, + suggestTransferDirection, } from "./balance.service"; const mockSelect = vi.fn(); @@ -974,3 +992,215 @@ describe("getAccountsPeriodAnchor", () => { expect(sql).not.toMatch(/WHERE\s+s\.snapshot_date/); }); }); + +// ----------------------------------------------------------------------------- +// Returns + transfers (Issue #142) +// ----------------------------------------------------------------------------- + +describe("computeAccountReturn", () => { + beforeEach(() => { + vi.mocked(loadProfiles).mockReset(); + vi.mocked(invoke).mockReset(); + }); + + it("invokes the Tauri command with the active profile's db_filename", async () => { + vi.mocked(loadProfiles).mockResolvedValueOnce({ + active_profile_id: "p1", + profiles: [ + { + id: "p1", + name: "Max", + color: "#fff", + pin_hash: null, + db_filename: "max.db", + created_at: "0", + }, + ], + }); + const fakeReturn = { + value_start: 1000, + value_end: 1100, + net_contributions: 0, + return_pct: 0.1, + annualized_pct: 0.42, + is_partial: false, + has_no_transfers_warning: true, + }; + vi.mocked(invoke).mockResolvedValueOnce(fakeReturn); + + const out = await computeAccountReturn(7, "2026-01-01", "2026-04-01"); + + expect(out).toEqual(fakeReturn); + expect(invoke).toHaveBeenCalledWith("compute_account_return", { + dbFilename: "max.db", + accountId: 7, + periodStart: "2026-01-01", + periodEnd: "2026-04-01", + }); + }); + + it("rejects malformed period dates before invoking the command", async () => { + vi.mocked(loadProfiles).mockResolvedValueOnce({ + active_profile_id: "p1", + profiles: [ + { + id: "p1", + name: "Max", + color: "#fff", + pin_hash: null, + db_filename: "max.db", + created_at: "0", + }, + ], + }); + await expect( + computeAccountReturn(1, "not-a-date", "2026-04-01") + ).rejects.toBeInstanceOf(BalanceServiceError); + expect(invoke).not.toHaveBeenCalled(); + }); + + it("throws transfer_active_profile_unknown when no active profile resolves", async () => { + vi.mocked(loadProfiles).mockResolvedValueOnce({ + active_profile_id: "missing", + profiles: [], + }); + await expect( + computeAccountReturn(1, "2026-01-01", "2026-04-01") + ).rejects.toMatchObject({ code: "transfer_active_profile_unknown" }); + expect(invoke).not.toHaveBeenCalled(); + }); +}); + +describe("suggestTransferDirection", () => { + it("maps negative bank amounts to 'in' (money left bank → arrived in account)", () => { + expect(suggestTransferDirection(-100)).toBe("in"); + }); + it("maps positive bank amounts to 'out' (money came back from account)", () => { + expect(suggestTransferDirection(50)).toBe("out"); + }); + it("treats zero as 'out' as a deterministic fallback", () => { + expect(suggestTransferDirection(0)).toBe("out"); + }); +}); + +describe("linkTransfer", () => { + it("rejects an invalid direction without touching the DB", async () => { + await expect( + // @ts-expect-error testing runtime guard + linkTransfer(1, 2, "sideways") + ).rejects.toBeInstanceOf(BalanceServiceError); + expect(mockExecute).not.toHaveBeenCalled(); + }); + + it("guards against duplicate links with a typed error", async () => { + mockSelect.mockResolvedValueOnce([{ id: 5 }]); + await expect(linkTransfer(1, 2, "in")).rejects.toMatchObject({ + code: "transfer_already_linked", + }); + expect(mockExecute).not.toHaveBeenCalled(); + }); + + it("inserts and returns the new transfer id", async () => { + mockSelect.mockResolvedValueOnce([]); + mockExecute.mockResolvedValueOnce({ lastInsertId: 99, rowsAffected: 1 }); + const id = await linkTransfer(1, 2, "out", " manual "); + expect(id).toBe(99); + const sql = mockExecute.mock.calls[0][0] as string; + expect(sql).toContain("INSERT INTO balance_account_transfers"); + expect(mockExecute.mock.calls[0][1]).toEqual([1, 2, "out", "manual"]); + }); + + it("normalizes empty notes to null", async () => { + mockSelect.mockResolvedValueOnce([]); + mockExecute.mockResolvedValueOnce({ lastInsertId: 1, rowsAffected: 1 }); + await linkTransfer(1, 2, "in", " "); + expect(mockExecute.mock.calls[0][1][3]).toBeNull(); + }); +}); + +describe("unlinkTransfer", () => { + it("throws transfer_not_linked when no row was deleted", async () => { + mockExecute.mockResolvedValueOnce({ lastInsertId: 0, rowsAffected: 0 }); + await expect(unlinkTransfer(1, 2)).rejects.toMatchObject({ + code: "transfer_not_linked", + }); + }); + + it("succeeds when one row is deleted", async () => { + mockExecute.mockResolvedValueOnce({ lastInsertId: 0, rowsAffected: 1 }); + await expect(unlinkTransfer(1, 2)).resolves.toBeUndefined(); + expect(mockExecute.mock.calls[0][1]).toEqual([1, 2]); + }); +}); + +describe("listAccountTransfers", () => { + it("filters by account_id only when no date range is supplied", async () => { + mockSelect.mockResolvedValueOnce([]); + await listAccountTransfers(7); + const sql = mockSelect.mock.calls[0][0] as string; + expect(sql).toContain("FROM balance_account_transfers bat"); + expect(sql).toContain("JOIN transactions t"); + expect(sql).toContain("JOIN balance_accounts a"); + expect(sql).toContain("WHERE bat.account_id = $1"); + expect(sql).not.toContain("t.date >="); + expect(mockSelect.mock.calls[0][1]).toEqual([7]); + }); + + it("appends inclusive date bounds when supplied", async () => { + mockSelect.mockResolvedValueOnce([]); + await listAccountTransfers(7, { from: "2026-01-01", to: "2026-04-01" }); + const sql = mockSelect.mock.calls[0][0] as string; + expect(sql).toContain("t.date >="); + expect(sql).toContain("t.date <="); + expect(mockSelect.mock.calls[0][1]).toEqual([7, "2026-01-01", "2026-04-01"]); + }); +}); + +describe("listLinkedTransactionIds", () => { + it("returns a Set of transaction ids", async () => { + mockSelect.mockResolvedValueOnce([ + { transaction_id: 5 }, + { transaction_id: 12 }, + ]); + const ids = await listLinkedTransactionIds(); + expect(ids).toBeInstanceOf(Set); + expect(ids.has(5)).toBe(true); + expect(ids.has(12)).toBe(true); + expect(ids.size).toBe(2); + }); +}); + +describe("listAllLinkedTransfersForTooltip", () => { + it("groups multiple links per transaction id", async () => { + mockSelect.mockResolvedValueOnce([ + { transaction_id: 1, account_id: 10, account_name: "TFSA", direction: "in" }, + { transaction_id: 1, account_id: 20, account_name: "RRSP", direction: "out" }, + { transaction_id: 2, account_id: 10, account_name: "TFSA", direction: "in" }, + ]); + const map = await listAllLinkedTransfersForTooltip(); + expect(map.get(1)).toHaveLength(2); + expect(map.get(2)).toHaveLength(1); + expect(map.get(1)?.[0].account_name).toBe("TFSA"); + }); +}); + +describe("isLinkedTransactionFkError", () => { + it("matches the canonical SQLite FK error text", () => { + expect( + isLinkedTransactionFkError(new Error("FOREIGN KEY constraint failed")) + ).toBe(true); + }); + + it("matches the wrapped tauri-plugin-sql variant", () => { + expect( + isLinkedTransactionFkError( + new Error("code: 787, message: FOREIGN KEY constraint failed") + ) + ).toBe(true); + }); + + it("does not match unrelated errors", () => { + expect(isLinkedTransactionFkError(new Error("something else"))).toBe(false); + expect(isLinkedTransactionFkError(undefined)).toBe(false); + }); +}); diff --git a/src/services/balance.service.ts b/src/services/balance.service.ts index f3244fd..fd241f3 100644 --- a/src/services/balance.service.ts +++ b/src/services/balance.service.ts @@ -9,14 +9,20 @@ // filesystem / OAuth / license / profile work and the future Modified Dietz // + price-fetch work in Issue #142. +import { invoke } from "@tauri-apps/api/core"; import { getDb } from "./db"; +import { loadProfiles } from "./profileService"; import type { + AccountReturn, BalanceAccount, + BalanceAccountTransfer, + BalanceAccountTransferWithTransaction, BalanceAccountWithCategory, BalanceCategory, BalanceCategoryKind, BalanceSnapshot, BalanceSnapshotLine, + BalanceTransferDirection, } from "../shared/types"; import { BALANCE_CURRENCY_CAD } from "../shared/types"; @@ -40,7 +46,13 @@ export type BalanceErrorCode = | "snapshot_priced_quantity_required" | "snapshot_priced_unit_price_required" | "snapshot_priced_value_mismatch" - | "snapshot_simple_must_be_scalar"; + | "snapshot_simple_must_be_scalar" + // Issue #142 — transfers + returns + | "transfer_direction_invalid" + | "transfer_already_linked" + | "transfer_not_linked" + | "transfer_active_profile_unknown" + | "transaction_linked_to_balance_account"; export class BalanceServiceError extends Error { readonly code: BalanceErrorCode; @@ -955,3 +967,251 @@ export async function getAccountsPeriodAnchor( params ); } + +// ----------------------------------------------------------------------------- +// Returns + transfers (Issue #142 / Bilan #4) +// ----------------------------------------------------------------------------- +// +// Two distinct surface areas: +// +// (1) `computeAccountReturn` — Modified Dietz return for one account over a +// period. Lives on the Rust side (`compute_account_return` Tauri command) +// because it needs to JOIN snapshots + transfers + transactions and +// apply day-precision weighting in a single short-lived connection. The +// TS shim resolves the active profile's `db_filename` from `loadProfiles` +// and forwards it to the command. +// +// (2) Transfer linking helpers — `linkTransfer`, `unlinkTransfer`, +// `listAccountTransfers`. Plain CRUD on `balance_account_transfers` via +// `getDb()`, same pattern as the rest of this file. + +/** + * Compute the Modified Dietz return for `accountId` over the period + * `[periodStart, periodEnd]` (both ISO `YYYY-MM-DD`). Returns the typed + * `AccountReturn` shape — see `src/shared/types/index.ts`. + * + * Resolves the active profile's `db_filename` from `loadProfiles()` so the + * caller doesn't have to thread it through every screen. Throws + * `transfer_active_profile_unknown` if no active profile is set (should be + * impossible in normal app flow, but the service guards it anyway). + */ +export async function computeAccountReturn( + accountId: number, + periodStart: string, + periodEnd: string +): Promise { + const startNorm = normalizeSnapshotDate(periodStart); + const endNorm = normalizeSnapshotDate(periodEnd); + const config = await loadProfiles(); + const profile = config.profiles.find( + (p) => p.id === config.active_profile_id + ); + if (!profile) { + throw new BalanceServiceError( + "transfer_active_profile_unknown", + "No active profile is set" + ); + } + return invoke("compute_account_return", { + dbFilename: profile.db_filename, + accountId, + periodStart: startNorm, + periodEnd: endNorm, + }); +} + +function normalizeDirection( + direction: BalanceTransferDirection +): BalanceTransferDirection { + if (direction !== "in" && direction !== "out") { + throw new BalanceServiceError( + "transfer_direction_invalid", + `Invalid transfer direction: ${direction}` + ); + } + return direction; +} + +/** + * Suggested direction for an unlinked transaction based on its signed amount. + * Pure helper so the `LinkTransfersModal` UI can pre-fill the direction + * column without round-tripping. Convention: in this codebase, expense + * transactions are stored with negative amounts (money leaving the bank). + * From the *balance account's* perspective: + * - negative bank amount = money left the bank → arrived at the balance + * account = `in` + * - positive bank amount = money entered the bank = the balance account + * gave it back = `out` + */ +export function suggestTransferDirection( + transactionAmount: number +): BalanceTransferDirection { + return transactionAmount < 0 ? "in" : "out"; +} + +/** + * Link a transaction to a balance account with the given direction. + * Throws `transfer_already_linked` if the (transaction, account) pair is + * already in the table (UNIQUE constraint). + */ +export async function linkTransfer( + accountId: number, + transactionId: number, + direction: BalanceTransferDirection, + notes?: string | null +): Promise { + const dir = normalizeDirection(direction); + const trimmedNotes = notes ? notes.trim() || null : null; + const db = await getDb(); + // Guard duplicate link with a SELECT — keeps the error typed instead of a + // raw "UNIQUE constraint failed" string. + const existing = await db.select<{ id: number }[]>( + `SELECT id FROM balance_account_transfers + WHERE account_id = $1 AND transaction_id = $2`, + [accountId, transactionId] + ); + if (existing.length > 0) { + throw new BalanceServiceError( + "transfer_already_linked", + `Transaction ${transactionId} is already linked to account ${accountId}` + ); + } + const result = await db.execute( + `INSERT INTO balance_account_transfers (account_id, transaction_id, direction, notes) + VALUES ($1, $2, $3, $4)`, + [accountId, transactionId, dir, trimmedNotes] + ); + return result.lastInsertId as number; +} + +/** + * Unlink a transaction from an account. Throws `transfer_not_linked` if the + * pair isn't in the table — keeps callers from silently no-op'ing on a stale + * UI state. + */ +export async function unlinkTransfer( + accountId: number, + transactionId: number +): Promise { + const db = await getDb(); + const result = await db.execute( + `DELETE FROM balance_account_transfers + WHERE account_id = $1 AND transaction_id = $2`, + [accountId, transactionId] + ); + if (result.rowsAffected === 0) { + throw new BalanceServiceError( + "transfer_not_linked", + `No transfer linked transaction ${transactionId} to account ${accountId}` + ); + } +} + +/** + * List every linked transfer for `accountId`, joined with the transaction + * table for date/description/amount. Optional `dateRange` (ISO YYYY-MM-DD, + * inclusive both sides) filters by `transactions.date`. + */ +export async function listAccountTransfers( + accountId: number, + dateRange?: { from?: string; to?: string } +): Promise { + const params: unknown[] = [accountId]; + const conditions: string[] = ["bat.account_id = $1"]; + if (dateRange?.from) { + conditions.push(`t.date >= $${params.length + 1}`); + params.push(normalizeSnapshotDate(dateRange.from)); + } + if (dateRange?.to) { + conditions.push(`t.date <= $${params.length + 1}`); + params.push(normalizeSnapshotDate(dateRange.to)); + } + const where = `WHERE ${conditions.join(" AND ")}`; + const db = await getDb(); + return db.select( + `SELECT bat.id AS id, + bat.account_id AS account_id, + bat.transaction_id AS transaction_id, + bat.direction AS direction, + bat.notes AS notes, + bat.created_at AS created_at, + t.date AS transaction_date, + t.description AS transaction_description, + t.amount AS transaction_amount, + a.name AS account_name + FROM balance_account_transfers bat + JOIN transactions t ON t.id = bat.transaction_id + JOIN balance_accounts a ON a.id = bat.account_id + ${where} + ORDER BY t.date DESC, bat.id DESC`, + params + ); +} + +/** + * Returns the set of `transaction_id`s currently linked to ANY balance + * account. Used by the transactions table to render the transfer icon + * without an N+1 query — the caller receives the full set once per render + * and does an in-memory `.has(id)` lookup. Cheap on real-world scales + * (typically < 1000 linked transfers per profile). + */ +export async function listLinkedTransactionIds(): Promise> { + const db = await getDb(); + const rows = await db.select<{ transaction_id: number }[]>( + `SELECT DISTINCT transaction_id FROM balance_account_transfers` + ); + return new Set(rows.map((r) => r.transaction_id)); +} + +/** + * Returns transfer info keyed by `transaction_id` for tooltip rendering in + * the transactions table. Each transaction maps to an array because a + * single transaction *could* be linked to several accounts in principle + * (the UNIQUE is on the pair, not on transaction alone). + */ +export interface LinkedTransferTooltipRow { + transaction_id: number; + account_id: number; + account_name: string; + direction: BalanceTransferDirection; +} + +export async function listAllLinkedTransfersForTooltip(): Promise< + Map +> { + const db = await getDb(); + const rows = await db.select( + `SELECT bat.transaction_id AS transaction_id, + bat.account_id AS account_id, + a.name AS account_name, + bat.direction AS direction + FROM balance_account_transfers bat + JOIN balance_accounts a ON a.id = bat.account_id + ORDER BY bat.transaction_id` + ); + const map = new Map(); + for (const r of rows) { + const list = map.get(r.transaction_id) ?? []; + list.push(r); + map.set(r.transaction_id, list); + } + return map; +} + +/** + * Detect whether the SQL error returned by `tauri-plugin-sql` is a FK + * RESTRICT violation from `balance_account_transfers.transaction_id`. The + * plugin surfaces the SQLite error message verbatim, so we match on the + * string. Used by `transactionService.deleteTransaction` to surface a + * clean i18n error instead of leaking the raw SQL. + */ +export function isLinkedTransactionFkError(error: unknown): boolean { + const msg = error instanceof Error ? error.message : String(error ?? ""); + // SQLite FK error messages look like: + // "FOREIGN KEY constraint failed" + // or + // "code: 787, message: FOREIGN KEY constraint failed" + // Both contain the canonical "FOREIGN KEY constraint failed" substring. + return /FOREIGN KEY constraint failed/i.test(msg); +} + diff --git a/src/shared/types/index.ts b/src/shared/types/index.ts index 3653a2f..0d0c451 100644 --- a/src/shared/types/index.ts +++ b/src/shared/types/index.ts @@ -630,3 +630,57 @@ export interface BalanceSnapshotLine { created_at: string; updated_at: string; } + +// Account transfers — added Issue #142 (Bilan #4). Links a transaction to a +// balance account so the Modified Dietz return calculator can separate +// contributions from gains. Direction follows the account's perspective: +// 'in' = capital added (deposit / buy) +// 'out' = capital removed (withdrawal / sell) +// `transaction_id` ON DELETE RESTRICT — preserves reproducibility of past +// returns, the UI must force the user to unlink before deleting the +// underlying transaction. +export type BalanceTransferDirection = "in" | "out"; + +export interface BalanceAccountTransfer { + id: number; + account_id: number; + transaction_id: number; + direction: BalanceTransferDirection; + notes: string | null; + created_at: string; +} + +/** Joined view used by LinkTransfersModal + transaction icon lookup. */ +export interface BalanceAccountTransferWithTransaction + extends BalanceAccountTransfer { + transaction_date: string; + transaction_description: string; + transaction_amount: number; + account_name: string; +} + +/** + * Modified Dietz return for one account over a period. + * Mirrors the Rust struct in `src-tauri/src/commands/return_calculator.rs`. + * + * - `value_start` / `value_end`: latest snapshot value ≤ each endpoint, or + * null when no snapshot exists. + * - `net_contributions`: signed sum of cash flows in the period. + * - `return_pct`: Modified Dietz return (0.05 = +5%); null if either + * endpoint is missing or denominator is non-positive. + * - `annualized_pct`: `(1 + R)^(365/T) - 1`; null for zero-length periods + * or whenever `return_pct` is null. + * - `is_partial`: true when one endpoint snapshot is missing. + * - `has_no_transfers_warning`: true when no transfers were tagged in the + * period — return collapses to simple `(V_end - V_start) / V_start` and + * the UI surfaces a warning so the user can verify nothing was forgotten. + */ +export interface AccountReturn { + value_start: number | null; + value_end: number | null; + net_contributions: number; + return_pct: number | null; + annualized_pct: number | null; + is_partial: boolean; + has_no_transfers_warning: boolean; +}