diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3b220720..ab2cfb56 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -11,6 +11,33 @@ permissions: jobs: ci: + # Org reusable workflow (Node 24): format check, lint, build, unit tests. uses: Sea-Haven-Industries/.github/.github/workflows/ci-typescript-frontend.yaml@main with: node-version: "24" + + governance: + # Repo-owned guarantee that every frontend quality gate runs from this + # repository, independent of (and in addition to) the reusable workflow. + # `npm run verify` is the single command that chains: format check, lint + # (--max-warnings=0), type-check + build, unit tests, then the governance + # checks in scripts/governance-check.mjs (godfile ratchet + changed-file + # maintainability gate). If the reusable workflow is later confirmed to run + # every gate, this job can be slimmed to `npm run governance`. + # + # GOVERNANCE_BASE points the changed-file gate at the right diff: + # PR -> the PR target branch (origin/) + # push-> the previous commit on the branch (github.event.before) + runs-on: ubuntu-latest + env: + GOVERNANCE_BASE: ${{ github.event_name == 'pull_request' && format('origin/{0}', github.base_ref) || github.event.before }} + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: "24" + cache: npm + - run: npm ci + - run: npm run verify diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..2803356c --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,81 @@ +# AGENTS.md — frontend mandatory conventions + +This file is the binding entry point for **every contributor — human or AI coding +or review agent** — working in this repository (`seahaven-new-app`, the SHOC +frontend). It makes the validated React/TypeScript conventions mandatory and +points to the operational documents and executable gates that enforce them. + +Read these before writing or reviewing code. They override generic "best +practice" suggestions from any agent or model. This repository contract may +strengthen, but never weaken, the workspace-level `AGENTS.md`. + +- [QUALITY_GATES.md](QUALITY_GATES.md) — the executable gates, the single + command, and the no-false-pass guarantees. +- [ARCHITECTURE_AND_CODE_QUALITY.md](ARCHITECTURE_AND_CODE_QUALITY.md) — the + MUST / MUST NOT conventions (conditional rendering, typography, forms, state, + data access, security, maintainability) with evidence and thresholds. +- [REVIEW_AND_PR_FRAMEWORK.md](REVIEW_AND_PR_FRAMEWORK.md) — the PR review + contract (exact-head review, board regression inventory, behavior-based + testing, security/performance/Big-O review, no style-only comments). +- [docs/FRONTEND_MAINTAINABILITY.md](docs/FRONTEND_MAINTAINABILITY.md) — the + detailed rationale for the conditional-rendering and typography rules. + +## One command to run every gate + +```bash +npm run verify +``` + +This chains the full set: Prettier check, ESLint (`--max-warnings=0`), TypeScript +build (`tsc -b && vite build`), unit tests (`vitest run`), and the governance +checks (`npm run governance`). **Do not claim a task is done until `npm run +verify` is green locally.** CI runs the same `npm run verify` in a repo-owned +`governance` job, so a green local run mirrors CI. + +## Non-negotiable rules (enforced; do not work around) + +These are already enforced by lint/build or the governance script. Disabling, +baselining, or per-line-suppressing them is forbidden (see exceptions below). + +- **No one-sided `cond ? : null`** — use `&&` or the `when` prop. +- **The left operand of `&&` in JSX must be entirely boolean** — coerce with + `Boolean(...)` / an explicit comparison; `{count && ...}` is rejected. +- **Shared `Text` for `p` / `h1`–`h6` / error typography** — raw `

`/`` and + the `vp-error` class outside `Text` are rejected. +- **Zero lint warnings** — `--max-warnings=0` makes a warning a failure; fix it, + do not silence it. +- **Hooks correctness** — the `react-hooks` recommended rules (including + `exhaustive-deps`) run under the zero-warnings gate. +- **Godfile ratchet** — no source file may exceed 500 lines. Existing named debt + has a frozen per-file cap that may only decrease. +- **Changed-file maintainability** — changed TS/TSX must meet + `complexity ≤ 20`, function `≤ 150` lines, `≤ 4` params, `≤ 4` depth. + +## How to add / change a convention + +1. Land it **green**: a new or tightened rule must ship with the codebase passing + it (a migration in the same change), not as a warning-only backlog. +2. If legacy would break, use changed-file enforcement or migrate it. Do not add + new baseline debt or raise a frozen cap. +3. Document the rule, its threshold, and its evidence in + [ARCHITECTURE_AND_CODE_QUALITY.md](ARCHITECTURE_AND_CODE_QUALITY.md). + +## Exceptions and the ADR process + +Exceptions are **not granted by disabling a rule inline**. To deviate: + +1. Record an **Architecture Decision Record** under `docs/adr/` + (`NNNN-title.md`: context, decision, consequences, alternatives). +2. Existing grandfathered entries may only be removed or have their caps + reduced. New entries and cap increases fail the governance gate. +3. Get it reviewed like any other change. The ADR + baseline entry is the + auditable record; an `eslint-disable` comment is not. + +## Toolchain (do not change without an ADR) + +React 19, TypeScript 6, Vite, Tailwind 4 + MUI, TanStack Query, React Hook Form + +Zod, Ky. Node ≥ 22.22.1 (CI runs Node 24); npm 11.16.0 (pinned via +`packageManager`, invoked through corepack). **No new dependencies without an +ADR** — prefer the libraries already established (see +[ARCHITECTURE_AND_CODE_QUALITY.md](ARCHITECTURE_AND_CODE_QUALITY.md) §Forms and +data access). diff --git a/ARCHITECTURE_AND_CODE_QUALITY.md b/ARCHITECTURE_AND_CODE_QUALITY.md new file mode 100644 index 00000000..3cd2aefc --- /dev/null +++ b/ARCHITECTURE_AND_CODE_QUALITY.md @@ -0,0 +1,125 @@ +# ARCHITECTURE_AND_CODE_QUALITY.md — MUST / MUST NOT conventions + +Mandatory React/TypeScript conventions for this repository. Each rule lists how +it is enforced. Detailed rationale for rendering and typography lives in +[docs/FRONTEND_MAINTAINABILITY.md](docs/FRONTEND_MAINTAINABILITY.md). When a rule +says "MUST", it is enforced by lint, build, or `npm run governance`; "SHOULD" +means it is a review-enforced convention backed by the +[PR review contract](REVIEW_AND_PR_FRAMEWORK.md). + +## Conditional rendering + +- **MUST** use `&&` or the `Text` `when` prop for one-sided conditions; never + `cond ? : null`. (ESLint `no-restricted-syntax`.) +- **MUST** make the left operand of `&&` in JSX entirely boolean. Coerce presence + with `Boolean(value)` (or `Boolean(a || b)`); preserve numeric/empty-string + semantics and TypeScript narrowing with `count > 0`, `value != null`. + `{count && }` renders `0` and is rejected by the type-aware + `seahaven/no-non-boolean-jsx-and` rule. (See FRONTEND_MAINTAINABILITY.md for + the approved guard forms.) +- **MUST NOT** widen typed element-slot props (`icon`, `action`, `actions`) back + to `ReactNode`; coerce with `Boolean(prop)` before `&&`. + +## Typography and feedback + +- **MUST** use `Text` from `@/components/ui/text` for paragraphs, headings, + descriptions, labels, captions, code, and async feedback. Raw `

` and + `

`–`

` are rejected. (ESLint `no-restricted-syntax`.) +- **MUST** use `Text variant="error"` for the `vp-error` styling; the + `vp-error` class on any non-`Text` element is rejected + (`seahaven/no-vp-error-outside-text`). Do not compose `vp-error` dynamically to + bypass the static check. +- **MUST NOT** pass `component`, `role`, or `aria-live` to `Text` — they are + omitted from `TextProps` so the variant contract (element, tone, live region) + cannot be overridden. Use `as` and `tone`. +- **SHOULD** keep the `feedback` (polite `status`) region mounted and toggle text + with `when`; mount `error` (assertive `alert`) on demand. + +## Forms and mutations + +- **MUST** use the libraries already established: **React Hook Form** for field + registration/lifecycle, **Zod** for validation and inferred types, **TanStack + Query** for server reads and mutations (pending/error state, invalidation, + retries). +- **MUST NOT** add **TanStack Form** alongside React Hook Form for one form. A + second form convention increases cognitive and dependency cost. Replacing + React Hook Form requires an approved repo-wide migration ADR with measured + benefits, a codemod/migration plan, and removal of the superseded dependency. +- **SHOULD** keep file upload selection/validation in a focused component and + drive upload progress/errors/retry through a TanStack Query mutation, not in a + route-sized page. + +## State ownership (bounded) + +- **MUST** keep query loading/error/empty state adjacent to the query result; + mutation pending/error state in the component that initiated the mutation. +- **MUST** compose focused state components instead of accumulating unrelated + booleans in a page. Route pages coordinate sections and navigation; reusable + sections own their interaction details. +- **SHOULD** render errors inline with accessible feedback (`Text +variant="error"`); reserve toasts for cross-page outcomes. + +## Data access + +- **MUST** keep TanStack Query keys **stable and descriptive** (a consistent + entity + identity tuple, co-located with the query). Unstable or ad-hoc keys + break caching and invalidation. (Review-enforced; a future lint rule is + tracked as a gap.) +- **MUST NOT** materialize large server collections into client state and then + filter/sort them in the component when the server (or a memoized, virtualized + layer) should own it. Prefer server-side filtering/pagination; if client-side + is required, memoize and avoid re-filtering on every render. (Review-enforced.) +- **MUST** invalidate the right query keys after a mutation so caches do not + show stale data. + +## Security + +- **MUST NOT** surface raw server error payloads, stack traces, or internal + identifiers to end users. Map server errors to a safe user-facing message + (e.g. `Text variant="error"`); log the full detail only to controlled channels. + (Review-enforced.) +- **MUST NOT** bake secrets, tokens, or account-specific values into the build. + Only `VITE_*` build-time vars are allowed, and they are baked into the bundle — + never put a secret in a `VITE_` variable. + +## Hooks, performance, and Big-O + +- **MUST** satisfy `react-hooks` recommended rules (including `exhaustive-deps`) + under the zero-warnings gate. +- **SHOULD** avoid `O(n²)` or worse work inside render/hot paths; memoize derived + data, key lists stably, and prefer server-side filtering for large sets + (reviewers flag algorithmic complexity — see REVIEW_AND_PR_FRAMEWORK.md). +- **SHOULD** keep renders pure; derive expensive values with `useMemo`/`useCallback` + only when measured to matter (no speculative memoization). + +## Maintainability (measured thresholds) + +Enforced by `npm run governance` ([QUALITY_GATES.md](QUALITY_GATES.md)). +These are conservative code-shape proxies for review focus. File length is not +proof of a god object, cyclomatic complexity is not runtime Big-O, and neither +substitutes for behavior tests, profiling, query-plan evidence, or reviewer +judgment. + +| Metric | Threshold | How enforced | +| --------------------- | ----------- | ------------------------------------------------------------------------------------------ | +| File length (godfile) | ≤ 500 lines | Whole-repo ratchet: any file over the cap not in `scripts/governance-baseline.json` fails. | +| Cyclomatic complexity | ≤ 20 | Changed-file ESLint `complexity`. | +| Function length | ≤ 150 lines | Changed-file ESLint `max-lines-per-function` (`skipComments`). | +| Parameters | ≤ 4 | Changed-file ESLint `max-params`. | +| Nesting depth | ≤ 4 | Changed-file ESLint `max-depth`. | + +**Why these are changed-file ratchets, not whole-repo errors:** measured against +the current codebase, the maintainability thresholds surface **72 violations +across ~51 legacy files** (45 function-length, 24 complexity, 3 params), and +**5 files exceed 500 lines**. Applying them repo-wide at `error` would block +legacy without a migration. Instead: + +- The **5 oversized files** are explicit grandfathered debt with frozen + per-file caps in `scripts/governance-baseline.json`. They may shrink but never + grow; new entries and cap increases fail. +- The **complexity/function-length/params/depth** rules apply to **changed** + TS/TSX on a PR/push, so new and modified code must comply while untouched + legacy is not blocked. Bring legacy into compliance when you next touch it. + +These thresholds are starting ratchets: tighten them (lower caps, fewer baseline +entries) as debt is paid down. Never raise them to make a change pass. diff --git a/QUALITY_GATES.md b/QUALITY_GATES.md new file mode 100644 index 00000000..b28babd0 --- /dev/null +++ b/QUALITY_GATES.md @@ -0,0 +1,54 @@ +# QUALITY_GATES.md — executable frontend gates + +The single command that runs **every** gate, locally and in CI: + +```bash +npm run verify +``` + +`verify` chains: `format:check` → `lint` → `build` (`tsc -b && vite build`) → +`test` (`vitest run`) → `governance`. A task is not done until this is green. + +## Gate matrix + +| Gate | Command / rule source | Enforced by | Scope | +| ----------------------------------- | ----------------------------------------------------------------------------------------------------------- | ---------------------- | ------------------------------------ | +| Formatting | `npm run format:check` (Prettier) | `verify` + lint-staged | Whole repo | +| Lint, zero warnings | `npm run lint` → `eslint . --max-warnings=0` | `verify` + CI | Governed TS/TSX (`eslint.config.js`) | +| Type-check + production build | `npm run build` → `tsc -b && vite build` | `verify` + CI | Whole app | +| Unit tests | `npm test` → `vitest run` | `verify` + CI | `src/test/**`, `config/**/*.test.ts` | +| Conditional rendering (no `: null`) | `no-restricted-syntax` in `eslint.config.js` | lint | Governed TSX | +| Boolean-only JSX `&&` | `seahaven/no-non-boolean-jsx-and` (type-aware) in `eslint-rules/` | lint | Governed TSX | +| Shared `Text` typography | `no-restricted-syntax` (raw `p`/`h1`–`h6`) + `seahaven/no-vp-error-outside-text` | lint | Governed TSX | +| Hooks correctness | `eslint-plugin-react-hooks` recommended (incl. `exhaustive-deps`) under zero-warnings | lint | Governed TS/TSX | +| Godfile ratchet (file length) | `scripts/governance-check.mjs` + `scripts/governance-baseline.json` | `governance` | `src/**`, `config/**` (non-test) | +| Changed-file maintainability | `scripts/governance-check.mjs` → ESLint (`complexity`, `max-lines-per-function`, `max-params`, `max-depth`) | `governance` | Changed TS/TSX vs base ref | + +## No-false-pass guarantees + +- **`--max-warnings=0`** — a warning is a failure. There is no "warning-only" + backlog; rules ship green (see AGENTS.md → How to add a convention). +- **Type-aware rules fail closed** — `seahaven/no-non-boolean-jsx-and` reports + when type services are unavailable rather than silently claiming safety. +- **Godfile ratchet is monotonic** — any new file over the cap, new baseline + entry, global cap increase, per-file cap increase, or growth beyond a frozen + legacy cap fails. Only cap reductions and entry removals are allowed. +- **Changed-file maintainability fails closed without a valid base** — in CI the + base ref is derived from `GITHUB_BASE_REF` (PR) or `github.event.before` + (push). An absent or unresolvable base is a failure, not a pass. + +## Where the gates run + +- **Locally:** `npm run verify`. `lint-staged` (via Husky) re-runs ESLint + + Prettier on staged files at commit; commitlint enforces Conventional Commits. +- **CI ([`.github/workflows/ci.yaml`](.github/workflows/ci.yaml)):** the org + reusable workflow (`ci-typescript-frontend.yaml`, Node 24) runs + format/lint/build/tests, **and** a repo-owned `governance` job runs + `npm run verify` so the maintainability ratchets are guaranteed from this + repository regardless of the reusable workflow. + +## Toolchain pin + +Node ≥ 22.22.1 (CI uses Node 24); npm 11.16.0 via `packageManager` (use +`corepack npm …` if your default `npm` is older). The lockfile is +`package-lock.json` v3; install with `npm ci`. diff --git a/README.md b/README.md index e380b5df..51fc21d2 100644 --- a/README.md +++ b/README.md @@ -95,15 +95,17 @@ The dev proxy expects the `shoc-backend` API at `http://localhost:5141`; override with `VITE_API_TARGET` (e.g. `https://api.dev.seahaven.com` to use the deployed dev API). -| Command | Description | -| ------------------------------------------ | ----------------------------------------------------- | -| `npm run dev` | Start Vite dev server on port 3000 | -| `npm run build` | Type-check (`tsc -b`) and production build to `dist/` | -| `npm run preview` | Preview the production build locally | -| `npm test` / `npm run test:watch` | Vitest unit tests (once / watch) | -| `npm run test:e2e` / `npm run test:e2e:ui` | Playwright e2e tests (headless / UI mode) | -| `npm run lint` / `npm run lint:fix` | ESLint (check / auto-fix) | -| `npm run format` / `npm run format:check` | Prettier (write / check) | +| Command | Description | +| ------------------------------------------ | -------------------------------------------------------- | +| `npm run dev` | Start Vite dev server on port 3000 | +| `npm run build` | Type-check (`tsc -b`) and production build to `dist/` | +| `npm run preview` | Preview the production build locally | +| `npm test` / `npm run test:watch` | Vitest unit tests (once / watch) | +| `npm run test:e2e` / `npm run test:e2e:ui` | Playwright e2e tests (headless / UI mode) | +| `npm run lint` / `npm run lint:fix` | ESLint (check / auto-fix) | +| `npm run format` / `npm run format:check` | Prettier (write / check) | +| `npm run governance` | Frontend governance checks (godfile + maintainability) | +| `npm run verify` | **All gates**: format + lint + build + test + governance | Husky + lint-staged run ESLint and Prettier on staged files at commit; commitlint enforces conventional commit messages. Run `npx tsc --noEmit` (or @@ -132,7 +134,13 @@ CI/CD uses the org's reusable workflows (no stored AWS keys — OIDC only): - **CI** ([`.github/workflows/ci.yaml`](.github/workflows/ci.yaml)) — on push and PRs to `main`/`dev`, calls `Sea-Haven-Industries/.github` → `ci-typescript-frontend.yaml` (Node 24): - format check, lint, build, tests. + format check, lint, build, tests; **and** runs a repo-owned `governance` job + that calls `npm run verify` so every gate (including the maintainability + ratchets in [`scripts/governance-check.mjs`](scripts/governance-check.mjs)) is + guaranteed from this repository. Conventions and gates are documented under + [`AGENTS.md`](AGENTS.md), [`QUALITY_GATES.md`](QUALITY_GATES.md), + [`ARCHITECTURE_AND_CODE_QUALITY.md`](ARCHITECTURE_AND_CODE_QUALITY.md), and + [`REVIEW_AND_PR_FRAMEWORK.md`](REVIEW_AND_PR_FRAMEWORK.md). - **CD** ([`.github/workflows/deploy.yml`](.github/workflows/deploy.yml)) — on push to `dev`, calls `Sea-Haven-Industries/.github` → `cd-cdk.yaml`, which runs `cdk deploy` on `infra/cdk` (stack `shoc-frontend-dev`, `us-east-1`) diff --git a/REVIEW_AND_PR_FRAMEWORK.md b/REVIEW_AND_PR_FRAMEWORK.md new file mode 100644 index 00000000..977582f4 --- /dev/null +++ b/REVIEW_AND_PR_FRAMEWORK.md @@ -0,0 +1,81 @@ +# REVIEW_AND_PR_FRAMEWORK.md — PR review contract + +Every PR is reviewed against this contract, by humans and by review agents. The +goal is high-signal review: catch real behavior, security, performance, and +regression problems — not re-lint what the gates already enforce. + +## 1. Review the exact head + +- **MUST** review the diff at the **current PR head**, not a stale checkout. + Re-pull before reviewing if new commits landed; stale approvals are dismissed + on push by branch protection. +- **MUST** read the full diff of every changed file, including renames and + generated/mapper code, not only the "interesting" components. + +## 2. Board-backed regression inventory + +- **MUST** check the change against the **Seahaven Jira SH board** inventory — + all visible tickets for this area, not only the current ticket. Behavior that + is Done / QA-approved / released is **protected scope**. +- **MUST** treat a plausible regression against protected behavior as a + **Blocker** until disproven with repo evidence (the accepting tests, the linked + PR/release, and a targeted check of the changed code paths). +- **MUST NOT** approve if board access or the relevant inventory is missing — + say so and block rather than infer a pass. A missing ticket link alone is not + a blocker, but known acceptance criteria must still be traced. + +The current Jira workflow has no `Ready for QA` transition. Merged work remains +in the documented pre-QA status until QA evidence supports `Done`; do not invent +a status or mark unverified work Done. + +## 3. Behavior-based testing + +- **MUST** test through **public behavior** (rendered output, user interactions, + query/mutation outcomes), not internal implementation details. Prefer + `@testing-library` queries and user-event flows; assert what users observe. +- **MUST NOT** add tests whose only purpose is to prove a tool (ESLint, the + governance script) executes — validate tooling by running the real gates + (`npm run verify`), not with assertion-free unit tests. +- **SHOULD** cover the meaningful branches of new logic: the happy path, the + error/empty state, and any boundary the change introduces. + +## 4. Security review + +- **MUST** confirm no raw server error payloads, stack traces, or internal IDs + leak to the UI (see ARCHITECTURE_AND_CODE_QUALITY.md §Security). +- **MUST** confirm no secrets/tokens are introduced into the build, and that no + `VITE_*` variable carries a secret (it is baked into the bundle). +- **SHOULD** check untrusted input is validated (Zod) before use and that + dangerously-set HTML / unescaped server strings are not introduced. + +## 5. Performance and Big-O review + +- **MUST** flag algorithmic regressions in hot paths: `O(n²)`+ loops over server + collections, re-filtering/sorting on every render, unbounded list rendering + without virtualization. +- **MUST** confirm TanStack Query keys are stable and that mutations invalidate + the correct keys (no stale cache, no redundant refetch storms). +- **SHOULD** question speculative `useMemo`/`useCallback` (add when measured) and + unstable identities passed to memoized children. + +## 6. No style-only comments + +- **MUST NOT** leave comments that only restate what Prettier or ESLint already + enforces (formatting, naming nits the linter catches). Style is settled by the + gates; review is for behavior, correctness, security, and architecture. +- **MUST** make every comment actionable: tie it to a behavior, a risk, or an + evidence-based convention in these docs, and offer a concrete fix or a + targeted question. Use GitHub suggestion blocks when safe. + +## 7. Review close-out + +A review is complete when it records, briefly: + +1. Findings ordered by severity (Blocker / Needs-change / Suggestion), or + "no findings". +2. Open questions and their owner. +3. Which of the above checks were run, and any that were skipped (and why). +4. Residual risk, if approving. + +Do not write a monolithic review body or a validation transcript into the PR +surface; keep comments inline and high-signal. diff --git a/docs/FRONTEND_MAINTAINABILITY.md b/docs/FRONTEND_MAINTAINABILITY.md new file mode 100644 index 00000000..9b125ecc --- /dev/null +++ b/docs/FRONTEND_MAINTAINABILITY.md @@ -0,0 +1,115 @@ +# Frontend maintainability conventions + +## Conditional rendering + +Use logical `&&` or the `when` prop on `Text` when JSX has only a rendered state and an empty +state. Use a ternary only when both branches render meaningful alternatives. + +```tsx +{ + error && {error.message}; +} + + + {description} +; +``` + +ESLint rejects `condition ? : null`. This keeps one-sided conditions visually +distinct from real either-or UI decisions. + +The left operand of `&&` must be entirely boolean. `&&` renders its left operand when it is falsy +and non-boolean (notably `{count && }` renders `0`), so a type-aware ESLint rule, +`seahaven/no-non-boolean-jsx-and`, is enforced at `error` across the repository. It asks the +TypeScript checker for the type of the left operand and reports unless every union constituent is +boolean-like, so `boolean | undefined` and `string | null` fail just as `number` does. The rule +fails closed: if type services are unavailable in a governed TSX file it reports rather than +silently claiming safety. The `when` prop on `Text` is typed `boolean`, so TypeScript enforces the +same constraint at that component boundary. + +Approved guard forms (choose by semantics, not as a blind codemod): + +- Presence-only values where falsy means "absent" — errors, optional strings shown only when set + (descriptions, notes, reasons), and optional objects (locations, detail records): coerce with + `Boolean(value)` (or `Boolean(a || b)` for a composite presence test) before `&&`. + `{Boolean(error) && }` and `{Boolean(description) && }` are the canonical forms. +- Values where `0` or `""` is meaningful, or where a non-null value must flow into a typed prop or + helper inside the branch: use an explicit nullish/range comparison so the operand is boolean and + TypeScript can still narrow. `{count > 0 && ...}`, `{value != null && ...}`, and + `{isEdit && id != null && }` keep `0`/`""` semantics and preserve + narrowing. +- Element-slot props (`icon`, `action`, `actions`) are typed `ReactElement`, not `ReactNode`: the + slot holds one element (or fragment), and the render branch coerces with `Boolean(prop)`. Do not + widen these back to `ReactNode`, since a slot is never a meaningful `0`/`""`. + +Never weaken, disable, baseline, or add per-line exceptions to the rule. A new one-sided condition +that needs a non-boolean operand must be rewritten into one of the approved forms above. + +## Typography and feedback + +Use `Text` from `@/components/ui/text` for headings, paragraphs, descriptions, labels, captions, +code, and asynchronous feedback. It owns: + +- semantic HTML for each visual variant; +- the display, body, and monospace font families; +- default, muted, error, success, and warning tones; +- accessible `alert` and `status` live regions for error and feedback text; +- one-sided conditional text through `when`. + +The variant contract is non-overridable: `component`, `role`, and `aria-live` are omitted from +`TextProps` (TypeScript blocks them) and the remaining props are spread before the variant-owned +attributes so the variant element, tone, and live-region role always win at runtime. Use `as` to +choose the rendered element and `tone` to choose the color; do not pass `component`, `role`, or +`aria-live` directly. + +Live-region behavior: + +- `feedback` (polite `status`) keeps the region mounted and toggles its text content via `when`, so + the polite region exists before its content changes and is announced reliably. Mounting an + already-populated status node on demand is not announceable on most screen-reader/browser pairs. +- `error` (assertive `alert`) mounts on demand (`when={false}` unmounts it). Alert-on-mount is the + expected error pattern, so the shipped `when={Boolean(error)}` usages are correct. + +ESLint rejects raw paragraph and heading elements. Existing MUI `Typography` usages remain valid, +but new shared UI should prefer `Text` so semantics and design tokens do not drift. + +The `vp-error` CSS token is presentational chrome for vendor-portal error cards and may only appear +on the `Text` component. ESLint flags any non-`Text` JSX element whose `className` is a static +string literal containing `vp-error` (e.g. `
`, `
`); pair the error message with `variant="error"`. The rule enforces the +static surface only. It cannot resolve dynamic or composed class values +(`className={cn("vp-error", ...)}`, template literals with expressions, or expression-wrapped +strings), so do not compose `vp-error` dynamically to bypass it — prefer `Text variant="error"`. + +## Forms and mutations + +Use the libraries already established in the application: + +- React Hook Form owns field registration, touched/dirty state, and client form lifecycle. +- Zod owns form validation and inferred form value types. +- TanStack Query owns server reads and mutations, including pending/error state, cache + invalidation, and retries where safe. + +Do not add TanStack Form alongside React Hook Form. It would create two form conventions without +removing any current dependency. Reconsider only as a deliberate repository-wide migration with +benchmarks, a codemod plan, and an approved deprecation path. + +File uploads are not ordinary form fields. Keep file selection and client validation in a focused +component, and use a TanStack Query mutation for upload progress, errors, completion refresh, and +retry state. Do not place upload orchestration in a route-sized page component. + +## Page state + +Pages should compose focused state components instead of accumulating unrelated booleans: + +- query loading, error, and empty states stay adjacent to the query result; +- mutation pending/error state belongs to the component that initiated the mutation; +- route pages coordinate sections and navigation; +- reusable sections own their interaction details; +- errors render inline with accessible feedback, with toasts reserved for cross-page outcomes. + +## Enforcement and rollout + +The lint rules are repository-wide and the initial violations were migrated in the same change. +`npm run lint`, `npm run build`, and the `Text` behavior tests are required gates. Future +maintainability rules must also land with a green migration rather than a warning-only backlog. diff --git a/e2e/vendors/vendors.spec.ts b/e2e/vendors/vendors.spec.ts new file mode 100644 index 00000000..29df8921 --- /dev/null +++ b/e2e/vendors/vendors.spec.ts @@ -0,0 +1,410 @@ +import { expect, test, type Page, type Route } from "@playwright/test"; + +const TEST_USER = { + token: "vendor-e2e-token", + expiration: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + email: "admin@seahavenind.com", + userRoles: "Admin", + phoneNumber: "5551234567", + fullname: "Vendor E2E Admin", + id: "1", +}; + +const activeVendors = [ + { + Id: 1, + CompanyId: 101, + CompanyName: "Gateway Plumbing", + ContactName: "Adam Whyte", + Email: "dispatch@gateway.test", + Phone: "314-555-0198", + CompanyPhone: "314-555-0100", + Address: "1 Market St", + City: "St. Louis", + State: "MO", + Zip: "63101", + TradeSpecialties: "Plumbing, Backflow Preventers", + GoogleMapsUrl: "https://maps.google.com/gateway", + Notes: "Preferred after-hours vendor", + TotalJobs: 72, + IsActive: true, + PreferredContact: "Email", + }, + { + Id: 2, + CompanyId: 102, + CompanyName: "Metro HVAC", + ContactName: "Maya Chen", + Email: "maya@metro.test", + Phone: "314-555-0112", + CompanyPhone: "314-555-0110", + Address: "2 Pine St", + City: "Clayton", + State: "MO", + Zip: "63105", + TradeSpecialties: "HVAC", + GoogleMapsUrl: "", + Notes: "", + TotalJobs: 18, + IsActive: true, + }, +]; + +const inactiveVendors = [ + { + ...activeVendors[0], + Id: 3, + CompanyName: "Drain Cleaning Bros", + ContactName: "Drew Banks", + TotalJobs: 151, + IsActive: false, + }, +]; + +interface MockState { + listUrls: string[]; + createdBody?: Record; + updatedBody?: Record; + deletedId?: string; +} + +async function fulfillJson(route: Route, body: unknown, status = 200) { + await route.fulfill({ status, contentType: "application/json", body: JSON.stringify(body) }); +} + +async function captureVisual(page: Page, path: string) { + if (!process.env.VENDOR_VISUAL_CAPTURE) return; + await page.waitForTimeout(400); + await page.screenshot({ path, fullPage: true }); +} + +async function mockVendorApi( + page: Page, + options: { listError?: boolean; deactivationBlocked?: boolean; deleteConflict?: boolean } = {}, +) { + const state: MockState = { listUrls: [] }; + + await page.addInitScript((user) => localStorage.setItem("auth", JSON.stringify(user)), TEST_USER); + + await page.route("**/api/DropdownOptions/ByCategory/Trade*", (route) => + fulfillJson(route, { + data: [ + { id: 1, value: "Plumbing" }, + { id: 2, value: "HVAC" }, + ], + }), + ); + await page.route("**/api/Vendor/facets*", (route) => + fulfillJson(route, { + data: { + companies: activeVendors.map((vendor) => ({ + companyId: vendor.CompanyId, + name: vendor.CompanyName, + companyPhone: vendor.CompanyPhone, + email: vendor.Email, + address: vendor.Address, + city: vendor.City, + state: vendor.State, + zip: vendor.Zip, + googleMapsUrl: vendor.GoogleMapsUrl, + })), + trades: ["Plumbing", "Backflow Preventers", "HVAC"], + locations: [ + { city: "St. Louis", state: "MO", label: "St. Louis, MO" }, + { city: "Clayton", state: "MO", label: "Clayton, MO" }, + ], + jobBuckets: [ + { id: "under-50", label: "Under 50" }, + { id: "50-99", label: "50–99" }, + { id: "100-149", label: "100–149" }, + { id: "150-plus", label: "150+" }, + ], + }, + }), + ); + await page.route("**/api/Vendor/GetVendorList*", async (route) => { + const url = new URL(route.request().url()); + state.listUrls.push(url.toString()); + if (options.listError && url.searchParams.get("pageSize") !== "1") { + await fulfillJson(route, { message: "Vendor directory unavailable" }, 500); + return; + } + + const isActive = url.searchParams.get("isActive") !== "false"; + const source = isActive ? activeVendors : inactiveVendors; + const search = (url.searchParams.get("search") ?? "").toLowerCase(); + const company = url.searchParams.get("companies[0]"); + const filtered = source.filter( + (vendor) => + (!search || + `${vendor.CompanyName} ${vendor.ContactName} ${vendor.TradeSpecialties}` + .toLowerCase() + .includes(search)) && + (!company || vendor.CompanyName === company), + ); + const isCount = url.searchParams.get("pageSize") === "1"; + await fulfillJson(route, { + Data: isCount ? filtered.slice(0, 1) : filtered, + TotalCount: filtered.length, + PageNumber: 1, + PageSize: Number(url.searchParams.get("pageSize") ?? 12), + }); + }); + await page.route(/\/api\/vendors\/\d+\/deactivation-impact$/, async (route) => { + const vendorId = Number(route.request().url().split("/").at(-2)); + await fulfillJson(route, { + data: { + vendorId, + canDeactivate: !options.deactivationBlocked, + openWorkOrders: options.deactivationBlocked + ? [ + { + workOrderId: 501, + workOrderNumber: "WO-501", + workOrderTitle: "Emergency boiler repair", + status: "Dispatched", + scheduledDate: "2026-07-24T14:00:00Z", + dispatchId: 801, + }, + ] + : [], + }, + }); + }); + await page.route(/\/api\/vendors\/\d+$/, async (route) => { + const id = route.request().url().split("/").pop() ?? ""; + if (route.request().method() === "PUT") { + state.updatedBody = route.request().postDataJSON(); + await fulfillJson(route, { + data: { ...activeVendors[0], ...state.updatedBody, Id: Number(id) }, + }); + return; + } + if (route.request().method() === "DELETE") { + if (options.deleteConflict) { + await fulfillJson( + route, + { message: "Vendor has open work orders and cannot be deactivated." }, + 409, + ); + return; + } + state.deletedId = id; + await fulfillJson(route, { status: "200" }); + return; + } + const vendor = [...activeVendors, ...inactiveVendors].find((item) => String(item.Id) === id); + await fulfillJson(route, { data: vendor }); + }); + await page.route(/\/api\/vendors\/?$/, async (route) => { + if (route.request().method() !== "POST") { + await route.fallback(); + return; + } + state.createdBody = route.request().postDataJSON(); + await fulfillJson(route, { data: { ...state.createdBody, id: 99, totalJobs: 0 } }); + }); + + return state; +} + +test.describe("Vendor directory prototype parity", () => { + test("covers status tabs, global counts, search, facets, table fields, and filtered footer", async ({ + page, + }) => { + const state = await mockVendorApi(page); + await page.goto("/vendors"); + + await expect(page.getByRole("heading", { name: "Vendors / Technicians" })).toBeVisible(); + await expect(page.getByText("3 technicians · 2 active")).toBeVisible(); + await expect(page.getByRole("tab", { name: "Active (2)" })).toHaveAttribute( + "aria-selected", + "true", + ); + for (const heading of [ + "Company", + "Technician", + "Location", + "Trade", + "Phone", + "Email", + "Total Jobs", + "Status", + ]) { + await expect(page.getByRole("columnheader", { name: heading })).toBeVisible(); + } + await expect(page.getByText("Gateway Plumbing")).toBeVisible(); + await expect(page.getByText("+1")).toBeVisible(); + await expect(page.getByText("2 active technicians · 2 active · 1 inactive")).toBeVisible(); + await captureVisual(page, "/tmp/seahaven-vendor-implemented-list.png"); + + await page.getByPlaceholder("Search name, company, trade...").fill("metro"); + await expect(page.getByText("Metro HVAC")).toBeVisible(); + await expect(page.getByText(/1 active technician · filtered/)).toBeVisible(); + expect(state.listUrls.some((url) => new URL(url).searchParams.get("search") === "metro")).toBe( + true, + ); + + await page.getByPlaceholder("Search name, company, trade...").clear(); + await page.getByRole("button", { name: "Open vendor filters" }).click(); + await expect(page.getByRole("heading", { name: "Filters" })).toBeVisible(); + await captureVisual(page, "/tmp/seahaven-vendor-implemented-filters.png"); + await page.getByRole("button", { name: /Company/ }).click(); + await page.getByPlaceholder("All companies").click(); + await page.getByRole("option", { name: /Gateway Plumbing/ }).click(); + await page.getByRole("button", { name: "Apply" }).click(); + await expect(page.getByText("Gateway Plumbing").first()).toBeVisible(); + await expect(page.getByText(/1 active technician · filtered/)).toBeVisible(); + expect( + state.listUrls.some( + (url) => new URL(url).searchParams.get("companies[0]") === "Gateway Plumbing", + ), + ).toBe(true); + + await page.getByRole("button", { name: "Deactivate vendor" }).first().click(); + await expect(page.getByRole("dialog", { name: "Deactivate Vendor" })).toContainText( + "history will be preserved", + ); + await page.getByRole("button", { name: "Deactivate", exact: true }).click(); + await expect.poll(() => state.deletedId).toBe("1"); + + await page.getByRole("tab", { name: "Inactive (1)" }).click(); + await expect(page.getByText("No vendors match the current filters.")).toBeVisible(); + await page.getByRole("button", { name: "Clear all" }).click(); + await expect(page.getByText("Drain Cleaning Bros")).toBeVisible(); + await expect(page.getByRole("button", { name: "Deactivate vendor" })).toHaveCount(0); + }); + + test("creates with company autofill, then views and edits every detail field", async ({ + page, + }) => { + const state = await mockVendorApi(page); + await page.goto("/vendors"); + + await page.getByRole("button", { name: "+ Add Vendor" }).click(); + await expect(page.getByText("Register a technician and their vendor company.")).toBeVisible(); + await captureVisual(page, "/tmp/seahaven-vendor-implemented-add.png"); + await expect(page.getByRole("button", { name: "Add Vendor" }).last()).toBeDisabled(); + + await page.getByLabel("Technician Name").fill("New Technician"); + await page.getByRole("combobox", { name: "Company" }).click(); + await page.getByRole("option", { name: "Gateway Plumbing" }).click(); + await expect(page.getByLabel("Company Phone")).toHaveValue("314-555-0100"); + await expect(page.getByRole("textbox", { name: "Email" })).toHaveValue("dispatch@gateway.test"); + await expect(page.getByLabel("Address")).toHaveValue("1 Market St"); + await expect(page.getByLabel("Google Maps URL")).toHaveValue("https://maps.google.com/gateway"); + await page.getByRole("combobox", { name: "Add Trade" }).fill("Plumbing"); + await page.getByRole("button", { name: "Add trade" }).click(); + await page.getByRole("combobox", { name: "Add Trade" }).fill("HVAC"); + await page.getByRole("button", { name: "Add trade" }).click(); + await page.getByRole("button", { name: "Move HVAC up" }).click(); + await page.getByLabel("Notes").fill("Created in browser E2E"); + await page.getByRole("button", { name: "Text" }).click(); + await expect(page.getByRole("button", { name: "Text" })).toHaveAttribute( + "aria-pressed", + "true", + ); + await page.getByRole("button", { name: "Add Vendor" }).last().click(); + await expect(page.getByRole("dialog", { name: "Add Vendor" })).toHaveCount(0); + expect(state.createdBody).toMatchObject({ + companyName: "Gateway Plumbing", + companyId: 101, + contactName: "New Technician", + companyPhone: "(314) 555-0100", + preferredContact: "Text", + tradeSpecialties: "HVAC, Plumbing", + notes: "Created in browser E2E", + }); + + await page.getByRole("button", { name: "View vendor Gateway Plumbing" }).click(); + const detailDrawer = page.locator(".MuiDrawer-paper").last(); + await expect(detailDrawer.getByRole("heading", { name: "Gateway Plumbing" })).toBeVisible(); + await expect(detailDrawer.getByText("Adam Whyte")).toBeVisible(); + await expect(detailDrawer.getByText("72")).toBeVisible(); + await expect( + detailDrawer.getByRole("link", { name: "Open in Google Maps" }).first(), + ).toHaveAttribute("target", "_blank"); + await captureVisual(page, "/tmp/seahaven-vendor-implemented-detail.png"); + + await detailDrawer.getByRole("button", { name: "Edit" }).click(); + await expect(detailDrawer.getByLabel("Total Jobs")).toHaveValue("72"); + await expect(detailDrawer.getByLabel("Total Jobs")).toHaveAttribute("readonly"); + await detailDrawer.getByLabel("Notes").fill("Updated in browser E2E"); + await detailDrawer.getByRole("switch", { name: "Active" }).click(); + await detailDrawer.getByRole("button", { name: "Save" }).click(); + await expect.poll(() => state.updatedBody?.notes).toBe("Updated in browser E2E"); + expect(state.updatedBody?.isActive).toBe(false); + }); + + test("supports keyboard dismissal and free-text company creation", async ({ page }) => { + const state = await mockVendorApi(page); + await page.goto("/vendors"); + + const addButton = page.getByRole("button", { name: "+ Add Vendor" }); + await addButton.focus(); + await page.keyboard.press("Enter"); + await expect(page.getByRole("dialog", { name: /Add Vendor/ })).toBeVisible(); + await page.keyboard.press("Escape"); + await expect(page.getByRole("dialog", { name: /Add Vendor/ })).toHaveCount(0); + await expect(addButton).toBeFocused(); + + await addButton.click(); + await expect(page.getByRole("button", { name: "Add Vendor" }).last()).toBeDisabled(); + await page.getByLabel("Technician Name").fill("Independent Tech"); + await page.getByRole("combobox", { name: "Company" }).fill("Independent Vendor LLC"); + await page.getByRole("button", { name: "Add Vendor" }).last().click(); + + await expect.poll(() => state.createdBody?.companyName).toBe("Independent Vendor LLC"); + expect(state.createdBody?.companyId).toBeNull(); + }); + + test("blocks deactivation for linked work orders and preserves the vendor on a raced 409", async ({ + page, + }) => { + const blockedState = await mockVendorApi(page, { deactivationBlocked: true }); + await page.goto("/vendors"); + + await page.getByRole("button", { name: "Deactivate vendor" }).first().click(); + const blockedDialog = page.getByRole("dialog", { name: "Deactivate Vendor" }); + await expect(blockedDialog).toContainText("WO-501 — Emergency boiler repair"); + await expect(blockedDialog.getByRole("button", { name: /^Deactivate$/ })).toBeDisabled(); + expect(blockedState.deletedId).toBeUndefined(); + + await blockedDialog.getByRole("button", { name: "Cancel" }).click(); + await page.unrouteAll({ behavior: "wait" }); + + const racedState = await mockVendorApi(page, { deleteConflict: true }); + await page.reload(); + await page.getByRole("button", { name: "Deactivate vendor" }).first().click(); + await page + .getByRole("dialog", { name: "Deactivate Vendor" }) + .getByRole("button", { + name: /^Deactivate$/, + }) + .click(); + + await expect(page.getByRole("dialog", { name: "Deactivate Vendor" })).toContainText( + /open work orders|conflict/i, + ); + expect(racedState.deletedId).toBeUndefined(); + await expect(page.getByText("Gateway Plumbing").first()).toBeVisible(); + }); + + test("shows API failure and keeps drawers usable at a mobile viewport", async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await mockVendorApi(page, { listError: true }); + await page.goto("/vendors"); + + await expect(page.getByRole("main").getByRole("alert")).toContainText( + /Vendor directory unavailable|failed|server error/i, + { timeout: 15_000 }, + ); + await page.getByRole("button", { name: "Open vendor filters" }).click(); + const filterDrawer = page.locator(".MuiDrawer-paper").filter({ hasText: "Total Jobs" }); + await expect(filterDrawer).toBeVisible(); + const box = await filterDrawer.boundingBox(); + expect(box?.width).toBeLessThanOrEqual(391); + await captureVisual(page, "/tmp/seahaven-vendor-implemented-mobile.png"); + await page.getByRole("button", { name: "Close filters" }).click(); + }); +}); diff --git a/e2e/work-orders/vendor-inline-editor.spec.ts b/e2e/work-orders/vendor-inline-editor.spec.ts new file mode 100644 index 00000000..95639b66 --- /dev/null +++ b/e2e/work-orders/vendor-inline-editor.spec.ts @@ -0,0 +1,90 @@ +import { expect, test, type Route } from "@playwright/test"; + +const TEST_USER = { + token: "work-order-vendor-e2e-token", + expiration: new Date(Date.now() + 60 * 60 * 1000).toISOString(), + email: "admin@seahavenind.com", + userRoles: "Admin", + phoneNumber: "5551234567", + fullname: "Work Order E2E Admin", + id: "1", +}; + +async function fulfillJson(route: Route, body: unknown, status = 200) { + await route.fulfill({ status, contentType: "application/json", body: JSON.stringify(body) }); +} + +test("edits only the vendor assigned to the current work order", async ({ page }) => { + let updateRequest: { url: string; body: Record } | undefined; + + await page.addInitScript((user) => localStorage.setItem("auth", JSON.stringify(user)), TEST_USER); + await page.route("**/api/WorkOrder/GetWorkorderById*", (route) => + fulfillJson(route, { + Id: 501, + InternalWONumber: "WO-501", + WorkerOrderNumber: "CUSTOMER-501", + WorkerOrderTitle: "Emergency boiler repair", + Status: "Dispatched", + Comments: [], + AuditLog: [], + Dispatches: [ + { + Id: 801, + DispatchNumber: "D-801", + VendorId: 7, + VendorName: "Gateway Plumbing", + VendorEmail: "dispatch@gateway.test", + Status: "Sent", + DispatchedAt: "2026-07-23T12:00:00Z", + }, + ], + }), + ); + await page.route("**/api/vendors/7/work-order-update", async (route) => { + updateRequest = { + url: route.request().url(), + body: route.request().postDataJSON() as Record, + }; + await fulfillJson(route, { data: { id: 7 } }); + }); + await page.route("**/api/vendors/7", (route) => + fulfillJson(route, { + data: { + Id: 7, + CompanyName: "Gateway Plumbing", + ContactName: "Adam Whyte", + PreferredContact: "Email", + Phone: "3145550198", + Email: "dispatch@gateway.test", + Notes: "Preferred after-hours vendor", + IsActive: true, + }, + }), + ); + await page.route("**/api/User*", (route) => fulfillJson(route, { data: [] })); + + await page.goto("/workorders/501"); + await expect(page.getByRole("heading", { name: "Emergency boiler repair" })).toBeVisible(); + await page.getByRole("button", { name: "Edit vendor" }).click(); + + await expect(page.getByRole("dialog", { name: "Edit assigned vendor" })).toBeVisible(); + await expect(page.getByLabel("Technician name")).toHaveValue("Adam Whyte"); + await page.getByLabel("Technician name").fill("Taylor Tech"); + await page.getByLabel("Preferred contact").click(); + await page.getByRole("option", { name: "Text" }).click(); + await page.getByLabel("Phone").fill("3145550111"); + await page.getByLabel("Email").fill("taylor@example.com"); + await page.getByLabel("Notes").fill("Text before arrival"); + await page.getByRole("button", { name: "Save" }).click(); + + await expect(page.getByRole("dialog", { name: "Edit assigned vendor" })).toBeHidden(); + expect(updateRequest?.url).toMatch(/\/api\/vendors\/7\/work-order-update$/); + expect(updateRequest?.body).toEqual({ + workOrderId: 501, + contactName: "Taylor Tech", + preferredContact: "Text", + phone: "(314) 555-0111", + email: "taylor@example.com", + notes: "Text before arrival", + }); +}); diff --git a/eslint-rules/no-non-boolean-jsx-and.js b/eslint-rules/no-non-boolean-jsx-and.js new file mode 100644 index 00000000..10408424 --- /dev/null +++ b/eslint-rules/no-non-boolean-jsx-and.js @@ -0,0 +1,95 @@ +import ts from "typescript"; + +/** + * Local type-aware rule: the left operand of `&&` in JSX must be entirely + * boolean. + * + * `{value && }` renders its left operand when it is falsy and + * non-boolean (notably `{count && }` renders `0`), so the left operand + * must be `boolean` (or boolean literals) across the whole union. The rule + * asks the TypeScript checker for the type of the left operand and reports + * unless every union constituent is boolean-like. + * + * Type-aware by design: a selector that cannot see operand types would give + * false confidence rather than enforcement. If type services are unexpectedly + * unavailable in a governed TSX file, the rule fails closed (reports) instead + * of silently claiming the expression is safe. + */ +const booleanLikeFlags = ts.TypeFlags.Boolean | ts.TypeFlags.BooleanLiteral; + +function isBooleanLikeType(type) { + return (type.flags & booleanLikeFlags) !== 0; +} + +function isEntirelyBoolean(type) { + if (type.isUnion()) { + return type.types.every((constituent) => isBooleanLikeType(constituent)); + } + return isBooleanLikeType(type); +} + +const transparentExpressionTypes = new Set([ + "ChainExpression", + "ParenthesizedExpression", + "TSAsExpression", + "TSNonNullExpression", + "TSTypeAssertion", +]); + +function isInRenderedPosition(node) { + let current = node; + while (current.parent) { + const parent = current.parent; + if (parent.type === "JSXExpressionContainer") { + return parent.parent?.type !== "JSXAttribute"; + } + if (parent.type === "LogicalExpression" || transparentExpressionTypes.has(parent.type)) { + current = parent; + continue; + } + return false; + } + return false; +} + +const rule = { + meta: { + type: "problem", + docs: { + description: "Require the left operand of `&&` in JSX to be entirely boolean", + }, + schema: [], + messages: { + nonBooleanLeft: + 'The left operand of `&&` in JSX must be boolean. Non-boolean falsy operands (e.g. `0`, `""`) render into the DOM; coerce with `Boolean(...)` or `!!` before `&&`.', + typeServicesUnavailable: + "Type-aware boolean-safety check is unavailable for this JSX expression. This file must be part of a TypeScript project with type information so the rule can enforce safely.", + }, + }, + create(context) { + const services = context.sourceCode?.parserServices ?? context.parserServices; + + return { + LogicalExpression(node) { + if (node.operator !== "&&") { + return; + } + if (!isInRenderedPosition(node)) { + return; + } + + if (!services || services.program === null) { + context.report({ node, messageId: "typeServicesUnavailable" }); + return; + } + + const leftType = services.getTypeAtLocation(node.left); + if (!isEntirelyBoolean(leftType)) { + context.report({ node: node.left, messageId: "nonBooleanLeft" }); + } + }, + }; + }, +}; + +export default rule; diff --git a/eslint-rules/no-vp-error-outside-text.js b/eslint-rules/no-vp-error-outside-text.js new file mode 100644 index 00000000..3498f5ea --- /dev/null +++ b/eslint-rules/no-vp-error-outside-text.js @@ -0,0 +1,55 @@ +function containsVpErrorToken(node) { + if (!node || typeof node !== "object") { + return false; + } + + if (node.type === "Literal" && typeof node.value === "string") { + return node.value.split(/\s+/u).includes("vp-error"); + } + + if (node.type === "TemplateElement") { + return node.value.raw.split(/\s+/u).includes("vp-error"); + } + + return Object.entries(node).some(([key, value]) => { + if (key === "parent") { + return false; + } + if (Array.isArray(value)) { + return value.some(containsVpErrorToken); + } + return containsVpErrorToken(value); + }); +} + +const rule = { + meta: { + type: "problem", + docs: { + description: "Require the Text component for legacy vp-error styling", + }, + schema: [], + messages: { + useText: + 'Use the Text component for the vp-error class; pair the error message with variant="error" so feedback stays accessible and consistent.', + }, + }, + create(context) { + return { + JSXAttribute(node) { + if (node.name?.name !== "className" || !containsVpErrorToken(node.value)) { + return; + } + + const elementName = node.parent?.name; + if (elementName?.type === "JSXIdentifier" && elementName.name === "Text") { + return; + } + + context.report({ node, messageId: "useText" }); + }, + }; + }, +}; + +export default rule; diff --git a/eslint.config.js b/eslint.config.js index 8d1f29c2..10c0aa46 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -5,6 +5,16 @@ import pluginReactRefresh from "eslint-plugin-react-refresh"; import globals from "globals"; import tseslint from "typescript-eslint"; +import noNonBooleanJsxAnd from "./eslint-rules/no-non-boolean-jsx-and.js"; +import noVpErrorOutsideText from "./eslint-rules/no-vp-error-outside-text.js"; + +const localRules = { + rules: { + "no-non-boolean-jsx-and": noNonBooleanJsxAnd, + "no-vp-error-outside-text": noVpErrorOutsideText, + }, +}; + const legacyIgnores = [ "src/pages/**", "src/app/store.js", @@ -54,6 +64,7 @@ export default tseslint.config( plugins: { "react-hooks": pluginReactHooks, "react-refresh": pluginReactRefresh, + seahaven: localRules, }, rules: { ...pluginReactHooks.configs.recommended.rules, @@ -62,6 +73,23 @@ export default tseslint.config( "react-refresh/only-export-components": ["warn", { allowConstantExport: true }], "@typescript-eslint/no-unused-vars": ["error", { argsIgnorePattern: "^_" }], "@typescript-eslint/no-explicit-any": "warn", + "seahaven/no-non-boolean-jsx-and": "error", + "seahaven/no-vp-error-outside-text": "error", + "no-restricted-syntax": [ + "error", + { + selector: + "JSXExpressionContainer > ConditionalExpression[alternate.type='Literal'][alternate.value=null]", + message: + "Use logical AND for one-sided JSX rendering instead of `condition ? element : null`.", + }, + { + selector: + ":matches(JSXOpeningElement[name.name='p'], JSXOpeningElement[name.name='h1'], JSXOpeningElement[name.name='h2'], JSXOpeningElement[name.name='h3'], JSXOpeningElement[name.name='h4'], JSXOpeningElement[name.name='h5'], JSXOpeningElement[name.name='h6'])", + message: + "Use the shared Text component so typography semantics, family, tone, and feedback behavior stay consistent.", + }, + ], }, }, { diff --git a/package.json b/package.json index ee9bb861..3df2f4da 100644 --- a/package.json +++ b/package.json @@ -11,10 +11,12 @@ "test:watch": "vitest", "test:e2e": "playwright test", "test:e2e:ui": "playwright test --ui", - "lint": "eslint .", - "lint:fix": "eslint . --fix", + "lint": "eslint . --max-warnings=0", + "lint:fix": "eslint . --fix --max-warnings=0", "format": "prettier --write .", "format:check": "prettier --check .", + "governance": "node scripts/governance-check.mjs", + "verify": "npm run format:check && npm run lint && npm run build && npm test && npm run governance", "prepare": "husky" }, "lint-staged": { diff --git a/scripts/governance-baseline.json b/scripts/governance-baseline.json new file mode 100644 index 00000000..197c24da --- /dev/null +++ b/scripts/governance-baseline.json @@ -0,0 +1,32 @@ +{ + "version": 1, + "purpose": "Frozen grandfathered debt inventory for the frontend governance checks. New entries and cap increases fail the governance comparison. Existing caps may only decrease and entries must be removed when compliant.", + "maxFileLines": 500, + "godfileDebt": [ + { + "path": "src/app/(protected)/workorders/[id].tsx", + "maxLines": 797, + "reason": "Work-order detail route page; legacy godfile targeted for decomposition into focused state components." + }, + { + "path": "src/app/(protected)/workorders/_components/dispatch-detail-modal.tsx", + "maxLines": 782, + "reason": "Dispatch detail modal; oversized legacy component pending extraction of sections/fields." + }, + { + "path": "src/app/v/[token]/dispatch/[id].tsx", + "maxLines": 651, + "reason": "Vendor-portal dispatch detail route; legacy page pending decomposition." + }, + { + "path": "src/app/(protected)/vendors/index.tsx", + "maxLines": 612, + "reason": "Vendors list route; legacy page with mixed query/filter/table state pending extraction." + }, + { + "path": "src/domain/work-orders/mappers/work-order-mapper.ts", + "maxLines": 572, + "reason": "API<->domain mapper; long but cohesive mapping logic, pending split by responsibility." + } + ] +} diff --git a/scripts/governance-check.mjs b/scripts/governance-check.mjs new file mode 100644 index 00000000..41bb1de9 --- /dev/null +++ b/scripts/governance-check.mjs @@ -0,0 +1,289 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); +const ROOT = path.resolve(SCRIPT_DIR, ".."); +const BASELINE_PATH = path.join(SCRIPT_DIR, "governance-baseline.json"); + +const MAX_FILE_LINES = 500; +const MAINTAINABILITY_RULES = [ + 'complexity: ["error", { "max": 20 }]', + 'max-lines-per-function: ["error", { "skipComments": true, "max": 150 }]', + 'max-params: ["error", 4]', + 'max-depth: ["error", 4]', +]; +const GOVERNED_ROOTS = ["src/", "config/"]; +const EXCLUDE_DIR = /(^|\/)(mocks|test|__mocks__|node_modules|dist|coverage|e2e)\//; +const EXCLUDE_NAME = /\.(mock|test|spec)\.(ts|tsx)$|\.d\.ts$/; + +function isGoverned(relativePath) { + return ( + GOVERNED_ROOTS.some((root) => relativePath.startsWith(root)) && + /\.(ts|tsx)$/.test(relativePath) && + !EXCLUDE_DIR.test(relativePath) && + !EXCLUDE_NAME.test(relativePath) + ); +} + +function gitText(args) { + return execFileSync("git", args, { cwd: ROOT, encoding: "utf8" }).trim(); +} + +function gitLines(args) { + return gitText(args).split("\n").filter(Boolean); +} + +function governedFiles() { + const tracked = gitLines(["ls-files"]); + const untracked = gitLines(["ls-files", "--others", "--exclude-standard"]); + return [...new Set([...tracked, ...untracked])].filter(isGoverned); +} + +function lineCount(relativePath) { + const content = readFileSync(path.join(ROOT, relativePath), "utf8"); + if (content.length === 0) return 0; + return content.endsWith("\n") ? content.split("\n").length - 1 : content.split("\n").length; +} + +function readBaseline() { + return JSON.parse(readFileSync(BASELINE_PATH, "utf8")); +} + +function readBaselineAtRef(ref) { + try { + const content = execFileSync("git", ["show", `${ref}:scripts/governance-baseline.json`], { + cwd: ROOT, + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }); + return JSON.parse(content); + } catch { + return null; + } +} + +function godfileRatchet(baseRef) { + const baseline = readBaseline(); + const cap = baseline.maxFileLines ?? MAX_FILE_LINES; + const debtEntries = new Map( + (baseline.godfileDebt ?? []).map((entry) => [entry.path, entry.maxLines]), + ); + const files = governedFiles(); + + const newDebt = []; + const grownDebt = []; + for (const file of files) { + const lines = lineCount(file); + const debtCap = debtEntries.get(file); + if (lines > cap && debtCap === undefined) { + newDebt.push({ path: file, lines }); + } else if (debtCap !== undefined && lines > debtCap) { + grownDebt.push({ path: file, lines, maxLines: debtCap }); + } + } + + const stale = []; + const remaining = []; + for (const [debtPath, maxLines] of debtEntries) { + const lines = files.includes(debtPath) ? lineCount(debtPath) : -1; + if (lines === -1 || lines <= cap) { + stale.push({ path: debtPath, lines }); + } else { + remaining.push({ path: debtPath, lines, maxLines }); + } + } + + const baselineLoosening = []; + const baseBaseline = baseRef ? readBaselineAtRef(baseRef) : null; + if (baseBaseline) { + const baseCap = baseBaseline.maxFileLines ?? MAX_FILE_LINES; + if (cap > baseCap) { + baselineLoosening.push(`global cap increased from ${baseCap} to ${cap}`); + } + const baseEntries = new Map( + (baseBaseline.godfileDebt ?? []).map((entry) => [entry.path, entry.maxLines]), + ); + for (const [debtPath, maxLines] of debtEntries) { + const priorMax = baseEntries.get(debtPath); + if (priorMax === undefined) { + baselineLoosening.push(`new debt entry: ${debtPath}`); + } else if (maxLines > priorMax) { + baselineLoosening.push(`cap increased for ${debtPath}: ${priorMax} -> ${maxLines}`); + } + } + } + + return { + cap, + newDebt, + grownDebt, + stale, + remaining, + baselineLoosening, + comparedBaseline: Boolean(baseBaseline), + }; +} + +function resolveBaseRef() { + if (process.env.GOVERNANCE_BASE) return process.env.GOVERNANCE_BASE; + if (process.env.GITHUB_BASE_REF) return `origin/${process.env.GITHUB_BASE_REF}`; + for (const candidate of ["origin/dev", "origin/main"]) { + try { + execFileSync("git", ["rev-parse", "--verify", candidate], { + cwd: ROOT, + encoding: "utf8", + stdio: "ignore", + }); + return candidate; + } catch { + // candidate ref not present locally; try the next + } + } + return null; +} + +function changedGovernedFiles(baseRef) { + let mergeBase; + try { + mergeBase = execFileSync("git", ["merge-base", baseRef, "HEAD"], { + cwd: ROOT, + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + } catch { + return null; + } + const diffed = gitLines(["diff", "--name-only", "--diff-filter=AMR", mergeBase, "HEAD"]); + const untracked = gitLines(["ls-files", "--others", "--exclude-standard"]); + return [...new Set([...diffed, ...untracked])].filter(isGoverned); +} + +function maintainabilityGate(files) { + if (files.length === 0) { + return { skipped: true, reason: "no changed governed TS/TSX files" }; + } + const eslintBin = path.join(ROOT, "node_modules", ".bin", "eslint"); + const ruleArgs = MAINTAINABILITY_RULES.flatMap((rule) => ["--rule", rule]); + const result = spawnSync( + eslintBin, + [ + ...files, + ...ruleArgs, + "--max-warnings=0", + "--no-warn-ignored", + "--no-error-on-unmatched-pattern", + ], + { cwd: ROOT, encoding: "utf8" }, + ); + return { + skipped: false, + status: result.status, + stdout: result.stdout?.trim() ?? "", + stderr: result.stderr?.trim() ?? "", + files, + }; +} + +function plural(count, word) { + return `${count} ${word}${count === 1 ? "" : "s"}`; +} + +function main() { + const failures = []; + const baseRef = resolveBaseRef(); + if (!baseRef) { + failures.push( + "base ref is required but was not found. Set GOVERNANCE_BASE to a valid commit or fetch origin/dev.", + ); + } else { + try { + gitText(["merge-base", baseRef, "HEAD"]); + } catch { + failures.push( + `base ref '${baseRef}' cannot be resolved against HEAD. Fetch it or set GOVERNANCE_BASE correctly.`, + ); + } + } + + console.log("─".repeat(64)); + console.log("godfile ratchet: legacy caps may only shrink"); + const god = godfileRatchet(baseRef); + console.log( + ` cap: ${god.cap} lines | grandfathered debt: ${plural(god.remaining.length, "file")} | new violations: ${god.newDebt.length}`, + ); + for (const entry of god.remaining) { + console.log(` debt ${String(entry.lines).padStart(4)}/${entry.maxLines} ${entry.path}`); + } + for (const entry of god.newDebt) { + console.log(` NEW ${String(entry.lines).padStart(4)} ${entry.path}`); + } + for (const entry of god.grownDebt) { + console.log(` GREW ${String(entry.lines).padStart(4)}/${entry.maxLines} ${entry.path}`); + } + if (god.newDebt.length > 0) { + failures.push( + `godfile ratchet: ${plural(god.newDebt.length, "file")} exceed ${god.cap} lines. Refactor them under the cap; new baseline debt is forbidden.`, + ); + } + if (god.grownDebt.length > 0) { + failures.push( + `godfile ratchet: ${plural(god.grownDebt.length, "grandfathered file")} exceeded its frozen line cap.`, + ); + } + if (god.baselineLoosening.length > 0) { + failures.push( + `governance baseline was loosened: ${god.baselineLoosening.join("; ")}. Only cap reductions and entry removals are allowed.`, + ); + } + if (!god.comparedBaseline) { + console.log(" baseline comparison unavailable (initial adoption or missing base file)"); + } + if (god.stale.length > 0) { + console.log(` stale baseline entries (now compliant — remove to ratchet tighter):`); + for (const entry of god.stale) { + console.log(` stale ${entry.path}`); + } + } + + console.log("─".repeat(64)); + if (!baseRef) { + console.log("changed-file maintainability gate: FAIL (no valid base ref)"); + } else { + const files = changedGovernedFiles(baseRef); + console.log( + `changed-file maintainability gate (base: ${baseRef}): ${files === null ? "unresolvable" : plural(files.length, "changed governed file")}`, + ); + if (files === null) { + console.log(" failed — base ref could not be resolved against HEAD"); + } else { + const gate = maintainabilityGate(files); + if (gate.skipped) { + console.log(` skipped — ${gate.reason}`); + } else { + const clean = gate.status === 0; + console.log( + ` result: ${clean ? "PASS" : "FAIL"} (complexity<=20, function<=150 lines, params<=4, depth<=4)`, + ); + if (!clean) { + if (gate.stdout) console.log(gate.stdout); + if (gate.stderr) console.log(gate.stderr); + failures.push( + "changed-file maintainability gate: see ESLint output above. Extract functions/components to meet the thresholds; do not relax the thresholds.", + ); + } + } + } + } + + console.log("─".repeat(64)); + if (failures.length > 0) { + console.log(`RESULT: FAIL (${plural(failures.length, "gate")})`); + for (const failure of failures) console.log(` - ${failure}`); + process.exit(1); + } + console.log("RESULT: PASS — all governance gates green"); +} + +main(); diff --git a/src/api/api-paths.ts b/src/api/api-paths.ts index 83a6b6aa..9b2f25eb 100644 --- a/src/api/api-paths.ts +++ b/src/api/api-paths.ts @@ -109,6 +109,8 @@ export const API_PATHS = { update: "Vendor/Update", delete: "Vendor/Delete", dropdown: "Vendor/Dropdown", + facets: "Vendor/facets", + deactivationImpact: "deactivation-impact", }, calendar: { getEventsForCalendar: "Calendar/GetEventsForCalendar", diff --git a/src/app/(auth)/_components/auth-card-header.tsx b/src/app/(auth)/_components/auth-card-header.tsx index 795ca0a7..aaf95cf6 100644 --- a/src/app/(auth)/_components/auth-card-header.tsx +++ b/src/app/(auth)/_components/auth-card-header.tsx @@ -1,5 +1,6 @@ import type { ComponentPropsWithoutRef } from "react"; +import { Text } from "@/components/ui/text"; import { cn } from "@/lib/utils"; export type AuthCardHeaderProps = ComponentPropsWithoutRef<"div"> & { @@ -10,12 +11,17 @@ export type AuthCardHeaderProps = ComponentPropsWithoutRef<"div"> & { export function AuthCardHeader({ title, subtitle, className, ...props }: AuthCardHeaderProps) { return (
-

{title}

- {subtitle && ( -

- {subtitle} -

- )} + + {title} + + + {subtitle} +
); } diff --git a/src/app/(auth)/_components/login-form.tsx b/src/app/(auth)/_components/login-form.tsx index e06ba713..3e3d8456 100644 --- a/src/app/(auth)/_components/login-form.tsx +++ b/src/app/(auth)/_components/login-form.tsx @@ -75,7 +75,9 @@ export function LoginForm({ disabled={isLoggingIn} error={Boolean(errors.password)} /> - {loginError && } + {Boolean(loginError) && ( + + )} - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load account"} diff --git a/src/app/(protected)/accounts/index.tsx b/src/app/(protected)/accounts/index.tsx index f4aea5da..7697c764 100644 --- a/src/app/(protected)/accounts/index.tsx +++ b/src/app/(protected)/accounts/index.tsx @@ -118,7 +118,7 @@ export default function AccountsListPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load accounts"} diff --git a/src/app/(protected)/assets/_components/asset-form.tsx b/src/app/(protected)/assets/_components/asset-form.tsx index d01ae284..513f1837 100644 --- a/src/app/(protected)/assets/_components/asset-form.tsx +++ b/src/app/(protected)/assets/_components/asset-form.tsx @@ -91,7 +91,7 @@ export default function AssetFormPage() { {isEdit ? "Edit Asset" : "Create Asset"} - {(error || optionsError) && ( + {Boolean(error || optionsError) && ( {error instanceof Error ? error.message diff --git a/src/app/(protected)/assets/index.tsx b/src/app/(protected)/assets/index.tsx index bea3dd35..e6d6451f 100644 --- a/src/app/(protected)/assets/index.tsx +++ b/src/app/(protected)/assets/index.tsx @@ -116,7 +116,7 @@ export default function AssetsListPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load assets"} diff --git a/src/app/(protected)/calendar/_components/event-form.tsx b/src/app/(protected)/calendar/_components/event-form.tsx index 183201d2..01134222 100644 --- a/src/app/(protected)/calendar/_components/event-form.tsx +++ b/src/app/(protected)/calendar/_components/event-form.tsx @@ -120,7 +120,7 @@ export default function EventFormPage() { {isEdit ? "Edit Event" : "Create Event"} - {error && Failed to load event} + {Boolean(error) && Failed to load event} diff --git a/src/app/(protected)/contacts/_components/contact-form.tsx b/src/app/(protected)/contacts/_components/contact-form.tsx index f2fefc39..401265d6 100644 --- a/src/app/(protected)/contacts/_components/contact-form.tsx +++ b/src/app/(protected)/contacts/_components/contact-form.tsx @@ -127,7 +127,7 @@ export default function ContactFormPage() { {isEdit ? "Edit Contact" : "Create Contact"} - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load contact"} diff --git a/src/app/(protected)/contacts/index.tsx b/src/app/(protected)/contacts/index.tsx index 54591c7e..e910c8f4 100644 --- a/src/app/(protected)/contacts/index.tsx +++ b/src/app/(protected)/contacts/index.tsx @@ -103,7 +103,7 @@ export default function ContactsListPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load contacts"} diff --git a/src/app/(protected)/dashboard.tsx b/src/app/(protected)/dashboard.tsx index 20be5454..e1af6bf9 100644 --- a/src/app/(protected)/dashboard.tsx +++ b/src/app/(protected)/dashboard.tsx @@ -70,7 +70,7 @@ function KpiCard({ title, description, value, icon, color, onClick, loading }: K {loading ? "..." : typeof value === "number" ? value.toLocaleString() : value} )} - {description && ( + {Boolean(description) && ( {isFetching && !isLoading && } - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load dashboard stats"} diff --git a/src/app/(protected)/employees/_components/employee-form.tsx b/src/app/(protected)/employees/_components/employee-form.tsx index 78ed8bc1..6d5413a8 100644 --- a/src/app/(protected)/employees/_components/employee-form.tsx +++ b/src/app/(protected)/employees/_components/employee-form.tsx @@ -83,7 +83,7 @@ function SelectField({ ))} - {helperText && ( + {Boolean(helperText) && ( {helperText} @@ -193,7 +193,7 @@ export default function EmployeeFormPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load employee"} diff --git a/src/app/(protected)/employees/index.tsx b/src/app/(protected)/employees/index.tsx index b570dbe3..ed56887b 100644 --- a/src/app/(protected)/employees/index.tsx +++ b/src/app/(protected)/employees/index.tsx @@ -124,7 +124,7 @@ export default function EmployeesListPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load employees"} diff --git a/src/app/(protected)/followups/_components/follow-up-form.tsx b/src/app/(protected)/followups/_components/follow-up-form.tsx index 603dd3f1..d7a6a86c 100644 --- a/src/app/(protected)/followups/_components/follow-up-form.tsx +++ b/src/app/(protected)/followups/_components/follow-up-form.tsx @@ -102,7 +102,7 @@ export default function FollowUpFormPage() { {isEdit ? "Edit Follow-up" : "Create Follow-up"} - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load follow-up"} diff --git a/src/app/(protected)/followups/index.tsx b/src/app/(protected)/followups/index.tsx index 69b42a59..30980fc0 100644 --- a/src/app/(protected)/followups/index.tsx +++ b/src/app/(protected)/followups/index.tsx @@ -129,7 +129,7 @@ export default function FollowUpsListPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load follow-ups"} diff --git a/src/app/(protected)/index.tsx b/src/app/(protected)/index.tsx index 20be5454..e1af6bf9 100644 --- a/src/app/(protected)/index.tsx +++ b/src/app/(protected)/index.tsx @@ -70,7 +70,7 @@ function KpiCard({ title, description, value, icon, color, onClick, loading }: K {loading ? "..." : typeof value === "number" ? value.toLocaleString() : value} )} - {description && ( + {Boolean(description) && ( {isFetching && !isLoading && } - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load dashboard stats"} diff --git a/src/app/(protected)/locations/_components/location-form.tsx b/src/app/(protected)/locations/_components/location-form.tsx index 79ef1acc..990fc75d 100644 --- a/src/app/(protected)/locations/_components/location-form.tsx +++ b/src/app/(protected)/locations/_components/location-form.tsx @@ -114,7 +114,7 @@ export default function LocationFormPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load location"} diff --git a/src/app/(protected)/locations/index.tsx b/src/app/(protected)/locations/index.tsx index df239574..097db2c8 100644 --- a/src/app/(protected)/locations/index.tsx +++ b/src/app/(protected)/locations/index.tsx @@ -118,7 +118,7 @@ export default function LocationsListPage() { - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load locations"} diff --git a/src/app/(protected)/pmschedules/_components/pm-schedule-form.tsx b/src/app/(protected)/pmschedules/_components/pm-schedule-form.tsx index d3b82f15..0253a149 100644 --- a/src/app/(protected)/pmschedules/_components/pm-schedule-form.tsx +++ b/src/app/(protected)/pmschedules/_components/pm-schedule-form.tsx @@ -80,8 +80,8 @@ export default function PmScheduleFormPage() { {isEdit ? "Edit PM Schedule" : "Create PM Schedule"} - {error && Failed to load PM schedule} - {optionsError && ( + {Boolean(error) && Failed to load PM schedule} + {Boolean(optionsError) && ( {optionsError instanceof Error ? optionsError.message : "Failed to load form options"} diff --git a/src/app/(protected)/pmschedules/index.tsx b/src/app/(protected)/pmschedules/index.tsx index 4517d4ed..826381e7 100644 --- a/src/app/(protected)/pmschedules/index.tsx +++ b/src/app/(protected)/pmschedules/index.tsx @@ -122,7 +122,7 @@ export default function PmSchedulesListPage() { )} - {error && ( + {Boolean(error) && ( {error instanceof Error ? error.message : "Failed to load PM schedules"} diff --git a/src/app/(protected)/settings/dropdowns.tsx b/src/app/(protected)/settings/dropdowns.tsx index 4e2c0df7..e4aef562 100644 --- a/src/app/(protected)/settings/dropdowns.tsx +++ b/src/app/(protected)/settings/dropdowns.tsx @@ -158,7 +158,7 @@ export default function DropdownOptionsPage() { alignItems: "stretch", }} > - {parentCategory && ( + {Boolean(parentCategory) && ( Parent Trade
-

+ This work order is past due. A new date must be scheduled before the status can be updated. -

+
- {tech && company && ( + {Boolean(tech && company) && (
diff --git a/src/app/(protected)/workorders/_components/list/table/cells/vendor-dialog.tsx b/src/app/(protected)/workorders/_components/list/table/cells/vendor-dialog.tsx index a6ea7f20..5f3e7827 100644 --- a/src/app/(protected)/workorders/_components/list/table/cells/vendor-dialog.tsx +++ b/src/app/(protected)/workorders/_components/list/table/cells/vendor-dialog.tsx @@ -8,6 +8,7 @@ import { } from "@/app/(protected)/workorders/_components/wizard/wizard-input-styles"; import { Detail } from "@/app/(protected)/workorders/_components/wizard/wizard-labels"; import { Button } from "@/components/ui/button"; +import { Text } from "@/components/ui/text"; import { dialogBlurBackdropSlotProps } from "@/components/ui/dialog-blur-backdrop"; import type { VendorDropdownItem } from "@/domain/work-orders/types/work-order"; @@ -165,7 +166,9 @@ export function VendorDialog({ Technician -

Search and select the technician

+ + Search and select the technician +
@@ -189,7 +192,7 @@ export function VendorDialog({ />
- {t && ( + {Boolean(t) && (
Technician Phone )} - {t && ( + {Boolean(t) && (
Company
{resolvedCompany || "—"}
- {companyEmail && } - {selectedVendor?.address && ( + {Boolean(companyEmail) && } + {selectedVendor != null && selectedVendor.address != null && ( {mk(Eye, "View details", onView, "#e6f4fb", "var(--primary)")} - {onEdit ? mk(Pencil, "Edit", onEdit, "#e6f4fb", "var(--primary)") : null} + {onEdit != null && mk(Pencil, "Edit", onEdit, "#e6f4fb", "var(--primary)")}
); } diff --git a/src/app/(protected)/workorders/_components/list/table/wo-table-section-header.tsx b/src/app/(protected)/workorders/_components/list/table/wo-table-section-header.tsx index 18301594..6b22e968 100644 --- a/src/app/(protected)/workorders/_components/list/table/wo-table-section-header.tsx +++ b/src/app/(protected)/workorders/_components/list/table/wo-table-section-header.tsx @@ -108,7 +108,7 @@ export function WoTableSectionHeader({ > {count} WO{count !== 1 ? "s" : ""} - {isToday && ( + {Boolean(isToday) && ( -

{message}

- {onAction && ( + + {message} + + {Boolean(onAction) && (
-

Upload photos and videos for this work order.

+ + Upload photos and videos for this work order. +
diff --git a/src/app/(protected)/workorders/_components/wizard/wizard-field-select.tsx b/src/app/(protected)/workorders/_components/wizard/wizard-field-select.tsx index d936e85d..460ecec5 100644 --- a/src/app/(protected)/workorders/_components/wizard/wizard-field-select.tsx +++ b/src/app/(protected)/workorders/_components/wizard/wizard-field-select.tsx @@ -2,6 +2,7 @@ import { useState, type ReactNode } from "react"; import { Check, ChevronDown } from "lucide-react"; import { Popover } from "@mui/material"; import { MenuAnchorButton } from "@/components/ui/menu-anchor-button"; +import { Text } from "@/components/ui/text"; import { useMenuAnchor } from "@/hooks/use-menu-anchor"; export interface WizardMenuOption { @@ -130,12 +131,13 @@ export function WizardFieldSelect({ )}
{filtered.length === 0 ? ( -

No matches -

+ ) : ( filtered.map((o) => (
@@ -55,13 +56,13 @@ function VendorPortalBody() { if (status === "error") { return (
-
-

Access Denied

-

+ + Access Denied + {error?.message || "This link is invalid or has expired. Please contact your dispatcher for a new link."} -

-
+ +
); } diff --git a/src/app/v/[token]/dashboard.tsx b/src/app/v/[token]/dashboard.tsx index a7dc74f9..eeb222a2 100644 --- a/src/app/v/[token]/dashboard.tsx +++ b/src/app/v/[token]/dashboard.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from "react"; import { useNavigate } from "react-router"; import { useQuery } from "@tanstack/react-query"; import { useVendorPortal } from "@/app/v/_components/vendor-portal-context"; +import { Text } from "@/components/ui/text"; import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; import { formatVendorPortalDateTime, @@ -54,7 +55,7 @@ export default function VendorPortalDashboardPage() { return ( <>
-

Your Work

+ Your Work
Active dispatches and work orders assigned to your company.
@@ -76,7 +77,9 @@ export default function VendorPortalDashboardPage() {
{isLoading &&
Loading dispatches…
} - {error &&
{error.message}
} + + {error?.message} + {!isLoading && !error && filtered.length === 0 && (
No dispatches in this view.
diff --git a/src/app/v/[token]/dispatch/[id].tsx b/src/app/v/[token]/dispatch/[id].tsx index 2edae8a1..a7000843 100644 --- a/src/app/v/[token]/dispatch/[id].tsx +++ b/src/app/v/[token]/dispatch/[id].tsx @@ -3,6 +3,7 @@ import { Link, useParams } from "react-router"; import { useQuery, useQueryClient } from "@tanstack/react-query"; import { SignaturePad } from "@/app/v/_components/signature-pad"; import { useVendorPortal } from "@/app/v/_components/vendor-portal-context"; +import { Text } from "@/components/ui/text"; import { vendorPortalApi } from "@/domain/vendor-portal/api/vendor-portal-api"; import { formatVendorPortalDateTime, @@ -63,7 +64,7 @@ function ChecklistSection({ - {item.isCompleted && item.completedAt && ( + {item.isCompleted && Boolean(item.completedAt) && ( {formatVendorPortalDateTime(item.completedAt)} @@ -224,7 +225,7 @@ function UpliftRequestsSection({
- {request.vendorReason && ( + {Boolean(request.vendorReason) && (
Reason {request.vendorReason} @@ -429,7 +430,11 @@ export default function VendorPortalDispatchPage() { } if (error && !data) { - return
{error.message}
; + return ( + + {error.message} + + ); } if (!data) return null; @@ -457,20 +462,20 @@ export default function VendorPortalDispatchPage() {
-

{data.dispatchNumber}

+ {data.dispatchNumber}
{data.workOrder?.workerOrderTitle}
{data.status}
- {data.workOrder?.internalWONumber && ( + {data.workOrder != null && Boolean(data.workOrder.internalWONumber) && (
WO # {data.workOrder.internalWONumber}
)} - {data.poNumber && ( + {Boolean(data.poNumber) && (
PO # {data.poNumber} @@ -482,7 +487,7 @@ export default function VendorPortalDispatchPage() { ${Number(data.nteAmount).toFixed(2)}
)} - {data.scheduledDate && ( + {Boolean(data.scheduledDate) && (
Scheduled {formatVendorPortalDateTime(data.scheduledDate)} @@ -490,9 +495,11 @@ export default function VendorPortalDispatchPage() { )}
- {data.location && ( + {data.location != null && ( <> -

Location

+ + Location +
{data.location.name}
{[ @@ -507,9 +514,11 @@ export default function VendorPortalDispatchPage() { )} - {(data.description || data.workOrder?.description) && ( + {Boolean(data.description || data.workOrder?.description) && ( <> -

Description

+ + Description +
{data.description || data.workOrder?.description}
)} @@ -570,7 +579,7 @@ export default function VendorPortalDispatchPage() {
-

Checklist

+ Checklist
-

NTE Uplift Requests

+ NTE Uplift Requests
-

Customer Signoff

+ Customer Signoff
-

Vendor Signoff

+ Vendor Signoff
-

Comments

+ Comments
-

Your Purchase Orders

+ Your Purchase Orders
Every PO issued to your company, with current status and dollar value.
@@ -113,7 +114,9 @@ export default function VendorPortalPosPage() {
{isLoading &&
Loading POs…
} - {error &&
{error.message}
} + + {error?.message} + {!isLoading && !error && visible.length === 0 && (
No POs match this view.
diff --git a/src/components/common/calendar/event-calendar.tsx b/src/components/common/calendar/event-calendar.tsx index ca7d11aa..0616c81b 100644 --- a/src/components/common/calendar/event-calendar.tsx +++ b/src/components/common/calendar/event-calendar.tsx @@ -22,7 +22,7 @@ export const EventCalendar = forwardRef(functi ) { return ( - {loading && ( + {Boolean(loading) && ( diff --git a/src/components/common/signature-capture.tsx b/src/components/common/signature-capture.tsx index c08123c2..9c14cf18 100644 --- a/src/components/common/signature-capture.tsx +++ b/src/components/common/signature-capture.tsx @@ -185,7 +185,7 @@ export function SignatureCapture({ onClose, onSave, title }: SignatureCapturePro fullWidth size="small" /> - {typedName && ( + {Boolean(typedName) && (
- {Icon && } + {Icon != null && } {label}
diff --git a/src/components/ui/empty-state.tsx b/src/components/ui/empty-state.tsx index 944591a7..5cf52b5f 100644 --- a/src/components/ui/empty-state.tsx +++ b/src/components/ui/empty-state.tsx @@ -1,13 +1,13 @@ -import type { ReactNode } from "react"; +import type { ReactElement } from "react"; import { Box, Typography } from "@mui/material"; import { cn } from "@/lib/utils"; type EmptyStateProps = { - icon?: ReactNode; + icon?: ReactElement; title: string; description?: string; - action?: ReactNode; + action?: ReactElement; className?: string; }; @@ -19,11 +19,11 @@ export function EmptyState({ icon, title, description, action, className }: Empt className, )} > - {icon && {icon}} + {Boolean(icon) && {icon}} {title} - {description && ( + {Boolean(description) && ( )} - {action && {action}} + {Boolean(action) && {action}} ); } diff --git a/src/components/ui/filter-popover.tsx b/src/components/ui/filter-popover.tsx index 59824344..2d8d5e8a 100644 --- a/src/components/ui/filter-popover.tsx +++ b/src/components/ui/filter-popover.tsx @@ -34,7 +34,7 @@ export function FilterPopover({
Filters - {activeCount > 0 && onClear && ( + {activeCount > 0 && Boolean(onClear) && ( + + ); +} + +const companyField = () => screen.getByRole("combobox", { name: "Company" }) as HTMLInputElement; +const companyIdOutput = () => screen.getByTestId("company-id"); + +describe("VendorFormFields company autocomplete state", () => { + it("clears a previously typed company when the form is reset (create-form reopen)", async () => { + renderWithProviders(, { + withAuth: false, + }); + + await userEvent.type(companyField(), "Leftover Co"); + expect(companyField()).toHaveValue("Leftover Co"); + + await userEvent.click(screen.getByRole("button", { name: "reset-form" })); + + expect(companyField()).toHaveValue(""); + }); + + it("shows the switched company value instead of previously typed text when the form is reset", async () => { + renderWithProviders( + , + { withAuth: false }, + ); + + await userEvent.type(companyField(), "Acme"); + expect(companyField()).toHaveValue("Acme"); + + await userEvent.click(screen.getByRole("button", { name: "reset-form" })); + + expect(companyField()).toHaveValue("Beta LLC"); + }); + + it("reflects the company provided by an external reset even when nothing was typed", async () => { + renderWithProviders( + , + { withAuth: false }, + ); + + expect(companyField()).toHaveValue("Acme Corp"); + + await userEvent.click(screen.getByRole("button", { name: "reset-form" })); + + expect(companyField()).toHaveValue("Beta LLC"); + }); + + it("updates the displayed company as the user types (freeSolo)", async () => { + renderWithProviders(, { + withAuth: false, + }); + + await userEvent.type(companyField(), "Acme"); + + expect(companyField()).toHaveValue("Acme"); + }); + + it("selects a company option, autofills company details, and shows the company name", async () => { + renderWithProviders(, { + withAuth: false, + }); + + await userEvent.type(companyField(), "Gate"); + const option = await screen.findByRole("option", { name: "Gateway Plumbing" }); + await userEvent.click(option); + + expect(companyField()).toHaveValue("Gateway Plumbing"); + expect(screen.getByRole("textbox", { name: "Company Phone" })).toHaveValue("(314) 555-0198"); + }); + + it("sets companyId when a known company is selected and clears it when the text changes", async () => { + renderWithProviders(, { + withAuth: false, + }); + + await userEvent.type(companyField(), "Gate"); + await userEvent.click(await screen.findByRole("option", { name: "Gateway Plumbing" })); + expect(companyIdOutput()).toHaveTextContent("co-1"); + + await userEvent.type(companyField(), "!"); + expect(companyIdOutput()).toHaveTextContent(""); + }); + + it("keeps companyId null for free-typed company text that matches no option", async () => { + renderWithProviders(, { + withAuth: false, + }); + + await userEvent.type(companyField(), "Independent Vendor LLC"); + expect(companyIdOutput()).toHaveTextContent(""); + }); + + it("formats technician and company phones into the (XXX) XXX-XXXX mask", async () => { + renderWithProviders(, { + withAuth: false, + }); + + const phone = screen.getByRole("textbox", { name: "Technician Phone" }); + await userEvent.type(phone, "3145550198"); + expect(phone).toHaveValue("(314) 555-0198"); + + const companyPhone = screen.getByRole("textbox", { name: "Company Phone" }); + await userEvent.type(companyPhone, "3145550100"); + expect(companyPhone).toHaveValue("(314) 555-0100"); + }); + + it("exposes a preferred contact selector with Phone, Email, and Text options", () => { + renderWithProviders(, { + withAuth: false, + }); + + expect(screen.getByRole("button", { name: "Phone" })).toHaveAttribute("aria-pressed", "true"); + expect(screen.getByRole("button", { name: "Email" })).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Text" })).toBeInTheDocument(); + }); +}); diff --git a/src/test/app/(protected)/vendors/vendors-list.test.tsx b/src/test/app/(protected)/vendors/vendors-list.test.tsx new file mode 100644 index 00000000..995dd5dc --- /dev/null +++ b/src/test/app/(protected)/vendors/vendors-list.test.tsx @@ -0,0 +1,163 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import VendorsListPage from "@/app/(protected)/vendors/index"; +import { renderWithProviders } from "@/test/test-utils"; + +const useVendorsList = vi.fn(); +const useVendorDetail = vi.fn(); +const useVendorDeactivationImpact = vi.fn(); +const mutate = vi.fn(); + +vi.mock("@/domain/vendors/use-cases/use-vendors-list", () => ({ + useVendorsList: (...args: unknown[]) => useVendorsList(...args), +})); + +vi.mock("@/domain/vendors/use-cases/use-vendor-detail", () => ({ + useVendorDetail: (...args: unknown[]) => useVendorDetail(...args), +})); + +vi.mock("@/domain/vendors/use-cases/use-vendor-deactivation-impact", () => ({ + useVendorDeactivationImpact: (...args: unknown[]) => useVendorDeactivationImpact(...args), +})); + +vi.mock("@/domain/vendors/use-cases/use-delete-vendor", () => ({ + useDeleteVendor: () => ({ mutate, isPending: false }), +})); + +vi.mock("@/hooks/use-debounce", () => ({ + useDebounce: (value: string) => value, +})); + +const activeVendor = { + id: 1, + companyId: "co-1", + companyName: "Gateway Plumbing", + contactName: "Adam Whyte", + email: "dispatch@gatewayplumbing.com", + phone: "(314) 555-0198", + city: "St. Louis", + state: "MO", + tradeSpecialties: "Backflow Preventers", + isActive: true, + preferredContact: "Email", +}; + +const inactiveVendor = { + ...activeVendor, + id: 2, + companyName: "Drain Cleaning Bros", + isActive: false, +}; + +function result(items: (typeof activeVendor)[], totalCount: number) { + return { + data: { items, totalCount, page: 1, pageSize: 12 }, + isLoading: false, + isFetching: false, + error: null, + }; +} + +function setupDefaults() { + useVendorDetail.mockReturnValue({ data: undefined, isLoading: false, error: null }); + useVendorDeactivationImpact.mockReturnValue({ + data: undefined, + isLoading: false, + error: null, + }); +} + +describe("VendorsListPage", () => { + it("renders prototype-aligned status tabs and location, then hides deactivation for inactive rows", async () => { + setupDefaults(); + useVendorsList.mockImplementation((params: { isActive?: boolean; pageSize?: number }) => { + if (params.pageSize === 1) return result([], params.isActive ? 1 : 1); + return params.isActive ? result([activeVendor], 1) : result([inactiveVendor], 1); + }); + + renderWithProviders(, { route: "/vendors", withAuth: false }); + + expect(screen.getByRole("heading", { name: "Vendors / Technicians" })).toBeInTheDocument(); + expect(screen.getByText("St. Louis, MO")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Deactivate vendor" })).toBeInTheDocument(); + + await userEvent.click(screen.getByRole("tab", { name: "Inactive (1)" })); + + expect(screen.getByText("Drain Cleaning Bros")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Deactivate vendor" })).not.toBeInTheDocument(); + }); + + it("reports the filtered totalCount in the footer when page rows are fewer than total", () => { + setupDefaults(); + const pageVendors = Array.from({ length: 3 }, (_, index) => ({ + ...activeVendor, + id: 100 + index, + companyName: `Vendor ${index + 1}`, + })); + + useVendorsList.mockImplementation((params: { isActive?: boolean; pageSize?: number }) => { + if (params.pageSize === 1) return result([], params.isActive ? 25 : 0); + return result(pageVendors, 25); + }); + + renderWithProviders(, { route: "/vendors", withAuth: false }); + + expect(screen.getByText(/25 active technicians/)).toBeInTheDocument(); + expect(screen.queryByText(/3 active technicians/)).not.toBeInTheDocument(); + }); + + it("opens the detail drawer when a row is clicked or activated by keyboard", async () => { + setupDefaults(); + useVendorDetail.mockReturnValue({ data: activeVendor, isLoading: false, error: null }); + useVendorsList.mockImplementation((params: { isActive?: boolean; pageSize?: number }) => { + if (params.pageSize === 1) return result([], 1); + return params.isActive ? result([activeVendor], 1) : result([], 0); + }); + + renderWithProviders(, { route: "/vendors", withAuth: false }); + + const row = screen.getByRole("button", { + name: "Open vendor details for Gateway Plumbing", + }); + row.focus(); + await userEvent.keyboard("{Enter}"); + + expect(screen.getByRole("heading", { level: 2, name: "Gateway Plumbing" })).toBeInTheDocument(); + }); + + it("blocks deactivation when the preflight reports open work orders", async () => { + setupDefaults(); + useVendorDeactivationImpact.mockReturnValue({ + data: { + vendorId: 1, + canDeactivate: false, + openWorkOrders: [ + { + workOrderId: 101, + workOrderNumber: "WO-101", + workOrderTitle: "Boiler repair", + status: "Dispatched", + }, + ], + }, + isLoading: false, + error: null, + }); + useVendorsList.mockImplementation((params: { isActive?: boolean; pageSize?: number }) => { + if (params.pageSize === 1) return result([], 1); + return params.isActive ? result([activeVendor], 1) : result([], 0); + }); + + renderWithProviders(, { route: "/vendors", withAuth: false }); + + await userEvent.click(screen.getByRole("button", { name: "Deactivate vendor" })); + + expect( + screen.getByText(/cannot be deactivated because it still has open work orders/), + ).toBeInTheDocument(); + expect(screen.getByText(/Boiler repair/)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /^Deactivate$/ })).toBeDisabled(); + expect(mutate).not.toHaveBeenCalled(); + }); +}); diff --git a/src/test/components/ui/text.test.tsx b/src/test/components/ui/text.test.tsx new file mode 100644 index 00000000..04973d75 --- /dev/null +++ b/src/test/components/ui/text.test.tsx @@ -0,0 +1,82 @@ +import { screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { Text } from "@/components/ui/text"; +import { renderWithProviders } from "@/test/test-utils"; + +describe("Text", () => { + it("maps visual variants to semantic elements", () => { + renderWithProviders( + <> + Page title + Supporting copy + , + { withAuth: false }, + ); + + expect(screen.getByRole("heading", { level: 1, name: "Page title" })).toBeInTheDocument(); + expect(screen.getByText("Supporting copy").tagName).toBe("P"); + }); + + it("does not render conditional text when its condition is false", () => { + renderWithProviders( + + Saved + , + { withAuth: false }, + ); + + expect(screen.queryByText("Saved")).not.toBeInTheDocument(); + }); + + it("keeps the feedback live region mounted so toggling its content is announceable", () => { + const { rerender } = renderWithProviders( + + Saved + , + { withAuth: false }, + ); + + const region = screen.getByRole("status"); + expect(region).toHaveAttribute("aria-live", "polite"); + expect(region).toHaveAttribute("aria-atomic", "true"); + expect(region).toBeEmptyDOMElement(); + + rerender( + + Saved + , + ); + + expect(screen.getByRole("status")).toBe(region); + expect(region).toHaveTextContent("Saved"); + }); + + it("gives error and feedback messages accessible live-region semantics", () => { + renderWithProviders( + <> + Upload failed + Uploading + , + { withAuth: false }, + ); + + const alert = screen.getByRole("alert"); + const status = screen.getByRole("status"); + expect(alert).toHaveTextContent("Upload failed"); + expect(status).toHaveTextContent("Uploading"); + expect(alert).toHaveAttribute("aria-live", "assertive"); + expect(alert).toHaveAttribute("aria-atomic", "true"); + expect(status).toHaveAttribute("aria-live", "polite"); + expect(status).toHaveAttribute("aria-atomic", "true"); + }); + + it("does not attach live-region semantics to non-feedback variants", () => { + renderWithProviders(Saved, { withAuth: false }); + + const node = screen.getByText("Saved"); + expect(node).not.toHaveAttribute("role"); + expect(node).not.toHaveAttribute("aria-live"); + expect(node).not.toHaveAttribute("aria-atomic"); + }); +}); diff --git a/src/test/domain/vendors/api/vendors-api.test.ts b/src/test/domain/vendors/api/vendors-api.test.ts new file mode 100644 index 00000000..b66ce1bf --- /dev/null +++ b/src/test/domain/vendors/api/vendors-api.test.ts @@ -0,0 +1,269 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { API_PATHS } from "@/api/api-paths"; + +const apiGet = vi.fn(); +const apiPost = vi.fn(); +const apiPut = vi.fn(); +const apiDelete = vi.fn(); + +vi.mock("@/api/api", () => ({ + apiGet: (...args: unknown[]) => apiGet(...args), + apiPost: (...args: unknown[]) => apiPost(...args), + apiPut: (...args: unknown[]) => apiPut(...args), + apiDelete: (...args: unknown[]) => apiDelete(...args), +})); + +import { vendorsApi } from "@/domain/vendors/api/vendors-api"; + +describe("vendorsApi.getList", () => { + beforeEach(() => { + apiGet.mockReset(); + apiPost.mockReset(); + apiPut.mockReset(); + apiDelete.mockReset(); + apiGet.mockResolvedValue({ + Data: [ + { + Id: 7, + CompanyId: "co-1", + CompanyName: "Gateway Plumbing", + ContactName: "Adam Whyte", + City: "St. Louis", + State: "MO", + IsActive: false, + PreferredContact: "Email", + }, + ], + TotalCount: 1, + }); + }); + + it("forwards the status filter and maps list location fields", async () => { + const result = await vendorsApi.getList({ search: "gateway", isActive: false, page: 1 }); + + expect(apiGet).toHaveBeenCalledWith(API_PATHS.vendor.getList, { + searchParams: { search: "gateway", isActive: "false", page: 1, pageSize: 12 }, + }); + expect(result.items[0]).toMatchObject({ + city: "St. Louis", + state: "MO", + isActive: false, + companyId: "co-1", + preferredContact: "Email", + }); + }); + + it("forwards all selected facets as repeated indexed query parameters", async () => { + await vendorsApi.getList({ + companies: ["Gateway Plumbing", "Metro HVAC"], + trades: ["Plumbing"], + locations: ["St. Louis, MO"], + jobBuckets: ["50-99"], + }); + + expect(apiGet).toHaveBeenCalledWith(API_PATHS.vendor.getList, { + searchParams: { + search: "", + page: 1, + pageSize: 12, + "companies[0]": "Gateway Plumbing", + "companies[1]": "Metro HVAC", + "trades[0]": "Plumbing", + "locations[0]": "St. Louis, MO", + "jobBuckets[0]": "50-99", + }, + }); + }); + + it("maps company profiles and labels returned by the facets endpoint", async () => { + apiGet.mockResolvedValueOnce({ + data: { + companies: [ + { + name: "Gateway Plumbing", + companyPhone: "314-555-0100", + email: "dispatch@gateway.test", + address: "1 Market St", + city: "St. Louis", + state: "MO", + zip: "63101", + googleMapsUrl: "https://maps.google.com/gateway", + }, + ], + trades: ["Plumbing"], + locations: [{ city: "St. Louis", state: "MO" }], + jobBuckets: [{ id: "50-99", label: "50–99" }], + }, + }); + + const facets = await vendorsApi.getFacets(); + + expect(apiGet).toHaveBeenCalledWith(API_PATHS.vendor.facets, { searchParams: {} }); + expect(facets.companies[0]).toMatchObject({ + name: "Gateway Plumbing", + companyPhone: "314-555-0100", + googleMapsUrl: "https://maps.google.com/gateway", + }); + expect(facets.locations[0]?.label).toBe("St. Louis, MO"); + expect(facets.jobBuckets[0]).toEqual({ id: "50-99", label: "50–99" }); + }); + + it("serializes the requested facet scope as an isActive query parameter", async () => { + apiGet.mockResolvedValue({ + data: { companies: [], trades: [], locations: [], jobBuckets: [] }, + }); + + await vendorsApi.getFacets({ isActive: true }); + await vendorsApi.getFacets({ isActive: false }); + + expect(apiGet).toHaveBeenNthCalledWith(1, API_PATHS.vendor.facets, { + searchParams: { isActive: "true" }, + }); + expect(apiGet).toHaveBeenNthCalledWith(2, API_PATHS.vendor.facets, { + searchParams: { isActive: "false" }, + }); + }); + + it("sends all editable prototype fields when creating and updating", async () => { + const values = { + companyId: "42", + companyName: "Gateway Plumbing", + contactName: "Adam Whyte", + phone: "+1 314-555-0198", + companyPhone: "314-555-0100", + email: "dispatch@gateway.test", + address: "1 Market St", + city: "St. Louis", + state: "MO", + zip: "63101", + tradeSpecialties: "Plumbing, HVAC", + googleMapsUrl: "https://maps.google.com/gateway", + notes: "Preferred after-hours vendor", + totalJobs: 72, + isActive: true, + preferredContact: "Text", + }; + apiPost.mockResolvedValueOnce({ data: values }); + apiPut.mockResolvedValueOnce({ data: { ...values, isActive: false } }); + + await vendorsApi.create(values); + await vendorsApi.update(7, { ...values, isActive: false }); + + expect(apiPost).toHaveBeenCalledWith( + API_PATHS.rest.vendors, + expect.objectContaining({ + companyName: "Gateway Plumbing", + notes: values.notes, + companyId: 42, + preferredContact: "Text", + phone: "(314) 555-0198", + companyPhone: "(314) 555-0100", + }), + ); + expect(apiPut).toHaveBeenCalledWith( + `${API_PATHS.rest.vendors}/7`, + expect.objectContaining({ companyName: "Gateway Plumbing", isActive: false }), + ); + expect(apiPost.mock.calls[0]?.[1]).not.toHaveProperty("totalJobs"); + expect(apiPut.mock.calls[0]?.[1]).not.toHaveProperty("totalJobs"); + }); + + it("sends a null companyId when free-text company has no company link", async () => { + apiPost.mockResolvedValueOnce({ data: {} }); + await vendorsApi.create({ + companyName: "Independent Vendor LLC", + contactName: "Indy Tech", + preferredContact: "Phone", + }); + + expect(apiPost).toHaveBeenCalledWith( + API_PATHS.rest.vendors, + expect.objectContaining({ companyName: "Independent Vendor LLC", companyId: null }), + ); + }); + + it("does not send a non-numeric company id to the integer backend contract", async () => { + apiPost.mockResolvedValueOnce({ data: {} }); + + await vendorsApi.create({ + companyId: "co-1", + companyName: "Gateway Plumbing", + contactName: "Adam Whyte", + }); + + expect(apiPost).toHaveBeenCalledWith( + API_PATHS.rest.vendors, + expect.objectContaining({ companyId: null }), + ); + }); + + it("preserves a legacy phone on read and unrelated write", async () => { + apiGet.mockResolvedValueOnce({ + data: { + id: 7, + companyName: "Legacy Vendor", + contactName: "Taylor", + phone: "555-1234 ext 9", + }, + }); + apiPut.mockResolvedValueOnce({ data: {} }); + + const vendor = await vendorsApi.getById(7); + expect(vendor.phone).toBe("555-1234 ext 9"); + + await vendorsApi.update(7, { ...vendor, notes: "Updated only the notes" }); + expect(apiPut).toHaveBeenCalledWith( + `${API_PATHS.rest.vendors}/7`, + expect.objectContaining({ phone: "555-1234 ext 9", notes: "Updated only the notes" }), + ); + }); + + it("maps the deactivation-impact payload including linked work orders", async () => { + apiGet.mockResolvedValueOnce({ + data: { + vendorId: 7, + canDeactivate: false, + openWorkOrders: [ + { + workOrderId: 101, + workOrderNumber: "WO-101", + workOrderTitle: "Boiler repair", + status: "Dispatched", + scheduledDate: "2025-01-02", + dispatchId: 501, + }, + ], + }, + }); + + const impact = await vendorsApi.getDeactivationImpact(7); + + expect(apiGet).toHaveBeenCalledWith( + `${API_PATHS.rest.vendors}/7/${API_PATHS.vendor.deactivationImpact}`, + ); + expect(impact).toEqual({ + vendorId: 7, + canDeactivate: false, + openWorkOrders: [ + { + workOrderId: 101, + workOrderNumber: "WO-101", + workOrderTitle: "Boiler repair", + status: "Dispatched", + scheduledDate: "2025-01-02", + dispatchId: 501, + }, + ], + }); + }); + + it("deletes a vendor by id and surfaces the 409 error without retry swallowing", async () => { + apiDelete.mockResolvedValueOnce(undefined); + await vendorsApi.delete(7); + expect(apiDelete).toHaveBeenCalledWith(`${API_PATHS.rest.vendors}/7`); + + const conflict = Object.assign(new Error("Conflict"), { response: { status: 409 } }); + apiDelete.mockRejectedValueOnce(conflict); + await expect(vendorsApi.delete(8)).rejects.toThrow("Conflict"); + }); +}); diff --git a/src/test/domain/vendors/schemas/vendor-schema.test.ts b/src/test/domain/vendors/schemas/vendor-schema.test.ts new file mode 100644 index 00000000..244c2b8e --- /dev/null +++ b/src/test/domain/vendors/schemas/vendor-schema.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from "vitest"; +import { vendorSchema } from "@/domain/vendors/schemas/vendor-schema"; + +const validVendor = { + companyName: "Gateway Plumbing", + contactName: "Adam Whyte", + email: "dispatch@gateway.test", + phone: "(314) 555-0198", + googleMapsUrl: "https://maps.google.com/gateway", +}; + +describe("vendorSchema", () => { + it("requires both the company and technician identity", () => { + const result = vendorSchema.safeParse({ ...validVendor, companyName: "", contactName: "" }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.flatten().fieldErrors.companyName).toContain("Company is required"); + expect(result.error.flatten().fieldErrors.contactName).toContain( + "Technician name is required", + ); + } + }); + + it("accepts an empty optional email and HTTPS map URL", () => { + expect(vendorSchema.safeParse({ ...validVendor, email: "" }).success).toBe(true); + }); + + it("rejects malformed email and non-HTTPS map links", () => { + expect( + vendorSchema.safeParse({ + ...validVendor, + email: "not-an-email", + googleMapsUrl: "http://maps.google.com/gateway", + }).success, + ).toBe(false); + }); + + it("accepts canonicalizable North American phones and rejects partials", () => { + expect(vendorSchema.safeParse({ ...validVendor, phone: "", companyPhone: "" }).success).toBe( + true, + ); + expect(vendorSchema.safeParse({ ...validVendor, companyPhone: "(314) 555-0100" }).success).toBe( + true, + ); + expect(vendorSchema.safeParse({ ...validVendor, phone: "314-555-0198" }).success).toBe(true); + expect(vendorSchema.safeParse({ ...validVendor, phone: "+1 314 555 0198" }).success).toBe(true); + expect(vendorSchema.safeParse({ ...validVendor, phone: "(314)" }).success).toBe(false); + }); + + it("accepts a nullable companyId and a known preferred contact value", () => { + expect( + vendorSchema.safeParse({ ...validVendor, companyId: null, preferredContact: "Text" }).success, + ).toBe(true); + expect( + vendorSchema.safeParse({ ...validVendor, companyId: 42, preferredContact: "Email" }).success, + ).toBe(true); + }); + + it("rejects an unknown preferred contact value", () => { + expect(vendorSchema.safeParse({ ...validVendor, preferredContact: "Fax" }).success).toBe(false); + }); +}); diff --git a/src/test/domain/vendors/use-cases/use-vendor-facets.test.tsx b/src/test/domain/vendors/use-cases/use-vendor-facets.test.tsx new file mode 100644 index 00000000..c732e116 --- /dev/null +++ b/src/test/domain/vendors/use-cases/use-vendor-facets.test.tsx @@ -0,0 +1,86 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { renderHook, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactNode } from "react"; + +const getFacets = vi.fn(); + +vi.mock("@/domain/vendors/api/vendors-api", () => ({ + vendorsApi: { getFacets: (...args: unknown[]) => getFacets(...args) }, +})); + +import { useVendorFacets } from "@/domain/vendors/use-cases/use-vendor-facets"; +import { queryKeys } from "@/infra/query-key/query-key"; + +const emptyFacets = { companies: [], trades: [], locations: [], jobBuckets: [] }; +const activeFacets = { ...emptyFacets, companies: [{ name: "Active Co" }] }; +const inactiveFacets = { ...emptyFacets, companies: [{ name: "Inactive Co" }] }; + +function createClient(): QueryClient { + return new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); +} + +function makeWrapper(client: QueryClient) { + return function Wrapper({ children }: { children: ReactNode }) { + return {children}; + }; +} + +describe("useVendorFacets", () => { + beforeEach(() => { + getFacets.mockReset(); + }); + + it("switches facet requests true/false as the tab scope changes", async () => { + getFacets.mockResolvedValue(activeFacets); + const { rerender } = renderHook(({ isActive }) => useVendorFacets(isActive), { + initialProps: { isActive: true as boolean | undefined }, + wrapper: makeWrapper(createClient()), + }); + + await waitFor(() => expect(getFacets).toHaveBeenCalledWith({ isActive: true })); + + getFacets.mockResolvedValue(inactiveFacets); + rerender({ isActive: false }); + await waitFor(() => expect(getFacets).toHaveBeenCalledWith({ isActive: false })); + + getFacets.mockResolvedValue(activeFacets); + rerender({ isActive: undefined }); + await waitFor(() => expect(getFacets).toHaveBeenCalledWith(undefined)); + }); + + it("scopes the cache so active, inactive, and unscoped keys cannot collide", () => { + const active = queryKeys.vendors.facets(true); + const inactive = queryKeys.vendors.facets(false); + const unscoped = queryKeys.vendors.facets(); + + expect(active).not.toEqual(inactive); + expect(active).not.toEqual(unscoped); + expect(inactive).not.toEqual(unscoped); + expect(active[active.length - 1]).toBe(true); + expect(inactive[inactive.length - 1]).toBe(false); + expect(unscoped[unscoped.length - 1]).toBeNull(); + }); + + it("reuses the cached facet entry when returning to a previous scope", async () => { + getFacets.mockResolvedValue(activeFacets); + const client = createClient(); + const { result, rerender } = renderHook(({ isActive }) => useVendorFacets(isActive), { + initialProps: { isActive: true as boolean | undefined }, + wrapper: makeWrapper(client), + }); + + await waitFor(() => expect(getFacets).toHaveBeenCalledTimes(1)); + + getFacets.mockResolvedValue(inactiveFacets); + rerender({ isActive: false }); + await waitFor(() => expect(getFacets).toHaveBeenCalledTimes(2)); + + getFacets.mockClear(); + rerender({ isActive: true }); + await waitFor(() => expect(result.current.data).toEqual(activeFacets)); + expect(getFacets).not.toHaveBeenCalled(); + }); +}); diff --git a/src/test/domain/work-orders/api/work-orders-api.test.ts b/src/test/domain/work-orders/api/work-orders-api.test.ts index 32040a19..f44e7b30 100644 --- a/src/test/domain/work-orders/api/work-orders-api.test.ts +++ b/src/test/domain/work-orders/api/work-orders-api.test.ts @@ -2,9 +2,11 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { API_PATHS } from "@/api/api-paths"; const apiGet = vi.fn(); +const apiPut = vi.fn(); vi.mock("@/api/api", () => ({ apiGet: (...args: unknown[]) => apiGet(...args), + apiPut: (...args: unknown[]) => apiPut(...args), apiPost: vi.fn(), apiDelete: vi.fn(), apiPostNoContent: vi.fn(), @@ -15,6 +17,7 @@ import { workOrdersApi } from "@/domain/work-orders/api/work-orders-api"; describe("workOrdersApi.getById", () => { beforeEach(() => { apiGet.mockReset(); + apiPut.mockReset(); apiGet.mockResolvedValue({ Id: 10, title: "Test WO", @@ -22,6 +25,22 @@ describe("workOrdersApi.getById", () => { }); }); + it("updates only the assigned vendor through the work-order-scoped endpoint", async () => { + apiPut.mockResolvedValueOnce(undefined); + const payload = { + workOrderId: 10, + contactName: "Taylor Tech", + preferredContact: "Text" as const, + phone: "(314) 555-0199", + email: "taylor@example.com", + notes: "Text before arrival", + }; + + await workOrdersApi.updateVendorFromWorkOrder(7, payload); + + expect(apiPut).toHaveBeenCalledWith(`${API_PATHS.rest.vendors}/7/work-order-update`, payload); + }); + it("calls GetWorkorderById with id query param, not path suffix", async () => { await workOrdersApi.getById(10); diff --git a/src/test/domain/work-orders/mappers/work-order-mapper.test.ts b/src/test/domain/work-orders/mappers/work-order-mapper.test.ts index a9dfc9bf..632fa346 100644 --- a/src/test/domain/work-orders/mappers/work-order-mapper.test.ts +++ b/src/test/domain/work-orders/mappers/work-order-mapper.test.ts @@ -1,5 +1,26 @@ import { describe, expect, it } from "vitest"; -import { mapUpliftRequest } from "@/domain/work-orders/mappers/work-order-mapper"; +import { + mapUpliftRequest, + mapWorkOrderDispatchSummary, +} from "@/domain/work-orders/mappers/work-order-mapper"; + +describe("mapWorkOrderDispatchSummary", () => { + it("preserves the assigned vendor identity needed by inline editing", () => { + expect( + mapWorkOrderDispatchSummary({ + Id: 21, + VendorId: 7, + VendorName: "Gateway Plumbing", + VendorEmail: "dispatch@gateway.test", + }), + ).toMatchObject({ + id: 21, + vendorId: 7, + vendorName: "Gateway Plumbing", + vendorEmail: "dispatch@gateway.test", + }); + }); +}); describe("mapUpliftRequest", () => { it("maps decided uplift fields from PascalCase payload", () => {