diff --git a/web/src/routes/HelpPage.tsx b/web/src/routes/HelpPage.tsx index 15fd70035..122914298 100644 --- a/web/src/routes/HelpPage.tsx +++ b/web/src/routes/HelpPage.tsx @@ -165,9 +165,10 @@ export function HelpPage() {
  • Drill-downs work the same way. Open a flock's{" "} birds ledger to record a cull, an inventory item to - record a purchase, feed usage, or a stock correction, or an order to{" "} - record a payment — the ledger stays where it is and - the form comes to you. + record a purchase, feed usage, or a stock correction, an order to{" "} + record a payment, or a worker's{" "} + flocks to manage their access — the ledger stays + where it is and the form comes to you.
  • Cancel, Escape, or a click outside closes the popup diff --git a/web/src/routes/UsersPage.test.tsx b/web/src/routes/UsersPage.test.tsx index 464bfac37..8ba9753a2 100644 --- a/web/src/routes/UsersPage.test.tsx +++ b/web/src/routes/UsersPage.test.tsx @@ -195,14 +195,14 @@ describe("UsersPage flock scoping", () => { }); expect(mockListAssignments).toHaveBeenCalledWith("u-w"); - expect(await screen.findByText("Assigned flocks")).toBeInTheDocument(); + // F133: the per-worker panel now opens in the shared dialog, titled with the + // worker's email (its accessible name). + const panel = await screen.findByRole("dialog", { name: /Flock access — worker@farm.test/ }); // The assignment's flock id resolves to a name via the loaded flocks list. // Scope to the
  • so it doesn't collide with the dropdown's "Coop A" option. - const item = screen.getByRole("listitem"); + const item = within(panel).getByRole("listitem"); expect(within(item).getByText("Coop A")).toBeInTheDocument(); expect(within(item).getByRole("button", { name: "remove" })).toBeInTheDocument(); - // Toggle flips to "hide flocks". - expect(within(workerRow).getByRole("button", { name: "hide flocks" })).toBeInTheDocument(); }); it("shows the empty-assignments hint (account-wide access) and lists only ACTIVE flocks to assign", async () => { @@ -294,18 +294,81 @@ describe("UsersPage flock scoping", () => { expect(await screen.findByText(/No assignments — account-wide access/)).toBeInTheDocument(); }); - it("collapses the panel again on 'hide flocks'", async () => { + it("resets the assign dropdown to the first active flock each time the dialog opens", async () => { + mockListAssignments.mockResolvedValue([]); + await renderReady(ADMIN); + + const workerRow = screen.getByRole("row", { name: /worker@farm.test/ }); + // Open, switch the pick off the default (fl1 -> fl2), then close. + await act(async () => { + fireEvent.click(within(workerRow).getByRole("button", { name: "flocks" })); + }); + await screen.findByRole("dialog", { name: /Flock access/ }); + fireEvent.change(flockSelect(), { target: { value: "fl2" } }); + expect(flockSelect()).toHaveValue("fl2"); + fireEvent.click(screen.getByRole("button", { name: "Done" })); + + // Reopen — the dropdown is back on the first active flock, not the stale fl2, + // so a distracted admin can't assign the previous worker's pick by accident. + await act(async () => { + fireEvent.click(within(workerRow).getByRole("button", { name: "flocks" })); + }); + await screen.findByRole("dialog", { name: /Flock access/ }); + expect(flockSelect()).toHaveValue("fl1"); + }); + + it("closes the flock dialog on Done", async () => { mockListAssignments.mockResolvedValue([ASSIGN_1]); await renderReady(ADMIN); - const workerRow = () => screen.getByRole("row", { name: /worker@farm.test/ }); + const workerRow = screen.getByRole("row", { name: /worker@farm.test/ }); + await act(async () => { + fireEvent.click(within(workerRow).getByRole("button", { name: "flocks" })); + }); + const panel = await screen.findByRole("dialog", { name: /Flock access/ }); + + fireEvent.click(within(panel).getByRole("button", { name: "Done" })); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + it("discards a stale refresh from a worker whose dialog was closed and reopened for another", async () => { + const WORKER_2: User = { id: "u-w2", email: "worker2@farm.test", displayName: "Walt", role: "Worker" }; + mockListUsers.mockResolvedValue([WORKER_USER, WORKER_2, ADMIN_USER]); + // Call 1: open A (has Coop A). Call 2: A's post-remove refresh — hung. + // Call 3: open B (empty). If the guard fails, A's refresh overwrites B. + let resolveStale!: (v: FlockAssignment[]) => void; + mockListAssignments + .mockResolvedValueOnce([ASSIGN_1]) + .mockImplementationOnce(() => new Promise((r) => { resolveStale = r; })) + .mockResolvedValueOnce([]); + mockUnassignFlock.mockResolvedValue(undefined); + + renderWithProviders(, { token: ADMIN }); + await screen.findByText("worker@farm.test"); + + // Open worker A, remove its assignment — the refresh now hangs (busy). + const rowA = screen.getByRole("row", { name: /worker@farm\.test/ }); + await act(async () => { + fireEvent.click(within(rowA).getByRole("button", { name: "flocks" })); + }); + const panelA = await screen.findByRole("dialog", { name: /Flock access — worker@farm\.test/ }); + await act(async () => { + fireEvent.click(within(panelA).getByRole("button", { name: "remove" })); + }); + + // Close A and open worker B while A's refresh is still in flight. + fireEvent.click(within(panelA).getByRole("button", { name: "Done" })); + const rowB = screen.getByRole("row", { name: /worker2@farm\.test/ }); await act(async () => { - fireEvent.click(within(workerRow()).getByRole("button", { name: "flocks" })); + fireEvent.click(within(rowB).getByRole("button", { name: "flocks" })); }); - expect(await screen.findByText("Assigned flocks")).toBeInTheDocument(); + const panelB = await screen.findByRole("dialog", { name: /Flock access — worker2@farm\.test/ }); + expect(within(panelB).getByText(/No assignments — account-wide access/)).toBeInTheDocument(); - fireEvent.click(within(workerRow()).getByRole("button", { name: "hide flocks" })); - expect(screen.queryByText("Assigned flocks")).not.toBeInTheDocument(); + // A's refresh finally resolves — it must NOT splice A's list into B's dialog. + await act(async () => { resolveStale([ASSIGN_1]); }); + expect(within(panelB).queryByRole("listitem")).toBeNull(); + expect(within(panelB).getByText(/No assignments — account-wide access/)).toBeInTheDocument(); }); it("surfaces an error when an assign fails, keeping the panel open", async () => { @@ -323,7 +386,7 @@ describe("UsersPage flock scoping", () => { }); expect(await screen.findByText(/already assigned|Conflict/)).toBeInTheDocument(); - expect(screen.getByText("Assigned flocks")).toBeInTheDocument(); // panel stayed open + expect(screen.getByRole("dialog")).toBeInTheDocument(); // dialog stayed open }); }); diff --git a/web/src/routes/UsersPage.tsx b/web/src/routes/UsersPage.tsx index 8b44b0491..5df5fd04c 100644 --- a/web/src/routes/UsersPage.tsx +++ b/web/src/routes/UsersPage.tsx @@ -30,6 +30,12 @@ export function UsersPage() { // #103 flock scoping: expand a worker row to manage assignments. const [openUser, setOpenUser] = useState(null); const [assignments, setAssignments] = useState([]); + // The user the dialog is CURRENTLY for, tracked synchronously (before state + // commits). An assign/remove refresh, or a load, that resolves after the + // dialog was closed and reopened for a different worker must not splice the + // old worker's list or error into the new one (#154 review) — post-await + // writes commit only while this still matches the request's target. + const activeUser = useRef(null); const [flocks, setFlocks] = useState([]); const [assignFlockId, setAssignFlockId] = useState(""); @@ -59,47 +65,66 @@ export function UsersPage() { .catch((err) => setError(errText(err))); }, []); - async function toggleAssignments(userId: string) { - if (openUser === userId) { - setOpenUser(null); - return; - } + // F133: flock scoping is a per-worker action, so it opens in the shared dialog + // like the other per-row surfaces (#131) — the row button opens it, the dialog + // closes it. Load the assignments before opening so the panel is never empty + // mid-flight; a load failure surfaces on the page and the dialog stays shut. + async function openAssignments(userId: string) { + activeUser.current = userId; try { - setAssignments(await listFlockAssignments(userId)); - setOpenUser(userId); + const list = await listFlockAssignments(userId); + if (activeUser.current !== userId) return; // superseded by another open/close + setAssignments(list); + // Start every worker's dialog on the first active flock. Without this the + // dropdown keeps the last worker's pick — open A, choose fl2, close, open + // B, and B shows fl2 — so a distracted admin could assign the wrong flock. + setAssignFlockId(flocks[0]?.id ?? ""); setError(null); + setOpenUser(userId); } catch (err) { + if (activeUser.current !== userId) return; + activeUser.current = null; // load failed → no dialog; surface on the page setError(errText(err)); } } + function closeAssignments() { + activeUser.current = null; + setOpenUser(null); + setError(null); + } + async function onAssign() { - if (!openUser || !assignFlockId || busy) return; + const target = openUser; + if (!target || !assignFlockId || busy) return; setBusy(true); setError(null); - const scope = `assign:${openUser}:${assignFlockId}`; + const scope = `assign:${target}:${assignFlockId}`; try { - await assignFlock(openUser, assignFlockId, keyFor(scope)); - setAssignments(await listFlockAssignments(openUser)); + await assignFlock(target, assignFlockId, keyFor(scope)); + const fresh = await listFlockAssignments(target); clearKey(scope); + if (activeUser.current === target) setAssignments(fresh); } catch (err) { - setError(errText(err)); + if (activeUser.current === target) setError(errText(err)); } finally { setBusy(false); } } async function onUnassign(assignmentId: string) { - if (!openUser || busy) return; + const target = openUser; + if (!target || busy) return; setBusy(true); setError(null); const scope = `unassign:${assignmentId}`; try { - await unassignFlock(openUser, assignmentId, keyFor(scope)); - setAssignments(await listFlockAssignments(openUser)); + await unassignFlock(target, assignmentId, keyFor(scope)); + const fresh = await listFlockAssignments(target); clearKey(scope); + if (activeUser.current === target) setAssignments(fresh); } catch (err) { - setError(errText(err)); + if (activeUser.current === target) setError(errText(err)); } finally { setBusy(false); } @@ -178,8 +203,8 @@ export function UsersPage() { - {/* The dialog carries its own copy while it is up. */} - {error && !creating &&

    {error}

    } + {/* Each dialog carries its own error copy while it is up. */} + {error && !creating && openUser === null &&

    {error}

    } {message &&

    {message}

    } @@ -194,8 +219,8 @@ export function UsersPage() { @@ -204,37 +229,42 @@ export function UsersPage() {
    {u.role} {u.role === "Worker" && ( - )}
    - {openUser !== null && ( - <> -

    Assigned flocks

    -

    - No assignments = the worker can record for any flock. The first - assignment narrows them to the listed flocks only. -

    - {assignments.length === 0 ? ( -

    No assignments — account-wide access.

    - ) : ( -
      - {assignments.map((a) => ( -
    • - {flockName(a.flockId)}{" "} - -
    • - ))} -
    - )} -
    - - -
    - - )} + u.id === openUser)?.email ?? ""}`} + onClose={closeAssignments} + > +

    + No assignments = the worker can record for any flock. The first + assignment narrows them to the listed flocks only. +

    + {assignments.length === 0 ? ( +

    No assignments — account-wide access.

    + ) : ( +
      + {assignments.map((a) => ( +
    • + {flockName(a.flockId)}{" "} + +
    • + ))} +
    + )} +
    + + +
    + {error &&

    {error}

    } +
    + +
    +
    ); }