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
3 changed files with 32 additions and 4 deletions
Showing only changes of commit 273cf36eb7 - Show all commits

View file

@ -283,7 +283,7 @@ Un `parent_id` en boucle (A sous B, B sous A, ou une catégorie sous elle-même)
- **Diagnostic** — `findOrphanCategories` lit **toutes** les catégories, actives ou non (`SELECT id, parent_id, is_active FROM categories`) : les autres requêtes ne gardent que les actives, ce qui ferait d'un parent désactivé un parent absent. Motifs : `cycle` (le membre de plus petit `id`, une entrée par boucle), `inactive_parent`, `missing_parent` ; seule la tête de chaque groupe est listée, sa descendance la suit. - **Diagnostic** — `findOrphanCategories` lit **toutes** les catégories, actives ou non (`SELECT id, parent_id, is_active FROM categories`) : les autres requêtes ne gardent que les actives, ce qui ferait d'un parent désactivé un parent absent. Motifs : `cycle` (le membre de plus petit `id`, une entrée par boucle), `inactive_parent`, `missing_parent` ; seule la tête de chaque groupe est listée, sa descendance la suit.
- **Validation** — `updateCategory` (seulement quand le parent change : un renommage d'une catégorie enregistrée avant ces règles n'est jamais refusé) et `reattachCategory` (toujours) passent par `checkNewParent` : `parentChangeRefusal` sur les catégories actives (`self_parent` ; `cycle` pour un parent pris dans la descendance ou sans profondeur ; `too_deep`), puis ce que porte le parent (`checkParentContent`, décision de Max du 2026-10-19), dans cet ordre, le premier trouvé faisant le refus : `parent_has_transactions` (des transactions directes), `parent_has_budget` (un montant non nul dans `budget_entries` ou `budget_template_entries`), `parent_has_keywords` (un mot-clé actif ; un mot-clé supprimé, `is_active = 0`, ne classe rien et ne compte pas). Devenu parent, il ne serait plus saisissable : ses transactions resteraient dessus, ses montants sortiraient de la grille du budget, et ses mots-clés continueraient d'y classer des transactions. Le contenu se vérifie quel que soit l'état du parent, feuille ou déjà parent. `createCategory` vérifie de même le parent d'une sous-catégorie, dans un `withTransaction` qui porte aussi l'insertion et le `is_inputable = 0` du parent ; sa vérification de profondeur (`getCategoryDepth`, erreur simple) reste avant la transaction. La racine est toujours acceptée : c'est la réparation de dernier recours d'une boucle. Un refus lève `CategoryHierarchyError` (`code`), que `useCategories` reconnaît par `instanceof` et affiche par la clé `categories.hierarchy.errors.<code>`. Lectures et écritures (la ligne, puis `is_inputable = 0` sur le nouveau parent, comme `createCategory`) se font dans **un** `withTransaction`, par son handle. Un parent inactif ou inexistant lève une erreur simple, hors d'atteinte de l'interface. - **Validation** — `updateCategory` (seulement quand le parent change : un renommage d'une catégorie enregistrée avant ces règles n'est jamais refusé) et `reattachCategory` (toujours) passent par `checkNewParent` : `parentChangeRefusal` sur les catégories actives (`self_parent` ; `cycle` pour un parent pris dans la descendance ou sans profondeur ; `too_deep`), puis ce que porte le parent (`checkParentContent`, décision de Max du 2026-10-19), dans cet ordre, le premier trouvé faisant le refus : `parent_has_transactions` (des transactions directes), `parent_has_budget` (un montant non nul dans `budget_entries` ou `budget_template_entries`), `parent_has_keywords` (un mot-clé actif ; un mot-clé supprimé, `is_active = 0`, ne classe rien et ne compte pas). Devenu parent, il ne serait plus saisissable : ses transactions resteraient dessus, ses montants sortiraient de la grille du budget, et ses mots-clés continueraient d'y classer des transactions. Le contenu se vérifie quel que soit l'état du parent, feuille ou déjà parent. `createCategory` vérifie de même le parent d'une sous-catégorie, dans un `withTransaction` qui porte aussi l'insertion et le `is_inputable = 0` du parent ; sa vérification de profondeur (`getCategoryDepth`, erreur simple) reste avant la transaction. La racine est toujours acceptée : c'est la réparation de dernier recours d'une boucle. Un refus lève `CategoryHierarchyError` (`code`), que `useCategories` reconnaît par `instanceof` et affiche par la clé `categories.hierarchy.errors.<code>`. Lectures et écritures (la ligne, puis `is_inputable = 0` sur le nouveau parent, comme `createCategory`) se font dans **un** `withTransaction`, par son handle. Un parent inactif ou inexistant lève une erreur simple, hors d'atteinte de l'interface.
- **`getCategoryDepth`** garde les `id` vus et rend `Infinity` sur une boucle : `createCategory` refuse alors ce parent. - **`getCategoryDepth`** garde les `id` vus et rend `Infinity` sur une boucle : `createCategory` refuse alors ce parent.
- **Fenêtre** — `HierarchyCheckModal` (présentationnelle) : nombre d'orphelines ou « aucune », une ligne par orpheline (nom, motif traduit, `CategoryCombobox` du nouveau parent alimenté par `buildParentOptions` avec l'orpheline en catégorie éditée, « Racine » présélectionnée en `extraOptions`, bouton « Rattacher »), bouton « Fermer ». La liste du combobox sort de la fenêtre par son portail (#429). À l'ouverture, `checkHierarchy` relit les catégories avant le diagnostic, sans l'état de chargement de la page (un formulaire ouvert derrière la fenêtre reste tel quel) : un glisser-déposer laisse dans `state.categories` ce que montre l'arbre (`SET_TREE`), sans les catégories qu'aucune racine n'atteint, que la fenêtre doit nommer et dont elle calcule les parents proposés. Après un rattachement, une ligne `logInfo` donne le motif et le nombre de catégories remises en place, sans nom de catégorie, puis la liste et l'arbre se rechargent. - **Fenêtre** — `HierarchyCheckModal` (présentationnelle) : nombre d'orphelines ou « aucune », une ligne par orpheline (nom, motif traduit, `CategoryCombobox` du nouveau parent alimenté par `buildParentOptions` avec l'orpheline en catégorie éditée, « Racine » présélectionnée en `extraOptions`, bouton « Rattacher »), bouton « Fermer ». La liste du combobox sort de la fenêtre par son portail (#429). À l'ouverture, `checkHierarchy` relit les catégories avant le diagnostic, une fois finies les écritures d'un glisser-déposer en cours (un `UPDATE` par catégorie, qu'une lecture à mi-chemin montrerait à moitié appliqué), sans l'état de chargement de la page (un formulaire ouvert derrière la fenêtre reste tel quel) : un glisser-déposer laisse dans `state.categories` ce que montre l'arbre (`SET_TREE`), sans les catégories qu'aucune racine n'atteint, que la fenêtre doit nommer et dont elle calcule les parents proposés. Après un rattachement, une ligne `logInfo` donne le motif et le nombre de catégories remises en place, sans nom de catégorie, puis la liste et l'arbre se rechargent.
- **Budget** — `buildBudgetYearRows` (`useBudget.ts`) et `getBudgetVsActualData` (`budgetService.ts`) placent au premier niveau une catégorie dont le parent n'est pas dans la liste active (`displaysAsRoot`), comme l'arbre et les sélecteurs ; une boucle y reste cachée, la fenêtre la répare. - **Budget** — `buildBudgetYearRows` (`useBudget.ts`) et `getBudgetVsActualData` (`budgetService.ts`) placent au premier niveau une catégorie dont le parent n'est pas dans la liste active (`displaysAsRoot`), comme l'arbre et les sélecteurs ; une boucle y reste cachée, la fenêtre la répare.
- **Hors portée** — le glisser-déposer : `updateCategorySortOrders` écrit `parent_id` sans cette validation, gardé par l'interface (`moveCategory` ne peut pas déposer une catégorie dans son propre sous-arbre) ; il ne refuse pas non plus un parent qui porte des transactions, des montants de budget ou des mots-clés, et ne touche pas `is_inputable`. La restauration d'une sauvegarde (`restoreCategories`) réinsère les `parent_id` tels quels, boucles comprises : la fenêtre répare après coup. - **Hors portée** — le glisser-déposer : `updateCategorySortOrders` écrit `parent_id` sans cette validation, gardé par l'interface (`moveCategory` ne peut pas déposer une catégorie dans son propre sous-arbre) ; il ne refuse pas non plus un parent qui porte des transactions, des montants de budget ou des mots-clés, et ne touche pas `is_inputable`. La restauration d'une sauvegarde (`restoreCategories`) réinsère les `parent_id` tels quels, boucles comprises : la fenêtre répare après coup.

View file

@ -47,6 +47,7 @@ import {
getAllCategoriesWithCounts, getAllCategoriesWithCounts,
reattachCategory, reattachCategory,
updateCategory, updateCategory,
updateCategorySortOrders,
} from "../services/categoryService"; } from "../services/categoryService";
import { logInfo } from "../services/logService"; import { logInfo } from "../services/logService";
import { markedT } from "../test/i18nMock"; import { markedT } from "../test/i18nMock";
@ -84,6 +85,8 @@ beforeEach(() => {
vi.mocked(createCategory).mockReset(); vi.mocked(createCategory).mockReset();
vi.mocked(findOrphanCategories).mockReset(); vi.mocked(findOrphanCategories).mockReset();
vi.mocked(reattachCategory).mockReset(); vi.mocked(reattachCategory).mockReset();
vi.mocked(updateCategorySortOrders).mockReset();
vi.mocked(updateCategorySortOrders).mockResolvedValue(undefined);
vi.mocked(logInfo).mockReset(); vi.mocked(logInfo).mockReset();
}); });
@ -278,6 +281,24 @@ describe("useCategories — « Vérifier la hiérarchie » (#393)", () => {
expect(types).not.toContain("SET_LOADING"); expect(types).not.toContain("SET_LOADING");
}); });
it("reads the categories once the drag-and-drop's writes are over, never halfway", async () => {
let written: () => void = () => undefined;
vi.mocked(updateCategorySortOrders).mockReturnValue(new Promise<void>((r) => (written = r)));
vi.mocked(findOrphanCategories).mockResolvedValue([]);
let api = await mount();
const moved = api.moveCategory(10, null, 0);
api = harness.render(useCategories);
const checked = api.checkHierarchy();
await flush();
// The mount's read only: the check waits for the writes.
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(1);
expect(findOrphanCategories).not.toHaveBeenCalled();
written();
await Promise.all([moved, checked]);
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(2);
expect(harness.render(useCategories).state.hierarchyCheck).toEqual({ orphans: [], error: null });
});
it("closes the window; a check that ends after it closed changes nothing", async () => { it("closes the window; a check that ends after it closed changes nothing", async () => {
let resolve: (orphans: OrphanCategory[]) => void = () => undefined; let resolve: (orphans: OrphanCategory[]) => void = () => undefined;
vi.mocked(findOrphanCategories).mockReturnValue(new Promise((r) => (resolve = r))); vi.mocked(findOrphanCategories).mockReturnValue(new Promise((r) => (resolve = r)));

View file

@ -204,6 +204,8 @@ export function useCategories() {
const [state, dispatch] = useReducer(reducer, initialState); const [state, dispatch] = useReducer(reducer, initialState);
const fetchIdRef = useRef(0); const fetchIdRef = useRef(0);
const duplicateCheckDone = useRef(false); const duplicateCheckDone = useRef(false);
/** The writes of the last drag-and-drop, settled or not: the hierarchy check reads after them. */
const pendingMoveRef = useRef<Promise<unknown>>(Promise.resolve());
const loadCategories = useCallback(async () => { const loadCategories = useCallback(async () => {
const fetchId = ++fetchIdRef.current; const fetchId = ++fetchIdRef.current;
@ -393,8 +395,10 @@ export function useCategories() {
} }
collectUpdates(newTree, null); collectUpdates(newTree, null);
const written = updateCategorySortOrders(updates);
pendingMoveRef.current = written.catch(() => undefined);
try { try {
await updateCategorySortOrders(updates); await written;
} catch { } catch {
// Revert on error // Revert on error
await loadCategories(); await loadCategories();
@ -408,12 +412,15 @@ export function useCategories() {
* The window names the orphans and offers their parents from * The window names the orphans and offers their parents from
* `state.categories`, which a drag-and-drop leaves with what the tree shows * `state.categories`, which a drag-and-drop leaves with what the tree shows
* only (`SET_TREE`), without the categories no root reaches: the categories * only (`SET_TREE`), without the categories no root reaches: the categories
* are read again first. Without the page's loading state, so a form open * are read again first, once the drag-and-drop's writes are over (one
* behind the window stays as it is; a load started meanwhile wins. * `UPDATE` per category: read halfway, the tree would show half the move).
* Without the page's loading state, so a form open behind the window stays
* as it is; a load started meanwhile wins.
*/ */
const checkHierarchy = useCallback(async () => { const checkHierarchy = useCallback(async () => {
dispatch({ type: "OPEN_HIERARCHY_CHECK" }); dispatch({ type: "OPEN_HIERARCHY_CHECK" });
try { try {
await pendingMoveRef.current;
const fetchId = fetchIdRef.current; const fetchId = fetchIdRef.current;
const rows = await getAllCategoriesWithCounts(); const rows = await getAllCategoriesWithCounts();
if (fetchId === fetchIdRef.current) { if (fetchId === fetchIdRef.current) {