refactor(categories) : unifier la machine a etats des 2 arbres de categories sur useCollapsibleGroups #294

Closed

maximus wants to merge 1 commit from issue-290-categories-unify into issue-289-budget-collapse

maximus commented

2026-07-16 01:30:43 +00:00

Owner

Resolves #290

Stacked on #293 (issue-289-budget-collapse) — base branch is issue-289-budget-collapse, not main. Review/merge #293 first.

Unifies the collapse state machine of the two category trees onto useCollapsibleGroups (a strict superset after #288), keeping each tree's distinct recursive render and CategoryTree's drag-and-drop untouched.

CategoryTree (Categories page) → hook with storageKey: null + defaultExpanded: true (expanded by default, no seeding); drops the local Set + collectExpandable.
CategoryTaxonomyTree + guide page → hook with storageKey: null + defaultExpanded: false (collapsed by default); shared TAXONOMY_COLLAPSE_ACCESSORS exported from the tree.
Fixes the guide button bug (allExpanded = expanded.size > 0 flipped after one node → hook's correct allExpanded), and migrates a 4th consumer (StepDiscover, same bug) onto the hook for a green build + consistency.

Behaviours preserved: Categories opens expanded (DnD intact), guide/wizard open collapsed, and opening one node no longer flips the button.

Build 0 TS errors · 828 vitest pass · cargo check clean. No DB migration.

Generated autonomously by /autopilot run of 2026-07-15

Resolves #290 **Stacked on #293** (`issue-289-budget-collapse`) — base branch is `issue-289-budget-collapse`, not `main`. Review/merge #293 first. Unifies the collapse **state machine** of the two category trees onto `useCollapsibleGroups` (a strict superset after #288), keeping each tree's distinct recursive render and CategoryTree's drag-and-drop untouched. - **CategoryTree** (Categories page) → hook with `storageKey: null` + `defaultExpanded: true` (expanded by default, no seeding); drops the local `Set` + `collectExpandable`. - **CategoryTaxonomyTree + guide page** → hook with `storageKey: null` + `defaultExpanded: false` (collapsed by default); shared `TAXONOMY_COLLAPSE_ACCESSORS` exported from the tree. - Fixes the guide button bug (`allExpanded = expanded.size > 0` flipped after one node → hook's correct `allExpanded`), and migrates a 4th consumer (`StepDiscover`, same bug) onto the hook for a green build + consistency. Behaviours preserved: Categories opens expanded (DnD intact), guide/wizard open collapsed, and opening one node no longer flips the button. Build 0 TS errors · **828 vitest pass** · cargo check clean. No DB migration. Generated autonomously by /autopilot run of 2026-07-15

maximus added 1 commit 2026-07-16 01:30:44 +00:00

refactor(categories): unify both category trees' collapse state onto useCollapsibleGroups 9f628aa9f4

Replace the two hand-rolled Set-of-ids collapse state machines in the category
trees with the shared useCollapsibleGroups hook (a strict superset after #288),
keeping each tree's distinct recursive render and CategoryTree's drag-and-drop
untouched.

- CategoryTree (Categories page): storageKey null + defaultExpanded true, so
  every parent opens with no seeding; drops the local Set + collectExpandable.
- CategoryTaxonomyTree + guide page: storageKey null + defaultExpanded false
  (collapsed by default); exports a shared TAXONOMY_COLLAPSE_ACCESSORS.
- Fix the guide's button bug: allExpanded = expanded.size > 0 flipped to
  "Collapse all" after opening a single node; now uses the hook's correct
  allExpanded (every group must be open).
- Also migrate StepDiscover (4th consumer of CategoryTaxonomyTree, same button
  bug) onto the hook for a green build and consistency.

Both trees pass a flattened node list to the bulk ops; behaviour preserved:
Categories opens expanded, the guide/wizard open collapsed.

Resolves #290

Generated autonomously by /autopilot run of 2026-07-15

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maximus added the

autopilot:pending-human

label 2026-07-16 01:31:17 +00:00

maximus referenced this pull request

2026-07-16 01:44:15 +00:00

docs(collapse) : ADR 0016 persistance UI par-profil + guide utilisateur + architecture #295

maximus commented

2026-07-18 18:27:52 +00:00

Author

Owner

/pr-review — PR #294 · Verdict : APPROVE ✅

Review adversariale, head-vs-base (issue-289-budget-collapse). Pile stackée : #293 à reviewer/merger d'abord.

Résumé — Refactor propre et fidèle : la machine à états de collapse des deux arbres de catégories est unifiée sur useCollapsibleGroups (superset strict après #288), le rendu récursif distinct de chaque arbre et le drag-and-drop de CategoryTree restant intacts. Le bug du bouton #290 est corrigé par délégation au primitive testé. Une seule suggestion non bloquante.

Correction — vérifié

Polarité correcte des deux côtés. CategoryTree : defaultExpanded: true + set vide ⇒ tous les parents dépliés sans seeding (réplique l'ancien collectExpandable + Set local). Guide + StepDiscover : defaultExpanded: false + set vide ⇒ tout replié (réplique l'ancien Set vide initial).
Fix #290 correct. L'ancien allExpanded = expanded.size > 0 basculait le bouton dès un seul nœud déplié ; le nouveau groups.allExpanded(flatNodes) = keys.length > 0 && keys.every(!isCollapsedFor) n'est vrai que si tous les parents sont ouverts. Les atomes (isCollapsedFor deux polarités, collapsibleKeys) sont couverts par collapsibleRows.test.ts.
flattenNodes/flatNodes alimentent correctement les bulk ops du hook — collapsibleKeys n'est pas récursif, donc les consommateurs doivent lui passer une liste aplatie. Les feuilles incluses dans l'aplatissement sont filtrées sans effet par isParent.
Changement de signature de prop entièrement propagé. Seuls 2 consommateurs de CategoryTaxonomyTree (CategoriesStandardGuidePage, StepDiscover), tous deux mis à jour ; les Props publiques de CategoryTree restent inchangées ⇒ CategoriesPage correctement non touchée. noUnusedLocals + noUnusedParameters: true combinés au « 0 erreur TS » annoncé garantissent l'absence de code mort ou de consommateur oublié.
Stubs d'accessors documentés et inoffensifs. depthOf: () => 0 n'est jamais appelé sur le chemin des arbres ; visibleRows/parentKeyOf non plus (les deux arbres rendent récursivement et gatent chaque nœud sur isCollapsed). CategoryTree fournit même un parentKeyOf correct via parent_id.
Réactivité du toggle intacte. L'identité de groups.isCollapsed change avec flipped, les accessors module-const gardent les callbacks stables, les arbres se re-rendent bien au toggle.

Tests — un manque non bloquant

Le PR n'ajoute aucun test (diff = CHANGELOG + 4 composants). La logique allExpanded vit dans useCollapsibleGroups, sans fichier de test dédié : ses briques sont testées, mais le collage précis qui corrige #290 (length > 0 && every(!isCollapsedFor)) ne l'est pas directement. Un test ciblé du hook (1-sur-N déplié ⇒ faux ; tous ⇒ vrai ; vide ⇒ faux) serait le vrai garde-fou de régression. Non bloquant : #290 est étiqueté type:refactor, le fix est une pure délégation à des primitives déjà couvertes, sans logique nouvelle.

Observations mineures (non bloquantes)

CategoryTree — léger gain comportemental. Un nœud devenu parent après le montage s'ouvre désormais déplié (defaultExpanded: true) au lieu de replié (ancien Set seedé une seule fois). Meilleure UX, pas une régression de l'invariant « ouvre déplié ».
CHANGELOG : « wizard's overview step » désigne l'étape Discover (categoriesSeed.migration.discover). Trivial.
Pile stackée — gap CI : check.yml ne tourne que sur les PR vers main, donc ce maillon intermédiaire n'est pas CI-validé. Valider le tip cumulé en local (tsc && vite build, vitest run, cargo check) avant de merger la pile.

Qualité / Sécurité / Données

CHANGELOG bilingue sous [Unreleased], ordre Keep-a-Changelog correct (Changed avant Fixed). ✓
Aucune chaîne en dur (clés i18n réutilisées). ✓
Aucune surface sécurité (refactor d'état front pur, pas de SQL/IO/secret). ✓
Aucune migration DB. ✓
Commit conventionnel refactor(categories): … · Resolves #290. ✓

Verdict : APPROVE — 1 suggestion non bloquante (test de régression du hook allExpanded).

## /pr-review — PR #294 · Verdict : **APPROVE** ✅ *Review adversariale, head-vs-base (`issue-289-budget-collapse`). Pile stackée : #293 à reviewer/merger d'abord.* **Résumé** — Refactor propre et fidèle : la machine à états de collapse des deux arbres de catégories est unifiée sur `useCollapsibleGroups` (superset strict après #288), le rendu récursif distinct de chaque arbre et le drag-and-drop de `CategoryTree` restant intacts. Le bug du bouton #290 est corrigé par délégation au primitive testé. Une seule suggestion non bloquante. ### Correction — vérifié 1. **Polarité correcte des deux côtés.** `CategoryTree` : `defaultExpanded: true` + set vide ⇒ tous les parents dépliés sans seeding (réplique l'ancien `collectExpandable` + `Set` local). Guide + `StepDiscover` : `defaultExpanded: false` + set vide ⇒ tout replié (réplique l'ancien `Set` vide initial). 2. **Fix #290 correct.** L'ancien `allExpanded = expanded.size > 0` basculait le bouton dès un seul nœud déplié ; le nouveau `groups.allExpanded(flatNodes)` = `keys.length > 0 && keys.every(!isCollapsedFor)` n'est vrai que si **tous** les parents sont ouverts. Les atomes (`isCollapsedFor` deux polarités, `collapsibleKeys`) sont couverts par `collapsibleRows.test.ts`. 3. **`flattenNodes`/`flatNodes` alimentent correctement les bulk ops** du hook — `collapsibleKeys` n'est **pas** récursif, donc les consommateurs doivent lui passer une liste aplatie. Les feuilles incluses dans l'aplatissement sont filtrées sans effet par `isParent`. 4. **Changement de signature de prop entièrement propagé.** Seuls 2 consommateurs de `CategoryTaxonomyTree` (`CategoriesStandardGuidePage`, `StepDiscover`), tous deux mis à jour ; les `Props` publiques de `CategoryTree` restent inchangées ⇒ `CategoriesPage` correctement non touchée. `noUnusedLocals` + `noUnusedParameters: true` combinés au « 0 erreur TS » annoncé garantissent l'absence de code mort ou de consommateur oublié. 5. **Stubs d'accessors documentés et inoffensifs.** `depthOf: () => 0` n'est jamais appelé sur le chemin des arbres ; `visibleRows`/`parentKeyOf` non plus (les deux arbres rendent récursivement et gatent chaque nœud sur `isCollapsed`). `CategoryTree` fournit même un `parentKeyOf` correct via `parent_id`. 6. **Réactivité du toggle intacte.** L'identité de `groups.isCollapsed` change avec `flipped`, les accessors module-const gardent les callbacks stables, les arbres se re-rendent bien au toggle. ### Tests — un manque non bloquant - Le PR **n'ajoute aucun test** (diff = CHANGELOG + 4 composants). La logique `allExpanded` vit dans `useCollapsibleGroups`, **sans fichier de test dédié** : ses briques sont testées, mais le collage précis qui corrige #290 (`length > 0 && every(!isCollapsedFor)`) ne l'est pas directement. Un test ciblé du hook (1-sur-N déplié ⇒ faux ; tous ⇒ vrai ; vide ⇒ faux) serait le vrai garde-fou de régression. **Non bloquant** : #290 est étiqueté `type:refactor`, le fix est une pure délégation à des primitives déjà couvertes, sans logique nouvelle. ### Observations mineures (non bloquantes) - **`CategoryTree` — léger gain comportemental.** Un nœud devenu parent *après* le montage s'ouvre désormais déplié (`defaultExpanded: true`) au lieu de replié (ancien `Set` seedé une seule fois). Meilleure UX, pas une régression de l'invariant « ouvre déplié ». - **CHANGELOG** : « wizard's overview step » désigne l'étape *Discover* (`categoriesSeed.migration.discover`). Trivial. - **Pile stackée — gap CI** : `check.yml` ne tourne que sur les PR vers `main`, donc ce maillon intermédiaire n'est pas CI-validé. Valider le tip cumulé en local (`tsc && vite build`, `vitest run`, `cargo check`) avant de merger la pile. ### Qualité / Sécurité / Données - CHANGELOG bilingue sous `[Unreleased]`, ordre Keep-a-Changelog correct (Changed avant Fixed). ✓ - Aucune chaîne en dur (clés i18n réutilisées). ✓ - Aucune surface sécurité (refactor d'état front pur, pas de SQL/IO/secret). ✓ - Aucune migration DB. ✓ - Commit conventionnel `refactor(categories): …` · `Resolves #290`. ✓ --- *Verdict : **APPROVE** — 1 suggestion non bloquante (test de régression du hook `allExpanded`).*

maximus commented

2026-07-18 18:47:38 +00:00

Author

Owner

Mergé en fast-forward dans main (pile #292→#295), commit 9f628aa. Forgejo ne détecte pas le merge local → fermeture manuelle. L'issue liée s'est auto-fermée via Resolves #N.

Mergé en fast-forward dans `main` (pile #292→#295), commit `9f628aa`. Forgejo ne détecte pas le merge local → fermeture manuelle. L'issue liée s'est auto-fermée via `Resolves #N`.

maximus closed this pull request

2026-07-18 18:47:39 +00:00