From 715a794201706b3e20d55c1c0b703cd741932e1b Mon Sep 17 00:00:00 2001 From: Jerome Date: Tue, 7 Apr 2026 16:09:42 +0200 Subject: [PATCH 1/3] feat(customers): add customer list, create/edit form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements Task 11 — customer management feature for customer managers. Adds useCustomers/useCreateCustomer/useUpdateCustomer hooks, CustomerForm with zod validation and error display, CustomerList with inline edit, and wires /customers route. Also adds docs/WORKFLOW.md for the one-task-per-branch workflow. Co-Authored-By: Claude Sonnet 4.6 --- docs/WORKFLOW.md | 37 ++++++ .../customers/components/CustomerForm.tsx | 104 ++++++++++++++++ .../customers/components/CustomerList.tsx | 111 ++++++++++++++++++ src/features/customers/hooks/useCustomers.ts | 72 ++++++++++++ src/features/customers/types.ts | 8 ++ src/routes/index.tsx | 3 +- 6 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 docs/WORKFLOW.md create mode 100644 src/features/customers/components/CustomerForm.tsx create mode 100644 src/features/customers/components/CustomerList.tsx create mode 100644 src/features/customers/hooks/useCustomers.ts create mode 100644 src/features/customers/types.ts diff --git a/docs/WORKFLOW.md b/docs/WORKFLOW.md new file mode 100644 index 0000000..ebc3f7c --- /dev/null +++ b/docs/WORKFLOW.md @@ -0,0 +1,37 @@ +# Development Workflow + +## One Task at a Time + +Each plan task is implemented on its own branch, then submitted as a PR for review. + +### Steps + +1. **Create a branch** from the current base (last merged or last feature branch): + ```bash + git checkout -b feat/taskN- + ``` + +2. **Implement the task** following the plan steps exactly. + +3. **Verify** — run lint and build before committing: + ```bash + npm run lint + npm run build + ``` + +4. **Commit** with a descriptive message, then **open a PR** for agent review: + ```bash + git push -u origin feat/taskN- + gh pr create --title "..." --body "..." + ``` + +5. **After PR is reviewed and merged**, start the next task from the updated base branch. + +### Branch Naming + +`feat/taskN-` — e.g., `feat/task11-customers` + +### PR Review + +PRs should be reviewed by an agent (code-reviewer) before merging. +The PR body should summarize what was implemented and include a test plan. diff --git a/src/features/customers/components/CustomerForm.tsx b/src/features/customers/components/CustomerForm.tsx new file mode 100644 index 0000000..b8be693 --- /dev/null +++ b/src/features/customers/components/CustomerForm.tsx @@ -0,0 +1,104 @@ +import { useForm } from "react-hook-form"; +import { zodResolver } from "@hookform/resolvers/zod"; +import { z } from "zod"; +import { Button } from "@/components/ui/button"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { useCreateCustomer, useUpdateCustomer } from "../hooks/useCustomers"; +import type { Customer } from "@/lib/types"; + +const customerSchema = z.object({ + name: z.string().min(1, "Name is required"), + email: z.string().email("Please enter a valid email"), +}); + +type CustomerValues = z.infer; + +type CustomerFormProps = { + companyId: string; + customer?: Customer; + onClose: () => void; +}; + +export const CustomerForm = ({ + companyId, + customer, + onClose, +}: CustomerFormProps) => { + const createCustomer = useCreateCustomer(); + const updateCustomer = useUpdateCustomer(); + const isEditing = !!customer; + + const { + register, + handleSubmit, + formState: { errors }, + } = useForm({ + resolver: zodResolver(customerSchema), + defaultValues: customer + ? { name: customer.name, email: customer.email } + : undefined, + }); + + const onSubmit = async (values: CustomerValues) => { + try { + if (isEditing) { + await updateCustomer.mutateAsync({ + id: customer.id, + name: values.name, + email: values.email, + }); + } else { + await createCustomer.mutateAsync({ + name: values.name, + email: values.email, + companyId, + }); + } + onClose(); + } catch { + // error displayed via mutation.error below + } + }; + + const isPending = createCustomer.isPending || updateCustomer.isPending; + const mutationError = createCustomer.error ?? updateCustomer.error; + + return ( + + + {isEditing ? "Edit Customer" : "New Customer"} + + +
+
+ + + {errors.name && ( +

{errors.name.message}

+ )} +
+
+ + + {errors.email && ( +

{errors.email.message}

+ )} +
+ {mutationError && ( +

{mutationError.message}

+ )} +
+ + +
+
+ + + ); +}; diff --git a/src/features/customers/components/CustomerList.tsx b/src/features/customers/components/CustomerList.tsx new file mode 100644 index 0000000..57dc06c --- /dev/null +++ b/src/features/customers/components/CustomerList.tsx @@ -0,0 +1,111 @@ +import { useState } from "react"; +import { Button } from "@/components/ui/button"; +import { + Table, + TableBody, + TableCell, + TableHead, + TableHeader, + TableRow, +} from "@/components/ui/table"; +import { useCustomers } from "../hooks/useCustomers"; +import { useAuthContext } from "@/features/auth/AuthProvider"; +import { supabase } from "@/lib/supabase"; +import { useQuery } from "@tanstack/react-query"; +import { CustomerForm } from "./CustomerForm"; +import type { Customer } from "@/lib/types"; + +export const CustomerList = () => { + const { data: customers, isLoading, isError, error } = useCustomers(); + const { user } = useAuthContext(); + const [showCreate, setShowCreate] = useState(false); + const [editingCustomer, setEditingCustomer] = useState(null); + + // Get the CM's company ID + const { data: userCompany } = useQuery({ + queryKey: ["user-company", user?.id], + queryFn: async () => { + if (!user) throw new Error("Not authenticated"); + const { data, error } = await supabase + .from("user_companies") + .select("company_id") + .eq("user_id", user.id) + .single(); + if (error) throw error; + return data; + }, + enabled: !!user, + }); + + if (isLoading) { + return

Loading customers...

; + } + + if (isError) { + return

{error.message}

; + } + + const companyId = userCompany?.company_id; + + return ( +
+
+

Customers

+ +
+ + {showCreate && companyId && ( + setShowCreate(false)} + /> + )} + + {editingCustomer && companyId && ( + setEditingCustomer(null)} + /> + )} + + + + + Name + Email + Created + + + + + {customers?.map((customer) => ( + + {customer.name} + {customer.email} + + {new Date(customer.created_at).toLocaleDateString()} + + + + + + ))} + {customers?.length === 0 && ( + + + No customers yet + + + )} + +
+
+ ); +}; diff --git a/src/features/customers/hooks/useCustomers.ts b/src/features/customers/hooks/useCustomers.ts new file mode 100644 index 0000000..b364c1b --- /dev/null +++ b/src/features/customers/hooks/useCustomers.ts @@ -0,0 +1,72 @@ +import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; +import { supabase } from "@/lib/supabase"; +import type { Customer } from "@/lib/types"; + +export const useCustomers = () => { + return useQuery({ + queryKey: ["customers"], + queryFn: async () => { + const { data, error } = await supabase + .from("customers") + .select("*") + .order("name"); + if (error) throw error; + return data; + }, + }); +}; + +export const useCreateCustomer = () => { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: async ({ + name, + email, + companyId, + }: { + name: string; + email: string; + companyId: string; + }) => { + const { data, error } = await supabase + .from("customers") + .insert({ name, email, company_id: companyId }) + .select() + .single(); + if (error) throw error; + return data; + }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["customers"] }); + }, + }); +}; + +export const useUpdateCustomer = () => { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: async ({ + id, + name, + email, + }: { + id: string; + name: string; + email: string; + }) => { + const { data, error } = await supabase + .from("customers") + .update({ name, email }) + .eq("id", id) + .select() + .single(); + if (error) throw error; + return data; + }, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["customers"] }); + }, + }); +}; diff --git a/src/features/customers/types.ts b/src/features/customers/types.ts new file mode 100644 index 0000000..3977d7b --- /dev/null +++ b/src/features/customers/types.ts @@ -0,0 +1,8 @@ +import type { Customer } from "@/lib/types"; + +export type CustomerFormValues = { + name: string; + email: string; +}; + +export type { Customer }; diff --git a/src/routes/index.tsx b/src/routes/index.tsx index dc55be5..2d4a8e4 100644 --- a/src/routes/index.tsx +++ b/src/routes/index.tsx @@ -8,6 +8,7 @@ import { SignupForm } from "@/features/auth/components/SignupForm"; import { CompanyList } from "@/features/companies/components/CompanyList"; import { CompanyDetail } from "@/features/companies/components/CompanyDetail"; import { AgentList } from "@/features/agents/components/AgentList"; +import { CustomerList } from "@/features/customers/components/CustomerList"; export const router = createBrowserRouter([ { @@ -51,7 +52,7 @@ export const router = createBrowserRouter([ children: [ { path: "/customers", - element:
Customers — coming soon
, + element: , }, ], }, From f42d00b91d50918cfb4b5e46e5dc341c5f4fd1d8 Mon Sep 17 00:00:00 2001 From: Jerome Date: Tue, 7 Apr 2026 16:38:05 +0200 Subject: [PATCH 2/3] fix: address all code review issues + add tests + fix invite signup bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review fixes: - feat(customers): disable New Customer button until companyId resolves - feat(customers): use TablesInsert/TablesUpdate in useCustomers mutations - feat(customers): remove unused CustomerFormValues from types.ts - feat(customers): add noValidate to CustomerForm (prevents browser constraint validation from blocking zodResolver in tests) RLS fixes: - fix(db): add WITH CHECK to customers_update_cm policy (migration 00010) Signup bug fix: - fix(db): normalize invite email to lowercase in create_invite (migration 00009) - fix(db): make handle_new_user trigger compare emails case-insensitively Root cause: Supabase Auth normalizes emails to lowercase in auth.users but invites stored the email exactly as typed by the admin (e.g. testCM0@test.com vs testcm0@test.com), causing the trigger lookup to fail every time. Tests: - test(customers): add useCustomers.test.tsx (hooks — fetch, create, update, error) - test(customers): add CustomerForm.test.tsx (validation, submit, cancel, edit mode) - test(customers): add CustomerList.test.tsx (loading/error/empty/populated states, button guard, form toggle) - test(customers): add tests/e2e/customers.spec.ts (full CM CRUD flow + access control) Docs: - docs: add required-tests-per-feature table to CLAUDE.md testing section - docs: add userEvent.setup() and noValidate conventions to CLAUDE.md Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 15 ++ .../components/CustomerForm.test.tsx | 142 ++++++++++++++++++ .../customers/components/CustomerForm.tsx | 2 +- .../components/CustomerList.test.tsx | 126 ++++++++++++++++ .../customers/components/CustomerList.tsx | 4 +- .../customers/hooks/useCustomers.test.tsx | 122 +++++++++++++++ src/features/customers/hooks/useCustomers.ts | 11 +- src/features/customers/types.ts | 9 +- .../00009_fix_invite_email_case.sql | 105 +++++++++++++ .../migrations/00010_fix_customers_rls.sql | 15 ++ tests/e2e/customers.spec.ts | 88 +++++++++++ 11 files changed, 627 insertions(+), 12 deletions(-) create mode 100644 src/features/customers/components/CustomerForm.test.tsx create mode 100644 src/features/customers/components/CustomerList.test.tsx create mode 100644 src/features/customers/hooks/useCustomers.test.tsx create mode 100644 supabase/migrations/00009_fix_invite_email_case.sql create mode 100644 supabase/migrations/00010_fix_customers_rls.sql create mode 100644 tests/e2e/customers.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 4c3b6fc..060674d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -145,6 +145,21 @@ tests/ - **Test wrapper**: `src/test-utils.tsx` provides `renderWithProviders` (QueryClient + MemoryRouter) and `createTestQueryClient`. - **Mock `AuthProvider`** in component tests by mocking `"../AuthProvider"` with a `useAuthContext` that returns controlled values — avoids needing real Supabase auth. - **Prefer testing RPC call shapes** over mocking Supabase query chains — RPC mocks are simpler (`vi.fn()` on `supabase.rpc`) and verify the contract with the database. +- **Use `userEvent.setup()`** for user interaction tests — call `const user = userEvent.setup()` then `await user.click(...)` / `await user.type(...)`. The legacy direct API (`userEvent.click(...)`) can cause timing issues. +- **Add `noValidate` to forms** — prevents browser constraint validation from blocking react-hook-form's schema validation in tests and in the app. Always add `noValidate` to any `
` that uses zodResolver. + +### Required tests per feature + +Every feature module must ship with: + +| Test file | What to cover | +|-----------|---------------| +| `hooks/useFoo.test.tsx` | Query fetches data; mutation calls Supabase with correct payload (including camelCase→snake_case mapping); error propagation; cache invalidation | +| `components/FooForm.test.tsx` | Render in create vs edit mode; validation errors for empty/invalid fields; `mutateAsync` called with correct args; `onClose` called on success; Cancel button | +| `components/FooList.test.tsx` | Loading / error / empty states; row rendering; form toggle interactions; role guards if applicable | +| `tests/e2e/foo.spec.ts` | Full CRUD flow logged in as the correct role; access control (wrong role cannot access the page) | + +Run `npm test && npm run lint && npm run build` before every commit. ## Supabase Guidelines diff --git a/src/features/customers/components/CustomerForm.test.tsx b/src/features/customers/components/CustomerForm.test.tsx new file mode 100644 index 0000000..cc8f11b --- /dev/null +++ b/src/features/customers/components/CustomerForm.test.tsx @@ -0,0 +1,142 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { renderWithProviders } from "@/test-utils"; +import { CustomerForm } from "./CustomerForm"; +import type { Customer } from "@/lib/types"; + +const mockCreateMutateAsync = vi.fn(); +const mockUpdateMutateAsync = vi.fn(); + +vi.mock("../hooks/useCustomers", () => ({ + useCreateCustomer: () => ({ + mutateAsync: mockCreateMutateAsync, + isPending: false, + error: null, + }), + useUpdateCustomer: () => ({ + mutateAsync: mockUpdateMutateAsync, + isPending: false, + error: null, + }), +})); + +const mockOnClose = vi.fn(); +const COMPANY_ID = "co-1"; + +const EXISTING_CUSTOMER: Customer = { + id: "cust-1", + name: "Alice Smith", + email: "alice@acme.com", + company_id: COMPANY_ID, + created_at: "2026-01-01T00:00:00Z", +}; + +describe("CustomerForm — create mode", () => { + beforeEach(() => vi.clearAllMocks()); + + it("renders New Customer title with empty fields", () => { + renderWithProviders( + , + ); + expect(screen.getByText("New Customer")).toBeTruthy(); + expect((screen.getByLabelText("Name") as HTMLInputElement).value).toBe(""); + expect((screen.getByLabelText("Email") as HTMLInputElement).value).toBe(""); + expect(screen.getByRole("button", { name: "Create" })).toBeTruthy(); + }); + + it("shows validation error for empty name", async () => { + const user = userEvent.setup(); + renderWithProviders( + , + ); + await user.click(screen.getByRole("button", { name: "Create" })); + await waitFor(() => expect(screen.getByText("Name is required")).toBeTruthy()); + expect(mockCreateMutateAsync).not.toHaveBeenCalled(); + }); + + it("shows validation error for invalid email", async () => { + const user = userEvent.setup(); + renderWithProviders( + , + ); + await user.type(screen.getByLabelText("Name"), "Bob"); + await user.type(screen.getByLabelText("Email"), "not-an-email"); + await user.click(screen.getByRole("button", { name: "Create" })); + await waitFor(() => + expect(screen.getByText("Please enter a valid email")).toBeTruthy(), + ); + expect(mockCreateMutateAsync).not.toHaveBeenCalled(); + }); + + it("calls createCustomer.mutateAsync with correct args and then onClose", async () => { + const user = userEvent.setup(); + mockCreateMutateAsync.mockResolvedValue({}); + renderWithProviders( + , + ); + await user.type(screen.getByLabelText("Name"), "Bob Jones"); + await user.type(screen.getByLabelText("Email"), "bob@acme.com"); + await user.click(screen.getByRole("button", { name: "Create" })); + await waitFor(() => + expect(mockCreateMutateAsync).toHaveBeenCalledWith({ + name: "Bob Jones", + email: "bob@acme.com", + companyId: COMPANY_ID, + }), + ); + expect(mockOnClose).toHaveBeenCalled(); + }); + + it("calls onClose when Cancel is clicked without submitting", async () => { + const user = userEvent.setup(); + renderWithProviders( + , + ); + await user.click(screen.getByRole("button", { name: "Cancel" })); + expect(mockOnClose).toHaveBeenCalled(); + expect(mockCreateMutateAsync).not.toHaveBeenCalled(); + }); +}); + +describe("CustomerForm — edit mode", () => { + beforeEach(() => vi.clearAllMocks()); + + it("renders Edit Customer title with pre-filled fields", () => { + renderWithProviders( + , + ); + expect(screen.getByText("Edit Customer")).toBeTruthy(); + expect((screen.getByLabelText("Name") as HTMLInputElement).value).toBe("Alice Smith"); + expect((screen.getByLabelText("Email") as HTMLInputElement).value).toBe("alice@acme.com"); + expect(screen.getByRole("button", { name: "Save" })).toBeTruthy(); + }); + + it("calls updateCustomer.mutateAsync with id, name, email — no companyId", async () => { + const user = userEvent.setup(); + mockUpdateMutateAsync.mockResolvedValue({}); + renderWithProviders( + , + ); + const nameField = screen.getByLabelText("Name"); + await user.clear(nameField); + await user.type(nameField, "Alice Updated"); + await user.click(screen.getByRole("button", { name: "Save" })); + await waitFor(() => + expect(mockUpdateMutateAsync).toHaveBeenCalledWith({ + id: "cust-1", + name: "Alice Updated", + email: "alice@acme.com", + }), + ); + expect(mockOnClose).toHaveBeenCalled(); + }); +}); diff --git a/src/features/customers/components/CustomerForm.tsx b/src/features/customers/components/CustomerForm.tsx index b8be693..81859cd 100644 --- a/src/features/customers/components/CustomerForm.tsx +++ b/src/features/customers/components/CustomerForm.tsx @@ -71,7 +71,7 @@ export const CustomerForm = ({ {isEditing ? "Edit Customer" : "New Customer"} - +
diff --git a/src/features/customers/components/CustomerList.test.tsx b/src/features/customers/components/CustomerList.test.tsx new file mode 100644 index 0000000..6e7eeac --- /dev/null +++ b/src/features/customers/components/CustomerList.test.tsx @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { renderWithProviders } from "@/test-utils"; +import { CustomerList } from "./CustomerList"; +import type { Customer } from "@/lib/types"; + +// Mock supabase for the inline user_companies query +const mockFrom = vi.fn(); +vi.mock("@/lib/supabase", () => ({ + supabase: { from: (...args: unknown[]) => mockFrom(...args) }, +})); + +// Mock AuthProvider +vi.mock("@/features/auth/AuthProvider", () => ({ + useAuthContext: () => ({ user: { id: "user-1" } }), +})); + +// Mock useCustomers hook +const mockUseCustomers = vi.fn(); +vi.mock("../hooks/useCustomers", () => ({ + useCustomers: () => mockUseCustomers(), + useCreateCustomer: () => ({ mutateAsync: vi.fn(), isPending: false, error: null }), + useUpdateCustomer: () => ({ mutateAsync: vi.fn(), isPending: false, error: null }), +})); + +const CUSTOMERS: Customer[] = [ + { id: "c1", name: "Alice Smith", email: "alice@acme.com", company_id: "co1", created_at: "2026-01-01T00:00:00Z" }, + { id: "c2", name: "Bob Jones", email: "bob@acme.com", company_id: "co1", created_at: "2026-02-01T00:00:00Z" }, +]; + +const makeUserCompanyChain = (companyId: string | null) => { + const chain: Record = {}; + chain.select = vi.fn().mockReturnValue(chain); + chain.eq = vi.fn().mockReturnValue(chain); + chain.single = vi.fn().mockResolvedValue({ + data: companyId ? { company_id: companyId } : null, + error: null, + }); + return chain; +}; + +describe("CustomerList", () => { + beforeEach(() => vi.clearAllMocks()); + + it("shows loading state", () => { + mockUseCustomers.mockReturnValue({ isLoading: true, isError: false, data: undefined, error: null }); + mockFrom.mockReturnValue(makeUserCompanyChain("co1")); + + renderWithProviders(); + expect(screen.getByText("Loading customers...")).toBeTruthy(); + }); + + it("shows error state", () => { + mockUseCustomers.mockReturnValue({ + isLoading: false, + isError: true, + data: undefined, + error: { message: "Permission denied" }, + }); + mockFrom.mockReturnValue(makeUserCompanyChain("co1")); + + renderWithProviders(); + expect(screen.getByText("Permission denied")).toBeTruthy(); + }); + + it("shows empty state", async () => { + mockUseCustomers.mockReturnValue({ isLoading: false, isError: false, data: [], error: null }); + mockFrom.mockReturnValue(makeUserCompanyChain("co1")); + + renderWithProviders(); + await waitFor(() => expect(screen.getByText("No customers yet")).toBeTruthy()); + }); + + it("renders a row per customer with name and email", async () => { + mockUseCustomers.mockReturnValue({ isLoading: false, isError: false, data: CUSTOMERS, error: null }); + mockFrom.mockReturnValue(makeUserCompanyChain("co1")); + + renderWithProviders(); + await waitFor(() => expect(screen.getByText("Alice Smith")).toBeTruthy()); + expect(screen.getByText("alice@acme.com")).toBeTruthy(); + expect(screen.getByText("Bob Jones")).toBeTruthy(); + expect(screen.getByText("bob@acme.com")).toBeTruthy(); + }); + + it("New Customer button is disabled while companyId is loading", async () => { + mockUseCustomers.mockReturnValue({ isLoading: false, isError: false, data: [], error: null }); + // userCompany query returns null (loading / unresolved) + const chain: Record = {}; + chain.select = vi.fn().mockReturnValue(chain); + chain.eq = vi.fn().mockReturnValue(chain); + chain.single = vi.fn().mockReturnValue(new Promise(() => {})); // never resolves + mockFrom.mockReturnValue(chain); + + renderWithProviders(); + await waitFor(() => { + const btn = screen.getByRole("button", { name: "New Customer" }); + expect((btn as HTMLButtonElement).disabled).toBe(true); + }); + }); + + it("New Customer button enabled when companyId resolves, and clicking shows form", async () => { + mockUseCustomers.mockReturnValue({ isLoading: false, isError: false, data: CUSTOMERS, error: null }); + mockFrom.mockReturnValue(makeUserCompanyChain("co1")); + + renderWithProviders(); + const btn = await screen.findByRole("button", { name: "New Customer" }); + await waitFor(() => expect((btn as HTMLButtonElement).disabled).toBe(false)); + await userEvent.click(btn); + expect(screen.getByText("New Customer", { selector: "div" })).toBeTruthy(); + }); + + it("clicking Edit shows pre-filled form for that customer", async () => { + mockUseCustomers.mockReturnValue({ isLoading: false, isError: false, data: CUSTOMERS, error: null }); + mockFrom.mockReturnValue(makeUserCompanyChain("co1")); + + renderWithProviders(); + await waitFor(() => expect(screen.getByText("Alice Smith")).toBeTruthy()); + const user = userEvent.setup(); + const editButtons = screen.getAllByRole("button", { name: "Edit" }); + const firstEdit = editButtons[0]; + if (!firstEdit) throw new Error("No Edit button found"); + await user.click(firstEdit); + expect(screen.getByText("Edit Customer")).toBeTruthy(); + }); +}); diff --git a/src/features/customers/components/CustomerList.tsx b/src/features/customers/components/CustomerList.tsx index 57dc06c..4197fa5 100644 --- a/src/features/customers/components/CustomerList.tsx +++ b/src/features/customers/components/CustomerList.tsx @@ -51,7 +51,9 @@ export const CustomerList = () => {

Customers

- +
{showCreate && companyId && ( diff --git a/src/features/customers/hooks/useCustomers.test.tsx b/src/features/customers/hooks/useCustomers.test.tsx new file mode 100644 index 0000000..f62abb7 --- /dev/null +++ b/src/features/customers/hooks/useCustomers.test.tsx @@ -0,0 +1,122 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { type ReactNode } from "react"; +import { useCustomers, useCreateCustomer, useUpdateCustomer } from "./useCustomers"; + +const mockFrom = vi.fn(); + +vi.mock("@/lib/supabase", () => ({ + supabase: { + from: (...args: unknown[]) => mockFrom(...args), + }, +})); + +const makeChain = (terminal: () => Promise) => { + const chain: Record = {}; + chain.select = vi.fn().mockReturnValue(chain); + chain.insert = vi.fn().mockReturnValue(chain); + chain.update = vi.fn().mockReturnValue(chain); + chain.eq = vi.fn().mockReturnValue(chain); + chain.order = vi.fn().mockImplementation(terminal); + chain.single = vi.fn().mockImplementation(terminal); + return chain; +}; + +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return ({ children }: { children: ReactNode }) => ( + {children} + ); +}; + +describe("useCustomers", () => { + beforeEach(() => vi.clearAllMocks()); + + it("fetches customers ordered by name", async () => { + const customers = [ + { id: "c1", name: "Alice", email: "alice@acme.com", company_id: "co1", created_at: "" }, + ]; + const terminal = vi.fn().mockResolvedValue({ data: customers, error: null }); + mockFrom.mockReturnValue(makeChain(terminal)); + + const { result } = renderHook(() => useCustomers(), { wrapper: createWrapper() }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(result.current.data).toEqual(customers); + }); + + it("propagates query error", async () => { + const terminal = vi.fn().mockResolvedValue({ data: null, error: { message: "Permission denied" } }); + mockFrom.mockReturnValue(makeChain(terminal)); + + const { result } = renderHook(() => useCustomers(), { wrapper: createWrapper() }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + }); +}); + +describe("useCreateCustomer", () => { + beforeEach(() => vi.clearAllMocks()); + + it("inserts with correct payload mapping camelCase to snake_case", async () => { + const created = { id: "c1", name: "Bob", email: "bob@acme.com", company_id: "co1", created_at: "" }; + const terminal = vi.fn().mockResolvedValue({ data: created, error: null }); + const insertChain = makeChain(terminal); + mockFrom.mockReturnValue(insertChain); + + const { result } = renderHook(() => useCreateCustomer(), { wrapper: createWrapper() }); + + result.current.mutate({ name: "Bob", email: "bob@acme.com", companyId: "co1" }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(insertChain.insert).toHaveBeenCalledWith({ + name: "Bob", + email: "bob@acme.com", + company_id: "co1", + }); + }); + + it("propagates mutation error", async () => { + const terminal = vi.fn().mockResolvedValue({ data: null, error: { message: "Unique violation" } }); + mockFrom.mockReturnValue(makeChain(terminal)); + + const { result } = renderHook(() => useCreateCustomer(), { wrapper: createWrapper() }); + + result.current.mutate({ name: "Bob", email: "bob@acme.com", companyId: "co1" }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + }); +}); + +describe("useUpdateCustomer", () => { + beforeEach(() => vi.clearAllMocks()); + + it("updates with name and email only — no company_id in payload", async () => { + const updated = { id: "c1", name: "Alice Updated", email: "a@acme.com", company_id: "co1", created_at: "" }; + const terminal = vi.fn().mockResolvedValue({ data: updated, error: null }); + const updateChain = makeChain(terminal); + mockFrom.mockReturnValue(updateChain); + + const { result } = renderHook(() => useUpdateCustomer(), { wrapper: createWrapper() }); + + result.current.mutate({ id: "c1", name: "Alice Updated", email: "a@acme.com" }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(updateChain.update).toHaveBeenCalledWith({ name: "Alice Updated", email: "a@acme.com" }); + expect(updateChain.eq).toHaveBeenCalledWith("id", "c1"); + }); + + it("propagates mutation error", async () => { + const terminal = vi.fn().mockResolvedValue({ data: null, error: { message: "Not found" } }); + mockFrom.mockReturnValue(makeChain(terminal)); + + const { result } = renderHook(() => useUpdateCustomer(), { wrapper: createWrapper() }); + + result.current.mutate({ id: "c1", name: "Alice", email: "a@acme.com" }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + }); +}); diff --git a/src/features/customers/hooks/useCustomers.ts b/src/features/customers/hooks/useCustomers.ts index b364c1b..ffaae56 100644 --- a/src/features/customers/hooks/useCustomers.ts +++ b/src/features/customers/hooks/useCustomers.ts @@ -1,6 +1,7 @@ import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; import { supabase } from "@/lib/supabase"; import type { Customer } from "@/lib/types"; +import type { TablesInsert, TablesUpdate } from "@/lib/database.types"; export const useCustomers = () => { return useQuery({ @@ -29,9 +30,14 @@ export const useCreateCustomer = () => { email: string; companyId: string; }) => { + const payload: TablesInsert<"customers"> = { + name, + email, + company_id: companyId, + }; const { data, error } = await supabase .from("customers") - .insert({ name, email, company_id: companyId }) + .insert(payload) .select() .single(); if (error) throw error; @@ -56,9 +62,10 @@ export const useUpdateCustomer = () => { name: string; email: string; }) => { + const payload: TablesUpdate<"customers"> = { name, email }; const { data, error } = await supabase .from("customers") - .update({ name, email }) + .update(payload) .eq("id", id) .select() .single(); diff --git a/src/features/customers/types.ts b/src/features/customers/types.ts index 3977d7b..e33c066 100644 --- a/src/features/customers/types.ts +++ b/src/features/customers/types.ts @@ -1,8 +1 @@ -import type { Customer } from "@/lib/types"; - -export type CustomerFormValues = { - name: string; - email: string; -}; - -export type { Customer }; +export type { Customer } from "@/lib/types"; diff --git a/supabase/migrations/00009_fix_invite_email_case.sql b/supabase/migrations/00009_fix_invite_email_case.sql new file mode 100644 index 0000000..03fb6ac --- /dev/null +++ b/supabase/migrations/00009_fix_invite_email_case.sql @@ -0,0 +1,105 @@ +-- Fix: Supabase Auth normalizes emails to lowercase in auth.users, but +-- create_invite stored emails as-is from admin input. The trigger's +-- case-sensitive email comparison would fail for mixed-case invite emails. +-- Fix: normalize email to lowercase at invite creation time, and make the +-- trigger lookup case-insensitive for existing invites. + +-- 1. Update create_invite to normalize email to lowercase +CREATE OR REPLACE FUNCTION public.create_invite( + p_email text, + p_role app_role, + p_company_ids uuid[] DEFAULT '{}'::uuid[] +) +RETURNS TABLE (invite_id uuid, token text) +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = public, extensions +AS $$ +DECLARE + _invite_id uuid; + _token text; + _user_id uuid; +BEGIN + IF public.user_role() <> 'admin' THEN + RAISE EXCEPTION 'Only admins can create invites'; + END IF; + + _user_id := auth.uid(); + _token := encode(gen_random_bytes(32), 'hex'); + + INSERT INTO invites (email, role, token, invited_by, expires_at) + VALUES (lower(p_email), p_role, _token, _user_id, now() + interval '7 days') + RETURNING id INTO _invite_id; + + IF array_length(p_company_ids, 1) IS NOT NULL THEN + INSERT INTO invite_companies (invite_id, company_id) + SELECT _invite_id, unnest(p_company_ids); + END IF; + + RETURN QUERY SELECT _invite_id, _token; +END; +$$; + +GRANT EXECUTE ON FUNCTION public.create_invite(text, app_role, uuid[]) TO authenticated; + +-- 2. Update handle_new_user to compare emails case-insensitively +-- (handles any existing invites that may have mixed-case emails) +CREATE OR REPLACE FUNCTION handle_new_user() +RETURNS trigger +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = public +AS $$ +DECLARE + _profile_count int; + _invite record; + _user_name text; + _invite_token text; +BEGIN + _user_name := COALESCE( + NEW.raw_user_meta_data ->> 'name', + split_part(NEW.email, '@', 1) + ); + + PERFORM pg_advisory_xact_lock(42); + + SELECT count(*) INTO _profile_count FROM profiles; + + IF _profile_count = 0 THEN + INSERT INTO profiles (user_id, name, role) + VALUES (NEW.id, _user_name, 'admin'); + RETURN NEW; + END IF; + + _invite_token := NEW.raw_user_meta_data ->> 'invite_token'; + + IF _invite_token IS NULL THEN + RAISE EXCEPTION 'No invite token provided during signup'; + END IF; + + -- Compare emails case-insensitively to handle mixed-case invite emails + SELECT * INTO _invite + FROM invites + WHERE lower(email) = lower(NEW.email) + AND token = _invite_token + AND used_at IS NULL + AND expires_at > now() + LIMIT 1; + + IF _invite IS NULL THEN + RAISE EXCEPTION 'No valid invite found for % with the provided token', NEW.email; + END IF; + + INSERT INTO profiles (user_id, name, role) + VALUES (NEW.id, _user_name, _invite.role); + + INSERT INTO user_companies (user_id, company_id) + SELECT NEW.id, ic.company_id + FROM invite_companies ic + WHERE ic.invite_id = _invite.id; + + UPDATE invites SET used_at = now() WHERE id = _invite.id; + + RETURN NEW; +END; +$$; diff --git a/supabase/migrations/00010_fix_customers_rls.sql b/supabase/migrations/00010_fix_customers_rls.sql new file mode 100644 index 0000000..cf92c9e --- /dev/null +++ b/supabase/migrations/00010_fix_customers_rls.sql @@ -0,0 +1,15 @@ +-- Fix: customers_update_cm lacked WITH CHECK, allowing a CM to move a +-- customer to a company they don't belong to during an UPDATE. + +DROP POLICY IF EXISTS customers_update_cm ON customers; + +CREATE POLICY customers_update_cm ON customers + FOR UPDATE + USING ( + public.user_role() = 'customer_manager' + AND company_id IN (SELECT public.user_company_ids()) + ) + WITH CHECK ( + public.user_role() = 'customer_manager' + AND company_id IN (SELECT public.user_company_ids()) + ); diff --git a/tests/e2e/customers.spec.ts b/tests/e2e/customers.spec.ts new file mode 100644 index 0000000..197d3cc --- /dev/null +++ b/tests/e2e/customers.spec.ts @@ -0,0 +1,88 @@ +import { test, expect } from "@playwright/test"; + +// All tests log in as the seeded customer manager (Diana Patel, assigned to Acme Corp) +test.describe("Customers — customer manager flow", () => { + test.beforeEach(async ({ page }) => { + await page.goto("/login"); + await page.getByLabel("Email").fill("cm@serverdesk.local"); + await page.getByLabel("Password").fill("password123"); + await page.getByRole("button", { name: /sign in/i }).click(); + await page.waitForURL("**/"); + await expect(page.getByText("cm@serverdesk.local")).toBeVisible({ + timeout: 10000, + }); + }); + + test("customer manager can navigate to customers page", async ({ page }) => { + await page.getByRole("link", { name: /customers/i }).click(); + await page.waitForURL("**/customers"); + await expect(page.getByText("Customers")).toBeVisible(); + }); + + test("customers page lists seeded customers for the CM company", async ({ page }) => { + await page.goto("/customers"); + // Acme Corp has John Smith and Sarah Jones in seed data + await expect(page.getByText("John Smith")).toBeVisible({ timeout: 10000 }); + await expect(page.getByText("Sarah Jones")).toBeVisible(); + }); + + test("customer manager can create a new customer", async ({ page }) => { + await page.goto("/customers"); + // Wait for the New Customer button to be enabled (companyId resolved) + const newBtn = page.getByRole("button", { name: /new customer/i }); + await expect(newBtn).toBeEnabled({ timeout: 10000 }); + await newBtn.click(); + + await page.getByLabel("Name").fill("Test Customer"); + await page.getByLabel("Email").fill("testcustomer@acmecorp.com"); + await page.getByRole("button", { name: /create/i }).click(); + + // After creation, the new customer appears in the list + await expect(page.getByText("Test Customer")).toBeVisible({ timeout: 10000 }); + await expect(page.getByText("testcustomer@acmecorp.com")).toBeVisible(); + }); + + test("customer manager can edit an existing customer", async ({ page }) => { + await page.goto("/customers"); + await expect(page.getByText("John Smith")).toBeVisible({ timeout: 10000 }); + + // Click Edit on the first row + const editButtons = page.getByRole("button", { name: /edit/i }); + await editButtons.first().click(); + + // Form should pre-fill with existing data + await expect(page.getByLabel("Name")).not.toHaveValue(""); + + // Update the name + await page.getByLabel("Name").fill("John Smith Updated"); + await page.getByRole("button", { name: /save/i }).click(); + + // Updated name appears in the list + await expect(page.getByText("John Smith Updated")).toBeVisible({ timeout: 10000 }); + }); + + test("create form shows validation errors for invalid input", async ({ page }) => { + await page.goto("/customers"); + const newBtn = page.getByRole("button", { name: /new customer/i }); + await expect(newBtn).toBeEnabled({ timeout: 10000 }); + await newBtn.click(); + + // Submit without filling anything + await page.getByRole("button", { name: /create/i }).click(); + await expect(page.getByText(/name is required/i)).toBeVisible(); + }); +}); + +test.describe("Customers — access control", () => { + test("admin cannot access /customers (role guard)", async ({ page }) => { + await page.goto("/login"); + await page.getByLabel("Email").fill("admin@serverdesk.local"); + await page.getByLabel("Password").fill("password123"); + await page.getByRole("button", { name: /sign in/i }).click(); + await page.waitForURL("**/"); + + await page.goto("/customers"); + // RoleGuard should redirect or show access denied — not show the customers UI + await expect(page.getByText("Customers")).not.toBeVisible({ timeout: 5000 }); + }); +}); From 7b98a260ffb7ee9c635b471dc901459d8429a1ea Mon Sep 17 00:00:00 2001 From: Jerome Date: Tue, 7 Apr 2026 16:42:34 +0200 Subject: [PATCH 3/3] docs: auto-reset DB after new migrations + apply pending migrations - Added rule to CLAUDE.md: always run npx supabase db reset immediately after writing a new migration, without waiting for the user to ask - Applied migrations 00009 and 00010 to local Supabase (db reset done) Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index 060674d..7674d48 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -168,6 +168,7 @@ Run `npm test && npm run lint && npm run build` before every commit. - Use database triggers (PL/pgSQL) for side effects (e.g., auto-creating profile on signup). - Use Edge Functions for email webhook processing and sending. - Migrations go in `supabase/migrations/` with descriptive names. +- **After writing any new migration, always run `npx supabase db reset` immediately** — do not wait for the user to ask. This applies the migration and re-seeds the local database so changes take effect right away. - Run `supabase db push` to apply migrations locally, `supabase db reset` to reset. ### RLS Security Rules