From a93747f11947a5b44be2b9b328a27721f7bcb9d2 Mon Sep 17 00:00:00 2001 From: npal Date: Thu, 17 Sep 2026 18:05:06 -0500 Subject: [PATCH 1/3] refactor(vendor-portal,settings): move state and fetch logic into hooks (SH-378) vendor-portal-provider.tsx and task-templates.tsx mixed data-fetching, state, and rendering in one file, with no caching and hand-rolled form state. This is a pure refactor with no behavior change. - vendor-portal-provider.tsx: replace manual useEffect/useState fetch with useVendorPortalSession (useQuery), closing the token race condition the old key={token} remount was working around - task-templates.tsx: split into useTaskTemplateEditor hook plus 4 presentational components; replace hand-rolled form state with react-hook-form + zod validation; replace the "new" string sentinel with a NEW_TEMPLATE_ID constant - add 29 tests (hook, component, and full-page integration) confirming identical behavior to the original code --- .../delete-task-template-dialog.tsx | 39 +++ .../task-template-detail-form.tsx | 70 ++++ .../task-template-items-field.tsx | 87 +++++ .../task-templates/task-template-list.tsx | 54 +++ .../(protected)/settings/task-templates.tsx | 307 +++--------------- .../v/_components/vendor-portal-provider.tsx | 31 +- .../constants/task-template-constants.ts | 9 + .../schemas/task-template-schema.ts | 4 +- .../task-templates/types/task-template.ts | 4 + .../use-cases/use-task-template-editor.ts | 118 +++++++ .../use-cases/use-vendor-portal-session.ts | 30 ++ .../settings/task-templates.test.tsx | 178 ++++++++++ .../app/v/vendor-portal-provider.test.tsx | 89 +++++ .../use-task-template-editor.test.tsx | 173 ++++++++++ .../use-vendor-portal-session.test.tsx | 101 ++++++ 15 files changed, 1001 insertions(+), 293 deletions(-) create mode 100644 src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx create mode 100644 src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx create mode 100644 src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx create mode 100644 src/app/(protected)/settings/_components/task-templates/task-template-list.tsx create mode 100644 src/domain/settings/task-templates/constants/task-template-constants.ts create mode 100644 src/domain/settings/task-templates/use-cases/use-task-template-editor.ts create mode 100644 src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts create mode 100644 src/test/app/(protected)/settings/task-templates.test.tsx create mode 100644 src/test/app/v/vendor-portal-provider.test.tsx create mode 100644 src/test/domain/settings/task-templates/use-task-template-editor.test.tsx create mode 100644 src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx diff --git a/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx b/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx new file mode 100644 index 00000000..0432d943 --- /dev/null +++ b/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx @@ -0,0 +1,39 @@ +import { + Button, + Dialog, + DialogActions, + DialogContent, + DialogContentText, + DialogTitle, +} from "@mui/material"; + +interface DeleteTaskTemplateDialogProps { + open: boolean; + templateName: string; + onCancel: () => void; + onConfirm: () => void; +} + +export function DeleteTaskTemplateDialog({ + open, + templateName, + onCancel, + onConfirm, +}: DeleteTaskTemplateDialogProps) { + return ( + + Delete Template + + + Are you sure you want to delete "{templateName}"? + + + + + + + + ); +} diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx new file mode 100644 index 00000000..d8b8781a --- /dev/null +++ b/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx @@ -0,0 +1,70 @@ +import type { UseFormReturn } from "react-hook-form"; +import { Box, Button, Paper, Stack, TextField, Typography } from "@mui/material"; +import type { TaskTemplateFormValues } from "@/domain/settings/task-templates/types/task-template"; +import { TaskTemplateItemsField } from "./task-template-items-field"; + +interface TaskTemplateDetailFormProps { + form: UseFormReturn; + isNew: boolean; + isSelected: boolean; + isSaving: boolean; + onDelete: () => void; + onSubmit: () => void; +} + +export function TaskTemplateDetailForm({ + form, + isNew, + isSelected, + isSaving, + onDelete, + onSubmit, +}: TaskTemplateDetailFormProps) { + const { + register, + control, + formState: { isValid }, + } = form; + + if (!isSelected) { + return ( + + + Select a template or create a new one + + + ); + } + + return ( + + + + + + + + + + + + {!isNew && ( + + )} + + + + + + ); +} diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx new file mode 100644 index 00000000..1b36d983 --- /dev/null +++ b/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx @@ -0,0 +1,87 @@ +import { useState } from "react"; +import { useFieldArray, type Control } from "react-hook-form"; +import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; +import ArrowUpwardIcon from "@mui/icons-material/ArrowUpward"; +import DeleteOutlineIcon from "@mui/icons-material/DeleteOutlined"; +import { Box, Button, IconButton, Stack, TextField, Typography } from "@mui/material"; +import type { TaskTemplateFormValues } from "@/domain/settings/task-templates/types/task-template"; + +interface TaskTemplateItemsFieldProps { + control: Control; +} + +export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) { + const { fields, append, remove, swap } = useFieldArray({ control, name: "items" }); + const [newItemText, setNewItemText] = useState(""); + + const handleAddItem = () => { + const text = newItemText.trim(); + if (!text) return; + append({ text, sortOrder: fields.length }); + setNewItemText(""); + }; + + return ( + <> + + Checklist Items ({fields.length}) + + + + {fields.map((field, index) => ( + + + {index + 1} + + + {field.text} + + swap(index, index - 1)} + > + + + swap(index, index + 1)} + > + + + remove(index)} + > + + + + ))} + + + + setNewItemText(event.target.value)} + onKeyDown={(event) => { + if (event.key === "Enter") { + event.preventDefault(); + handleAddItem(); + } + }} + placeholder="Add checklist item..." + /> + + + + ); +} diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx new file mode 100644 index 00000000..54d62e2a --- /dev/null +++ b/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx @@ -0,0 +1,54 @@ +import { + Box, + Button, + CircularProgress, + List, + ListItemButton, + ListItemText, + Paper, +} from "@mui/material"; +import type { + SelectedTaskTemplateId, + TaskTemplate, +} from "@/domain/settings/task-templates/types/task-template"; + +interface TaskTemplateListProps { + isLoading: boolean; + selectedId: SelectedTaskTemplateId; + templates: TaskTemplate[]; + onNew: () => void; + onSelect: (template: TaskTemplate) => void; +} + +export function TaskTemplateList({ + isLoading, + selectedId, + templates, + onNew, + onSelect, +}: TaskTemplateListProps) { + return ( + + + {isLoading ? ( + + + + ) : ( + + {templates.map((template) => ( + onSelect(template)} + > + + + ))} + + )} + + ); +} diff --git a/src/app/(protected)/settings/task-templates.tsx b/src/app/(protected)/settings/task-templates.tsx index d9888509..ed1b00f9 100644 --- a/src/app/(protected)/settings/task-templates.tsx +++ b/src/app/(protected)/settings/task-templates.tsx @@ -1,128 +1,36 @@ -import { useState } from "react"; -import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; -import ArrowUpwardIcon from "@mui/icons-material/ArrowUpward"; -import DeleteOutlineIcon from "@mui/icons-material/DeleteOutlined"; -import { - Alert, - Box, - Button, - CircularProgress, - Dialog, - DialogActions, - DialogContent, - DialogContentText, - DialogTitle, - IconButton, - List, - ListItemButton, - ListItemText, - Paper, - Stack, - TextField, - Typography, -} from "@mui/material"; +import { Alert, Box, Stack, Typography } from "@mui/material"; import { SettingsNav } from "@/components/common/settings-nav"; -import { mapTaskTemplateToFormValues } from "@/domain/settings/task-templates/mappers/task-template-mapper"; -import type { - TaskTemplate, - TaskTemplateFormValues, -} from "@/domain/settings/task-templates/types/task-template"; -import { - useCreateTaskTemplate, - useDeleteTaskTemplate, - useUpdateTaskTemplate, -} from "@/domain/settings/task-templates/use-cases/use-task-template-mutations"; -import { useTaskTemplates } from "@/domain/settings/task-templates/use-cases/use-task-templates"; - -const emptyForm: TaskTemplateFormValues = { name: "", description: "", items: [] }; +import { useTaskTemplateEditor } from "@/domain/settings/task-templates/use-cases/use-task-template-editor"; +import { DeleteTaskTemplateDialog } from "./_components/task-templates/delete-task-template-dialog"; +import { TaskTemplateDetailForm } from "./_components/task-templates/task-template-detail-form"; +import { TaskTemplateList } from "./_components/task-templates/task-template-list"; export default function TaskTemplatesPage() { - const { data: templates = [], isLoading, error } = useTaskTemplates(); - const createTemplate = useCreateTaskTemplate(); - const updateTemplate = useUpdateTaskTemplate(); - const deleteTemplate = useDeleteTaskTemplate(); + const { + templatesQuery, + templates, + selectedId, + isNew, + isSaving, + form, + deleteConfirmOpen, + templateName, + selectTemplate, + startNewTemplate, + submitForm, + requestDelete, + cancelDelete, + confirmDelete, + } = useTaskTemplateEditor(); - const [selectedId, setSelectedId] = useState(null); - const [form, setForm] = useState(emptyForm); - const [newItemText, setNewItemText] = useState(""); - const [confirmDelete, setConfirmDelete] = useState(false); - - const isNew = selectedId === "new"; - const isSaving = createTemplate.isPending || updateTemplate.isPending; - - const handleSelect = (template: TaskTemplate) => { - setSelectedId(template.id); - setForm(mapTaskTemplateToFormValues(template)); - setNewItemText(""); - }; - - const handleNew = () => { - setSelectedId("new"); - setForm(emptyForm); - setNewItemText(""); - }; - - const handleAddItem = () => { - if (!newItemText.trim()) return; - setForm((prev) => ({ - ...prev, - items: [...prev.items, { text: newItemText.trim(), sortOrder: prev.items.length }], - })); - setNewItemText(""); - }; - - const handleRemoveItem = (index: number) => { - setForm((prev) => ({ - ...prev, - items: prev.items.filter((_, i) => i !== index).map((item, i) => ({ ...item, sortOrder: i })), - })); - }; - - const handleMoveItem = (index: number, direction: -1 | 1) => { - setForm((prev) => { - const items = [...prev.items]; - const target = index + direction; - if (target < 0 || target >= items.length) return prev; - [items[index], items[target]] = [items[target], items[index]]; - return { ...prev, items: items.map((item, i) => ({ ...item, sortOrder: i })) }; - }); - }; - - const handleSave = () => { - if (!form.name.trim()) return; - if (isNew) { - createTemplate.mutate(form, { - onSuccess: (result) => setSelectedId(result.id), - }); - return; - } - if (selectedId && selectedId !== "new") { - updateTemplate.mutate({ id: selectedId, values: form }); - } - }; - - const handleDelete = () => { - if (!selectedId || selectedId === "new") return; - deleteTemplate.mutate(selectedId, { - onSuccess: () => { - setSelectedId(null); - setForm(emptyForm); - setConfirmDelete(false); - }, - }); - }; + const { isLoading, error } = templatesQuery; return ( Task List Templates - + Create reusable checklists for work order dispatches @@ -132,155 +40,28 @@ export default function TaskTemplatesPage() { )} - - - {isLoading ? ( - - - - ) : ( - - {templates.map((template) => ( - handleSelect(template)} - > - - - ))} - - )} - - - - {!selectedId ? ( - - Select a template or create a new one - - ) : ( - - - setForm((f) => ({ ...f, name: e.target.value }))} - placeholder="e.g., HVAC Inspection" - /> - setForm((f) => ({ ...f, description: e.target.value }))} - /> - - - - Checklist Items ({form.items.length}) - - - - {form.items.map((item, index) => ( - - - {index + 1} - - - {item.text} - - handleMoveItem(index, -1)} - > - - - handleMoveItem(index, 1)} - > - - - handleRemoveItem(index)}> - - - - ))} - - - - setNewItemText(e.target.value)} - onKeyDown={(e) => { - if (e.key === "Enter") { - e.preventDefault(); - handleAddItem(); - } - }} - placeholder="Add checklist item..." - /> - - - - - {!isNew && ( - - )} - - - - )} - + + - setConfirmDelete(false)}> - Delete Template - - - Are you sure you want to delete "{form.name}"? - - - - - - - + ); } diff --git a/src/app/v/_components/vendor-portal-provider.tsx b/src/app/v/_components/vendor-portal-provider.tsx index 036e58dd..4eb66044 100644 --- a/src/app/v/_components/vendor-portal-provider.tsx +++ b/src/app/v/_components/vendor-portal-provider.tsx @@ -1,38 +1,13 @@ -import { useEffect, useMemo, useState, type ReactNode } from "react"; +import { useMemo, type ReactNode } from "react"; import { useParams } from "react-router"; -import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; -import type { VendorPortalSession } from "@/domain/vendor-portal/types/vendor-portal"; +import { useVendorPortalSession } from "@/domain/vendor-portal/use-cases/use-vendor-portal-session"; import { VendorPortalContext, type VendorPortalContextValue, - type VendorPortalStatus, } from "@/app/v/_components/vendor-portal-context"; function VendorPortalSessionLoader({ token, children }: { token: string; children: ReactNode }) { - const [vendor, setVendor] = useState(null); - const [status, setStatus] = useState("loading"); - const [error, setError] = useState(null); - - useEffect(() => { - let cancelled = false; - - vendorPortalApi - .session(token) - .then((data) => { - if (cancelled) return; - setVendor(data); - setStatus("ready"); - }) - .catch((err: Error) => { - if (cancelled) return; - setError(err); - setStatus("error"); - }); - - return () => { - cancelled = true; - }; - }, [token]); + const { vendor, status, error } = useVendorPortalSession(token); const value = useMemo( () => ({ token, vendor, status, error }), diff --git a/src/domain/settings/task-templates/constants/task-template-constants.ts b/src/domain/settings/task-templates/constants/task-template-constants.ts new file mode 100644 index 00000000..4c5263e6 --- /dev/null +++ b/src/domain/settings/task-templates/constants/task-template-constants.ts @@ -0,0 +1,9 @@ +import type { TaskTemplateFormValues } from "@/domain/settings/task-templates/types/task-template"; + +export const NEW_TEMPLATE_ID = "new" as const; + +export const EMPTY_TASK_TEMPLATE_FORM: TaskTemplateFormValues = { + name: "", + description: "", + items: [], +}; diff --git a/src/domain/settings/task-templates/schemas/task-template-schema.ts b/src/domain/settings/task-templates/schemas/task-template-schema.ts index 07d4c344..3695f5ba 100644 --- a/src/domain/settings/task-templates/schemas/task-template-schema.ts +++ b/src/domain/settings/task-templates/schemas/task-template-schema.ts @@ -7,8 +7,8 @@ const taskTemplateItemSchema = z.object({ export const taskTemplateSchema = z.object({ name: z.string().min(1, "Template name is required"), - description: z.string().optional().default(""), - items: z.array(taskTemplateItemSchema).default([]), + description: z.string(), + items: z.array(taskTemplateItemSchema), }); export type TaskTemplateSchemaValues = z.infer; diff --git a/src/domain/settings/task-templates/types/task-template.ts b/src/domain/settings/task-templates/types/task-template.ts index 6044559f..67918b65 100644 --- a/src/domain/settings/task-templates/types/task-template.ts +++ b/src/domain/settings/task-templates/types/task-template.ts @@ -1,3 +1,5 @@ +import type { NEW_TEMPLATE_ID } from "@/domain/settings/task-templates/constants/task-template-constants"; + export interface TaskTemplateItem { itemText: string; sortOrder: number; @@ -21,3 +23,5 @@ export interface TaskTemplateFormValues { description: string; items: TaskTemplateFormItem[]; } + +export type SelectedTaskTemplateId = string | number | typeof NEW_TEMPLATE_ID | null; diff --git a/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts b/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts new file mode 100644 index 00000000..7dc7be13 --- /dev/null +++ b/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts @@ -0,0 +1,118 @@ +import { useState } from "react"; +import { useForm, useWatch, type UseFormReturn } from "react-hook-form"; +import { zodResolver } from "@hookform/resolvers/zod"; +import type { UseQueryResult } from "@tanstack/react-query"; +import { + EMPTY_TASK_TEMPLATE_FORM, + NEW_TEMPLATE_ID, +} from "@/domain/settings/task-templates/constants/task-template-constants"; +import { mapTaskTemplateToFormValues } from "@/domain/settings/task-templates/mappers/task-template-mapper"; +import { taskTemplateSchema } from "@/domain/settings/task-templates/schemas/task-template-schema"; +import type { + SelectedTaskTemplateId, + TaskTemplate, + TaskTemplateFormValues, +} from "@/domain/settings/task-templates/types/task-template"; +import { + useCreateTaskTemplate, + useDeleteTaskTemplate, + useUpdateTaskTemplate, +} from "@/domain/settings/task-templates/use-cases/use-task-template-mutations"; +import { useTaskTemplates } from "@/domain/settings/task-templates/use-cases/use-task-templates"; + +export interface UseTaskTemplateEditorResult { + templatesQuery: UseQueryResult; + templates: TaskTemplate[]; + selectedId: SelectedTaskTemplateId; + isNew: boolean; + isSaving: boolean; + isDeleting: boolean; + form: UseFormReturn; + deleteConfirmOpen: boolean; + templateName: string; + selectTemplate: (template: TaskTemplate) => void; + startNewTemplate: () => void; + submitForm: () => void; + requestDelete: () => void; + cancelDelete: () => void; + confirmDelete: () => void; +} + +export function useTaskTemplateEditor(): UseTaskTemplateEditorResult { + const templatesQuery = useTaskTemplates(); + const createTemplate = useCreateTaskTemplate(); + const updateTemplate = useUpdateTaskTemplate(); + const deleteTemplate = useDeleteTaskTemplate(); + + const [selectedId, setSelectedId] = useState(null); + const [deleteConfirmOpen, setDeleteConfirmOpen] = useState(false); + + const form = useForm({ + resolver: zodResolver(taskTemplateSchema), + defaultValues: EMPTY_TASK_TEMPLATE_FORM, + mode: "onChange", + }); + + const isNew = selectedId === NEW_TEMPLATE_ID; + const templateName = useWatch({ control: form.control, name: "name" }); + + const selectTemplate = (template: TaskTemplate) => { + setSelectedId(template.id); + form.reset(mapTaskTemplateToFormValues(template)); + }; + + const startNewTemplate = () => { + setSelectedId(NEW_TEMPLATE_ID); + form.reset(EMPTY_TASK_TEMPLATE_FORM); + }; + + const onSubmit = (values: TaskTemplateFormValues) => { + const normalized: TaskTemplateFormValues = { + ...values, + items: values.items.map((item, index) => ({ ...item, sortOrder: index })), + }; + if (isNew) { + createTemplate.mutate(normalized, { + onSuccess: (result) => setSelectedId(result.id), + }); + return; + } + if (selectedId !== null) { + updateTemplate.mutate({ id: selectedId, values: normalized }); + } + }; + + const submitForm = form.handleSubmit(onSubmit); + + const requestDelete = () => setDeleteConfirmOpen(true); + const cancelDelete = () => setDeleteConfirmOpen(false); + + const confirmDelete = () => { + if (selectedId === null || isNew) return; + deleteTemplate.mutate(selectedId, { + onSuccess: () => { + setSelectedId(null); + form.reset(EMPTY_TASK_TEMPLATE_FORM); + setDeleteConfirmOpen(false); + }, + }); + }; + + return { + templatesQuery, + templates: templatesQuery.data ?? [], + selectedId, + isNew, + isSaving: createTemplate.isPending || updateTemplate.isPending, + isDeleting: deleteTemplate.isPending, + form, + deleteConfirmOpen, + templateName, + selectTemplate, + startNewTemplate, + submitForm, + requestDelete, + cancelDelete, + confirmDelete, + }; +} diff --git a/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts b/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts new file mode 100644 index 00000000..63c440a0 --- /dev/null +++ b/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts @@ -0,0 +1,30 @@ +import { useQuery, type QueryStatus } from "@tanstack/react-query"; +import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; +import type { VendorPortalSession } from "@/domain/vendor-portal/types/vendor-portal"; +import type { VendorPortalStatus } from "@/app/v/_components/vendor-portal-context"; +import { queryKeys } from "@/infra/query-key/query-key"; + +const VENDOR_PORTAL_STATUS_BY_QUERY_STATUS: Record = { + pending: "loading", + error: "error", + success: "ready", +}; + +interface UseVendorPortalSessionResult { + vendor: VendorPortalSession | null; + status: VendorPortalStatus; + error: Error | null; +} + +export function useVendorPortalSession(token: string): UseVendorPortalSessionResult { + const { data, status, error } = useQuery({ + queryKey: queryKeys.vendorPortal.session(token), + queryFn: () => vendorPortalApi.session(token), + }); + + return { + vendor: data ?? null, + status: VENDOR_PORTAL_STATUS_BY_QUERY_STATUS[status], + error: error ?? null, + }; +} diff --git a/src/test/app/(protected)/settings/task-templates.test.tsx b/src/test/app/(protected)/settings/task-templates.test.tsx new file mode 100644 index 00000000..876a5cd1 --- /dev/null +++ b/src/test/app/(protected)/settings/task-templates.test.tsx @@ -0,0 +1,178 @@ +import { fireEvent, screen, waitFor, within } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import TaskTemplatesPage from "@/app/(protected)/settings/task-templates"; +import { taskTemplatesApi } from "@/domain/settings/task-templates/api/task-templates-api"; +import type { TaskTemplate } from "@/domain/settings/task-templates/types/task-template"; +import { renderWithProviders } from "@/test/test-utils"; + +const existingTemplate: TaskTemplate = { + id: "5", + name: "HVAC Inspection", + description: "Standard checklist", + isActive: true, + items: [ + { itemText: "Check filters", sortOrder: 0 }, + { itemText: "Check refrigerant", sortOrder: 1 }, + ], +}; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function renderPage(templates: TaskTemplate[] = [existingTemplate]) { + vi.spyOn(taskTemplatesApi, "getAll").mockResolvedValue(templates); + return renderWithProviders(, { withAuth: false }); +} + +describe("TaskTemplatesPage", () => { + it("shows a placeholder until a template is selected or created", async () => { + renderPage(); + + expect(await screen.findByText("HVAC Inspection")).toBeInTheDocument(); + expect(screen.getByText("Select a template or create a new one")).toBeInTheDocument(); + }); + + it("selecting a template from the list populates the detail form", async () => { + renderPage(); + + fireEvent.click(await screen.findByText("HVAC Inspection")); + + expect(await screen.findByLabelText(/^Template Name/)).toHaveValue("HVAC Inspection"); + expect(screen.getByLabelText("Description")).toHaveValue("Standard checklist"); + expect(screen.getByText("Checklist Items (2)")).toBeInTheDocument(); + expect(screen.getByText("Check filters")).toBeInTheDocument(); + expect(screen.getByText("Check refrigerant")).toBeInTheDocument(); + }); + + it("starting a new template shows an empty form with Create Template action", async () => { + renderPage(); + await screen.findByText("HVAC Inspection"); + + fireEvent.click(screen.getByRole("button", { name: "+ New Template" })); + + expect(await screen.findByLabelText(/^Template Name/)).toHaveValue(""); + expect(screen.getByRole("button", { name: "Create Template" })).toBeDisabled(); + expect(screen.queryByRole("button", { name: "Delete" })).not.toBeInTheDocument(); + }); + + it("enables Create Template only once a name is entered", async () => { + renderPage(); + await screen.findByText("HVAC Inspection"); + fireEvent.click(screen.getByRole("button", { name: "+ New Template" })); + + const nameInput = await screen.findByLabelText(/^Template Name/); + fireEvent.change(nameInput, { target: { value: "New Checklist" } }); + + await waitFor(() => + expect(screen.getByRole("button", { name: "Create Template" })).toBeEnabled(), + ); + }); + + it("adds a checklist item and appends it to the end of the list", async () => { + renderPage(); + fireEvent.click(await screen.findByText("HVAC Inspection")); + await screen.findByText("Checklist Items (2)"); + + fireEvent.change(screen.getByPlaceholderText("Add checklist item..."), { + target: { value: "New Item" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Add" })); + + expect(await screen.findByText("Checklist Items (3)")).toBeInTheDocument(); + const rows = screen.getAllByText(/^(Check filters|Check refrigerant|New Item)$/); + expect(rows.map((row) => row.textContent)).toEqual([ + "Check filters", + "Check refrigerant", + "New Item", + ]); + }); + + it("moves an item up and down without changing the item count", async () => { + renderPage(); + fireEvent.click(await screen.findByText("HVAC Inspection")); + await screen.findByText("Checklist Items (2)"); + + fireEvent.click(screen.getByRole("button", { name: "Move item 2 up" })); + + let rows = screen.getAllByText(/^(Check filters|Check refrigerant)$/); + expect(rows.map((row) => row.textContent)).toEqual(["Check refrigerant", "Check filters"]); + + fireEvent.click(screen.getByRole("button", { name: "Move item 1 down" })); + + rows = screen.getAllByText(/^(Check filters|Check refrigerant)$/); + expect(rows.map((row) => row.textContent)).toEqual(["Check filters", "Check refrigerant"]); + }); + + it("removes an item from the checklist", async () => { + renderPage(); + fireEvent.click(await screen.findByText("HVAC Inspection")); + await screen.findByText("Checklist Items (2)"); + + fireEvent.click(screen.getByRole("button", { name: "Remove item 1" })); + + expect(await screen.findByText("Checklist Items (1)")).toBeInTheDocument(); + expect(screen.queryByText("Check filters")).not.toBeInTheDocument(); + expect(screen.getByText("Check refrigerant")).toBeInTheDocument(); + }); + + it("saves an edited template through the update mutation", async () => { + vi.spyOn(taskTemplatesApi, "update").mockResolvedValue(existingTemplate); + renderPage(); + + fireEvent.click(await screen.findByText("HVAC Inspection")); + const descriptionInput = await screen.findByLabelText("Description"); + fireEvent.change(descriptionInput, { target: { value: "Updated checklist" } }); + + fireEvent.click(screen.getByRole("button", { name: "Save Changes" })); + + await waitFor(() => + expect(taskTemplatesApi.update).toHaveBeenCalledWith( + "5", + expect.objectContaining({ description: "Updated checklist" }), + ), + ); + }); + + it("deletes a template after confirming in the dialog", async () => { + vi.spyOn(taskTemplatesApi, "delete").mockResolvedValue(undefined); + renderPage(); + + fireEvent.click(await screen.findByText("HVAC Inspection")); + fireEvent.click(await screen.findByRole("button", { name: "Delete" })); + + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByText(/Are you sure you want to delete/)).toHaveTextContent( + "HVAC Inspection", + ); + + fireEvent.click(within(dialog).getByRole("button", { name: "Delete" })); + + await waitFor(() => expect(taskTemplatesApi.delete).toHaveBeenCalledWith("5")); + await waitFor(() => + expect(screen.getByText("Select a template or create a new one")).toBeInTheDocument(), + ); + }); + + it("cancelling the delete dialog keeps the template intact", async () => { + const deleteSpy = vi.spyOn(taskTemplatesApi, "delete"); + renderPage(); + + fireEvent.click(await screen.findByText("HVAC Inspection")); + fireEvent.click(await screen.findByRole("button", { name: "Delete" })); + const dialog = await screen.findByRole("dialog"); + + fireEvent.click(within(dialog).getByRole("button", { name: "Cancel" })); + + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + expect(deleteSpy).not.toHaveBeenCalled(); + expect(screen.getByLabelText(/^Template Name/)).toHaveValue("HVAC Inspection"); + }); + + it("surfaces a load error banner when the template list fails to fetch", async () => { + vi.spyOn(taskTemplatesApi, "getAll").mockRejectedValue(new Error("Network down")); + renderWithProviders(, { withAuth: false }); + + expect(await screen.findByRole("alert")).toHaveTextContent("Network down"); + }); +}); diff --git a/src/test/app/v/vendor-portal-provider.test.tsx b/src/test/app/v/vendor-portal-provider.test.tsx new file mode 100644 index 00000000..264b0219 --- /dev/null +++ b/src/test/app/v/vendor-portal-provider.test.tsx @@ -0,0 +1,89 @@ +import { screen } from "@testing-library/react"; +import { Route, Routes } from "react-router"; +import { describe, expect, it, vi } from "vitest"; +import { useVendorPortal } from "@/app/v/_components/vendor-portal-context"; +import { VendorPortalProvider } from "@/app/v/_components/vendor-portal-provider"; +import type { VendorPortalSession } from "@/domain/vendor-portal/types/vendor-portal"; +import { renderWithProviders } from "@/test/test-utils"; + +const useVendorPortalSession = vi.fn(); + +vi.mock("@/domain/vendor-portal/use-cases/use-vendor-portal-session", () => ({ + useVendorPortalSession: (...args: unknown[]) => useVendorPortalSession(...args), +})); + +function VendorPortalConsumer() { + const { token, vendor, status, error } = useVendorPortal(); + return ( +
+ {token} + {status} + {vendor?.companyName ?? "none"} + {error?.message ?? "none"} +
+ ); +} + +function renderPortal(token: string) { + return renderWithProviders( + + + + + } + /> + , + { + route: `/v/${token}`, + routerProps: { initialEntries: [`/v/${token}`] }, + withAuth: false, + }, + ); +} + +describe("VendorPortalProvider", () => { + it("exposes loading status while the session query is pending", () => { + useVendorPortalSession.mockReturnValue({ vendor: null, status: "loading", error: null }); + + renderPortal("token-a"); + + expect(screen.getByTestId("token")).toHaveTextContent("token-a"); + expect(screen.getByTestId("status")).toHaveTextContent("loading"); + expect(screen.getByTestId("vendor-name")).toHaveTextContent("none"); + }); + + it("exposes ready status with the resolved vendor once the session loads", () => { + const vendor: VendorPortalSession = { companyName: "Acme Vendor", vendorId: 42 }; + useVendorPortalSession.mockReturnValue({ vendor, status: "ready", error: null }); + + renderPortal("token-b"); + + expect(screen.getByTestId("status")).toHaveTextContent("ready"); + expect(screen.getByTestId("vendor-name")).toHaveTextContent("Acme Vendor"); + expect(screen.getByTestId("error")).toHaveTextContent("none"); + }); + + it("exposes error status and message when the session lookup fails", () => { + useVendorPortalSession.mockReturnValue({ + vendor: null, + status: "error", + error: new Error("Invalid or expired link"), + }); + + renderPortal("token-c"); + + expect(screen.getByTestId("status")).toHaveTextContent("error"); + expect(screen.getByTestId("error")).toHaveTextContent("Invalid or expired link"); + }); + + it("passes the token straight from the route param through to the hook", () => { + useVendorPortalSession.mockReturnValue({ vendor: null, status: "loading", error: null }); + + renderPortal("route-token-123"); + + expect(useVendorPortalSession).toHaveBeenCalledWith("route-token-123"); + }); +}); diff --git a/src/test/domain/settings/task-templates/use-task-template-editor.test.tsx b/src/test/domain/settings/task-templates/use-task-template-editor.test.tsx new file mode 100644 index 00000000..6d292bd9 --- /dev/null +++ b/src/test/domain/settings/task-templates/use-task-template-editor.test.tsx @@ -0,0 +1,173 @@ +import { QueryClientProvider } from "@tanstack/react-query"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { taskTemplatesApi } from "@/domain/settings/task-templates/api/task-templates-api"; +import type { TaskTemplate } from "@/domain/settings/task-templates/types/task-template"; +import { useTaskTemplateEditor } from "@/domain/settings/task-templates/use-cases/use-task-template-editor"; +import { createTestQueryClient } from "@/test/test-utils"; + +const existingTemplate: TaskTemplate = { + id: "5", + name: "HVAC Inspection", + description: "Standard checklist", + isActive: true, + items: [ + { itemText: "Check filters", sortOrder: 0 }, + { itemText: "Check refrigerant", sortOrder: 1 }, + ], +}; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function wrapperFor() { + const queryClient = createTestQueryClient(); + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +function renderEditor() { + vi.spyOn(taskTemplatesApi, "getAll").mockResolvedValue([existingTemplate]); + return renderHook(() => useTaskTemplateEditor(), { wrapper: wrapperFor() }); +} + +describe("useTaskTemplateEditor", () => { + it("starts with nothing selected and an empty form", async () => { + const { result } = renderEditor(); + + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + expect(result.current.selectedId).toBeNull(); + expect(result.current.isNew).toBe(false); + expect(result.current.form.getValues()).toEqual({ name: "", description: "", items: [] }); + }); + + it("selectTemplate loads the mapped form values for that template", async () => { + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + act(() => result.current.selectTemplate(existingTemplate)); + + expect(result.current.selectedId).toBe("5"); + expect(result.current.isNew).toBe(false); + expect(result.current.form.getValues()).toEqual({ + name: "HVAC Inspection", + description: "Standard checklist", + items: [ + { text: "Check filters", sortOrder: 0 }, + { text: "Check refrigerant", sortOrder: 1 }, + ], + }); + }); + + it("startNewTemplate marks isNew and resets the form to empty", async () => { + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + act(() => result.current.selectTemplate(existingTemplate)); + + act(() => result.current.startNewTemplate()); + + expect(result.current.isNew).toBe(true); + expect(result.current.form.getValues()).toEqual({ name: "", description: "", items: [] }); + }); + + it("does not call create when the name is blank (schema validation blocks submit)", async () => { + const createSpy = vi.spyOn(taskTemplatesApi, "create"); + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + act(() => result.current.startNewTemplate()); + await act(async () => result.current.submitForm()); + + expect(createSpy).not.toHaveBeenCalled(); + }); + + it("creates a new template and normalizes item sortOrder by array position", async () => { + const created: TaskTemplate = { ...existingTemplate, id: "9", name: "New Template" }; + const createSpy = vi.spyOn(taskTemplatesApi, "create").mockResolvedValue(created); + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + act(() => result.current.startNewTemplate()); + act(() => { + result.current.form.setValue("name", "New Template", { shouldValidate: true }); + result.current.form.setValue( + "items", + [ + { text: "Second", sortOrder: 99 }, + { text: "First", sortOrder: 1 }, + ], + { shouldValidate: true }, + ); + }); + + await act(async () => result.current.submitForm()); + + expect(createSpy).toHaveBeenCalledWith({ + name: "New Template", + description: "", + items: [ + { text: "Second", sortOrder: 0 }, + { text: "First", sortOrder: 1 }, + ], + }); + await waitFor(() => expect(result.current.selectedId).toBe("9")); + }); + + it("updates the selected template instead of creating when not new", async () => { + const updateSpy = vi.spyOn(taskTemplatesApi, "update").mockResolvedValue(existingTemplate); + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + act(() => result.current.selectTemplate(existingTemplate)); + act(() => { + result.current.form.setValue("description", "Updated description", { shouldValidate: true }); + }); + + await act(async () => result.current.submitForm()); + + expect(updateSpy).toHaveBeenCalledWith( + "5", + expect.objectContaining({ description: "Updated description" }), + ); + }); + + it("toggles the delete confirmation dialog open and closed", async () => { + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + act(() => result.current.requestDelete()); + expect(result.current.deleteConfirmOpen).toBe(true); + + act(() => result.current.cancelDelete()); + expect(result.current.deleteConfirmOpen).toBe(false); + }); + + it("confirmDelete removes the template and clears the selection", async () => { + const deleteSpy = vi.spyOn(taskTemplatesApi, "delete").mockResolvedValue(undefined); + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + act(() => result.current.selectTemplate(existingTemplate)); + act(() => result.current.requestDelete()); + + await act(async () => result.current.confirmDelete()); + + expect(deleteSpy).toHaveBeenCalledWith("5"); + await waitFor(() => expect(result.current.selectedId).toBeNull()); + expect(result.current.deleteConfirmOpen).toBe(false); + expect(result.current.form.getValues()).toEqual({ name: "", description: "", items: [] }); + }); + + it("does nothing on confirmDelete when nothing is selected", async () => { + const deleteSpy = vi.spyOn(taskTemplatesApi, "delete"); + const { result } = renderEditor(); + await waitFor(() => expect(result.current.templates).toHaveLength(1)); + + await act(async () => result.current.confirmDelete()); + + expect(deleteSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx b/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx new file mode 100644 index 00000000..bd924f33 --- /dev/null +++ b/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx @@ -0,0 +1,101 @@ +import { QueryClientProvider } from "@tanstack/react-query"; +import { renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; +import type { VendorPortalSession } from "@/domain/vendor-portal/types/vendor-portal"; +import { useVendorPortalSession } from "@/domain/vendor-portal/use-cases/use-vendor-portal-session"; +import { queryKeys } from "@/infra/query-key/query-key"; +import { createTestQueryClient } from "@/test/test-utils"; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function wrapperFor(queryClient = createTestQueryClient()) { + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +describe("useVendorPortalSession", () => { + it("starts in loading status with a null vendor", () => { + vi.spyOn(vendorPortalApi, "session").mockReturnValue(new Promise(() => {})); + + const { result } = renderHook(() => useVendorPortalSession("token-1"), { + wrapper: wrapperFor(), + }); + + expect(result.current.status).toBe("loading"); + expect(result.current.vendor).toBeNull(); + expect(result.current.error).toBeNull(); + }); + + it("maps a successful fetch to ready status with the vendor payload", async () => { + const vendor: VendorPortalSession = { companyName: "Acme Vendor", vendorId: 7 }; + vi.spyOn(vendorPortalApi, "session").mockResolvedValue(vendor); + + const { result } = renderHook(() => useVendorPortalSession("token-1"), { + wrapper: wrapperFor(), + }); + + await waitFor(() => expect(result.current.status).toBe("ready")); + expect(result.current.vendor).toEqual(vendor); + expect(result.current.error).toBeNull(); + }); + + it("maps a rejected fetch to error status with the error preserved", async () => { + vi.spyOn(vendorPortalApi, "session").mockRejectedValue(new Error("Invalid link")); + + const { result } = renderHook(() => useVendorPortalSession("token-1"), { + wrapper: wrapperFor(), + }); + + await waitFor(() => expect(result.current.status).toBe("error")); + expect(result.current.vendor).toBeNull(); + expect(result.current.error).toBeInstanceOf(Error); + expect(result.current.error?.message).toBe("Invalid link"); + }); + + it("caches by the centralized vendorPortal.session query key, keyed per token", async () => { + const sessionSpy = vi + .spyOn(vendorPortalApi, "session") + .mockResolvedValue({ companyName: "Acme Vendor" }); + // staleTime: Infinity isolates cache-reuse from React Query's default + // background-refetch-on-mount behavior, which is a separate concern. + const queryClient = createTestQueryClient(); + queryClient.setDefaultOptions({ queries: { retry: false, gcTime: 0, staleTime: Infinity } }); + const wrapper = wrapperFor(queryClient); + + const { result: first } = renderHook(() => useVendorPortalSession("shared-token"), { wrapper }); + await waitFor(() => expect(first.current.status).toBe("ready")); + + const { result: second } = renderHook(() => useVendorPortalSession("shared-token"), { + wrapper, + }); + await waitFor(() => expect(second.current.status).toBe("ready")); + + expect(sessionSpy).toHaveBeenCalledTimes(1); + expect(queryClient.getQueryData(queryKeys.vendorPortal.session("shared-token"))).toEqual({ + companyName: "Acme Vendor", + }); + }); + + it("fetches independently for a different token instead of reusing the cache", async () => { + const sessionSpy = vi + .spyOn(vendorPortalApi, "session") + .mockResolvedValue({ companyName: "Acme Vendor" }); + const queryClient = createTestQueryClient(); + const wrapper = wrapperFor(queryClient); + + const { result: first } = renderHook(() => useVendorPortalSession("token-x"), { wrapper }); + await waitFor(() => expect(first.current.status).toBe("ready")); + + const { result: second } = renderHook(() => useVendorPortalSession("token-y"), { wrapper }); + await waitFor(() => expect(second.current.status).toBe("ready")); + + expect(sessionSpy).toHaveBeenCalledTimes(2); + expect(sessionSpy).toHaveBeenNthCalledWith(1, "token-x"); + expect(sessionSpy).toHaveBeenNthCalledWith(2, "token-y"); + }); +}); From 62e5d46b0f6022b3dc39d7701131e134090e1038 Mon Sep 17 00:00:00 2001 From: npal Date: Fri, 18 Sep 2026 10:46:16 -0500 Subject: [PATCH 2/3] fix: address PR #224 review findings and extract task-template copy (SH-378) Three real behavior deltas flagged in review, each closing a gap against the "no behavior change" claim: - use-vendor-portal-session.ts: add retry:false and meta:{suppressErrorToast:true} so an invalid/expired token fails fast with a single request and no duplicate global toast (this was also the root cause of the failing e2e test - the toast and the inline error message both had role="alert", tripping a strict-mode locator match) - task-template-schema.ts: trim the name before validating so a whitespace-only name is rejected, matching the old manual form.name.trim() check - task-template-detail-form.tsx: block Enter-triggered implicit submission on the Template Name / Description inputs, since the original page had no
element and Enter did nothing - task-template-items-field.tsx: surface a visible error when a loaded item has empty text, since the schema already blocked save in that case but gave no way to see why; use-task-template-editor.ts now eagerly validates after loading a template so this reflects reality immediately instead of only after an unrelated edit Also extract every user-facing string in the task-templates feature into TASK_TEMPLATE_COPY (task-template-constants.ts) instead of inline literals scattered across 5 files. 6 new regression tests cover all of the above; full suite (36 tests across the 2 refactored areas) still green. --- .../delete-task-template-dialog.tsx | 11 ++--- .../task-template-detail-form.tsx | 32 ++++++++++--- .../task-template-items-field.tsx | 22 ++++++--- .../task-templates/task-template-list.tsx | 3 +- .../(protected)/settings/task-templates.tsx | 7 +-- .../constants/task-template-constants.ts | 26 ++++++++++ .../schemas/task-template-schema.ts | 3 +- .../use-cases/use-task-template-editor.ts | 1 + .../use-cases/use-vendor-portal-session.ts | 2 + .../settings/task-templates.test.tsx | 48 +++++++++++++++++++ .../use-vendor-portal-session.test.tsx | 38 +++++++++++++++ 11 files changed, 168 insertions(+), 25 deletions(-) diff --git a/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx b/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx index 0432d943..d8ac7285 100644 --- a/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx +++ b/src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx @@ -6,6 +6,7 @@ import { DialogContentText, DialogTitle, } from "@mui/material"; +import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; interface DeleteTaskTemplateDialogProps { open: boolean; @@ -22,16 +23,14 @@ export function DeleteTaskTemplateDialog({ }: DeleteTaskTemplateDialogProps) { return ( - Delete Template + {TASK_TEMPLATE_COPY.deleteDialogTitle} - - Are you sure you want to delete "{templateName}"? - + {TASK_TEMPLATE_COPY.deleteDialogBody(templateName)} - + diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx index d8b8781a..889be812 100644 --- a/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx +++ b/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx @@ -1,8 +1,14 @@ import type { UseFormReturn } from "react-hook-form"; import { Box, Button, Paper, Stack, TextField, Typography } from "@mui/material"; +import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; import type { TaskTemplateFormValues } from "@/domain/settings/task-templates/types/task-template"; import { TaskTemplateItemsField } from "./task-template-items-field"; +function getSubmitLabel(isSaving: boolean, isNew: boolean): string { + if (isSaving) return TASK_TEMPLATE_COPY.savingLabel; + return isNew ? TASK_TEMPLATE_COPY.createButton : TASK_TEMPLATE_COPY.saveButton; +} + interface TaskTemplateDetailFormProps { form: UseFormReturn; isNew: boolean; @@ -30,7 +36,7 @@ export function TaskTemplateDetailForm({ return ( - Select a template or create a new one + {TASK_TEMPLATE_COPY.emptySelection} ); @@ -38,17 +44,29 @@ export function TaskTemplateDetailForm({ return ( - + { + if (event.key === "Enter" && event.target instanceof HTMLInputElement) { + event.preventDefault(); + } + }} + > - + @@ -56,11 +74,11 @@ export function TaskTemplateDetailForm({ {!isNew && ( )} diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx index 1b36d983..384b229c 100644 --- a/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx +++ b/src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx @@ -1,9 +1,10 @@ import { useState } from "react"; -import { useFieldArray, type Control } from "react-hook-form"; +import { useFieldArray, useFormState, type Control } from "react-hook-form"; import ArrowDownwardIcon from "@mui/icons-material/ArrowDownward"; import ArrowUpwardIcon from "@mui/icons-material/ArrowUpward"; import DeleteOutlineIcon from "@mui/icons-material/DeleteOutlined"; import { Box, Button, IconButton, Stack, TextField, Typography } from "@mui/material"; +import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; import type { TaskTemplateFormValues } from "@/domain/settings/task-templates/types/task-template"; interface TaskTemplateItemsFieldProps { @@ -12,7 +13,9 @@ interface TaskTemplateItemsFieldProps { export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) { const { fields, append, remove, swap } = useFieldArray({ control, name: "items" }); + const { errors } = useFormState({ control, name: "items" }); const [newItemText, setNewItemText] = useState(""); + const hasInvalidItem = Array.isArray(errors.items) && errors.items.some(Boolean); const handleAddItem = () => { const text = newItemText.trim(); @@ -24,8 +27,13 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) return ( <> - Checklist Items ({fields.length}) + {TASK_TEMPLATE_COPY.checklistItemsLabel(fields.length)} + {hasInvalidItem && ( + + {TASK_TEMPLATE_COPY.invalidItemError} + + )} {fields.map((field, index) => ( @@ -39,7 +47,7 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) swap(index, index - 1)} > @@ -47,7 +55,7 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) swap(index, index + 1)} > @@ -55,7 +63,7 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) remove(index)} > @@ -76,10 +84,10 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps) handleAddItem(); } }} - placeholder="Add checklist item..." + placeholder={TASK_TEMPLATE_COPY.addItemPlaceholder} /> diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx index 54d62e2a..0afab3ca 100644 --- a/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx +++ b/src/app/(protected)/settings/_components/task-templates/task-template-list.tsx @@ -7,6 +7,7 @@ import { ListItemText, Paper, } from "@mui/material"; +import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; import type { SelectedTaskTemplateId, TaskTemplate, @@ -30,7 +31,7 @@ export function TaskTemplateList({ return ( {isLoading ? ( diff --git a/src/app/(protected)/settings/task-templates.tsx b/src/app/(protected)/settings/task-templates.tsx index ed1b00f9..f368d828 100644 --- a/src/app/(protected)/settings/task-templates.tsx +++ b/src/app/(protected)/settings/task-templates.tsx @@ -1,5 +1,6 @@ import { Alert, Box, Stack, Typography } from "@mui/material"; import { SettingsNav } from "@/components/common/settings-nav"; +import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; import { useTaskTemplateEditor } from "@/domain/settings/task-templates/use-cases/use-task-template-editor"; import { DeleteTaskTemplateDialog } from "./_components/task-templates/delete-task-template-dialog"; import { TaskTemplateDetailForm } from "./_components/task-templates/task-template-detail-form"; @@ -29,14 +30,14 @@ export default function TaskTemplatesPage() { - Task List Templates + {TASK_TEMPLATE_COPY.pageTitle} - Create reusable checklists for work order dispatches + {TASK_TEMPLATE_COPY.pageSubtitle} {Boolean(error) && ( - {error instanceof Error ? error.message : "Failed to load templates"} + {error instanceof Error ? error.message : TASK_TEMPLATE_COPY.loadError} )} diff --git a/src/domain/settings/task-templates/constants/task-template-constants.ts b/src/domain/settings/task-templates/constants/task-template-constants.ts index 4c5263e6..c59dec9a 100644 --- a/src/domain/settings/task-templates/constants/task-template-constants.ts +++ b/src/domain/settings/task-templates/constants/task-template-constants.ts @@ -7,3 +7,29 @@ export const EMPTY_TASK_TEMPLATE_FORM: TaskTemplateFormValues = { description: "", items: [], }; + +export const TASK_TEMPLATE_COPY = { + pageTitle: "Task List Templates", + pageSubtitle: "Create reusable checklists for work order dispatches", + loadError: "Failed to load templates", + newTemplateButton: "+ New Template", + emptySelection: "Select a template or create a new one", + nameLabel: "Template Name", + namePlaceholder: "e.g., HVAC Inspection", + nameRequiredError: "Template name is required", + descriptionLabel: "Description", + deleteButton: "Delete", + savingLabel: "Saving...", + createButton: "Create Template", + saveButton: "Save Changes", + checklistItemsLabel: (count: number) => `Checklist Items (${count})`, + invalidItemError: "One or more checklist items are empty. Remove it to save.", + addItemPlaceholder: "Add checklist item...", + addButton: "Add", + moveItemUpLabel: (position: number) => `Move item ${position} up`, + moveItemDownLabel: (position: number) => `Move item ${position} down`, + removeItemLabel: (position: number) => `Remove item ${position}`, + deleteDialogTitle: "Delete Template", + deleteDialogBody: (name: string) => `Are you sure you want to delete "${name}"?`, + cancelButton: "Cancel", +} as const; diff --git a/src/domain/settings/task-templates/schemas/task-template-schema.ts b/src/domain/settings/task-templates/schemas/task-template-schema.ts index 3695f5ba..dfc56f7f 100644 --- a/src/domain/settings/task-templates/schemas/task-template-schema.ts +++ b/src/domain/settings/task-templates/schemas/task-template-schema.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; const taskTemplateItemSchema = z.object({ text: z.string().min(1), @@ -6,7 +7,7 @@ const taskTemplateItemSchema = z.object({ }); export const taskTemplateSchema = z.object({ - name: z.string().min(1, "Template name is required"), + name: z.string().trim().min(1, TASK_TEMPLATE_COPY.nameRequiredError), description: z.string(), items: z.array(taskTemplateItemSchema), }); diff --git a/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts b/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts index 7dc7be13..c056fbc2 100644 --- a/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts +++ b/src/domain/settings/task-templates/use-cases/use-task-template-editor.ts @@ -59,6 +59,7 @@ export function useTaskTemplateEditor(): UseTaskTemplateEditorResult { const selectTemplate = (template: TaskTemplate) => { setSelectedId(template.id); form.reset(mapTaskTemplateToFormValues(template)); + void form.trigger(); }; const startNewTemplate = () => { diff --git a/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts b/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts index 63c440a0..54be29b9 100644 --- a/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts +++ b/src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts @@ -20,6 +20,8 @@ export function useVendorPortalSession(token: string): UseVendorPortalSessionRes const { data, status, error } = useQuery({ queryKey: queryKeys.vendorPortal.session(token), queryFn: () => vendorPortalApi.session(token), + retry: false, + meta: { suppressErrorToast: true }, }); return { diff --git a/src/test/app/(protected)/settings/task-templates.test.tsx b/src/test/app/(protected)/settings/task-templates.test.tsx index 876a5cd1..785185c8 100644 --- a/src/test/app/(protected)/settings/task-templates.test.tsx +++ b/src/test/app/(protected)/settings/task-templates.test.tsx @@ -175,4 +175,52 @@ describe("TaskTemplatesPage", () => { expect(await screen.findByRole("alert")).toHaveTextContent("Network down"); }); + + it("keeps Create Template disabled for a whitespace-only name", async () => { + renderPage(); + await screen.findByText("HVAC Inspection"); + fireEvent.click(screen.getByRole("button", { name: "+ New Template" })); + + const nameInput = await screen.findByLabelText(/^Template Name/); + fireEvent.change(nameInput, { target: { value: " " } }); + + await waitFor(() => + expect(screen.getByRole("button", { name: "Create Template" })).toBeDisabled(), + ); + }); + + it("does not submit the form when Enter is pressed in the Template Name field", async () => { + const updateSpy = vi.spyOn(taskTemplatesApi, "update"); + renderPage(); + + fireEvent.click(await screen.findByText("HVAC Inspection")); + const nameInput = await screen.findByLabelText(/^Template Name/); + fireEvent.keyDown(nameInput, { key: "Enter", code: "Enter" }); + + expect(updateSpy).not.toHaveBeenCalled(); + }); + + it("shows a recovery message and disables Save when a loaded item has empty text", async () => { + const templateWithBlankItem: TaskTemplate = { + ...existingTemplate, + items: [{ itemText: "", sortOrder: 0 }], + }; + renderPage([templateWithBlankItem]); + + fireEvent.click(await screen.findByText("HVAC Inspection")); + + expect( + await screen.findByText("One or more checklist items are empty. Remove it to save."), + ).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save Changes" })).toBeDisabled(); + + fireEvent.click(screen.getByRole("button", { name: "Remove item 1" })); + + await waitFor(() => + expect( + screen.queryByText("One or more checklist items are empty. Remove it to save."), + ).not.toBeInTheDocument(), + ); + expect(screen.getByRole("button", { name: "Save Changes" })).toBeEnabled(); + }); }); diff --git a/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx b/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx index bd924f33..a91312bc 100644 --- a/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx +++ b/src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx @@ -6,10 +6,18 @@ import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; import type { VendorPortalSession } from "@/domain/vendor-portal/types/vendor-portal"; import { useVendorPortalSession } from "@/domain/vendor-portal/use-cases/use-vendor-portal-session"; import { queryKeys } from "@/infra/query-key/query-key"; +import { createAppQueryClient } from "@/lib/query/query-client"; import { createTestQueryClient } from "@/test/test-utils"; +const toastMocks = vi.hoisted(() => ({ error: vi.fn() })); + +vi.mock("react-toastify", () => ({ + toast: { error: toastMocks.error, success: vi.fn() }, +})); + afterEach(() => { vi.restoreAllMocks(); + toastMocks.error.mockReset(); }); function wrapperFor(queryClient = createTestQueryClient()) { @@ -98,4 +106,34 @@ describe("useVendorPortalSession", () => { expect(sessionSpy).toHaveBeenNthCalledWith(1, "token-x"); expect(sessionSpy).toHaveBeenNthCalledWith(2, "token-y"); }); + + it("does not retry a rejected request, even under the app's default retry:1 policy", async () => { + const sessionSpy = vi + .spyOn(vendorPortalApi, "session") + .mockRejectedValue(new Error("Invalid link")); + // Uses the real app query client (default retry: 1) instead of the test + // client, so this only passes if the hook opts out of retries itself. + const queryClient = createAppQueryClient(); + + const { result } = renderHook(() => useVendorPortalSession("token-1"), { + wrapper: wrapperFor(queryClient), + }); + + await waitFor(() => expect(result.current.status).toBe("error")); + expect(sessionSpy).toHaveBeenCalledTimes(1); + }); + + it("does not show the global error toast, since the layout renders its own inline error UI", async () => { + vi.spyOn(vendorPortalApi, "session").mockRejectedValue(new Error("Invalid link")); + // Uses the real app query client so the QueryCache's global onError + // handler actually runs and would toast unless suppressed. + const queryClient = createAppQueryClient(); + + const { result } = renderHook(() => useVendorPortalSession("token-1"), { + wrapper: wrapperFor(queryClient), + }); + + await waitFor(() => expect(result.current.status).toBe("error")); + expect(toastMocks.error).not.toHaveBeenCalled(); + }); }); From 86e43b66f42570427e78e4b6cc8c4fb5410ecc8a Mon Sep 17 00:00:00 2001 From: npal Date: Mon, 21 Sep 2026 20:04:33 -0500 Subject: [PATCH 3/3] fix: reset add-item draft on template selection change (SH-378) TaskTemplateItemsField owns newItemText locally but stays mounted across selectTemplate/startNewTemplate (which only call form.reset()), so a typed draft survived switching templates - the base page cleared this draft explicitly in both handleSelect and handleNew. Remount TaskTemplateItemsField on selection change via key={selectedId} instead of lifting the draft into the editor hook: it's transient input-only state, not form data, so this keeps the fix local and lets React's own remount semantics reset it. Confirmed the two new regression tests fail without the key and pass with it. --- .../task-template-detail-form.tsx | 9 ++++- .../(protected)/settings/task-templates.tsx | 1 + .../settings/task-templates.test.tsx | 40 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx b/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx index 889be812..301195ed 100644 --- a/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx +++ b/src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx @@ -1,7 +1,10 @@ import type { UseFormReturn } from "react-hook-form"; import { Box, Button, Paper, Stack, TextField, Typography } from "@mui/material"; import { TASK_TEMPLATE_COPY } from "@/domain/settings/task-templates/constants/task-template-constants"; -import type { TaskTemplateFormValues } from "@/domain/settings/task-templates/types/task-template"; +import type { + SelectedTaskTemplateId, + TaskTemplateFormValues, +} from "@/domain/settings/task-templates/types/task-template"; import { TaskTemplateItemsField } from "./task-template-items-field"; function getSubmitLabel(isSaving: boolean, isNew: boolean): string { @@ -14,6 +17,7 @@ interface TaskTemplateDetailFormProps { isNew: boolean; isSelected: boolean; isSaving: boolean; + selectedId: SelectedTaskTemplateId; onDelete: () => void; onSubmit: () => void; } @@ -23,6 +27,7 @@ export function TaskTemplateDetailForm({ isNew, isSelected, isSaving, + selectedId, onDelete, onSubmit, }: TaskTemplateDetailFormProps) { @@ -69,7 +74,7 @@ export function TaskTemplateDetailForm({ /> - + {!isNew && ( diff --git a/src/app/(protected)/settings/task-templates.tsx b/src/app/(protected)/settings/task-templates.tsx index f368d828..bbc98752 100644 --- a/src/app/(protected)/settings/task-templates.tsx +++ b/src/app/(protected)/settings/task-templates.tsx @@ -53,6 +53,7 @@ export default function TaskTemplatesPage() { isNew={isNew} isSelected={selectedId !== null} isSaving={isSaving} + selectedId={selectedId} onDelete={requestDelete} onSubmit={submitForm} /> diff --git a/src/test/app/(protected)/settings/task-templates.test.tsx b/src/test/app/(protected)/settings/task-templates.test.tsx index 785185c8..eb57b26e 100644 --- a/src/test/app/(protected)/settings/task-templates.test.tsx +++ b/src/test/app/(protected)/settings/task-templates.test.tsx @@ -16,6 +16,14 @@ const existingTemplate: TaskTemplate = { ], }; +const secondTemplate: TaskTemplate = { + id: "9", + name: "Plumbing Inspection", + description: "Basic checklist", + isActive: true, + items: [{ itemText: "Check for leaks", sortOrder: 0 }], +}; + afterEach(() => { vi.restoreAllMocks(); }); @@ -88,6 +96,38 @@ describe("TaskTemplatesPage", () => { ]); }); + it("clears the unsaved add-item draft when switching to a different template", async () => { + renderPage([existingTemplate, secondTemplate]); + fireEvent.click(await screen.findByText("HVAC Inspection")); + await screen.findByText("Checklist Items (2)"); + + fireEvent.change(screen.getByPlaceholderText("Add checklist item..."), { + target: { value: "Check filter" }, + }); + expect(screen.getByPlaceholderText("Add checklist item...")).toHaveValue("Check filter"); + + fireEvent.click(screen.getByText("Plumbing Inspection")); + + await screen.findByText("Checklist Items (1)"); + expect(screen.getByPlaceholderText("Add checklist item...")).toHaveValue(""); + }); + + it("clears the unsaved add-item draft when starting a new template", async () => { + renderPage(); + fireEvent.click(await screen.findByText("HVAC Inspection")); + await screen.findByText("Checklist Items (2)"); + + fireEvent.change(screen.getByPlaceholderText("Add checklist item..."), { + target: { value: "Check filter" }, + }); + expect(screen.getByPlaceholderText("Add checklist item...")).toHaveValue("Check filter"); + + fireEvent.click(screen.getByRole("button", { name: "+ New Template" })); + + await screen.findByText("Checklist Items (0)"); + expect(screen.getByPlaceholderText("Add checklist item...")).toHaveValue(""); + }); + it("moves an item up and down without changing the item count", async () => { renderPage(); fireEvent.click(await screen.findByText("HVAC Inspection"));