diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 444903b..a6d6990 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,23 +1,3 @@ -### TASK-QUALITY-01 — `KVEditor` Uses `entryKeys.join(",")` as Effect Dependency - -**Where found** -`frontend/src/components/config/KVEditor.tsx`. An effect dependency is computed as `entryKeys.join(",")`. This works for most key values but produces incorrect results (false equality) when any key contains a comma character — two different key sets could produce the same joined string. - -**Goal** -Replace the join-based comparison with a stable serialisation that cannot produce false equality. The simplest correct option is `JSON.stringify(entryKeys)`, which handles commas, empty strings, and special characters correctly. Alternatively, use `useDeepCompareEffect` from a utility library, or maintain a counter that increments whenever keys change. - -**Possible traps and issues** -- `JSON.stringify` on a large array is marginally more expensive than `join`. For a config editor with typically fewer than 50 keys this cost is negligible. -- Ensure the dependency is the full keys array (not the joined string) and let React's referential equality handle the common case; only reach for `JSON.stringify` if the array reference itself is not stable. - -**Docs changes needed** -None required. - -**Why this is needed** -A KV entry key containing a comma (e.g. `"a,b"` vs separate keys `"a"` and `"b"`) would cause the effect to not fire when it should, silently failing to update derived state. - ---- - ### TASK-QUALITY-02 — `useConfigItem.save()` Briefly Shows Session-Expiry as Save Error **Where found** diff --git a/frontend/src/hooks/__tests__/useConfigItem.test.ts b/frontend/src/hooks/__tests__/useConfigItem.test.ts index 39876a5..f8bfbdb 100644 --- a/frontend/src/hooks/__tests__/useConfigItem.test.ts +++ b/frontend/src/hooks/__tests__/useConfigItem.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { renderHook, act } from "@testing-library/react"; import { useConfigItem } from "../useConfigItem"; +import { ApiError } from "../../api/client"; describe("useConfigItem", () => { beforeEach(() => { @@ -85,4 +86,40 @@ describe("useConfigItem", () => { expect(result.current.saveError).toBe("save failed"); }); + + it("auth errors are rethrown without setting saveError", async () => { + const fetchFn = vi.fn().mockResolvedValue("ok"); + const authError = new ApiError(401, "Unauthorized"); + const saveFn = vi.fn().mockRejectedValue(authError); + + const { result } = renderHook(() => useConfigItem({ fetchFn, saveFn })); + + await act(async () => { + await Promise.resolve(); + }); + + await act(async () => { + await expect(result.current.save("test")).rejects.toThrow(authError); + }); + + expect(result.current.saveError).toBeNull(); + }); + + it("403 errors are rethrown without setting saveError", async () => { + const fetchFn = vi.fn().mockResolvedValue("ok"); + const forbiddenError = new ApiError(403, "Forbidden"); + const saveFn = vi.fn().mockRejectedValue(forbiddenError); + + const { result } = renderHook(() => useConfigItem({ fetchFn, saveFn })); + + await act(async () => { + await Promise.resolve(); + }); + + await act(async () => { + await expect(result.current.save("test")).rejects.toThrow(forbiddenError); + }); + + expect(result.current.saveError).toBeNull(); + }); }); diff --git a/frontend/src/hooks/useConfigItem.ts b/frontend/src/hooks/useConfigItem.ts index 1916d50..a292d52 100644 --- a/frontend/src/hooks/useConfigItem.ts +++ b/frontend/src/hooks/useConfigItem.ts @@ -3,6 +3,7 @@ */ import { useCallback, useEffect, useRef, useState } from "react"; import { handleFetchError } from "../utils/fetchError"; +import { isAuthError } from "../api/client"; export interface UseConfigItemResult { data: T | null; @@ -71,6 +72,7 @@ export function useConfigItem( setData((prevData) => mergeOnSave(prevData, update)); } } catch (err: unknown) { + if (isAuthError(err)) throw err; // let auth handler deal with it const message = err instanceof Error ? err.message : "Failed to save data"; setSaveError(message); throw err;