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() {
{u.role}
{u.role === "Worker" && (
-
@@ -204,37 +229,42 @@ export function UsersPage() {
- {openUser !== null && (
- <>
-
Assigned flocks
-
- No assignments = the worker can record for any flock. The first
- assignment narrows them to the listed flocks only.
-