From 0ac8c4473e7029f3236e14bb7df6c5a8fba1f784 Mon Sep 17 00:00:00 2001 From: Codex Review Integration Date: Wed, 16 Sep 2026 23:14:51 -0300 Subject: [PATCH] fix(team-members): harden team member flows (SH-325) --- e2e/work-orders/work-orders.visual.spec.ts | 18 +++++++-- .../_components/add-team-member-dialog.tsx | 39 ++++++++++++------- .../_components/team-members-table.tsx | 11 ++++++ src/app/(protected)/team-members/index.tsx | 8 +++- .../add-team-member-dialog.test.tsx | 39 +++++++++++++++++++ 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/e2e/work-orders/work-orders.visual.spec.ts b/e2e/work-orders/work-orders.visual.spec.ts index 547723a6..17f8d49b 100644 --- a/e2e/work-orders/work-orders.visual.spec.ts +++ b/e2e/work-orders/work-orders.visual.spec.ts @@ -204,6 +204,12 @@ async function openWorkOrderPage(page: Page, mode: "default" | "empty" | "error" }); } +async function expectWorkOrderPageReady(page: Page) { + await expect(page.getByRole("heading", { name: "Work Orders" })).toBeVisible({ + timeout: 90_000, + }); +} + async function expectStableScreenshot(page: Page, name: string) { await page.waitForTimeout(250); await page.evaluate( @@ -217,15 +223,18 @@ async function expectStableScreenshot(page: Page, name: string) { } test.describe("Work Orders deterministic pixel regression", () => { + test.setTimeout(120_000); + test("list", async ({ page }) => { await openWorkOrderPage(page); - await expect(page.getByRole("heading", { name: "Work Orders" })).toBeVisible(); + await expectWorkOrderPageReady(page); await expect(page.getByText("WO-501").first()).toBeVisible(); await expectStableScreenshot(page, "wo-list.png"); }); test("filters", async ({ page }) => { await openWorkOrderPage(page); + await expectWorkOrderPageReady(page); await page.getByRole("button", { name: "Advanced Filters" }).click(); await expect(page.getByRole("dialog", { name: "Advanced Filters" })).toBeVisible(); await expectStableScreenshot(page, "wo-filters.png"); @@ -233,6 +242,7 @@ test.describe("Work Orders deterministic pixel regression", () => { test("new", async ({ page }) => { await openWorkOrderPage(page); + await expectWorkOrderPageReady(page); await page.getByRole("button", { name: "New WO" }).click(); await expect(page.getByRole("heading", { name: "Type & schedule" })).toBeVisible(); await expectStableScreenshot(page, "wo-new.png"); @@ -240,6 +250,7 @@ test.describe("Work Orders deterministic pixel regression", () => { test("detail", async ({ page }) => { await openWorkOrderPage(page); + await expectWorkOrderPageReady(page); const row = page.locator("#wo-row-1"); await row.hover(); await row.getByRole("button", { name: "View details" }).click(); @@ -249,7 +260,7 @@ test.describe("Work Orders deterministic pixel regression", () => { test("empty", async ({ page }) => { await openWorkOrderPage(page, "empty"); - await expect(page.getByRole("heading", { name: "Work Orders" })).toBeVisible(); + await expectWorkOrderPageReady(page); await page.getByLabel("Search work orders").fill("zz"); await expect(page.getByText("No work orders match your search")).toBeVisible(); await expectStableScreenshot(page, "wo-empty.png"); @@ -257,7 +268,7 @@ test.describe("Work Orders deterministic pixel regression", () => { test("error", async ({ page }) => { await openWorkOrderPage(page, "error"); - await expect(page.getByRole("heading", { name: "Work Orders" })).toBeVisible(); + await expectWorkOrderPageReady(page); const alert = page.getByRole("main").getByRole("alert"); await expect(alert).toBeVisible(); await expect(alert).toContainText(/server error/i); @@ -267,6 +278,7 @@ test.describe("Work Orders deterministic pixel regression", () => { test("mobile", async ({ page }) => { await page.setViewportSize({ width: 390, height: 844 }); await openWorkOrderPage(page); + await expectWorkOrderPageReady(page); await expect(page.getByText("WO-501").first()).toBeVisible(); await expectStableScreenshot(page, "wo-mobile.png"); diff --git a/src/app/(protected)/team-members/_components/add-team-member-dialog.tsx b/src/app/(protected)/team-members/_components/add-team-member-dialog.tsx index 7180f3b4..8ee4ac73 100644 --- a/src/app/(protected)/team-members/_components/add-team-member-dialog.tsx +++ b/src/app/(protected)/team-members/_components/add-team-member-dialog.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState, type Dispatch, type SetStateAction } from "react"; +import { useEffect, useMemo, useRef, useState, type Dispatch, type SetStateAction } from "react"; import { Accordion, AccordionDetails, @@ -357,15 +357,25 @@ function TeamMemberFormFields({ export function AddTeamMemberDialog({ open, onClose }: { open: boolean; onClose: () => void }) { const [form, setForm] = useState(emptyForm); const [submitted, setSubmitted] = useState(false); - const createTeamMember = useCreateTeamMember(); - const mutationError = createTeamMember.error; + const submissionId = useRef(0); + const { error: mutationError, isPending, mutate, reset } = useCreateTeamMember(); useEffect(() => { + submissionId.current += 1; + reset(); if (open) { setForm(emptyForm()); setSubmitted(false); } - }, [open]); + }, [open, reset]); + + const handleClose = () => { + if (isPending) return; + + submissionId.current += 1; + reset(); + onClose(); + }; const emailIsValid = /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(form.email.trim()); const errors = useMemo( @@ -419,11 +429,18 @@ export function AddTeamMemberDialog({ open, onClose }: { open: boolean; onClose: serviceAreas: form.serviceAreas, permissionOverrides: permissionOverrides(form.role, form.permissions), }; - createTeamMember.mutate(input, { onSuccess: onClose }); + const currentSubmissionId = submissionId.current; + mutate(input, { + onSuccess: () => { + if (submissionId.current !== currentSubmissionId) return; + reset(); + onClose(); + }, + }); }; return ( - + Add Member @@ -444,15 +461,11 @@ export function AddTeamMemberDialog({ open, onClose }: { open: boolean; onClose: /> - - diff --git a/src/app/(protected)/team-members/_components/team-members-table.tsx b/src/app/(protected)/team-members/_components/team-members-table.tsx index b608b9de..777aeea4 100644 --- a/src/app/(protected)/team-members/_components/team-members-table.tsx +++ b/src/app/(protected)/team-members/_components/team-members-table.tsx @@ -22,11 +22,13 @@ function memberStatusLabel(member: TeamMemberListItem) { export function TeamMembersTable({ rows, isLoading, + hasError, tab, onOpen, }: { rows: TeamMemberListItem[]; isLoading: boolean; + hasError: boolean; tab: "active" | "inactive"; onOpen: (member: TeamMemberListItem) => void; }) { @@ -52,6 +54,15 @@ export function TeamMembersTable({ + ) : hasError ? ( + + + Unable to load team members + + Check your connection and try again. + + + ) : rows.length === 0 ? ( diff --git a/src/app/(protected)/team-members/index.tsx b/src/app/(protected)/team-members/index.tsx index ee8a6467..61990aa7 100644 --- a/src/app/(protected)/team-members/index.tsx +++ b/src/app/(protected)/team-members/index.tsx @@ -149,7 +149,13 @@ export default function TeamMembersListPage() { )} - + ({ describe("AddTeamMemberDialog", () => { const mutate = vi.fn(); + const reset = vi.fn(); beforeEach(() => { mutate.mockReset(); + reset.mockReset(); vi.mocked(useCreateTeamMember).mockReturnValue({ mutate, + reset, isPending: false, error: null, } as unknown as ReturnType); @@ -62,4 +65,40 @@ describe("AddTeamMemberDialog", () => { expect.any(Object), ); }, 15000); + + it("does not close while a member is being created", async () => { + const user = userEvent.setup(); + const onClose = vi.fn(); + vi.mocked(useCreateTeamMember).mockReturnValue({ + mutate, + reset, + isPending: true, + error: null, + } as unknown as ReturnType); + renderWithProviders(, { withAuth: false }); + + await user.keyboard("{Escape}"); + + expect(onClose).not.toHaveBeenCalled(); + }); + + it("clears a previous mutation error when reopened", () => { + const mutation = { + mutate, + reset, + isPending: false, + error: new Error("Email is already in use."), + } as unknown as ReturnType; + vi.mocked(useCreateTeamMember).mockReturnValue(mutation); + const { rerender } = renderWithProviders(, { + withAuth: false, + }); + + expect(screen.getByText("Email is already in use.")).toBeInTheDocument(); + mutation.error = null; + rerender(); + rerender(); + + expect(screen.queryByText("Email is already in use.")).not.toBeInTheDocument(); + }); });