cdaa2e2b25
- Standardize quote number format from Q-YYYY-MM-NNN to QYYYYMMNNN across all client, server, test and API code - Add atomic backfill logic for quote counters to avoid sequence collisions with existing records - Rearrange Delivery Notes and Repairs to the Operations sidebar section - Add supplier count badge to pricelist catalogue and improve filter tab overflow styling - Optimize font loading in root layout to skip preloading unused font weights - Improve table styling for customer and company pages with better text wrapping and empty states - Fix rep display logic in quotes page to show "Unassigned" when no rep is assigned - Add supporting documentation, PocketBase counter schemas and setup migration scripts - Remove embedded SupplierMap from suppliers page for cleaner layout - Improve invoice card layout for company and customer info spacing - Improve filter label styling and accessibility in collection map page
327 lines
23 KiB
Markdown
327 lines
23 KiB
Markdown
# WFDASH QA Health Check
|
||
|
||
**Date:** 2026-07-10
|
||
**Branch:** `minimax_ui_rev1`
|
||
**HEAD:** `47b01bd feat: add picking slips, bulk print tracking, supplier map`
|
||
**Auditor:** Coder (senior QA pass against `Ultimate Debugger and QA Engineer` P0–P3 framework)
|
||
|
||
---
|
||
|
||
## 🔍 Debugging Report
|
||
|
||
### Summary
|
||
|
||
WFDASH is a high-effort, feature-rich Next.js 16 / React 19 / PocketBase CRM with very strong conventions (test scaffold, route guard helpers, sanitizers, async params). But three concrete production-blocking issues surface in **type-check and tests**: the **quote-number formatter** returns `Q202607001` instead of the documented `Q-2026-07-001` shape, the **order-number generator** uses `now` month for its prefix even when the matching sequence comes from a prior month, and the **invoice-details type test** fails TypeScript excess-property checking. Combined with a custom regex HTML sanitizer that the author themselves flags as "not a general-purpose sanitizer", the codebase is in **"ship-with-known-bugs"** territory rather than "in order". Fix the three logic bugs and migrate the sanitizer to DOMPurify before considering the next release.
|
||
|
||
**Verdict:** **Needs fixes** — 3 P1 functional bugs in production code paths + 1 P1 sanitizer risk + manageable secondary issues.
|
||
|
||
---
|
||
|
||
### Issues Identified (by severity, descending)
|
||
|
||
#### [P1] — Quote number formatter produces wrong shape
|
||
|
||
- **Location:** `src/lib/server/quotes-service.ts:156-158`
|
||
- **Severity:** P1 — every new quote, customer-facing, breaks the documented contract.
|
||
- **Reproduction (unit test, fails today):**
|
||
```
|
||
npm run test:run
|
||
FAIL src/lib/server/quotes-service.test.ts > generateQuoteNumberServer > starts the month at 001
|
||
expected 'Q202607001' to be 'Q-2026-07-001'
|
||
```
|
||
Three of the four failure messages are the same root cause (variants of `increments on each call`, `concurrent calls no race`, `createQuoteFromEmail creates a new quote, customer, company, items, and attachments`).
|
||
- **Root cause:**
|
||
```ts
|
||
function formatQuoteNumber(year, month, seq) {
|
||
return `Q${year}${String(month).padStart(2, "0")}${String(seq).padStart(3, "0")}`;
|
||
}
|
||
```
|
||
Missing the two dashes that every other surface in the codebase assumes. `AGENT_INSTRUCTIONS.md` line 153-156 and `docs/SHIPPED.md` line 161 both document `Q-YYYY-MM-NNN`. The n8n v5 side indexes against this regex (`src/lib/server/quotes-service.test.ts:367`: `/^Q-\d{4}-\d{2}-001$/`).
|
||
- **Recommended fix:**
|
||
```ts
|
||
return `Q-${year}-${String(month).padStart(2, "0")}-${String(seq).padStart(3, "0")}`;
|
||
```
|
||
- **Verification:** `npx vitest run src/lib/server/quotes-service.test.ts` shows all four previously-failing tests now green. Run `scripts/smoke/counter.ts` and `counter-concurrent.ts` against dev PB to confirm no other downstream consumer relied on the wrong shape.
|
||
|
||
---
|
||
|
||
#### [P1] — Order-number generator picks wrong month on rollover
|
||
|
||
- **Location:** `src/lib/server/orders-service.ts:41-55`
|
||
- **Severity:** P1 — silently resets the sequence at every month boundary; also racy by construction.
|
||
- **Reproduction (unit test, fails today):**
|
||
```
|
||
npm run test:run
|
||
FAIL src/lib/server/orders-service.test.ts:228:57
|
||
expected 'ORD-2026-07-008' to be 'ORD-2026-06-008'
|
||
```
|
||
- **Root cause:** `prefix = ` is built from `new Date()` even when the matched sequence comes from a different month. Two failure modes follow:
|
||
1. The unit test pins the contract: when the latest matching order sits in June and we're now in July, the next order should still be `ORD-2026-06-008` — i.e. **the prefix is derived from the matched order, not wall-clock time**. Today the implementation breaks that.
|
||
2. Even with a fixed prefix, the generator reads-then-writes without atomic increment — two concurrent `createOrderFromQuote` calls can both observe `ORD-...-007` and both write `ORD-...-008`. (`quotes-service.ts` gets atomicity right via `quote_counters` + `last_seq+` — the orders-side has no equivalent.)
|
||
- **Recommended fix:** (a) Derive `year`/`month` from the matched order's prefix, fall back to `now` when the table is empty, (b) introduce an `order_counters` collection (mirroring `quote_counters`) for the cross-month rollover, or (c) accept the restart-at-001 behaviour but pin a test that documents it as intentional. The current "use `now` always" path is the worst of the three options.
|
||
- **Verification:** `npx vitest run src/lib/server/orders-service.test.ts`; then a manual `createOrderFromQuote` after midnight on 2026-06-30/2026-07-01 to confirm what the operator actually sees.
|
||
|
||
---
|
||
|
||
#### [P1] — TypeScript excess-property error in collection-confirm test
|
||
|
||
- **Location:** `src/app/(app)/collection-confirm/view-model.test.ts:278` (vs `src/app/(app)/collection-confirm/view-model.ts:210`)
|
||
- **Severity:** P1 — `npx tsc --noEmit` fails; CI cannot pass.
|
||
- **Reproduction:**
|
||
```
|
||
src/app/(app)/collection-confirm/view-model.test.ts(278,63): error TS2353:
|
||
Object literal may only specify known properties, and 'setBy' does not exist in type 'Pick<InvoiceDetails, "number" | "date">'.
|
||
```
|
||
- **Root cause:** `buildConfirmNote` is declared `Pick<InvoiceDetails, "date" | "number">`; the test passes `{ date, number, setBy, setByName, setAt }`. Two fixes are defensible — pick the one that matches intent.
|
||
- **Recommended fix (pick one):**
|
||
- Drop the extra fields from the test:
|
||
```ts
|
||
buildConfirmNote({ date: "2026-07-08", number: "INV-2" })
|
||
```
|
||
- Or change `buildConfirmNote` to accept the full type if those fields are needed later:
|
||
```ts
|
||
export function buildConfirmNote(invoice: InvoiceDetails): string { … }
|
||
```
|
||
- **Verification:** `npx tsc --noEmit` shows zero errors.
|
||
|
||
---
|
||
|
||
#### [P1] — Custom regex-based HTML sanitizer is bypassable
|
||
|
||
- **Location:** `src/components/quote-email-card.tsx:166-233` (`sanitizeHtml`)
|
||
- **Severity:** P1 — defence-in-depth layer only; author documents this openly but it is invoked with `dangerouslySetInnerHTML` on remote email bodies from arbitrary senders.
|
||
- **Reproduction (conceptual, not exploited live):** a hostile email body containing `javascript:alert(1)` inside an `<a href="…">` tag, or attribute injection via mismatched quotes (the regex on line 211 accepts `'` and unquoted values, which can smuggle payloads past the per-tag whitelist).
|
||
- **Root cause:** Regex substitution cannot reliably parse HTML. The author already flags this at line 171-174 ("NOT a general-purpose sanitizer (use DOMPurify for that)"). It survives today because it sees friendly RFQ traffic, but the email pipeline is the canonical XSS ingress for a CRM and the cost of getting it wrong is high.
|
||
- **Recommended fix:** add `dompurify` + `isomorphic-dompurify`, replace `sanitizeHtml` body with:
|
||
```ts
|
||
import DOMPurify from "isomorphic-dompurify";
|
||
return DOMPurify.sanitize(input, {
|
||
ALLOWED_TAGS: [...ALLOWED_TAGS],
|
||
ALLOWED_ATTR: [...ALLOWED_ATTRS.a, ...ALLOWED_ATTRS.img, ...ALLOWED_ATTRS.th, ...ALLOWED_ATTRS.td, ...ALLOWED_ATTRS.table],
|
||
ALLOW_DATA_ATTR: false,
|
||
});
|
||
```
|
||
- **Verification:** add a vitest with two payloads — `javascript:` href, `javascript:alert(1)` href, both should be neutralised.
|
||
|
||
---
|
||
|
||
#### [P1] — `scripts/setup-pocketbase.ts` is destructive with no backup guard
|
||
|
||
- **Location:** `scripts/setup-pocketbase.ts:50-63`
|
||
- **Severity:** P1 — operator footgun. Hard-coded admin credentials (line 4-5) `david.els1@outlook.com / 1234567890` and the script unconditionally `DELETE`s 12+ collections against `http://127.0.0.1:8090`.
|
||
- **Reproduction:** run `npm run setup` against any PB instance that already has data — every listed collection is wiped.
|
||
- **Root cause:** the comment at line 48-49 acknowledges the destructive intent ("delete all existing (including stale ones from other projects)") but there is no confirmation prompt, no backup step, no `dryRun` flag.
|
||
- **Recommended fix:**
|
||
- Move the hardcoded creds to env reads (`PB_ADMIN_EMAIL`, `PB_ADMIN_PASSWORD`) with safe fallback only in `NODE_ENV !== "production"`.
|
||
- Add a `--confirm-wipe` flag and refuse to run without it.
|
||
- Add a `takeSnapshot()` call that writes `pb-exports/pb_schema-pre-setup-{ts}.json` first.
|
||
- **Verification:** run the script against a sandbox PB with non-empty data and confirm: (a) it refuses without the flag, (b) with the flag it produces a snapshot.
|
||
|
||
---
|
||
|
||
#### [P2] — 50 ESLint errors, dominated by React 19 hook violations
|
||
|
||
- **Location:** see table below
|
||
- **Severity:** P2 — the build succeeds but CI lint gates will fail, and the new `react-hooks/set-state-in-effect` rule is real: it flags code that triggers cascading renders.
|
||
- **Reproduction:** `npm run lint`.
|
||
- **Counts:**
|
||
| Rule | Count |
|
||
|---|---|
|
||
| `react-hooks/set-state-in-effect` | 27 |
|
||
| `@typescript-eslint/no-explicit-any` | 17 |
|
||
| `jsx-a11y/alt-text` (false positives on `@react-pdf/renderer` `Image`) | 12 |
|
||
| `react/display-name` | 1 |
|
||
| `@typescript-eslint/no-require-imports` | 1 |
|
||
| `Cannot access variable before it is declared` | 1 |
|
||
| `react/no-unescaped-entities` | 1 |
|
||
| **Total** | **50 errors / 86 warnings** |
|
||
- **Top offender files:**
|
||
- `src/components/order-form.tsx` (initNewOrder + 4 sub-effects → cascading renders when the form loads)
|
||
- `src/components/quote-pdf-viewer.tsx:47` (sync reset on close path)
|
||
- `src/components/supplier-map.tsx:150` (fetch-on-mount in effect)
|
||
- `src/components/ui/quote-search.tsx:157` (fetch on query change)
|
||
- `src/app/(app)/collection-map/page.tsx:437,441,504` (4 instances)
|
||
- `src/app/(app)/quotes/[id]/page.tsx:859,867` (2 instances)
|
||
- `src/lib/pdf/purchase-order-document.tsx:122` and `quote-document.tsx:120` — false-positive alt warnings on `@react-pdf` `<Image>` (PDF renderer, not HTML)
|
||
- **Recommended fix:**
|
||
- Where sync state writes happen in cleanup, move them to `useLayoutEffect` (mounted → unmount) or compute from props instead.
|
||
- Add `// eslint-disable-next-line jsx-a11y/alt-text` on the two PDF `<Image>` lines or scope the rule to `.tsx` files matching `**/page.tsx` only.
|
||
- Replace `any` annotations with `unknown` + narrow — or `as` for genuinely-opaque third-party shapes.
|
||
- **Verification:** `npm run lint` shows 0 errors, 0 warnings (or warnings only).
|
||
|
||
---
|
||
|
||
#### [P2] — 6 Vitest failures in production code paths
|
||
|
||
- **Location:** see counts
|
||
- **Severity:** P2 — same root causes as P1 #1, #2 (quote/order numbering) and P1 #3 (collection-confirm test). Plus one UI-only test.
|
||
- **Reproduction:** `npx vitest run`.
|
||
```
|
||
Test Files 3 failed | 52 passed (55)
|
||
Tests 6 failed | 483 passed (489)
|
||
```
|
||
- **Failures:**
|
||
| File | Test | Root cause |
|
||
|---|---|---|
|
||
| `src/lib/server/quotes-service.test.ts` | `generateQuoteNumberServer > starts the month at 001` | See P1 #1 |
|
||
| `src/lib/server/quotes-service.test.ts` | `generateQuoteNumberServer > increments on each call` | See P1 #1 |
|
||
| `src/lib/server/quotes-service.test.ts` | `generateQuoteNumberServer > produces unique numbers under concurrent calls (no race)` | See P1 #1 (and possibly also a race) |
|
||
| `src/lib/server/quotes-service.test.ts` | `createQuoteFromEmail > creates a new quote, customer, company, items, and attachments` | See P1 #1 |
|
||
| `src/lib/server/orders-service.test.ts` | `createOrderFromQuote > increments the sequence using the latest matching order_number` | See P1 #2 |
|
||
| `src/app/(app)/collection-pool/page.test.tsx` | `CollectionPoolPage sort control > defaults to operational sort and reorders rows when nearby suppliers is selected` | UI: rendered DOM shows rows `[Alpha, Charlie]` not `[Alpha, Bravo, Charlie]` — likely the "nearby" sort colocates Bravo/Charlie by shared coords in the test fixture |
|
||
- **Recommended fix:** the first 5 fall out of the P1 fixes. For the sort test, inspect `src/app/(app)/collection-pool/page.test.tsx` fixture and the `nearby` comparator at `src/app/(app)/collection-pool/page.tsx` (search for `nearby`).
|
||
- **Verification:** `npx vitest run` reports 0 failures.
|
||
|
||
---
|
||
|
||
#### [P2] — All pages are `"use client"`; nothing is server-rendered
|
||
|
||
- **Location:** every file under `src/app/(app)/**/page.tsx` (~24 files), `src/app/layout.tsx` wraps `AuthProvider` (client), and `src/app/(app)/layout.tsx:1` is `"use client"`.
|
||
- **Severity:** P2 — performance + SEO + first-paint hit. Dashboard, list pages, PDF view all hydrate fresh in the browser.
|
||
- **Recommended fix:** the (app)/layout auth check should move to a server component that reads the PB auth cookie via `await cookies()` (per Next 16 docs, `01-app/.../cookies.md`), letting list pages render server-side. The `<Sidebar>` and provider wrappers stay client-side.
|
||
- **Verification:** Lighthouse mobile run on `/dashboard` and `/quotes` shows improved LCP and a smaller initial JS payload.
|
||
|
||
---
|
||
|
||
#### [P2] — `og-image.png` and `og-square.png` are too large for social cards
|
||
|
||
- **Location:** `public/og-image.png` (732K), `public/og-square.png` (1.1M), `public/twitter-image.png` (772K).
|
||
- **Severity:** P2 — page weight + slow social-share unfurls. `og-image` is referenced on every `metadata.openGraph.images` so it's effectively eagerly fetched by social previews.
|
||
- **Recommended fix:** re-export at ≤ 200K each as JPEG/WebP using the existing `letterhead` assets. Next/Image `priority` is not used here; add a `public/og-square.{webp,jpg}` and reference in `layout.tsx:111-119,123`.
|
||
- **Verification:** `du -sh public/og-*` ≤ 200K and a manual Slack/Twitter card preview.
|
||
|
||
---
|
||
|
||
#### [P2] — E2E coverage is narrow vs page surface
|
||
|
||
- **Location:** `e2e/` (only 5 specs: `_helpers`, `agent-api`, `auth`, `health`, `management`).
|
||
- **Severity:** P2 — 22+ pages have no E2E coverage. Recent feature work on `invoicing`, `collection-confirm`, `collection-map` (date filter + unassign), `collection-pool` (nearby sort), `sprint` (categories + claims), `purchase-orders` (PDF tokens + additional attachments), and `companies-coordinates` is entirely uncovered at the E2E layer.
|
||
- **Recommended fix:** add at minimum specs for `invoicing`, `purchase-orders/{id}/pdf`, `collection-pool` (sort + Mark Ready dialog), `sprint/items/{id}/claim`. The unit coverage is strong (`483/489`) but it tests PB services and isolated components, not composed flows.
|
||
- **Verification:** `npx playwright test --list` shows the new specs and they pass locally.
|
||
|
||
---
|
||
|
||
#### [P2] — Aggressive PB filter string interpolation
|
||
|
||
- **Location:** `src/lib/pb-helpers.ts` (multiple sites: lines 437, 577, 947, 1066, 1436, 1439).
|
||
- **Severity:** P2 (the dominant flows use it correctly, but the surface is large).
|
||
- **Reproduction (conceptual):** in `await pb.collection("orders").getList(1, 50, { filter: `order_number ~ "${search}"` })`, a `search` containing `"` is escaped (`\"`) and newlines/backslashes are not. The resulting filter is well-formed today because every call site uses IDs or short alphanumeric tokens, but the pattern does not survive a refactor.
|
||
- **Root cause:** `pb-helpers.ts:1436-1439` shows two calls in a row:
|
||
```ts
|
||
clauses.push(`category = "${filters.category.replace(/"/g, '\\"')}"`);
|
||
clauses.push(`brand = "${filters.brand.replace(/"/g, '\\"')}"`);
|
||
```
|
||
NB: PB string literals also treat `\` specially; the escape omits that. The PB native SDK offers `pb.filter(expr, params)`-style helpers in 0.21+ but the project uses raw strings.
|
||
- **Recommended fix:** introduce one internal `pbFilter(field, value, op)` helper in `src/lib/pb-helpers.ts` that handles quote/backslash escaping once and test it.
|
||
- **Verification:** add a unit test passing `'; --` and `\\` as inputs; the resulting filter string either succeeds (escaped) or throws the expected PB error envelope.
|
||
|
||
---
|
||
|
||
#### [P2] — `lucide-react` dependency version looks unusual
|
||
|
||
- **Location:** `package.json:58` declares `"lucide-react": "^1.16.0"`.
|
||
- **Severity:** P2 — verified on npm that lucide-react actually publishes 1.x via the maintainers (1.24.0 is current); package-lock confirms 1.16.0 is the installed version. **Not a real bug**, but flag for a fresh install — if the upstream version diverges from the lockfile's lockSpec pattern, imports break.
|
||
- **Recommended fix:** in CI, `npm ci` (strict lockfile) so an upstream version drift never surprises a build. Add `engines.npm` if not present.
|
||
- **Verification:** `npm ci` on a fresh checkout succeeds.
|
||
|
||
---
|
||
|
||
#### [P2] — Destructive "wipe" in `scripts/setup-pocketbase.ts`
|
||
|
||
(See also P1 #6 — that's the same script; the duplication is intentional here to call out the operator UX angle separately for triaging.) Already covered.
|
||
|
||
---
|
||
|
||
#### [P3] — Loose PNG/screenshot files in the repo root
|
||
|
||
- **Location:** repo root: `Screenshot 2026-07-09 at 01.12.32.png` (80K), `bulk_modal.png` (177K), `invoicing-*.png` × 6 (~800K total), `page-overview.png` (101K), `single_modal.png` (183K).
|
||
- **Severity:** P3 — repo bloat, accidentally-committed investigation screenshots. Not referenced by code.
|
||
- **Recommended fix:** move to `docs/_screenshots/` or `.gitignore` patterns like `Screenshot *.png`, `bulk_modal.png`, `single_modal.png`, `invoicing*.png`, `page-overview.png`.
|
||
- **Verification:** `git ls-files | grep -E '\.(png|jpg)$' | grep -v public/` returns nothing.
|
||
|
||
---
|
||
|
||
#### [P3] — Stray large JPG inside `scripts/`
|
||
|
||
- **Location:** `scripts/mrbxnim4-telegram-cloud-photo-size-4-6016860144018328845-y.jpg` (52K)
|
||
- **Severity:** P3 — looks like a Telegram-saved reference photo that escaped review.
|
||
- **Recommended fix:** move to `docs/_references/` or delete.
|
||
|
||
---
|
||
|
||
#### [P3] — Loose ESM debug scripts at `scripts/dev/`
|
||
|
||
- **Location:** `scripts/dev/debug-*.{ts,tsx}` (12 files).
|
||
- **Severity:** P3 — pulled into ESLint scope (driving the "50 errors" count up). Code in them is incidental.
|
||
- **Recommended fix:** move to `scripts/dev/_archive/` or add `scripts/dev/**` to `eslint.config.mjs` `globalIgnores`. Either is fine — naming should signal "operator-only, not shipped".
|
||
- **Verification:** `npm run lint` count drops; nothing in production import-path references them.
|
||
|
||
---
|
||
|
||
#### [P3] — `tsconfig.json` excludes `scripts/**` but ESLint scans them
|
||
|
||
- **Location:** `tsconfig.json:42` vs `eslint.config.mjs:9-15`.
|
||
- **Severity:** P3 — symptom, not cause. Many of the 50 lint errors live in scripts that aren't type-checked. Either fix the scripts or exempt them.
|
||
- **Recommended fix:** add `scripts/**` to the ESLint `globalIgnores`.
|
||
|
||
---
|
||
|
||
#### [P3] — Modal escape-on-backdrop-click behaviour
|
||
|
||
- **Location:** Base UI `Dialog` (in `src/components/ui/dialog.tsx`) doesn't ship escape-on-backdrop-click out of the box on this version's setting; custom escape handler in `quote-pdf-viewer.tsx:90-97` only.
|
||
- **Severity:** P3 — minor UX inconsistency. Verify by clicking outside the dialog or pressing `Esc`.
|
||
- **Recommended fix:** confirm with a manual smoke pass and standardise on the Base UI default.
|
||
|
||
---
|
||
|
||
#### [P3] — `ThemeBootstrapScript` uses `dangerouslySetInnerHTML`
|
||
|
||
- **Location:** `src/components/theme-bootstrap.tsx` (script with `dangerouslySetInnerHTML={{__html: code}}`)
|
||
- **Severity:** P3 — acceptable because `code` is a literal authored string in the same file (no remote content), but flag for future readers.
|
||
|
||
---
|
||
|
||
### What's Working Well ✅
|
||
|
||
- **Test scaffolding is healthy.** 483/489 unit tests pass; failures are concentrated in 3 files and tied to the P1 findings (not flaky code).
|
||
- **Next.js 16 async params** are used consistently in every `[id]/route.ts` and `[id]/page.tsx`.
|
||
- **shadcn/ui base-nova + Base UI primitives** are used consistently across all 14 base components (`button`, `dialog`, `dropdown-menu`, `input`, `popover`, `scroll-area`, `select`, `separator`, `sheet`, `switch`, `tabs`, `avatar`, `badge`, `checkbox`). Zero Radix in `src/components/ui/`.
|
||
- **Heavy API routes declare `runtime = "nodejs"` + `dynamic = "force-dynamic"` correctly** (`/api/v1/quotes/[id]/pdf`, `/purchase-orders/[id]/dispatch`, `/purchase-orders/[id]/send-email`, etc.).
|
||
- **PB filter fallback pattern** for the legacy vs template schema is in place (`pb-helpers.ts:228-260`, "Try the new filter first; fall back to legacy").
|
||
- **File proxy at `/api/v1/files/[collection]/[recordId]/[filename]`** has an explicit whitelist, inline-vs-attachment disposition, and cache headers (`no-cache`).
|
||
- **Theme bootstrap script** prevents FOUC via `suppressHydrationWarning` + pre-paint script.
|
||
- **`.gitignore`** correctly excludes `.env*`, `.mcp.json`, `test-results/`, `.next/`, `tsbuildinfo`.
|
||
- **No `TODO`/`FIXME`/`XXX`/`HACK` markers** anywhere under `src/` or `scripts/`.
|
||
- **PWA scaffold** present (`/manifest.json`, `service-worker-registration`, `/sw.js`).
|
||
- **Standalone Next.js output** in `next.config.ts:4` — matches the Dockerfile multi-stage build.
|
||
- **`allowedDevOrigins` includes `192.168.1.122`, `192.168.1.128`** — operator explicitly accommodated LAN phone testing; correct.
|
||
|
||
---
|
||
|
||
### Recommendations
|
||
|
||
1. **Adopt a `react-dom/test-utils` lint baseline for `react-hooks/set-state-in-effect`.** The 27 `set-state-in-effect` violations are a real React 19 smell. Convert the cleanest 3-5 first (`order-form.tsx`, `quote-search.tsx`, `supplier-map.tsx`'s `fetchSuppliers()`) to a `useEffect(() => { let cancelled = false; ... }, [])` pattern with derived state via `useState` initialiser; lint baseline should land at zero (or scoped-down per directory).
|
||
2. **Move auth gating server-side.** Replace the `(app)/layout.tsx` client redirect (`src/app/(app)/layout.tsx:12-16`) with a server component that reads the PB auth cookie via `await cookies()`. This unlocks server-side data fetching for the 24 `use client` pages — biggest single perf win available.
|
||
3. **Atomic counter for orders** (mirroring `quote_counters`). Even if the month-rollover logic is intentionally fixed to use `now`-month only, two simultaneous `createOrderFromQuote` calls still race. Add `scripts/migrations/2026-07-10-create-order-counters.ts`.
|
||
4. **DOMPurify swap-in for the email sanitizer.** Use `isomorphic-dompurify`; same ALLOWED_TAGS/ALLOWED_ATTRS. Add 2 vitest cases covering `javascript:` and `:` payloads.
|
||
5. **Convert tests under `src/lib/server/quotes-service.test.ts` and `orders-service.test.ts` to a CI gate.** Add a `lint:fix` script that auto-applies the 29 fixable warnings and gates the build.
|
||
6. **Add E2E coverage for invoicing and PO PDF dispatch flows.** The new PDF-token + attachment work has unit coverage but no composed-flow coverage; one Playwright spec per flow closes the gap.
|
||
|
||
---
|
||
|
||
### Next Steps
|
||
|
||
> Tackle in this order:
|
||
>
|
||
> 1. **Fix `formatQuoteNumber`** — 1 line, unblocks 4 tests + downstream n8n.
|
||
> 2. **Fix `buildConfirmNote` test (or type)** — 1 line, unblocks `tsc --noEmit`.
|
||
> 3. **Fix `createOrderFromQuote` month derivation** — clarify intent with the team (extend latest vs. reset-at-month-boundary), pin a test, ship.
|
||
> 4. **Move auth gating server-side** — biggest perf win, broad but mechanical.
|
||
> 5. **DOMPurify** — small, immediate XSS risk reduction.
|
||
> 6. **Lint zero** — schedule a 1-2 hour pass to land `react-hooks/set-state-in-effect` + `no-explicit-any` cleanups.
|
||
> 7. **E2E gap-fill** for invoicing + PO dispatch.
|
||
>
|
||
> Want me to:
|
||
> - Generate the fixes for the top 3 P1s?
|
||
> - Open follow-up issues for the lint + e2e gaps?
|
||
> - Run a security audit pass (XSS, CSRF, rate limiting)?
|