chore(balance): post-merge cleanup of #182-#185 reviews (#187) #195
8 changed files with 160 additions and 99 deletions
|
|
@ -19,6 +19,7 @@
|
|||
- Bilan : le sélecteur de date sur `/balance/snapshot` se ferme maintenant après la sélection sur Linux (WebKitGTK) au lieu de rester ouvert jusqu'à ce que l'utilisateur appuie sur Échap. Le contournement appelle `blur()` sur le champ après chaque changement — sans effet sur Windows WebView2 / macOS WKWebView, où le sélecteur se ferme déjà automatiquement (#177).
|
||||
- Mise à jour de la dépendance `postcss` (8.5.6 → 8.5.13) pour corriger l'avis de sécurité de sévérité modérée GHSA-qx2v-qp2m-jg93 (XSS via `</style>` non échappé dans le stringifier CSS). Transitive via vite, build-time uniquement — aucun impact runtime sur le binaire Tauri livré (#180).
|
||||
- Contournement du sélecteur de date WebKitGTK étendu aux 7 autres champs `<input type="date">` natifs répartis sur 4 composants (barre de filtres Transactions, formulaire Ajustements, modal de Liaison de transferts, sélecteur de période). Chaque handler onChange appelle désormais `e.currentTarget.blur()` pour fermer le popup natif sur Linux Tauri WebView — sans effet sur Windows WebView2 / macOS WKWebView. Même approche que #177 (#188).
|
||||
- Bilan : nettoyage post-merge des suggestions issues des reviews des PR #182-#185. Six corrections groupées : (1) `getStarterCollisions` filtre désormais `archived_at IS NULL`, donc recréer un compte starter volontairement archivé n'est plus bloqué ; (2) `proposeStarterAccounts` re-vérifie chaque collision (nom, catégorie) en transaction avant l'INSERT en défense-in-depth (saut silencieux si déjà présent, aucune contrainte UNIQUE ajoutée) ; (3) les nouveaux profils reçoivent désormais `balance_starter_proposed` pré-seedé dans `consolidated_schema.sql` pour que le StarterAccountsModal ne s'ouvre jamais brièvement avec uniquement des collisions à la première visite de /balance ; (4) `/balance` cache maintenant le sélecteur de période, le graphique d'évolution et le tableau des comptes tant que la carte d'onboarding vide est affichée (évite trois messages vides empilés) ; (5) `BalanceOnboardingCard.Step` appelle directement `useTranslation()` au lieu de recevoir `t` en prop ; (6) le bloc de doc de la formule Modified Dietz dans `return_calculator.rs` est maintenant entouré d'une fence `text` pour que `cargo test --doc` n'essaie plus de compiler la pseudo-math comme du Rust (#187).
|
||||
|
||||
## [0.9.0] - 2026-04-29
|
||||
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@
|
|||
- Bilan: snapshot date picker on `/balance/snapshot` now closes after a date is selected on Linux (WebKitGTK), instead of staying open until the user pressed Esc. Workaround calls `blur()` on the input after each change — no-op on Windows WebView2 / macOS WKWebView, where the picker already auto-closes (#177).
|
||||
- Updated `postcss` dependency (8.5.6 → 8.5.13) to address moderate severity advisory GHSA-qx2v-qp2m-jg93 (XSS via unescaped `</style>` in CSS stringifier). Transitive via vite, build-time only — no runtime impact on the shipped Tauri binary (#180).
|
||||
- WebKitGTK date picker workaround extended to the remaining 7 native `<input type="date">` fields across 4 components (Transactions filter bar, Adjustments form, Link Transfers modal, Period selector). Each onChange handler now calls `e.currentTarget.blur()` to dismiss the native popup on Linux Tauri WebView — no-op on Windows WebView2 / macOS WKWebView. Same approach as #177 (#188).
|
||||
- Bilan: post-merge cleanup of suggestions raised in the #182-#185 reviews. Six fixes bundled: (1) `getStarterCollisions` now filters `archived_at IS NULL` so re-creating a voluntarily archived starter is no longer blocked; (2) `proposeStarterAccounts` re-checks each (name, category) collision in-transaction before INSERT as defense-in-depth (skips silently on hit, no UNIQUE constraint added); (3) brand-new profiles now get `balance_starter_proposed` pre-seeded in `consolidated_schema.sql` so the StarterAccountsModal never briefly opens with all-collisions on first /balance visit; (4) `/balance` now hides the period selector, evolution chart and accounts table while the empty-state onboarding card is shown (avoids three stacked empty messages); (5) `BalanceOnboardingCard.Step` now calls `useTranslation()` directly instead of receiving `t` as a prop; (6) `return_calculator.rs` Modified Dietz formula doc block is wrapped in a `text` fence so `cargo test --doc` no longer fails to compile pseudo-math as Rust (#187).
|
||||
|
||||
## [0.9.0] - 2026-04-29
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,9 @@
|
|||
//!
|
||||
//! Modified Dietz formula:
|
||||
//!
|
||||
//! R = (V_end - V_start - sum(CF_i)) / (V_start + sum(W_i * CF_i))
|
||||
//! ```text
|
||||
//! R = (V_end - V_start - sum(CF_i)) / (V_start + sum(W_i * CF_i))
|
||||
//! ```
|
||||
//!
|
||||
//! where `W_i = (T - t_i) / T`, `T = period_days`, `t_i = days from period_start
|
||||
//! to flow date`. A flow on day 0 is fully invested for the whole period
|
||||
|
|
|
|||
|
|
@ -288,6 +288,12 @@ INSERT OR IGNORE INTO user_preferences (key, value) VALUES ('theme', 'light');
|
|||
INSERT OR IGNORE INTO user_preferences (key, value) VALUES ('currency', 'EUR');
|
||||
INSERT OR IGNORE INTO user_preferences (key, value) VALUES ('date_format', 'DD/MM/YYYY');
|
||||
INSERT OR REPLACE INTO user_preferences (key, value) VALUES ('categories_schema_version', 'v1');
|
||||
-- Suppress StarterAccountsModal on first /balance visit for new profiles
|
||||
-- (Issue #179). The 4 starter accounts are already seeded above, so the
|
||||
-- modal would only show 4 collision rows with no actionable choice. Pre-
|
||||
-- writing the pref skips that briefly-empty UX entirely. Suggestion S1
|
||||
-- from PR #185 review (#187).
|
||||
INSERT OR IGNORE INTO user_preferences (key, value) VALUES ('balance_starter_proposed', '{"shown_at":"seed","accepted":[]}');
|
||||
|
||||
-- ============================================================================
|
||||
-- Seed v1 — IPC Statistique Canada-aligned, 3 levels, Canada/Québec
|
||||
|
|
|
|||
|
|
@ -16,7 +16,6 @@
|
|||
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { Link } from "react-router-dom";
|
||||
import type { TFunction } from "i18next";
|
||||
import { Wallet, FileText, Check, ArrowRight } from "lucide-react";
|
||||
|
||||
interface BalanceOnboardingCardProps {
|
||||
|
|
@ -81,7 +80,6 @@ export default function BalanceOnboardingCard({
|
|||
description={t("balance.onboarding.step1.description")}
|
||||
ctaLabel={t("balance.onboarding.step1.cta")}
|
||||
ctaHref="/balance/accounts"
|
||||
t={t}
|
||||
/>
|
||||
<Step
|
||||
number={2}
|
||||
|
|
@ -92,7 +90,6 @@ export default function BalanceOnboardingCard({
|
|||
ctaLabel={t("balance.onboarding.step2.cta")}
|
||||
ctaHref="/balance/snapshot"
|
||||
disabledHint={t("balance.onboarding.step2.disabledHint")}
|
||||
t={t}
|
||||
/>
|
||||
</ol>
|
||||
</div>
|
||||
|
|
@ -112,7 +109,6 @@ interface StepProps {
|
|||
ctaLabel: string;
|
||||
ctaHref: string;
|
||||
disabledHint?: string;
|
||||
t: TFunction;
|
||||
}
|
||||
|
||||
function Step({
|
||||
|
|
@ -124,8 +120,8 @@ function Step({
|
|||
ctaLabel,
|
||||
ctaHref,
|
||||
disabledHint,
|
||||
t,
|
||||
}: StepProps) {
|
||||
const { t } = useTranslation();
|
||||
const isDone = state === "done";
|
||||
const isActive = state === "active";
|
||||
const isDisabled = state === "disabled";
|
||||
|
|
|
|||
|
|
@ -73,6 +73,13 @@ describe("getStarterCollisions", () => {
|
|||
expect(result.has("tfsa")).toBe(false);
|
||||
expect(result.has("cash")).toBe(false); // name "CELI" != "Compte chèque"
|
||||
});
|
||||
|
||||
it("excludes archived accounts via SQL filter", async () => {
|
||||
mockSelect.mockResolvedValueOnce([]);
|
||||
await getStarterCollisions();
|
||||
const sql = mockSelect.mock.calls[0][0];
|
||||
expect(sql).toMatch(/archived_at IS NULL/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("proposeStarterAccounts", () => {
|
||||
|
|
@ -85,10 +92,12 @@ describe("proposeStarterAccounts", () => {
|
|||
it("inserts selected starters atomically and returns their ids", async () => {
|
||||
// BEGIN
|
||||
mockExecute.mockResolvedValueOnce({ rowsAffected: 0, lastInsertId: 0 });
|
||||
// For each starter: SELECT id FROM balance_categories + INSERT
|
||||
// For each starter: SELECT category id, SELECT in-txn collision check, INSERT
|
||||
mockSelect
|
||||
.mockResolvedValueOnce([{ id: 11 }]) // cash category
|
||||
.mockResolvedValueOnce([{ id: 13 }]); // rrsp category
|
||||
.mockResolvedValueOnce([{ id: 11 }]) // cash category lookup
|
||||
.mockResolvedValueOnce([{ count: 0 }]) // S3 collision check for cash
|
||||
.mockResolvedValueOnce([{ id: 13 }]) // rrsp category lookup
|
||||
.mockResolvedValueOnce([{ count: 0 }]); // S3 collision check for rrsp
|
||||
mockExecute
|
||||
.mockResolvedValueOnce({ rowsAffected: 1, lastInsertId: 100 }) // INSERT cash
|
||||
.mockResolvedValueOnce({ rowsAffected: 1, lastInsertId: 101 }) // INSERT rrsp
|
||||
|
|
@ -103,9 +112,33 @@ describe("proposeStarterAccounts", () => {
|
|||
expect(sqls.filter((s) => /INSERT INTO balance_accounts/.test(s))).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("skips silently when in-txn collision check finds an existing account (S3)", async () => {
|
||||
// BEGIN
|
||||
mockExecute.mockResolvedValueOnce({ rowsAffected: 0, lastInsertId: 0 });
|
||||
// First starter "cash": category lookup succeeds, collision check returns count=1 → skip
|
||||
mockSelect
|
||||
.mockResolvedValueOnce([{ id: 11 }]) // cash category lookup
|
||||
.mockResolvedValueOnce([{ count: 1 }]) // S3 collision: cash already exists
|
||||
// Second starter "rrsp": category lookup + clean collision check
|
||||
.mockResolvedValueOnce([{ id: 13 }]) // rrsp category lookup
|
||||
.mockResolvedValueOnce([{ count: 0 }]); // rrsp clean
|
||||
mockExecute
|
||||
.mockResolvedValueOnce({ rowsAffected: 1, lastInsertId: 101 }) // INSERT rrsp
|
||||
.mockResolvedValueOnce({ rowsAffected: 0, lastInsertId: 0 }); // COMMIT
|
||||
|
||||
const result = await proposeStarterAccounts(["cash", "rrsp"]);
|
||||
expect(result).toEqual([101]); // only rrsp inserted, cash skipped silently
|
||||
|
||||
const sqls = mockExecute.mock.calls.map((c) => c[0]);
|
||||
expect(sqls.filter((s) => /INSERT INTO balance_accounts/.test(s))).toHaveLength(1);
|
||||
expect(sqls).toContain("COMMIT"); // no rollback — skip is normal flow
|
||||
});
|
||||
|
||||
it("rolls back on insert failure", async () => {
|
||||
mockExecute.mockResolvedValueOnce({ rowsAffected: 0, lastInsertId: 0 }); // BEGIN
|
||||
mockSelect.mockResolvedValueOnce([{ id: 11 }]);
|
||||
mockSelect
|
||||
.mockResolvedValueOnce([{ id: 11 }]) // cash category
|
||||
.mockResolvedValueOnce([{ count: 0 }]); // S3 collision check clean
|
||||
mockExecute.mockRejectedValueOnce(new Error("disk full"));
|
||||
mockExecute.mockResolvedValueOnce({ rowsAffected: 0, lastInsertId: 0 }); // ROLLBACK
|
||||
|
||||
|
|
|
|||
|
|
@ -58,10 +58,9 @@ export default function BalancePage() {
|
|||
|
||||
// Issue #179 — one-shot starter-accounts modal for existing profiles. The
|
||||
// pref `balance_starter_proposed` is written once (confirmed or dismissed),
|
||||
// so the modal never re-appears. New profiles get the 4 starters seeded
|
||||
// directly via consolidated_schema.sql and never hit this branch (the
|
||||
// first /balance visit will write the pref with accepted=[] silently
|
||||
// since collisions match all 4).
|
||||
// so the modal never re-appears. New profiles get both the 4 starters AND
|
||||
// the pref pre-seeded via consolidated_schema.sql, so they never hit this
|
||||
// branch at all (S1 fix from #187).
|
||||
const [showStarterModal, setShowStarterModal] = useState(false);
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
|
|
@ -174,96 +173,104 @@ export default function BalancePage() {
|
|||
</div>
|
||||
)}
|
||||
|
||||
<div className="space-y-6">
|
||||
{(() => {
|
||||
// Issue #178 — show a 2-step onboarding card while the user has no
|
||||
// accounts or no snapshots yet. We probe accountsLatest for ANY
|
||||
// snapshot date so the empty-state guard is independent of the
|
||||
// active period filter (state.period).
|
||||
const accountsCount = state.accountsLatest.length;
|
||||
const hasAnySnapshot = state.accountsLatest.some(
|
||||
(a) => a.latest_snapshot_date != null
|
||||
);
|
||||
if (accountsCount === 0 || !hasAnySnapshot) {
|
||||
return (
|
||||
{/* Issue #178 — empty-state guard. We probe accountsLatest for ANY
|
||||
snapshot date so the guard is independent of the active period
|
||||
filter (state.period). When empty, we render only the onboarding
|
||||
card — period selector, chart and accounts table would all show
|
||||
empty states stacked under it (S2 from #187). */}
|
||||
{(() => {
|
||||
const accountsCount = state.accountsLatest.length;
|
||||
const hasAnySnapshot = state.accountsLatest.some(
|
||||
(a) => a.latest_snapshot_date != null
|
||||
);
|
||||
const isEmpty = accountsCount === 0 || !hasAnySnapshot;
|
||||
|
||||
if (isEmpty) {
|
||||
return (
|
||||
<div className="space-y-6">
|
||||
<BalanceOnboardingCard
|
||||
accountsCount={accountsCount}
|
||||
snapshotsCount={hasAnySnapshot ? 1 : 0}
|
||||
/>
|
||||
);
|
||||
}
|
||||
return <BalanceOverviewCard totals={state.evolutionTotals} />;
|
||||
})()}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
<div className="flex flex-col sm:flex-row sm:items-center sm:justify-between gap-3">
|
||||
{/* Period selector */}
|
||||
<div
|
||||
role="group"
|
||||
aria-label={t("balance.period.legend")}
|
||||
className="inline-flex rounded-lg border border-[var(--border)] overflow-hidden"
|
||||
>
|
||||
{PERIOD_OPTIONS.map((p) => (
|
||||
<button
|
||||
key={p}
|
||||
type="button"
|
||||
onClick={() => setPeriod(p)}
|
||||
className={`px-3 py-1.5 text-sm font-medium ${
|
||||
state.period === p
|
||||
? "bg-[var(--primary)] text-white"
|
||||
: "bg-[var(--card)] text-[var(--foreground)] hover:bg-[var(--muted)]/40"
|
||||
}`}
|
||||
aria-pressed={state.period === p}
|
||||
return (
|
||||
<div className="space-y-6">
|
||||
<BalanceOverviewCard totals={state.evolutionTotals} />
|
||||
|
||||
<div className="flex flex-col sm:flex-row sm:items-center sm:justify-between gap-3">
|
||||
{/* Period selector */}
|
||||
<div
|
||||
role="group"
|
||||
aria-label={t("balance.period.legend")}
|
||||
className="inline-flex rounded-lg border border-[var(--border)] overflow-hidden"
|
||||
>
|
||||
{t(`balance.period.${p}`)}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
{PERIOD_OPTIONS.map((p) => (
|
||||
<button
|
||||
key={p}
|
||||
type="button"
|
||||
onClick={() => setPeriod(p)}
|
||||
className={`px-3 py-1.5 text-sm font-medium ${
|
||||
state.period === p
|
||||
? "bg-[var(--primary)] text-white"
|
||||
: "bg-[var(--card)] text-[var(--foreground)] hover:bg-[var(--muted)]/40"
|
||||
}`}
|
||||
aria-pressed={state.period === p}
|
||||
>
|
||||
{t(`balance.period.${p}`)}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
|
||||
{/* Chart mode toggle */}
|
||||
<div
|
||||
role="group"
|
||||
aria-label={t("balance.chart.modeLegend")}
|
||||
className="inline-flex rounded-lg border border-[var(--border)] overflow-hidden"
|
||||
>
|
||||
{(["line", "stacked"] as BalanceChartMode[]).map((mode) => (
|
||||
<button
|
||||
key={mode}
|
||||
type="button"
|
||||
onClick={() => setChartMode(mode)}
|
||||
className={`px-3 py-1.5 text-sm font-medium ${
|
||||
state.chartMode === mode
|
||||
? "bg-[var(--primary)] text-white"
|
||||
: "bg-[var(--card)] text-[var(--foreground)] hover:bg-[var(--muted)]/40"
|
||||
}`}
|
||||
aria-pressed={state.chartMode === mode}
|
||||
{/* Chart mode toggle */}
|
||||
<div
|
||||
role="group"
|
||||
aria-label={t("balance.chart.modeLegend")}
|
||||
className="inline-flex rounded-lg border border-[var(--border)] overflow-hidden"
|
||||
>
|
||||
{t(`balance.chart.mode.${mode}`)}
|
||||
</button>
|
||||
))}
|
||||
{(["line", "stacked"] as BalanceChartMode[]).map((mode) => (
|
||||
<button
|
||||
key={mode}
|
||||
type="button"
|
||||
onClick={() => setChartMode(mode)}
|
||||
className={`px-3 py-1.5 text-sm font-medium ${
|
||||
state.chartMode === mode
|
||||
? "bg-[var(--primary)] text-white"
|
||||
: "bg-[var(--card)] text-[var(--foreground)] hover:bg-[var(--muted)]/40"
|
||||
}`}
|
||||
aria-pressed={state.chartMode === mode}
|
||||
>
|
||||
{t(`balance.chart.mode.${mode}`)}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<BalanceEvolutionChart
|
||||
mode={state.chartMode}
|
||||
totals={state.evolutionTotals}
|
||||
byCategory={state.evolutionByCategory}
|
||||
categoryLabels={categoryLabels}
|
||||
transferMarkers={allTransferMarkers}
|
||||
/>
|
||||
|
||||
<div>
|
||||
<h2 className="text-lg font-semibold mb-3">
|
||||
{t("balance.overview.accountsTitle")}
|
||||
</h2>
|
||||
<BalanceAccountsTable
|
||||
accounts={state.accountsLatest}
|
||||
periodAnchor={state.accountsPeriodAnchor}
|
||||
sinceCreationDate={earliestSnapshotDate}
|
||||
onArchiveAccount={(acc) => handleArchiveAccount(acc.account_id)}
|
||||
onLinkTransfers={(acc) => setLinkTarget(acc)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<BalanceEvolutionChart
|
||||
mode={state.chartMode}
|
||||
totals={state.evolutionTotals}
|
||||
byCategory={state.evolutionByCategory}
|
||||
categoryLabels={categoryLabels}
|
||||
transferMarkers={allTransferMarkers}
|
||||
/>
|
||||
|
||||
<div>
|
||||
<h2 className="text-lg font-semibold mb-3">
|
||||
{t("balance.overview.accountsTitle")}
|
||||
</h2>
|
||||
<BalanceAccountsTable
|
||||
accounts={state.accountsLatest}
|
||||
periodAnchor={state.accountsPeriodAnchor}
|
||||
sinceCreationDate={earliestSnapshotDate}
|
||||
onArchiveAccount={(acc) => handleArchiveAccount(acc.account_id)}
|
||||
onLinkTransfers={(acc) => setLinkTarget(acc)}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
})()}
|
||||
|
||||
<StarterAccountsModal
|
||||
isOpen={showStarterModal}
|
||||
|
|
|
|||
|
|
@ -487,7 +487,8 @@ export async function getStarterCollisions(): Promise<Set<string>> {
|
|||
`SELECT c.key AS key, a.name AS account_name
|
||||
FROM balance_accounts a
|
||||
INNER JOIN balance_categories c ON c.id = a.balance_category_id
|
||||
WHERE c.key IN ('cash','tfsa','rrsp','other')`
|
||||
WHERE c.key IN ('cash','tfsa','rrsp','other')
|
||||
AND a.archived_at IS NULL`
|
||||
);
|
||||
const collisions = new Set<string>();
|
||||
for (const starter of STARTER_ACCOUNTS) {
|
||||
|
|
@ -508,9 +509,11 @@ export async function getStarterCollisions(): Promise<Set<string>> {
|
|||
* in BEGIN/COMMIT — on any failure ROLLBACK is issued and the original error
|
||||
* is re-thrown. Returns the inserted account ids in input order.
|
||||
*
|
||||
* Callers MUST pre-filter `selectedKeys` against `getStarterCollisions()` so
|
||||
* we never INSERT a duplicate (the table has no UNIQUE on (name, category),
|
||||
* so collisions would silently create dupes if not guarded upstream).
|
||||
* Callers SHOULD pre-filter `selectedKeys` against `getStarterCollisions()`
|
||||
* to keep the UI honest, but each iteration ALSO re-checks for an existing
|
||||
* (name, category) account inside the transaction and skips silently on a
|
||||
* hit — a defense-in-depth guard since the table has no UNIQUE constraint
|
||||
* on (name, balance_category_id). Returned ids exclude any skipped starter.
|
||||
*/
|
||||
export async function proposeStarterAccounts(
|
||||
selectedKeys: string[]
|
||||
|
|
@ -537,6 +540,18 @@ export async function proposeStarterAccounts(
|
|||
`Seeded category '${starter.categoryKey}' missing — expected v9 schema`
|
||||
);
|
||||
}
|
||||
// Defense-in-depth: re-check collision in-txn before INSERT so we
|
||||
// never create a silent duplicate even if the upstream pre-filter
|
||||
// raced or was bypassed (S3 from PR #185 review).
|
||||
const existing = await db.select<{ count: number }[]>(
|
||||
`SELECT COUNT(*) AS count FROM balance_accounts
|
||||
WHERE name = $1 AND balance_category_id = $2
|
||||
AND archived_at IS NULL`,
|
||||
[starter.name, catRows[0].id]
|
||||
);
|
||||
if ((existing[0]?.count ?? 0) > 0) {
|
||||
continue;
|
||||
}
|
||||
const result = await db.execute(
|
||||
`INSERT INTO balance_accounts (balance_category_id, name, currency, is_active)
|
||||
VALUES ($1, $2, 'CAD', 1)`,
|
||||
|
|
|
|||
Loading…
Reference in a new issue