From 871768593d215228f20f4a2c9b69f1370f2208eb Mon Sep 17 00:00:00 2001 From: le king fu Date: Tue, 21 Apr 2026 20:58:53 -0400 Subject: [PATCH] fix(reports): render category combobox in hierarchical DFS order (#126) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The by-category report combobox (`/reports/category`) was showing its full category list with scrambled indentation — parents from one sub-tree interleaved with children from another. Root cause: `getAllCategoriesWithCounts` returns rows via `ORDER BY sort_order, name`, which is a *global* sort; two different roots with sort_order=1 would be followed by their respective children in the same bucket, mixing depths together. Add a pure `sortHierarchical(categories, resolveName)` helper in `CategoryCombobox.tsx` that rebuilds the display order as a DFS walk of the tree: each parent is emitted immediately followed by its descendants, with siblings within a group sorted by `sort_order` then localized display name. Orphans (parent filtered out or missing) are appended at the end so nothing disappears. The helper runs client-side inside the combobox's `useMemo`, so the fix is scoped to this component and doesn't affect other consumers of `getAllCategoriesWithCounts`. Filtering on the input query remains unchanged. Covered by 7 unit tests on the helper (empty list, single root, the exact bug-reproducing scrambled case, sort_order + name tiebreak, 3-level hierarchy, orphans, idempotence). Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.fr.md | 3 + CHANGELOG.md | 3 + .../shared/CategoryCombobox.test.ts | 114 ++++++++++++++++++ src/components/shared/CategoryCombobox.tsx | 78 +++++++++++- 4 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 src/components/shared/CategoryCombobox.test.ts diff --git a/CHANGELOG.fr.md b/CHANGELOG.fr.md index 8f57a1a..5d3fcf9 100644 --- a/CHANGELOG.fr.md +++ b/CHANGELOG.fr.md @@ -2,6 +2,9 @@ ## [Non publié] +### Corrigé +- **Rapport Zoom catégorie** (`/reports/category`) : la liste déroulante du combobox des catégories affiche désormais la liste complète dans un ordre hiérarchique DFS correct — chaque racine est émise avant ses descendants, et les frères et sœurs sont triés par `sort_order` puis nom affiché. Auparavant la liste était triée globalement par `sort_order` (via un `ORDER BY sort_order, name` SQL), ce qui entrelaçait des parents et enfants de sous-arbres différents partageant le même `sort_order`, d'où l'indentation incohérente et l'impression d'arbre cassé. La recherche filtrée (insensible aux accents) conserve le même comportement (#126) + ## [0.8.4] - 2026-04-21 ### Ajouté diff --git a/CHANGELOG.md b/CHANGELOG.md index 69d24d9..1511e35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## [Unreleased] +### Fixed +- **Category zoom report** (`/reports/category`): the category combobox dropdown now renders the full list in proper hierarchical DFS order — each root is emitted before its descendants, with siblings sorted by `sort_order` then display name. Previously the list was ordered by `sort_order` globally (from a SQL `ORDER BY sort_order, name`), which interleaved parents and children from different sub-trees that shared the same `sort_order`, producing scrambled indentation and a mis-leading tree. Filtering (accent-insensitive search) still behaves identically (#126) + ## [0.8.4] - 2026-04-21 ### Added diff --git a/src/components/shared/CategoryCombobox.test.ts b/src/components/shared/CategoryCombobox.test.ts new file mode 100644 index 0000000..854e52c --- /dev/null +++ b/src/components/shared/CategoryCombobox.test.ts @@ -0,0 +1,114 @@ +import { describe, it, expect } from "vitest"; +import { sortHierarchical } from "./CategoryCombobox"; +import type { Category } from "../../shared/types"; + +function cat( + id: number, + name: string, + parentId: number | null, + sortOrder: number, +): Category { + return { + id, + name, + parent_id: parentId ?? undefined, + type: "expense", + is_active: true, + is_inputable: true, + sort_order: sortOrder, + created_at: "", + }; +} + +const displayName = (c: Category) => c.name; + +describe("sortHierarchical", () => { + it("returns [] for empty input", () => { + expect(sortHierarchical([], displayName)).toEqual([]); + }); + + it("orders a single root before its children (parent-first DFS)", () => { + const input = [ + cat(10, "Paie", 1, 1), + cat(1, "Revenus", null, 1), + cat(11, "Autres revenus", 1, 2), + ]; + const ordered = sortHierarchical(input, displayName).map((c) => c.id); + expect(ordered).toEqual([1, 10, 11]); + }); + + it("keeps each root fully grouped with its sub-tree, roots ordered by sort_order", () => { + // Reproduces the reported bug: a flat list coming back globally ordered by + // (sort_order, name) would interleave roots and children that share the + // same sort_order. DFS must un-scramble that. + const input: Category[] = [ + // Roots + cat(1, "Revenus", null, 1), + cat(2, "Dépenses récurrentes", null, 2), + // Children of Revenus (sort_order 1 & 2 within that parent) + cat(10, "Paie", 1, 1), + cat(11, "Autres revenus", 1, 2), + // Children of Dépenses récurrentes (sort_order 1 & 2 within that parent) + cat(20, "Loyer", 2, 1), + cat(21, "Électricité", 2, 2), + ]; + + // Simulate the SQL artifact: global sort by (sort_order, name), which is + // exactly what triggered the bug. + const scrambled = [...input].sort((a, b) => { + if (a.sort_order !== b.sort_order) return a.sort_order - b.sort_order; + return a.name.localeCompare(b.name); + }); + + const ordered = sortHierarchical(scrambled, displayName).map((c) => c.id); + expect(ordered).toEqual([1, 10, 11, 2, 20, 21]); + }); + + it("orders siblings by sort_order, then by display name as tiebreaker", () => { + const input: Category[] = [ + cat(1, "Root", null, 1), + cat(12, "Beta", 1, 5), + cat(10, "Zulu", 1, 5), // same sort_order as 12 -> name tiebreak + cat(11, "Alpha", 1, 1), + ]; + const ordered = sortHierarchical(input, displayName).map((c) => c.id); + // Under Root, order should be: Alpha(sort=1), Beta(sort=5,B intermediate -> leaf)", () => { + const input: Category[] = [ + cat(2, "Dépenses", null, 2), + cat(31, "Assurances", 2, 12), + cat(310, "Assurance-auto", 31, 1), + cat(311, "Assurance-habitation", 31, 2), + cat(32, "Pharmacie", 2, 13), + ]; + const ordered = sortHierarchical(input, displayName).map((c) => c.id); + expect(ordered).toEqual([2, 31, 310, 311, 32]); + }); + + it("appends orphans (parent filtered out / missing) at the end", () => { + const input: Category[] = [ + cat(1, "Revenus", null, 1), + cat(10, "Paie", 1, 1), + // Orphan: parent_id 999 not in the set + cat(500, "Orphan", 999, 1), + ]; + const ordered = sortHierarchical(input, displayName).map((c) => c.id); + expect(ordered).toEqual([1, 10, 500]); + }); + + it("is stable (does not duplicate) when called with already-ordered input", () => { + const input: Category[] = [ + cat(1, "Revenus", null, 1), + cat(10, "Paie", 1, 1), + cat(2, "Dépenses", null, 2), + cat(20, "Loyer", 2, 1), + ]; + const once = sortHierarchical(input, displayName); + const twice = sortHierarchical(once, displayName); + expect(twice.map((c) => c.id)).toEqual(once.map((c) => c.id)); + expect(twice).toHaveLength(input.length); + }); +}); diff --git a/src/components/shared/CategoryCombobox.tsx b/src/components/shared/CategoryCombobox.tsx index a8ad312..3eddb1e 100644 --- a/src/components/shared/CategoryCombobox.tsx +++ b/src/components/shared/CategoryCombobox.tsx @@ -44,6 +44,70 @@ function computeDepths(categories: Category[]): Map { return depths; } +/** + * Order a flat list of categories in hierarchical DFS order: each root is + * emitted immediately followed by its descendants (depth-first, parent before + * children). Siblings within a group are ordered by `sort_order` ascending, + * then by `resolveName(cat)` for stable tiebreaking. + * + * A plain `ORDER BY sort_order, name` in SQL mixes parents and children from + * different sub-trees that happen to share the same `sort_order`, producing + * the scrambled indentation we saw in the by-category report combobox. + * Doing the DFS client-side keeps rendering correct regardless of query shape. + * + * Orphans (category whose parent is missing or inactive / filtered out) are + * emitted at the end, each treated as a pseudo-root, so nothing disappears. + */ +export function sortHierarchical( + categories: Category[], + resolveName: (cat: Category) => string, +): Category[] { + if (categories.length === 0) return []; + + const ids = new Set(); + for (const c of categories) ids.add(c.id); + + // Group by parent bucket: root (`null`) or parent id. + const childrenByParent = new Map(); + const orphans: Category[] = []; + for (const c of categories) { + if (c.parent_id == null) { + const bucket = childrenByParent.get(null) ?? []; + bucket.push(c); + childrenByParent.set(null, bucket); + } else if (ids.has(c.parent_id)) { + const bucket = childrenByParent.get(c.parent_id) ?? []; + bucket.push(c); + childrenByParent.set(c.parent_id, bucket); + } else { + orphans.push(c); + } + } + + const compare = (a: Category, b: Category) => { + if (a.sort_order !== b.sort_order) return a.sort_order - b.sort_order; + return resolveName(a).localeCompare(resolveName(b)); + }; + for (const bucket of childrenByParent.values()) bucket.sort(compare); + orphans.sort(compare); + + const out: Category[] = []; + const visited = new Set(); + const visit = (cat: Category) => { + if (visited.has(cat.id)) return; // defensive against cycles + visited.add(cat.id); + out.push(cat); + const kids = childrenByParent.get(cat.id); + if (kids) for (const child of kids) visit(child); + }; + const roots = childrenByParent.get(null) ?? []; + for (const root of roots) visit(root); + // Append orphans last, still treated as pseudo-roots so their own children + // (if any were pulled in) follow them. + for (const orphan of orphans) visit(orphan); + return out; +} + export default function CategoryCombobox({ categories, value, @@ -75,7 +139,15 @@ export default function CategoryCombobox({ [t] ); - const selectedCategory = categories.find((c) => c.id === value); + // Re-order the (potentially sort_order-globally-sorted) input into proper + // hierarchical DFS order so parents always precede their children and + // siblings stay grouped under the same ancestor. + const orderedCategories = useMemo( + () => sortHierarchical(categories, displayName), + [categories, displayName], + ); + + const selectedCategory = orderedCategories.find((c) => c.id === value); const displayLabel = activeExtra != null ? extraOptions?.find((o) => o.value === activeExtra)?.label ?? "" @@ -85,8 +157,8 @@ export default function CategoryCombobox({ const normalizedQuery = normalize(query); const filtered = query - ? categories.filter((c) => normalize(displayName(c)).includes(normalizedQuery)) - : categories; + ? orderedCategories.filter((c) => normalize(displayName(c)).includes(normalizedQuery)) + : orderedCategories; const filteredExtras = extraOptions ? query -- 2.45.2