fix(categories): reload the categories before the hierarchy check (#393)

After a drag-and-drop, SET_TREE leaves state.categories with what the
tree shows, without the members of a loop: the window then showed "—"
for the orphan and offered a parent the service refused (review 25437,
point 1). checkHierarchy now reads the categories again before the
diagnostic, without the page's loading state, so a form open behind
the window stays as it is; a load started meanwhile wins. Pinned by a
drag-and-drop-then-check hook test, plus the refusal-then-success case
of the window's message (RM2).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
le king fu 2026-10-19 20:59:02 -04:00
parent c8c8c068fc
commit be4b923451
2 changed files with 63 additions and 4 deletions

View file

@ -181,7 +181,8 @@ describe("useCategories — « Vérifier la hiérarchie » (#393)", () => {
let api = await mount();
await api.checkHierarchy();
api = harness.render(useCategories);
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(1);
// The mount's read, then the check's.
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(2);
await api.reattachOrphan(CYCLE, 1);
@ -192,7 +193,7 @@ describe("useCategories — « Vérifier la hiérarchie » (#393)", () => {
expect(line).toContain("3 categories");
for (const row of ROWS) expect(line).not.toContain(row.name);
expect(findOrphanCategories).toHaveBeenCalledTimes(2);
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(2);
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(3);
const { state } = harness.render(useCategories);
expect(state.hierarchyCheck).toEqual({ orphans: [], error: null });
expect(state.isSaving).toBe(false);
@ -209,7 +210,8 @@ describe("useCategories — « Vérifier la hiérarchie » (#393)", () => {
await api.reattachOrphan(CYCLE, null);
expect(reattachCategory).toHaveBeenCalledWith(3, null);
expect(logInfo).toHaveBeenCalledTimes(1);
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(2);
// The mount's read, the check's, and the reload after the committed reattachment.
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(3);
const { state } = harness.render(useCategories);
expect(state.hierarchyCheck?.error).toEqual({ message: "database is locked" });
expect(state.isSaving).toBe(false);
@ -232,6 +234,50 @@ describe("useCategories — « Vérifier la hiérarchie » (#393)", () => {
expect(logInfo).not.toHaveBeenCalled();
});
it("a refusal, then a reattachment that goes through: the window's message goes", async () => {
vi.mocked(findOrphanCategories).mockResolvedValueOnce([CYCLE]).mockResolvedValueOnce([]);
vi.mocked(reattachCategory)
.mockRejectedValueOnce(new CategoryHierarchyError("parent_has_transactions"))
.mockResolvedValueOnce(3);
let api = await mount();
await api.checkHierarchy();
api = harness.render(useCategories);
await api.reattachOrphan(CYCLE, 1);
api = harness.render(useCategories);
expect(api.state.hierarchyCheck?.error).toEqual({ key: "categories.hierarchy.errors.parent_has_transactions" });
await api.reattachOrphan(CYCLE, null);
expect(harness.render(useCategories).state.hierarchyCheck).toEqual({ orphans: [], error: null });
});
it("after a drag-and-drop, reads the categories again: the window gets the members of a loop back", async () => {
// 1 with 10 under it, 2, and 3 <-> 5: no root reaches 3 nor 5.
const row = (id: number, parent_id: number | null) => ({ ...ROWS[1], id, name: `c${id}`, parent_id, sort_order: id });
vi.mocked(getAllCategoriesWithCounts).mockResolvedValue([
row(1, null),
row(10, 1),
row(2, null),
row(3, 5),
row(5, 3),
] as never);
vi.mocked(findOrphanCategories).mockResolvedValue([CYCLE]);
let api = await mount();
const ids = () => harness.render(useCategories).state.categories.map((c) => c.id).sort((a, b) => a - b);
expect(ids()).toEqual([1, 2, 3, 5, 10]);
await api.moveCategory(2, null, 0);
// The drag-and-drop keeps what the tree shows: the loop is gone from the list.
expect(ids()).toEqual([1, 2, 10]);
api = harness.render(useCategories);
harness.dispatch.mockClear();
await api.checkHierarchy();
expect(ids()).toEqual([1, 2, 3, 5, 10]);
expect(harness.render(useCategories).state.hierarchyCheck).toEqual({ orphans: [CYCLE], error: null });
// Read without the page's loading state: the tree and an open form stay on screen.
const types = harness.dispatch.mock.calls.map(([action]) => (action as { type: string }).type);
expect(types).not.toContain("SET_LOADING");
});
it("closes the window; a check that ends after it closed changes nothing", async () => {
let resolve: (orphans: OrphanCategory[]) => void = () => undefined;
vi.mocked(findOrphanCategories).mockReturnValue(new Promise((r) => (resolve = r)));

View file

@ -403,10 +403,23 @@ export function useCategories() {
[state.tree, loadCategories]
);
/** Opens the « Vérifier la hiérarchie » window and runs the diagnostic (#393). */
/**
* Opens the « Vérifier la hiérarchie » window and runs the diagnostic (#393).
* The window names the orphans and offers their parents from
* `state.categories`, which a drag-and-drop leaves with what the tree shows
* 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
* behind the window stays as it is; a load started meanwhile wins.
*/
const checkHierarchy = useCallback(async () => {
dispatch({ type: "OPEN_HIERARCHY_CHECK" });
try {
const fetchId = fetchIdRef.current;
const rows = await getAllCategoriesWithCounts();
if (fetchId === fetchIdRef.current) {
const flat = rows.map((r) => ({ ...r, children: [] as CategoryTreeNode[] }));
dispatch({ type: "SET_CATEGORIES", payload: { flat, tree: buildTree(flat) } });
}
const orphans = await findOrphanCategories();
dispatch({ type: "SET_ORPHANS", payload: orphans });
} catch (e) {