adam/shoc-frontend-new
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/shoc-frontend-new.git synced 2026-10-03 04:33:23 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

fix: address PR #224 review findings and extract task-template copy (SH-378)

Browse source 
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 <form> 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.
This commit is contained in:
npal 2026-09-18 10:46:16 -05:00
parent 5645c7e02b
commit 62e5d46b0f
11 changed files with 168 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/app/(protected)/settings/_components/task-templates/delete-task-template-dialog.tsx
View file

@ -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 (
<Dialog open={open} onClose={onCancel}>
<DialogTitle>Delete Template</DialogTitle>
<DialogTitle>{TASK_TEMPLATE_COPY.deleteDialogTitle}</DialogTitle>
<DialogContent>
<DialogContentText>
Are you sure you want to delete &quot;{templateName}&quot;?
</DialogContentText>
<DialogContentText>{TASK_TEMPLATE_COPY.deleteDialogBody(templateName)}</DialogContentText>
</DialogContent>
<DialogActions>
<Button onClick={onCancel}>Cancel</Button>
<Button onClick={onCancel}>{TASK_TEMPLATE_COPY.cancelButton}</Button>
<Button color="error" variant="contained" onClick={onConfirm}>
Delete
{TASK_TEMPLATE_COPY.deleteButton}
</Button>
</DialogActions>
</Dialog>

32
src/app/(protected)/settings/_components/task-templates/task-template-detail-form.tsx
View file

@ -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<TaskTemplateFormValues>;
isNew: boolean;
@ -30,7 +36,7 @@ export function TaskTemplateDetailForm({
return (
<Paper variant="outlined" className="p-4">
<Typography className="py-10 text-center" sx={{ color: "text.secondary" }}>
Select a template or create a new one
{TASK_TEMPLATE_COPY.emptySelection}
</Typography>
</Paper>
);
@ -38,17 +44,29 @@ export function TaskTemplateDetailForm({
return (
<Paper variant="outlined" className="p-4">
<Box component="form" onSubmit={onSubmit}>
<Box
component="form"
onSubmit={onSubmit}
onKeyDown={(event) => {
if (event.key === "Enter" && event.target instanceof HTMLInputElement) {
event.preventDefault();
}
}}
>
<Stack spacing={3}>
<Stack direction={{ xs: "column", sm: "row" }} spacing={2}>
<TextField
label="Template Name"
label={TASK_TEMPLATE_COPY.nameLabel}
fullWidth
required
placeholder="e.g., HVAC Inspection"
placeholder={TASK_TEMPLATE_COPY.namePlaceholder}
{...register("name")}
/>
<TextField label="Description" fullWidth {...register("description")} />
<TextField
label={TASK_TEMPLATE_COPY.descriptionLabel}
fullWidth
{...register("description")}
/>
</Stack>
<TaskTemplateItemsField control={control} />
@ -56,11 +74,11 @@ export function TaskTemplateDetailForm({
<Stack direction="row" spacing={2} sx={{ justifyContent: "flex-end" }}>
{!isNew && (
<Button variant="outlined" color="error" onClick={onDelete}>
Delete
{TASK_TEMPLATE_COPY.deleteButton}
</Button>
)}
<Button type="submit" variant="contained" disabled={isSaving || !isValid}>
{isSaving ? "Saving..." : isNew ? "Create Template" : "Save Changes"}
{getSubmitLabel(isSaving, isNew)}
</Button>
</Stack>
</Stack>

22
src/app/(protected)/settings/_components/task-templates/task-template-items-field.tsx
View file

@ -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 (
<>
<Typography variant="subtitle2" className="font-semibold">
Checklist Items ({fields.length})
{TASK_TEMPLATE_COPY.checklistItemsLabel(fields.length)}
</Typography>
{hasInvalidItem && (
<Typography variant="caption" color="error">
{TASK_TEMPLATE_COPY.invalidItemError}
</Typography>
)}
<Stack spacing={1}>
{fields.map((field, index) => (
@ -39,7 +47,7 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps)
<IconButton
size="small"
disabled={index === 0}
aria-label={`Move item ${index + 1} up`}
aria-label={TASK_TEMPLATE_COPY.moveItemUpLabel(index + 1)}
onClick={() => swap(index, index - 1)}
>
<ArrowUpwardIcon fontSize="small" />
@ -47,7 +55,7 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps)
<IconButton
size="small"
disabled={index === fields.length - 1}
aria-label={`Move item ${index + 1} down`}
aria-label={TASK_TEMPLATE_COPY.moveItemDownLabel(index + 1)}
onClick={() => swap(index, index + 1)}
>
<ArrowDownwardIcon fontSize="small" />
@ -55,7 +63,7 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps)
<IconButton
size="small"
color="error"
aria-label={`Remove item ${index + 1}`}
aria-label={TASK_TEMPLATE_COPY.removeItemLabel(index + 1)}
onClick={() => remove(index)}
>
<DeleteOutlineIcon fontSize="small" />
@ -76,10 +84,10 @@ export function TaskTemplateItemsField({ control }: TaskTemplateItemsFieldProps)
handleAddItem();
}
}}
placeholder="Add checklist item..."
placeholder={TASK_TEMPLATE_COPY.addItemPlaceholder}
/>
<Button variant="outlined" onClick={handleAddItem} disabled={!newItemText.trim()}>
Add
{TASK_TEMPLATE_COPY.addButton}
</Button>
</Stack>
</>

3
src/app/(protected)/settings/_components/task-templates/task-template-list.tsx
View file

@ -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 (
<Paper variant="outlined" className="p-3">
<Button fullWidth variant="contained" className="mb-3" onClick={onNew}>
+ New Template
{TASK_TEMPLATE_COPY.newTemplateButton}
</Button>
{isLoading ? (
<Box className="flex justify-center p-4">

7
src/app/(protected)/settings/task-templates.tsx
View file

@ -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() {
<Box className="p-4">
<SettingsNav />
<Stack spacing={1} className="mb-4">
<Typography variant="h5">Task List Templates</Typography>
<Typography variant="h5">{TASK_TEMPLATE_COPY.pageTitle}</Typography>
<Typography variant="body2" sx={{ color: "text.secondary" }}>
Create reusable checklists for work order dispatches
{TASK_TEMPLATE_COPY.pageSubtitle}
</Typography>
</Stack>
{Boolean(error) && (
<Alert severity="error" className="mb-4">
{error instanceof Error ? error.message : "Failed to load templates"}
{error instanceof Error ? error.message : TASK_TEMPLATE_COPY.loadError}
</Alert>
)}
<Box className="grid grid-cols-1 gap-4 md:grid-cols-[280px_1fr]">

26
src/domain/settings/task-templates/constants/task-template-constants.ts
View file

@ -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;

3
src/domain/settings/task-templates/schemas/task-template-schema.ts
View file

@ -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),
});

1
src/domain/settings/task-templates/use-cases/use-task-template-editor.ts
View file

@ -59,6 +59,7 @@ export function useTaskTemplateEditor(): UseTaskTemplateEditorResult {
const selectTemplate = (template: TaskTemplate) => {
setSelectedId(template.id);
form.reset(mapTaskTemplateToFormValues(template));
void form.trigger();
};
const startNewTemplate = () => {

2
src/domain/vendor-portal/use-cases/use-vendor-portal-session.ts
View file

@ -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 {

48
src/test/app/(protected)/settings/task-templates.test.tsx
View file

@ -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();
});
});

38
src/test/domain/vendor-portal/use-cases/use-vendor-portal-session.test.tsx
View file

@ -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();
});
});