fix: follow-ups of the lot-v018 stack reviews #482

Merged
maximus merged 25 commits from review-followups-lot-v018 into main 2026-10-21 00:31:46 +00:00
5 changed files with 313 additions and 50 deletions
Showing only changes of commit 1a36d4e618 - Show all commits

View file

@ -42,6 +42,7 @@ import {
} from "./useCategories"; } from "./useCategories";
import { import {
CategoryHierarchyError, CategoryHierarchyError,
createCategory,
findOrphanCategories, findOrphanCategories,
getAllCategoriesWithCounts, getAllCategoriesWithCounts,
reattachCategory, reattachCategory,
@ -80,6 +81,7 @@ beforeEach(() => {
vi.mocked(getAllCategoriesWithCounts).mockReset(); vi.mocked(getAllCategoriesWithCounts).mockReset();
vi.mocked(getAllCategoriesWithCounts).mockResolvedValue(ROWS as never); vi.mocked(getAllCategoriesWithCounts).mockResolvedValue(ROWS as never);
vi.mocked(updateCategory).mockReset(); vi.mocked(updateCategory).mockReset();
vi.mocked(createCategory).mockReset();
vi.mocked(findOrphanCategories).mockReset(); vi.mocked(findOrphanCategories).mockReset();
vi.mocked(reattachCategory).mockReset(); vi.mocked(reattachCategory).mockReset();
vi.mocked(logInfo).mockReset(); vi.mocked(logInfo).mockReset();
@ -92,6 +94,15 @@ describe("categoriesErrorOf / categoriesErrorText (#393)", () => {
expect(categoriesErrorText(error, markedT)).toBe("t(categories.hierarchy.errors.cycle)"); expect(categoriesErrorText(error, markedT)).toBe("t(categories.hierarchy.errors.cycle)");
}); });
it("names the refusals of a parent's budget amounts and keywords by their keys (decision of 2026-10-19)", () => {
expect(categoriesErrorOf(new CategoryHierarchyError("parent_has_budget"))).toEqual({
key: "categories.hierarchy.errors.parent_has_budget",
});
expect(categoriesErrorOf(new CategoryHierarchyError("parent_has_keywords"))).toEqual({
key: "categories.hierarchy.errors.parent_has_keywords",
});
});
it("keeps the message of any other failure", () => { it("keeps the message of any other failure", () => {
const error = categoriesErrorOf(new Error("database is locked")); const error = categoriesErrorOf(new Error("database is locked"));
expect(error).toEqual({ message: "database is locked" }); expect(error).toEqual({ message: "database is locked" });
@ -116,6 +127,21 @@ describe("useCategories — saving the form (#393)", () => {
expect(state.editingCategory).not.toBeNull(); expect(state.editingCategory).not.toBeNull();
}); });
it("shows a refused sub-category by its translated code, and keeps the creation form open", async () => {
vi.mocked(createCategory).mockRejectedValue(new CategoryHierarchyError("parent_has_keywords"));
let api = await mount();
api.startCreating();
api = harness.render(useCategories);
const child = { ...FORM, name: "Électricité", parent_id: 10 };
await api.saveCategory(child);
const { state } = harness.render(useCategories);
expect(createCategory).toHaveBeenCalledWith(expect.objectContaining({ name: "Électricité", parent_id: 10 }));
expect(state.error).toEqual({ key: "categories.hierarchy.errors.parent_has_keywords" });
expect(state.isSaving).toBe(false);
expect(state.isCreating).toBe(true);
expect(state.editingCategory).not.toBeNull();
});
it("shows any other failure by its message", async () => { it("shows any other failure by its message", async () => {
vi.mocked(updateCategory).mockRejectedValue(new Error("database is locked")); vi.mocked(updateCategory).mockRejectedValue(new Error("database is locked"));
let api = await mount(); let api = await mount();

View file

@ -466,7 +466,9 @@
"self_parent": "A category cannot be its own parent.", "self_parent": "A category cannot be its own parent.",
"cycle": "This parent would create a parent loop: it sits under this category, or inside a loop. Choose another parent.", "cycle": "This parent would create a parent loop: it sits under this category, or inside a loop. Choose another parent.",
"too_deep": "With this parent, the category or its subcategories would go beyond three levels. Choose a parent higher in the hierarchy.", "too_deep": "With this parent, the category or its subcategories would go beyond three levels. Choose a parent higher in the hierarchy.",
"parent_has_transactions": "This parent has transactions of its own: once a parent, it would no longer take input. Choose another parent, or recategorize its transactions first." "parent_has_transactions": "This parent has transactions of its own: once a parent, it would no longer take input. Choose another parent, or recategorize its transactions first.",
"parent_has_budget": "This parent has budget amounts of its own, in the budget or in a budget template: once a parent, it would no longer take input and those amounts would drop out of the grid. Choose another parent, or first set those amounts to zero and delete, on the Budget page, the templates that hold any.",
"parent_has_keywords": "This parent has keywords of its own: once a parent, it would no longer take input, yet its keywords would keep sorting transactions into it. Choose another parent, or first delete its keywords (add them to another category if needed)."
} }
}, },
"help": { "help": {
@ -1201,7 +1203,7 @@
"Use the All Keywords view to get a global overview of your categorization rules", "Use the All Keywords view to get a global overview of your categorization rules",
"Use Re-initialize to reset categories to defaults — this will unlink all transaction categories", "Use Re-initialize to reset categories to defaults — this will unlink all transaction categories",
"A category missing from the tree, the menus and the budget is often caught in a parent loop: Check hierarchy finds it, and its transactions, keywords and budget amounts come back with it", "A category missing from the tree, the menus and the budget is often caught in a parent loop: Check hierarchy finds it, and its transactions, keywords and budget amounts come back with it",
"A category with transactions of its own cannot become the parent of an existing category, since it would no longer take input: recategorize its transactions first, or choose another parent" "A category with transactions, budget amounts (in the budget or in a template) or keywords of its own cannot become a parent, of an existing category or of a new subcategory, since it would no longer take input: first recategorize its transactions, set its amounts to zero or delete its keywords, or choose another parent"
] ]
}, },
"adjustments": { "adjustments": {

View file

@ -466,7 +466,9 @@
"self_parent": "Une catégorie ne peut pas être son propre parent.", "self_parent": "Une catégorie ne peut pas être son propre parent.",
"cycle": "Ce parent créerait une boucle de parenté : il se trouve sous cette catégorie, ou dans une boucle. Choisissez un autre parent.", "cycle": "Ce parent créerait une boucle de parenté : il se trouve sous cette catégorie, ou dans une boucle. Choisissez un autre parent.",
"too_deep": "Avec ce parent, la catégorie ou ses sous-catégories dépasseraient trois niveaux. Choisissez un parent plus haut dans la hiérarchie.", "too_deep": "Avec ce parent, la catégorie ou ses sous-catégories dépasseraient trois niveaux. Choisissez un parent plus haut dans la hiérarchie.",
"parent_has_transactions": "Ce parent a des transactions à son nom : devenu parent, il ne serait plus saisissable. Choisissez un autre parent, ou reclassez d'abord ses transactions." "parent_has_transactions": "Ce parent a des transactions à son nom : devenu parent, il ne serait plus saisissable. Choisissez un autre parent, ou reclassez d'abord ses transactions.",
"parent_has_budget": "Ce parent a des montants de budget à son nom, dans le budget ou dans un modèle de budget : devenu parent, il ne serait plus saisissable et ces montants sortiraient de la grille. Choisissez un autre parent, ou remettez d'abord ces montants à zéro et supprimez, dans la page Budget, les modèles qui en contiennent.",
"parent_has_keywords": "Ce parent a des mots-clés à son nom : devenu parent, il ne serait plus saisissable, mais ses mots-clés continueraient d'y classer des transactions. Choisissez un autre parent, ou supprimez d'abord ses mots-clés (ajoutez-les au besoin à une autre catégorie)."
} }
}, },
"help": { "help": {
@ -1201,7 +1203,7 @@
"Utilisez la vue Tous les mots-clés pour avoir un aperçu global de vos règles de catégorisation", "Utilisez la vue Tous les mots-clés pour avoir un aperçu global de vos règles de catégorisation",
"Utilisez Réinitialiser pour revenir aux catégories par défaut — cela dissociera toutes les catégories des transactions", "Utilisez Réinitialiser pour revenir aux catégories par défaut — cela dissociera toutes les catégories des transactions",
"Une catégorie disparue de l'arbre, des menus et du budget est souvent prise dans une boucle de parenté : Vérifier la hiérarchie la retrouve, et ses transactions, ses mots-clés et ses montants de budget reviennent avec elle", "Une catégorie disparue de l'arbre, des menus et du budget est souvent prise dans une boucle de parenté : Vérifier la hiérarchie la retrouve, et ses transactions, ses mots-clés et ses montants de budget reviennent avec elle",
"Une catégorie qui a des transactions à son nom ne peut pas devenir le parent d'une catégorie existante, puisqu'elle ne serait plus saisissable : reclassez d'abord ses transactions, ou choisissez un autre parent" "Une catégorie qui a des transactions, des montants de budget (dans le budget ou dans un modèle) ou des mots-clés à son nom ne peut pas devenir un parent, ni d'une catégorie existante ni d'une nouvelle sous-catégorie, puisqu'elle ne serait plus saisissable : reclassez d'abord ses transactions, remettez ses montants à zéro ou supprimez ses mots-clés, ou choisissez un autre parent"
] ]
}, },
"adjustments": { "adjustments": {

View file

@ -1,14 +1,24 @@
// categoryService — the hierarchy rules at the write (#393): a new parent is // categoryService — the hierarchy rules at the write (#393): a new parent is
// checked by `updateCategory` and `reattachCategory`, the reads and the writes // checked by `updateCategory` and `reattachCategory`, a parent's content by
// run in one transaction through its handle, the orphan diagnostic reads every // `createCategory` too, the reads and the writes run in one transaction
// category, and `getCategoryDepth` ends on a cycle. // through its handle, the orphan diagnostic reads every category, and
// `getCategoryDepth` ends on a cycle.
// //
// The base is a fake: a `categories` table in memory, the direct transaction // The base is a fake: a `categories` table in memory and BEGIN/COMMIT/ROLLBACK.
// count of each category, BEGIN/COMMIT/ROLLBACK. `withTransaction` hands the // What a parent holds (its transactions, budget amounts and keywords) lives in
// callback its own handle; the global connection of `getDb()` throws while a // a real SQLite database (`node:sqlite`, built into Node, no package), whose
// transaction runs, as the real lock would deadlock (docs/architecture.md, // tables are created by their CREATE TABLE of the schema of a new profile
// « Accès DB et transactions »). // (`consolidated_schema.sql`): the checks' SQL runs as written, so a condition
// dropped from a query (`amount <> 0`, `is_active = 1`) turns a test red.
// `withTransaction` hands the callback its own handle; the global connection
// of `getDb()` throws while a transaction runs, as the real lock would
// deadlock (docs/architecture.md, « Accès DB et transactions »).
import { describe, it, expect, vi, beforeEach } from "vitest"; import { describe, it, expect, vi, beforeEach } from "vitest";
import { readFileSync } from "fs";
import { resolve } from "path";
import { DatabaseSync } from "node:sqlite";
import fr from "../i18n/locales/fr.json";
import en from "../i18n/locales/en.json";
const fake = vi.hoisted(() => ({ const fake = vi.hoisted(() => ({
inTransaction: false, inTransaction: false,
@ -30,6 +40,7 @@ vi.mock("./db", () => ({
import { withTransaction } from "./db"; import { withTransaction } from "./db";
import { import {
CATEGORY_HIERARCHY_ERROR_CODES,
CategoryHierarchyError, CategoryHierarchyError,
categoryHierarchyErrorKey, categoryHierarchyErrorKey,
createCategory, createCategory,
@ -60,21 +71,70 @@ const row = (id: number, parent_id: number | null, extra: Partial<Row> = {}): Ro
const HIERARCHY_SQL = "SELECT id, parent_id, is_active FROM categories"; const HIERARCHY_SQL = "SELECT id, parent_id, is_active FROM categories";
/** What categories hold in their own name, for the checks of a parent's content. */
interface Content {
/** Budget amounts of a year, as [category, amount]. */
budget?: Array<[number, number]>;
/** Amounts of budget templates, as [category, amount]. */
templates?: Array<[number, number]>;
/** Keywords, as [category, is_active]. */
keywords?: Array<[number, 0 | 1]>;
}
/** The tables the content checks read. */
const CONTENT_TABLES = ["transactions", "budget_entries", "budget_template_entries", "keywords"];
const CONTENT_SQL = /\bFROM (transactions|budget_entries|budget_template_entries|keywords)\b/;
const SCHEMA = readFileSync(
resolve(import.meta.dirname, "../../src-tauri/src/database/consolidated_schema.sql"),
"utf-8",
);
/** The CREATE TABLE of `table` in the schema of a new profile. */
function createTableSql(table: string): string {
const found = SCHEMA.match(new RegExp(`CREATE TABLE IF NOT EXISTS ${table} \\([\\s\\S]*?\\n\\);`));
if (!found) throw new Error(`no CREATE TABLE for ${table} in consolidated_schema.sql`);
return found[0];
}
/** /**
* The categories table and the direct transaction counts. Every statement is * A SQLite database with the real content tables (no categories table: the
* logged with the connection it went through ("handle" or "global"). * foreign keys are off) holding `transactions` direct transactions per
* category and `content`.
*/ */
function useFakeDb(initial: Row[], transactions: Record<number, number> = {}) { function contentDb(transactions: Record<number, number>, content: Content): DatabaseSync {
const db = new DatabaseSync(":memory:", { enableForeignKeyConstraints: false });
for (const table of CONTENT_TABLES) db.exec(createTableSql(table));
const tx = db.prepare("INSERT INTO transactions (date, description, amount, category_id) VALUES ('2026-09-01', 'tx', -10, ?)");
for (const [category, count] of Object.entries(transactions)) {
for (let i = 0; i < count; i++) tx.run(Number(category));
}
const budget = db.prepare("INSERT INTO budget_entries (category_id, year, month, amount) VALUES (?, 2026, ?, ?)");
(content.budget ?? []).forEach(([category, amount], i) => budget.run(category, (i % 12) + 1, amount));
const template = db.prepare("INSERT INTO budget_template_entries (template_id, category_id, amount) VALUES (?, ?, ?)");
(content.templates ?? []).forEach(([category, amount], i) => template.run(i + 1, category, amount));
const keyword = db.prepare("INSERT INTO keywords (keyword, category_id, priority, is_active) VALUES (?, ?, 0, ?)");
(content.keywords ?? []).forEach(([category, active], i) => keyword.run(`KW${i}`, category, active));
return db;
}
/**
* The categories table, and what categories hold. Every statement is logged
* with the connection it went through ("handle" or "global").
*/
function useFakeDb(initial: Row[], transactions: Record<number, number> = {}, content: Content = {}) {
let rows = initial.map((r) => ({ ...r })); let rows = initial.map((r) => ({ ...r }));
let beforeTransaction: Row[] | null = null; let beforeTransaction: Row[] | null = null;
const log: Array<{ via: "handle" | "global"; sql: string; params: unknown[] }> = []; const log: Array<{ via: "handle" | "global"; sql: string; params: unknown[] }> = [];
const held = contentDb(transactions, content);
const select = async (sql: string, params: unknown[] = []) => { const select = async (sql: string, params: unknown[] = []) => {
if (sql === HIERARCHY_SQL) { if (sql === HIERARCHY_SQL) {
return rows.map(({ id, parent_id, is_active }) => ({ id, parent_id, is_active })); return rows.map(({ id, parent_id, is_active }) => ({ id, parent_id, is_active }));
} }
if (sql.includes("COUNT(*) AS cnt FROM transactions WHERE category_id = $1")) { if (CONTENT_SQL.test(sql)) {
return [{ cnt: transactions[params[0] as number] ?? 0 }]; // The query as written, its `$1` bound by name.
return held.prepare(sql).all(Object.fromEntries(params.map((p, i) => [`$${i + 1}`, p as number])));
} }
if (sql.includes("SELECT parent_id FROM categories WHERE id = $1 AND is_active = 1")) { if (sql.includes("SELECT parent_id FROM categories WHERE id = $1 AND is_active = 1")) {
const found = rows.find((r) => r.id === params[0] && r.is_active === 1); const found = rows.find((r) => r.id === params[0] && r.is_active === 1);
@ -261,16 +321,31 @@ describe("updateCategory — regression of #393: A -> B -> A", () => {
}); });
it("does not check an unchanged parent: a rename under a parent with transactions or at a fourth level is saved", async () => { it("does not check an unchanged parent: a rename under a parent with transactions or at a fourth level is saved", async () => {
// Stored before the rules: 2 holds transactions and has a child; 1000 sits at level 3. // Stored before the rules: 2 holds transactions, a budget amount and a
const db = useFakeDb([...TREE, row(1000, 100)], { 2: 5 }); // keyword, and has a child; 1000 sits at level 3.
const db = useFakeDb([...TREE, row(1000, 100)], { 2: 5 }, { budget: [[2, 40]], keywords: [[2, 1]] });
await updateCategory(20, form(2, { name: "Rent" })); await updateCategory(20, form(2, { name: "Rent" }));
await updateCategory(1000, form(100, { name: "Deep" })); await updateCategory(1000, form(100, { name: "Deep" }));
expect(db.rows().find((r) => r.id === 20)?.name).toBe("Rent"); expect(db.rows().find((r) => r.id === 20)?.name).toBe("Rent");
expect(db.rows().find((r) => r.id === 1000)?.name).toBe("Deep"); expect(db.rows().find((r) => r.id === 1000)?.name).toBe("Deep");
expect(db.log.some((l) => l.sql.includes("FROM transactions"))).toBe(false); expect(db.log.some((l) => CONTENT_SQL.test(l.sql))).toBe(false);
expect(db.writes().filter((w) => w === "UPDATE categories SET is_inputable = 0")).toEqual([]); expect(db.writes().filter((w) => w === "UPDATE categories SET is_inputable = 0")).toEqual([]);
}); });
it("counts the depth on the active categories only: a category under a deleted parent is a root", async () => {
// 3 hangs under 2, deactivated: the tree shows 3 at the root, and so do the
// pickers. Root 1 carries 10: under 3 (level 0), 10 lands at level 2.
const db = useFakeDb([row(1, null), row(10, 1), row(2, null, { is_active: 0 }), row(3, 2)]);
await updateCategory(1, form(3));
expect(db.rows().find((r) => r.id === 1)?.parent_id).toBe(3);
expect(db.writes()).toEqual([
"BEGIN",
"UPDATE categories SET name = $1, type = $2, color = $3, parent_id = $4, is_inputable = $5, sort_order = $6",
"UPDATE categories SET is_inputable = 0",
"COMMIT",
]);
});
it("refuses a deactivated parent with a plain error, out of reach of the pickers", async () => { it("refuses a deactivated parent with a plain error, out of reach of the pickers", async () => {
const db = useFakeDb([...TREE, row(3, null, { is_active: 0 })]); const db = useFakeDb([...TREE, row(3, null, { is_active: 0 })]);
const error = await updateCategory(20, form(3)).then( const error = await updateCategory(20, form(3)).then(
@ -283,7 +358,120 @@ describe("updateCategory — regression of #393: A -> B -> A", () => {
}); });
}); });
describe("a parent's content: transactions, budget amounts, keywords (#393, decision of 2026-10-19)", () => {
/** A sub-category of `parent` created from the form. */
const newChild = (parent: number) => ({
name: "New",
type: "expense",
color: "#000",
parent_id: parent,
is_inputable: true,
sort_order: 1,
});
// The three ways a category becomes a parent. In TREE, 11 is a leaf under
// root 1 and 20 a leaf under root 2: each puts 20, or a new category, under 11.
const ENTRY_POINTS: Array<[string, () => Promise<unknown>]> = [
["updateCategory (parent changed)", () => updateCategory(20, form(11))],
["reattachCategory", () => reattachCategory(20, 11)],
["createCategory (sub-category)", () => createCategory(newChild(11))],
];
const HOLDINGS: Array<[string, Record<number, number>, Content]> = [
["parent_has_transactions", { 11: 2 }, {}],
["parent_has_budget", {}, { budget: [[11, 25]] }],
["parent_has_keywords", {}, { keywords: [[11, 1]] }],
];
describe.each(ENTRY_POINTS)("%s", (_name, act) => {
it.each(HOLDINGS)("refuses %s, writing nothing", async (code, transactions, content) => {
const db = useFakeDb(TREE, transactions, content);
expect((await refusal(act())).code).toBe(code);
expect(db.writes()).toEqual(["BEGIN", "ROLLBACK"]);
expect(db.rows()).toHaveLength(TREE.length);
expect(db.rows().find((r) => r.id === 20)?.parent_id).toBe(2);
expect(db.rows().find((r) => r.id === 11)?.is_inputable).toBe(1);
// Every check went through the transaction's handle.
expect(db.log.filter((l) => CONTENT_SQL.test(l.sql)).every((l) => l.via === "handle")).toBe(true);
});
it("accepts a parent whose amounts are all zero and whose keywords are all deleted", async () => {
const db = useFakeDb(TREE, {}, {
budget: [[11, 0], [11, -0]],
templates: [[11, 0]],
keywords: [[11, 0], [11, 0]],
});
await act();
expect(db.rows().find((r) => r.id === 11)?.is_inputable).toBe(0);
const writes = db.writes();
expect([writes[0], writes[writes.length - 1]]).toEqual(["BEGIN", "COMMIT"]);
});
});
it("refuses a budget amount of a template, even with no amount in the budget itself", async () => {
useFakeDb(TREE, {}, { templates: [[11, 120]] });
expect((await refusal(updateCategory(20, form(11)))).code).toBe("parent_has_budget");
});
it("reads the budget and the keywords of the parent only, never of the moved category", async () => {
// 20 itself holds an amount, a keyword and transactions: moving it changes nothing for them.
const db = useFakeDb(TREE, { 20: 4 }, { budget: [[20, 50]], templates: [[20, 50]], keywords: [[20, 1]] });
await updateCategory(20, form(11));
expect(db.rows().find((r) => r.id === 20)?.parent_id).toBe(11);
});
it("checks transactions, then budget amounts, then keywords: the first found is the refusal", async () => {
useFakeDb(TREE, { 11: 1 }, { budget: [[11, 10]], keywords: [[11, 1]] });
expect((await refusal(updateCategory(20, form(11)))).code).toBe("parent_has_transactions");
useFakeDb(TREE, {}, { budget: [[11, 10]], keywords: [[11, 1]] });
expect((await refusal(updateCategory(20, form(11)))).code).toBe("parent_has_budget");
});
it("checks the content after the hierarchy rules: a parent that loops is refused as a loop", async () => {
const db = useFakeDb(TREE, { 10: 3 }, { keywords: [[10, 1]] });
expect((await refusal(updateCategory(1, form(10)))).code).toBe("cycle");
expect(db.log.some((l) => CONTENT_SQL.test(l.sql))).toBe(false);
});
it("creates a sub-category in one transaction through its handle: the parent's content, the row, the parent's flag", async () => {
const db = useFakeDb(TREE);
const id = await createCategory(newChild(11));
const created = db.rows().find((r) => r.id === id);
expect(created).toMatchObject({ name: "New", parent_id: 11, is_inputable: 1 });
expect(db.rows().find((r) => r.id === 11)?.is_inputable).toBe(0);
expect(db.writes()).toEqual(["BEGIN", "INSERT INTO categories (name, type, color, parent_id, is_inputable, sort_order) VALUES ($1, $2, $3, $4, $5, $6)", "UPDATE categories SET is_inputable = 0", "COMMIT"]);
// The depth is read before the transaction; everything after BEGIN goes through the handle.
const begin = db.log.findIndex((l) => l.sql === "BEGIN");
expect(db.log.slice(0, begin).every((l) => l.via === "global" && l.sql.startsWith("SELECT parent_id FROM categories"))).toBe(true);
expect(db.log.slice(begin).every((l) => l.via === "handle")).toBe(true);
expect(db.log.slice(begin).filter((l) => CONTENT_SQL.test(l.sql))).toHaveLength(3);
});
it("creates a root category without any content check", async () => {
const db = useFakeDb(TREE, { 1: 3 }, { keywords: [[1, 1]] });
await createCategory({ ...newChild(11), parent_id: null });
expect(db.log.some((l) => CONTENT_SQL.test(l.sql))).toBe(false);
expect(db.writes()).toEqual(["BEGIN", "INSERT INTO categories (name, type, color, parent_id, is_inputable, sort_order) VALUES ($1, $2, $3, $4, $5, $6)", "COMMIT"]);
});
it("checks a parent that is already a parent the same way", async () => {
// 1 already has children (10, 11), and a keyword stored before these rules.
useFakeDb(TREE, {}, { keywords: [[1, 1]] });
expect((await refusal(createCategory(newChild(1)))).code).toBe("parent_has_keywords");
});
});
describe("CategoryHierarchyError", () => { describe("CategoryHierarchyError", () => {
it("has a message for each of its codes, in French and in English", () => {
const errorsOf = (locale: unknown) =>
(locale as { categories: { hierarchy: { errors: Record<string, string> } } }).categories.hierarchy.errors;
for (const code of CATEGORY_HIERARCHY_ERROR_CODES) {
expect(errorsOf(fr)[code], `fr ${code}`).toMatch(/\S/);
expect(errorsOf(en)[code], `en ${code}`).toMatch(/\S/);
}
expect(Object.keys(errorsOf(fr)).sort()).toEqual([...CATEGORY_HIERARCHY_ERROR_CODES].sort());
});
it("carries its code and names the i18n key of its message", () => { it("carries its code and names the i18n key of its message", () => {
const error = new CategoryHierarchyError("too_deep"); const error = new CategoryHierarchyError("too_deep");
expect(error).toBeInstanceOf(Error); expect(error).toBeInstanceOf(Error);

View file

@ -12,15 +12,20 @@ import {
} from "../utils/categoryHierarchy"; } from "../utils/categoryHierarchy";
/** Why a new parent is refused (#393); each code has its message under `categories.hierarchy.errors`. */ /** Why a new parent is refused (#393); each code has its message under `categories.hierarchy.errors`. */
export type CategoryHierarchyErrorCode = export const CATEGORY_HIERARCHY_ERROR_CODES = [
| "self_parent" "self_parent",
| "cycle" "cycle",
| "too_deep" "too_deep",
| "parent_has_transactions"; "parent_has_transactions",
"parent_has_budget",
"parent_has_keywords",
] as const;
export type CategoryHierarchyErrorCode = (typeof CATEGORY_HIERARCHY_ERROR_CODES)[number];
/** /**
* Thrown when `updateCategory` or `reattachCategory` refuses a new parent * Thrown when `createCategory`, `updateCategory` or `reattachCategory` refuses
* (#393), before any write. The UI shows the translation of its code * a parent (#393), before any write. The UI shows the translation of its code
* (`categoryHierarchyErrorKey`), never `message`, which is for the tests and * (`categoryHierarchyErrorKey`), never `message`, which is for the tests and
* the developer console. * the developer console.
*/ */
@ -63,12 +68,48 @@ async function markAsParent(db: Database, parentId: number): Promise<void> {
); );
} }
/**
* What a parent may not hold in its own name (#393, decision of 2026-10-19):
* once a parent, it stops taking input, yet its direct transactions would stay
* on it, a non-zero budget amount (in a year of the budget or in a template)
* would drop out of the budget grid, and its active keywords would keep sorting
* transactions into it. A deleted keyword (`is_active = 0`) sorts nothing and
* no screen shows it, so it does not count. Checked in this order, and the
* first one found is the refusal: transactions first, as before these two.
*/
const PARENT_CONTENT_CHECKS: ReadonlyArray<{ code: CategoryHierarchyErrorCode; sql: string }> = [
{
code: "parent_has_transactions",
sql: `SELECT COUNT(*) AS cnt FROM transactions WHERE category_id = $1`,
},
{
code: "parent_has_budget",
sql: `SELECT (SELECT COUNT(*) FROM budget_entries WHERE category_id = $1 AND amount <> 0)
+ (SELECT COUNT(*) FROM budget_template_entries WHERE category_id = $1 AND amount <> 0) AS cnt`,
},
{
code: "parent_has_keywords",
sql: `SELECT COUNT(*) AS cnt FROM keywords WHERE category_id = $1 AND is_active = 1`,
},
];
/**
* Refuses `parentId` as a parent when it holds what `PARENT_CONTENT_CHECKS`
* lists, whatever it is today (a leaf, or already a parent), through the
* handle it is given. Throws before any write.
*/
async function checkParentContent(db: Database, parentId: number): Promise<void> {
for (const { code, sql } of PARENT_CONTENT_CHECKS) {
const found = await db.select<Array<{ cnt: number }>>(sql, [parentId]);
if ((found[0]?.cnt ?? 0) > 0) throw new CategoryHierarchyError(code);
}
}
/** /**
* Checks moving category `id` under `parentId` (#393), on rows read in the * Checks moving category `id` under `parentId` (#393), on rows read in the
* caller's transaction and through its handle: the hierarchy rules of * caller's transaction and through its handle: the hierarchy rules of
* `parentChangeRefusal`, then a parent that holds transactions of its own is * `parentChangeRefusal`, then what the parent holds (`checkParentContent`).
* refused (once a parent, it would stop taking input). The root is always * The root is always accepted. Throws before any write.
* accepted. Throws before any write.
*/ */
async function checkNewParent( async function checkNewParent(
db: Database, db: Database,
@ -84,11 +125,7 @@ async function checkNewParent(
} }
const refusal = parentChangeRefusal(active, id, parentId); const refusal = parentChangeRefusal(active, id, parentId);
if (refusal !== null) throw new CategoryHierarchyError(refusal); if (refusal !== null) throw new CategoryHierarchyError(refusal);
const used = await db.select<Array<{ cnt: number }>>( await checkParentContent(db, parentId);
`SELECT COUNT(*) AS cnt FROM transactions WHERE category_id = $1`,
[parentId]
);
if ((used[0]?.cnt ?? 0) > 0) throw new CategoryHierarchyError("parent_has_transactions");
} }
/** Runs `body` between BEGIN and COMMIT on the transaction handle, ROLLBACK on failure. */ /** Runs `body` between BEGIN and COMMIT on the transaction handle, ROLLBACK on failure. */
@ -159,6 +196,13 @@ export async function getCategoryDepth(categoryId: number): Promise<number> {
return depth; return depth;
} }
/**
* Creates a category. Under a parent: the depth is checked first, outside any
* transaction (`getCategoryDepth`); then, in one transaction through its
* handle, what the parent holds (#393, `checkParentContent`:
* `CategoryHierarchyError` on a refusal, nothing written), the new row, and
* the parent's `is_inputable`.
*/
export async function createCategory(data: { export async function createCategory(data: {
name: string; name: string;
type: string; type: string;
@ -167,27 +211,28 @@ export async function createCategory(data: {
is_inputable: boolean; is_inputable: boolean;
sort_order: number; sort_order: number;
}): Promise<number> { }): Promise<number> {
const db = await getDb(); const parentId = data.parent_id ?? null;
// Validate max depth: parent at depth 2 would create a 4th level // Validate max depth: parent at depth 2 would create a 4th level
if (data.parent_id !== null) { if (parentId !== null) {
const parentDepth = await getCategoryDepth(data.parent_id); const parentDepth = await getCategoryDepth(parentId);
if (parentDepth >= 2) { if (parentDepth >= 2) {
throw new Error("Cannot create category: maximum depth of 3 levels reached"); throw new Error("Cannot create category: maximum depth of 3 levels reached");
} }
} }
return withTransaction((db) =>
inTransaction(db, async () => {
if (parentId !== null) await checkParentContent(db, parentId);
const result = await db.execute( const result = await db.execute(
`INSERT INTO categories (name, type, color, parent_id, is_inputable, sort_order) VALUES ($1, $2, $3, $4, $5, $6)`, `INSERT INTO categories (name, type, color, parent_id, is_inputable, sort_order) VALUES ($1, $2, $3, $4, $5, $6)`,
[data.name, data.type, data.color, data.parent_id, data.is_inputable ? 1 : 0, data.sort_order] [data.name, data.type, data.color, parentId, data.is_inputable ? 1 : 0, data.sort_order]
); );
// Auto-manage is_inputable: when a child is created under a parent, set parent to is_inputable = 0 // Auto-manage is_inputable: when a child is created under a parent, set parent to is_inputable = 0
if (data.parent_id !== null) { if (parentId !== null) await markAsParent(db, parentId);
await markAsParent(db, data.parent_id);
}
return result.lastInsertId as number; return result.lastInsertId as number;
})
);
} }
/** /**
@ -197,8 +242,8 @@ export async function createCategory(data: {
* `is_inputable`) run in one transaction, through its handle. * `is_inputable`) run in one transaction, through its handle.
* *
* An unchanged parent is not checked: a name or a colour saved on a category * An unchanged parent is not checked: a name or a colour saved on a category
* stored before these rules (a fourth level, a parent with transactions) must * stored before these rules (a fourth level, a parent that holds transactions,
* not be refused. * budget amounts or keywords) must not be refused.
*/ */
export async function updateCategory( export async function updateCategory(
id: number, id: number,