fix(categories): a reattachment keeps a form open behind the window (#393)

Review 25921, point 5 (pre-existing since #472): after a reattachment,
reattachOrphan reloaded through loadCategories, whose loading state
unmounted the detail panel, so a category form open behind the window
lost its unsaved changes. The silent re-read of checkHierarchy becomes
readCategoriesAgain (waits for a drag-and-drop's writes, a newer load
wins, no loading state), used by both; a failed re-read after a
reattachment now shows in the window, where the user is.

Also pins the guard « a newer load wins » (review 25921, R8): a load
started during the check's read lands, the check's older answer is
dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
le king fu 2026-10-20 19:41:17 -04:00
parent 1386abd91e
commit bc6610103b
2 changed files with 59 additions and 16 deletions

View file

@ -281,6 +281,38 @@ describe("useCategories — « Vérifier la hiérarchie » (#393)", () => {
expect(types).not.toContain("SET_LOADING");
});
it("lets a load started during the check's read win over it", async () => {
let answer: (rows: unknown) => void = () => undefined;
const NEWER = [...ROWS, { ...ROWS[0], id: 2, name: "Épargne", sort_order: 2 }];
vi.mocked(findOrphanCategories).mockResolvedValue([]);
let api = await mount();
vi.mocked(getAllCategoriesWithCounts)
.mockReturnValueOnce(new Promise((r) => (answer = r)) as never)
.mockResolvedValueOnce(NEWER as never);
const checked = api.checkHierarchy();
await flush(); // the check's read is out, still unanswered
api = harness.render(useCategories);
await api.reinitializeCategories(); // a newer load: NEWER lands
answer(ROWS); // the check's older answer comes last
await checked;
const ids = harness.render(useCategories).state.categories.map((c) => c.id).sort((a, b) => a - b);
expect(ids).toEqual([1, 2, 10]);
});
it("a reattachment reads the categories again without the loading state: a form behind the window stays", async () => {
vi.mocked(findOrphanCategories).mockResolvedValueOnce([CYCLE]).mockResolvedValueOnce([]);
vi.mocked(reattachCategory).mockResolvedValue(3);
let api = await mount();
await api.checkHierarchy();
api = harness.render(useCategories);
harness.dispatch.mockClear();
await api.reattachOrphan(CYCLE, null);
const types = harness.dispatch.mock.calls.map(([action]) => (action as { type: string }).type);
expect(types).toContain("SET_CATEGORIES");
expect(types).not.toContain("SET_LOADING");
expect(getAllCategoriesWithCounts).toHaveBeenCalledTimes(3);
});
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)));

View file

@ -407,38 +407,49 @@ export function useCategories() {
[state.tree, loadCategories]
);
/**
* Reads the categories again for the « Vérifier la hiérarchie » window
* (#393), without the page's loading state: `loadCategories` would show
* « Chargement » in place of the tree and the detail panel, and a form open
* behind the window would come back from its stored values, its unsaved
* changes lost. Waits for a drag-and-drop's writes first (one `UPDATE` per
* category: read halfway, the tree would show half the move); a load
* started meanwhile wins. Throws when the read fails.
*/
const readCategoriesAgain = useCallback(async () => {
await pendingMoveRef.current;
const fetchId = fetchIdRef.current;
const rows = await getAllCategoriesWithCounts();
if (fetchId !== fetchIdRef.current) return;
const flat = rows.map((r) => ({ ...r, children: [] as CategoryTreeNode[] }));
dispatch({ type: "SET_CATEGORIES", payload: { flat, tree: buildTree(flat) } });
}, []);
/**
* 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, once the drag-and-drop's writes are over (one
* `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.
* are read again first (`readCategoriesAgain`).
*/
const checkHierarchy = useCallback(async () => {
dispatch({ type: "OPEN_HIERARCHY_CHECK" });
try {
await pendingMoveRef.current;
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) } });
}
await readCategoriesAgain();
const orphans = await findOrphanCategories();
dispatch({ type: "SET_ORPHANS", payload: orphans });
} catch (e) {
dispatch({ type: "SET_HIERARCHY_ERROR", payload: categoriesErrorOf(e) });
}
}, []);
}, [readCategoriesAgain]);
/**
* Reattaches an orphan under `parentId`, or to the root with `null` (#393).
* On success: one Journal line (counts, no name), then the orphan list and
* the tree reload; the tree reloads even if the new check fails, since the
* write is committed. A refusal shows in the window, by its translated code.
* the categories are read again; the categories are read even if the new
* check fails, since the write is committed. Read like at the opening
* (`readCategoriesAgain`), so a form open behind the window keeps its
* unsaved changes. A refusal, or a failed read, shows in the window.
*/
const reattachOrphan = useCallback(
async (orphan: OrphanCategory, parentId: number | null) => {
@ -450,14 +461,14 @@ export function useCategories() {
const orphans = await findOrphanCategories();
dispatch({ type: "SET_ORPHANS", payload: orphans });
} finally {
await loadCategories();
await readCategoriesAgain();
}
dispatch({ type: "SET_SAVING", payload: false });
} catch (e) {
dispatch({ type: "SET_HIERARCHY_ERROR", payload: categoriesErrorOf(e) });
}
},
[loadCategories]
[readCategoriesAgain]
);
const closeHierarchyCheck = useCallback(() => {