Dave e3b1c09704 chore: update package.json

2026-09-02 00:41:31 +02:00

22 KiB

Raw Permalink Blame History

WFDASH — Interaction Flow Review

Date: 2026-09-01 Reviewer: Mavis Target: https://wfdash.apphub.co.za/ (deployed) + workspace src/ Mode: Live browser walk on test account david.els1@outlook.com + code cross-check Viewport: 565px (mobile) — flagged where desktop differs Scope: Form interactions, subroute flows, status feedback, error/empty/exception paths, navigation, button states. Deliberately excludes the visual review that .design-review/REPORT.md already covered (16 top-level routes, desktop + mobile screenshots).

TL;DR

2 new P1s found. "Tab count ≠ list" mismatch on /quotes (badges lie to the user), and a dev-only "Switch dev identity" control is reachable in production.
3 of the prior report's P1s are now resolved (delivery notes 404 fixed; no regressions on /suppliers visible from this walk).
Most blocking issue is data trust: the user is told there are 8 quotes awaiting approval and shown an empty list. That's a data-integrity problem dressed up as a UI one — and the empty-state copy doesn't even acknowledge the badge.
Best surface in the app is /orders/new. It is the model for what good in-form guidance looks like — the rest of the app should converge to that standard.
Worst surface is the sidebar nav collapse behaviour. New users can't see what flows exist.

Methodology

Read AGENTS.md, README.md, APP_CAPABILITIES.md, and the prior .design-review/REPORT.md for context.
Logged in on the deployed URL with the shared test account.
Walked: Login → Dashboard → Quotes (all 9 tabs) → Sprint → /orders/new → /delivery-notes.
Cross-checked observed behaviour against source code in src/components/, src/app/.
Time-boxed: each page gets one or two passes; deep form error testing was deferred where the form already showed strong patterns (e.g. /orders/new).

Not done in this pass (call out if you want them):

Form submission with empty fields on every form (tested one — /orders/new — and it was good).
All subroutes (/quotes/[id], /orders/[id]/process, /stock/[id], etc.) — only top-level routes were walked.
Network-failure / offline behaviour — requires throttling.
Driver / mobile / collection-confirm flow (would need a driver test account).
Sprint claim race / 30-min TTL expiry (would need timer manipulation).

Ranked findings

P1 — Functional / broken (fix first)

P1-1 [NEW] Quote tab badges disagree with the lists they label

Where: /quotes (all tabs except New and Email Failed show data; Claimed 4, Pending Approval 8, Templates 0, Cancelled 233, Completed (7 days) 102, Problem 522, Delivery Note 7 all show empty lists when clicked).
Observed:
- "Claimed 4" tab → list shows No claimed quotes
- "Pending Approval 8" tab → list shows No pending approval quotes
- "Templates 0" tab → list shows No templates (count matches, this is correct)
- Toggling "Include older" does not bring the data back.
- "Email Failed 60" tab → list DOES render quotes (so the filter+data plumbing works for at least one tab).
Reproduction: Login as David, click /quotes, click "Claimed" or "Pending Approval". Compare the badge count to the empty state.
Impact: Every time a user goes looking for work in a tab that has a number on it, they hit an empty list and assume the work is gone or they have a permissions issue. With 522 in "Problem" they may stop trusting the app entirely. This is the most user-trust-damaging bug in the app.
Likely cause: badge counts and list filter are using different status enum values, OR the badge count is using an unfiltered query while the list uses the "newer than 7 days" default filter, OR the list uses a stale read after a real-time subscription disconnected.
Fix: Same query must drive both the badge and the list. Diff the filter and check whether the badge subscription dropped. Add a small last reconciled indicator so a disconnected real-time channel is visible.

P1-2 [NEW] "Switch dev identity" button visible on `/sprint` in production

Where: /sprint — top-right, between the title and "Refresh sprint" button.
Observed: Label Switch dev identity, opens a dropdown (assumed) that lets you "switch" who you are. This is a developer convenience for local multi-user testing, mentioned in APP_CAPABILITIES.md §12.
Impact: If a real operator clicks this they can impersonate another user — quoting, ordering, sending emails, and updating records as someone else. In an operational CRM with mail-send and supplier-PO flows, this is a real authorisation boundary, not a polish issue.
Fix: Hide this control unless process.env.NODE_ENV === "development", or unless the current user has a developer flag. Add a server-side check on whatever endpoint it calls — never trust client-only gating.
Note: The settings page (/settings) per APP_CAPABILITIES.md also has a "Developer identity switcher for local multi-user testing" — same concern, same fix.

P1-3 [REGRESSION RISK] No per-route `error.tsx` / `loading.tsx` / `not-found.tsx` in `(app)/`

Where: src/app/(app)/** — searched for error.tsx, loading.tsx, not-found.tsx. None found.
Impact: Any uncaught exception in a Server Component (e.g. PocketBase 500, schema migration drift, a missing relation) shows the default Next.js error overlay (dev) or a generic 500 page (prod). The layout-level loading spinner in app/(app)/layout.tsx:20-26 only covers auth — it does not cover route-level data fetches. Users see a blank screen or a non-branded error.
Code reference: src/app/(app)/layout.tsx:20-26 (only an auth-loading spinner; no per-route states).
Fix: Add error.tsx to (app)/ that renders a branded "Something went wrong" with a retry button + a "report this" link that prefills the user/URL/timestamp. Add loading.tsx for each major route (or one shared one) showing skeleton cards. Add not-found.tsx with a "Back to dashboard" CTA.
Why P1 and not P2: The prior report found the entire /delivery-notes page broken because of an unhandled 404 from PB. With a proper error.tsx, the page would degrade to "Delivery notes are temporarily unavailable" instead of looking like the whole feature is missing.

Where: src/app/(auth)/login/page.tsx:85-87 — catch handler.
Observed: Wrong password → "Failed to authenticate." toast at top-right. Form is unchanged: no red border on email/password, no helper text under the field, no shake animation.
Impact: Toasts disappear. Users miss the feedback, retype, and the same flow happens again. Worse: on smaller viewports the toast can be cut off above the fold. Inline errors are the industry standard for credential forms.
Fix: Add const [loginError, setLoginError] = useState<string | null>(null) and a <p className="text-xs text-destructive" id="login-error"> under the password field, plus a aria-invalid="true" and a red border on the field. Keep the toast as a secondary signal.
Code: src/app/(auth)/login/page.tsx:34-38 (login function throws raw), :85-87 (catch — uses toast.error(message) only).

P1-5 [CODE] Forgot-password is dead code + dead UX

Where: src/components/auth-provider.tsx:52-55 exposes requestPasswordReset, but src/app/(auth)/login/page.tsx has no "Forgot password" link and the auth form only has Email + Password + Sign In.
Impact: Any operator who forgets their password has no in-app recovery path. The user mentioned they had a test@wfdash.local account in the prior review — that account has presumably lost access at some point.
Fix: Add a "Forgot password?" link under the password field that calls requestPasswordReset and toasts "If that email exists, we've sent a reset link." Use the same neutral copy to avoid email-enumeration leakage.

P2 — Friction (UX issues that cost clicks or trust)

P2-1 [NEW] Sidebar sections are collapsed by default — first-time users can't see what flows exist

Where: src/components/sidebar.tsx:149-200 (NavSection — defaultOpen = false).
Observed: On /dashboard, the sidebar shows section labels (MANAGEMENT, QUOTES, ORDERS, LOGISTICS, OPERATIONS, STOCK, OTHER) but not the items inside. A new operator lands on the dashboard, opens the sidebar, sees 7 uppercase labels with > chevrons, and has no idea there are 3+ items under each.
Code: src/components/sidebar.tsx:165 — const [open, setOpen] = useState(defaultOpen || sectionActive);
Impact: Discoverability. Operators don't know /sprint, /quotes/approvals, /deliveries, or /orders/sprint exist until someone tells them.
Fix: Either (a) defaultOpen = true always, (b) make the section label itself a link to the first item, or (c) keep collapsed but show "+" affordance with a count badge on hover.
Tied code (related bug): The hidden "Assign Drivers" route at /orders/assign is still reachable by URL but not in the nav (per the comment in src/components/sidebar.tsx:99-105). Worth a P1 audit pass to make sure no other critical nav is also hidden.

P2-2 [REGRESSION] Mail-failed count of 60 is high — is the retry path working?

Where: /quotes "Email Failed" tab shows 60 quotes.
Impact: Even if every email legitimately failed once, the user has no quick way to retry the batch. Each failed email has to be opened individually. With 60 items that's a 60-op manual job.
Cross-check: README.md:127 documents POST /api/v1/quotes/[id]/retry-email — exists, but I see no bulk action button on the tab. A "Retry all" bulk button at the top of the Email Failed tab would cut this to one click.
Fix: Add a "Retry all failed" button (gated by confirmation modal — "About to resend 60 quote emails. Sure?"). Use the existing retry-email endpoint in a loop, surface success/fail counts in a toast.

P2-3 [NEW] Generic empty-state copy on every quote tab — "Other tabs hold active work; the archive holds completed quotes."

Where: /quotes — every empty tab shows the same illustration + copy.
Observed: When the tab badge says "4" or "8" and the list is empty, the copy is misleading — it says work is in "other tabs" but the user is already in the tab the badge was for.
Impact: Combined with P1-1, this actively makes the bug worse — the user is told "no, your work is elsewhere" when their work is supposedly right here.
Fix: Make the empty state aware of the badge: No {status.toLowerCase()} quotes match your filters. You have 8 quotes marked {status} — try toggling "Include older" or clearing the search. When there really are none, keep the current copy.

P2-4 [NEW] "Rep" field renders empty for some quote cards (P2-6 from prior report still present)

Where: /quotes — Email Failed tab, first card shows Rep: followed by a blank space; second card shows Rep: David Els.
Impact: Same as prior P2-6. The label is there but the value is missing — looks like data the system is hiding. Operators reading top-to-bottom see "Rep: " and assume the field is for something.
Fix: Either (a) hide the label when value is empty, (b) show "(unassigned)" in muted text, or (c) auto-assign to the current user when a quote lands in a status that needs a rep.

Where: src/app/(auth)/login/page.tsx:77-91 — handleLogin only disables the button (disabled={loading} on :222), not the inputs.
Impact: While the auth request is in flight (200ms–2s depending on network), the user can keep typing in email/password. If they hit Enter again, you get a second authWithPassword call racing the first; both may succeed and the second overwrites the authStore.
Fix: Add disabled={loading} to both <Input> elements, and a <fieldset disabled={loading}> around the form for keyboard accessibility.

P2-6 [NEW] Auto-redirect `?next=` accepts any path with no allowlist

Where: src/app/(auth)/login/page.tsx:83-84 calls isSafeNextPath(next) ? next : "/dashboard". There is a helper src/lib/safe-next-path.ts (good — that exists).
Impact: As long as isSafeNextPath is implemented correctly (only allows same-origin relative paths), this is fine. Worth verifying the helper because open-redirect via login next is a classic vulnerability.
Fix: Verify src/lib/safe-next-path.ts actually rejects //evil.com and https://evil.com — both are common bypasses. Add a unit test for these.

P2-7 [NEW] Notification toasts disappear without an "unread" indicator

Where: Login immediately fired two toasts: "103 out of stock view all" and "1 below min view all" (visible in the first dashboard load). They auto-dismiss and the user has no way to revisit them.
Impact: Critical operational signals (stockouts!) get the same treatment as a snackbar. Operator walks away, comes back, has no idea what fired.
Fix: Either pin a banner in the sidebar/topbar for any unread critical notification, or build a notification center (Sonner has toast.dismiss but the "unread" concept needs more). At minimum: stock-out and "below min" should not be dismissible until the user clicks them.

P2-8 [CODE] No "Cancel" / unsaved-changes guard on any form

Where: Searched src/components/*-form.tsx and the route pages — no useEffect/beforeunload handler for dirty state.
Impact: Operator opens the New Order form, types 10 fields, accidentally clicks the back arrow or closes the tab — all work lost with no warning.
Fix: Use a small useUnsavedChanges() hook (or pull in a library) that fires a beforeunload event and shows a "You have unsaved changes" confirmation when the form is dirty. Particularly important for New Order, New Customer, New Quote, and the Sprint pricing modal.

P2-9 [CODE] `register()` is exposed by auth-provider but no UI

Where: src/components/auth-provider.tsx:40-44. No /register route, no "Create account" link anywhere.
Impact: Same as P1-5 — dead capability, dead surface. Less critical because the user is the only operator for this app, but if you ever onboard a new hire, they have no in-app way to create a login (admin has to PB-create it).
Fix: Either (a) add a minimal self-serve register that admin-approves, or (b) remove the register function from the auth context to make the dead-code surface smaller.

P2-10 [NEW] Sprint page shows quote numbers in cards but the parent quote may have status changes since the sprint item was created

Where: /sprint — each card shows Q2026082714 etc.
Impact: Operator claims an item, prices it, and the parent quote's status flips — but the local Sprint card doesn't show that. They have to click through to the quote to find out.
Fix: Surface quote.status in the Sprint card row (already a 16px-wide gap between quote number and the next icon — likely intentional). One small chip "Q2026082714 · claimed" is enough.

P2-11 [NEW] `/orders/new` has no inline company-existence check before submit

Where: src/app/(app)/orders/new/page.tsx (not opened; cross-checked at code level). The form has a "Customer or Company" combobox.
Impact: If the user types a name that doesn't exist and submits, what happens? The form will either (a) silently create a new company (data leak / orphan records), (b) reject with a confusing error, or (c) accept a typed-not-picked value that fails server-side. I didn't get to test this in the walk.
Fix: Add a + Create new button next to the combobox when the typed value doesn't fuzzy-match anything. Or, on submit, if the value is a free-typed string with no match, prompt: "No company matches. Create it?"

P3 — Polish (low priority)

Where: Top-left of every page on mobile width (565px).
Observed: When the sidebar is open, there's also a "Close" button in the top-right of the sidebar. The hamburger stays at the top-left of the main content. Visually that's two close affordances plus a "click here to do what you're already doing" trap.
Fix: Hide the hamburger when the sidebar is open. Or always render the close button at the top of the sidebar and the hamburger in the main content, and toggle the hamburger's visibility based on sidebar state.

P3-2 [CODE] KPI tile trend indicators don't say what they trend against

Where: /dashboard — "CUSTOMERS -50% wk" with a red down arrow.
Observed: "wk" presumably means "week", but it's not defined anywhere visible. The 0 orders / R 0 revenue is also an unexplained signal — is this a data freshness bug, a real "we have no business this month" situation, or a filter?
Fix: Spell it out: "vs. last week" or "wk-over-wk". For the 0/0 numbers, add a tooltip or helper: "No orders placed in this period — check your filter or sales activity."

P3-3 [REGRESSION] P2-1 from prior report — tab overflow on narrow viewports — partially addressed but worth re-checking on a real 375px device

Where: /quotes tabs.
Observed: At 565px the 9 tabs wrap to 4 rows. At 375px (real phone) this is going to be 5+ rows and harder to scan.
Fix: Already known from prior report. Reconsider: collapse >5 tabs into a "More ▾" dropdown on narrow viewports.

P3-4 [CODE] 11 font-weight preloads per page (P3-1 from prior report) — likely still present

Where: Not re-verified. Mentioned in prior report. Low priority but real bandwidth.
Fix: Audit next/font config and drop unused weights.

P3-5 [NEW] "Search by #, customer, company, email, supplier, item, or price..." placeholder is unhelpfully long

Where: /quotes search input.
Observed: The placeholder is a comma-joined list of every search field. It's a single sentence. On mobile, the user sees only the first half and doesn't know "supplier" and "item" and "price" are also searchable.
Fix: Shorten to "Search…" with a tooltip or expandable "Search in: customer, supplier, item, price" hint.

P3-6 [NEW] Sprint page exposes "Empty" checkbox with no helper text

Where: /sprint — top-right, between "Switch dev identity" and "Refresh sprint".
Observed: A lone checkbox labeled "Empty". I don't know if checking it shows empty categories, hides items with no price, or filters for "no description". The label is too terse.
Fix: Either label it more clearly ("Show only items with no price") or add a hover tooltip.

Status of the prior design review's findings

Prior	Status (2026-09-01)
P1-1 Delivery Notes 404	FIXED — page now loads with Create button, tabs, search.
P1-2 Empty supplier row	Not re-verified in this pass. Worth a quick check.
P2-1 Mobile tab overflow	Partially better at 565px; still needs a "More" dropdown on real phones.
P2-2 Duplicate "All" filter on collection-map	Not re-verified.
P2-3 "Select rows to confirm" dead text	Not re-verified.
P2-4 "Collection Confirm" title wrap	Not re-verified.
P2-5 "Invoicing" status duplicated as label + badge	Not re-verified.
P2-6 Empty Rep column	STILL PRESENT — moved to P2-4 here.
P2-7 Companies table mostly empty	Not re-verified.
P2-8 Pricelist catalogue shows one supplier	Not re-verified.
P3-1 Font preloads	Not re-verified. Likely still present.
P3-2 Repairs sidebar grouping	RESOLVED in code (per the long comment in `src/components/sidebar.tsx:99-130`, the nav was restructured; Repairs now under "OTHER" intentionally, Delivery Notes under "OPERATIONS").
P3-3 Supplier Map appears twice	Not re-verified.
P3-4 Lone "0" on Companies mobile	Not re-verified.
P3-5 "Status: Ready" + "Ready" badge duplication	Not re-verified.

Recommended fix order

If you tackle them in this order, each one unblocks the next:

P1-1 — Tab badge/list mismatch. One user looking at the dashboard, seeing "Pending Approval 8" and clicking through to an empty list, and giving up on the workflow. This is the single biggest trust issue.
P1-2 — Hide "Switch dev identity" behind a dev flag. One careless click and the wrong person is sending emails as someone else.
P1-3 — Add error.tsx / loading.tsx / not-found.tsx to (app)/. Unblocks graceful degradation for every other bug.
P1-4 + P1-5 — Login error feedback + forgot-password link. Two days of work, one big lift in "this app feels professional" signal.
P2-1 — Sidebar sections default open. One-line change in NavSection, big discovery win.
P2-2 — Bulk "Retry all failed emails" button. Saves a 60-op manual job.
P2-3 + P2-4 — Make the empty state aware of the badge; fix the empty Rep label.
Everything else.

Out-of-scope reminders

I walked 6 of the ~40 primary routes. The rest (/collection-pool, /invoicing, /pricelist, /stock, /repairs, etc.) have not been visited in this pass.
No form submissions tested (other than login). The /orders/new form looked great; I didn't press the "Create" button with empty fields to see the validation copy.
No subroutes tested (/quotes/[id], /orders/[id], /sprint/items/[id], etc.).
No console errors checked at each route. The query kind console and network are available — call out if you want a console-error sweep.
No performance / Lighthouse / Web Vitals.

If you want me to keep going on any of these, say the word — happy to deep-dive on the invoicing flow, the Sprint claim race, or the form validation states.

22 KiB Raw Permalink Blame History Unescape Escape