diff --git a/.claude/project-context.md b/.claude/project-context.md index 5e3e252..fa38026 100644 --- a/.claude/project-context.md +++ b/.claude/project-context.md @@ -11,7 +11,7 @@ This is a **self-hosted** application running on a **Synology NAS**, deployed an ## What this is -A storefront for one-of-a-kind items (quantity 1 per item — once sold, it's gone). Public storefront + cart + checkout, with an authentik-SSO-gated admin panel for inventory/customer/settings management. +A storefront for one-of-a-kind items (quantity 1 per item — once sold, it's gone). Public storefront + cart + checkout, with an authentik-SSO-gated admin panel for inventory, category/tag taxonomy, customer, and settings management. The storefront filters items by category, tags, and price range. ## Tech stack @@ -27,11 +27,18 @@ A storefront for one-of-a-kind items (quantity 1 per item — once sold, it's go - **DB migrations**: `node-pg-migrate` — NOT Entity Framework (this is a Node/TS backend, EF doesn't apply here). Migration files live in `backend/migrations/`, run via `node migrate.js up` (bundled into the production image specifically so it can be invoked via `docker exec` against the live container). There's no `init.sql` anymore — it was deleted when migrations were introduced; the migration files are the single source of schema truth. - **Testing**: Jest (unit + integration against a disposable tmpfs Postgres via `node-pg-migrate`), Playwright (e2e) - **CI**: Gitea Actions — `sonarqube.yml` (static analysis + TS build check) and `tests.yml` (unit/integration/e2e with job summaries posted to Gitea's step summary) +- **Node**: 20+ required. **Node 18 cannot run this project's tooling** — `node-pg-migrate` pulls in a `lru-cache` that calls `diagnostics_channel.tracingChannel()`, which doesn't exist before Node 19.9, so `npm run migrate:up` dies with `TypeError: (0 , U.tracingChannel) is not a function`. This is a confusing failure because it looks like a broken dependency rather than a version problem. The dev machine has nvm4w with both 18.16.1 and 24.13.1 installed and **18 is the active default**, so a command can be run against the newer binary without switching globally: +```bash + export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH" +``` ## Data model (key tables) -- `items` — one-of-a-kind inventory. `status`: `available | reserved | sold` +- `items` — one-of-a-kind inventory. `status`: `available | reserved | sold`. `category_id` is nullable (`NULL` = Uncategorized) and `ON DELETE SET NULL` - `item_images` — multiple images per item (front/back/etc.), ordered +- `categories` — self-referencing tree (`parent_id`, `ON DELETE CASCADE`), arbitrary depth, one category per item. Two *partial* unique indexes enforce "siblings can't share a name" — a single plain unique constraint doesn't work, because a `NULL` `parent_id` compares unequal to every other `NULL` and duplicate root categories slip straight through +- `tags` — flexible labels with a `color`; unique on `lower(name)` +- `item_tags` — many-to-many between items and tags - `customers` — accounts, with `marketing_consent` (explicit opt-in, GDPR-style — unchecked by default, timestamped, unsubscribe token) - `admin_settings` — key/value store, currently just `cart_expiry_hours` (admin-configurable, default 24) - `carts` / `cart_items` — one cart per customer, items reserved with the admin-configurable expiry; unique constraint on `item_id` means an item can only be in one cart at a time @@ -48,6 +55,20 @@ A storefront for one-of-a-kind items (quantity 1 per item — once sold, it's go 5. **Admin panel security boundary**: NPM's Advanced nginx config only wraps `auth_request` around `location ~ ^/(admin|api/admin)` — everything else (storefront, cart, checkout, PayPal webhook) bypasses authentik entirely. Deliberate split — don't accidentally widen or narrow that regex without checking both directions. 6. **Cookie `secure` flag** is gated on `NODE_ENV === 'production'`, not hardcoded `true` — otherwise integration tests (plain HTTP, no TLS) silently fail to persist sessions. 7. **`app.ts`/`server.ts` split**: `app.ts` exports the Express app with no `.listen()` call, so tests can import and exercise it via `supertest` without binding a port. `server.ts` is the actual entry point — imports `app`, adds the cart-expiry sweep and cron job, calls `.listen()`. Any new background job or process-level concern goes in `server.ts`, not `app.ts`. +8. **Categories are manual metadata, deliberately not a rule engine.** Issue #23's wording ("rules that dictate how the app automatically organizes items") was explicitly resolved with the author to mean an admin-built tree with a per-item assignment — there is no predicate evaluation anywhere, and adding one would be a new feature, not a completion of this one. Categories are single-valued per item on purpose, to stay distinct from multi-valued tags. **Tag filtering is AND** ("must have all"), not OR. +9. **Item filtering happens in SQL, and malformed filter params return `400`** rather than being ignored — a broken filter link should show itself instead of quietly returning the whole catalogue. Parsing/SQL-building live in `backend/src/itemFilters.ts`, apart from the route so they're unit-testable with no database. Category filtering matches a node *and all descendants* via a recursive CTE; a materialized path column was rejected because reparenting would then have to rewrite every descendant's path, which is a standing drift risk. +10. **Migrations run automatically at container start**, via `CMD ["sh", "-c", "node migrate.js up && node dist/server.js"]` in the `Dockerfile`. Deployed code can therefore never be ahead of the schema. This replaced a separate manual `docker exec ... node migrate.js up` step that was easy to forget — and forgetting it took the storefront down (see the incident note below). `migrate.js` waits for Postgres to accept connections before running (the NAS routinely brings the DB container up slower than the app), and exits non-zero on failure, so `&&` stops a bad migration from serving against a half-migrated schema. +11. **Every async route is wrapped in `asyncRoute()` and the app mounts error middleware.** Express 4 does *not* forward a rejected promise from an async handler, and with no error middleware such a request **never responds at all** — it hangs until the client gives up. A hung request is indistinguishable from an empty result in the UI. New async routes must use the wrapper (`backend/src/asyncRoute.ts`); it becomes unnecessary only if the project moves to Express 5. Note the older route files predate this and are still unwrapped. +12. **Item SELECTs aggregate with scalar subqueries, never `LEFT JOIN` + `GROUP BY`** — see `backend/src/itemSelect.ts`, the single source for both the public and admin shapes. Joining two one-to-many relations in one query multiplies their rows together: with the old shape, an item with 2 images and 3 tags repeated every image three times. This bit once already when tags were added. If a third one-to-many relation is ever attached to items, extend `itemSelect.ts` the same way rather than adding a join. + +## Frontend gotchas + +- **antd's `defaultExpandAll` on `Tree` is evaluated once, at mount.** Any branch created *after* that renders collapsed, and its children become unreachable — in the admin Categories tab this meant adding a subcategory made it appear to vanish. Anywhere a tree outlives the data it displays, track `expandedKeys` in state instead and expand the parent when a node is added or moved. The storefront's filter drawer gets away with `defaultExpandAll` only because `destroyOnHidden` remounts it after the categories have loaded — that's a coincidence, not a pattern to copy. +- **`destroyOnClose` is deprecated in antd 5.29** in favour of `destroyOnHidden`. Several older files (`Admin.tsx`, `Cart.tsx`, `AuthPromptModal.tsx`) still use the old prop and log a console warning; new code uses `destroyOnHidden`. +- **A closed antd `Drawer` keeps its content in the DOM** unless `destroyOnHidden` is set, so duplicate control labels (two "Clear all" buttons, say) collide in Playwright's strict mode even when only one is on screen. The active-filter chip row is marked `role="group" aria-label="Active filters"` so its controls stay addressable independently of the drawer's. +- **An antd `Button` with an icon does not have the plain text as its accessible name** — the icon contributes leading whitespace, so `getByRole('button', { name: /^Filters/ })` matches nothing while `/Filters/` and the exact string `'Filters'` both work. Prefer a substring regex for icon buttons. +- **A failed request must never render as an empty result.** `App.tsx` tracks a `failed` flag separately from `loading`, and shows an error with a Retry rather than falling through to `Empty`. Saying "no items yet" when the server is broken hides an outage and reads to a customer as an empty shop — this is exactly how the 2026-08-17 incident presented. `fetchItems`/`fetchFilterOptions` throw on a non-OK response rather than returning the parsed error body, which would otherwise be set as the item list and crash the grid on `.map`. Covered by `tests/e2e/storefront-errors.spec.ts`. +- **antd import style is inconsistent across the codebase.** New files use deep imports (`import Drawer from 'antd/lib/drawer'`) per the user's standing rule; most older files still use barrel imports from `'antd'`. Don't churn existing files just to convert them. ## Deployment workflow — read this before touching the NAS @@ -117,6 +138,10 @@ name = Thom Lamb - **Always double check `git status` shows a file as staged before committing**, and check it again after committing to confirm a clean working tree before pushing. This project has repeatedly had files silently left out of a commit (most notably `Admin.tsx` once, and the entire cart feature's branch/push never happening at all despite believing it had) — the cost of that mistake compounds badly (hours of debugging "why doesn't the deployed code match what I was shown"), so the few seconds to verify `git status` at each step is well worth it. +- **Incident (2026-08-17): the storefront showed no inventory after a deploy.** No data was lost — the cause was deploying code that queried `categories` / `item_tags` / `items.category_id` against a database where that migration hadn't been run yet. Because Express 4 doesn't forward async errors and no error middleware existed, every item query **hung with no response**, and the frontend rendered that as an empty catalogue. Three changes now prevent this class: migrations run at container start (so code can't outrun the schema), an error middleware guarantees a response, and the storefront distinguishes "request failed" from "no items". **The general lesson is broader than migrations: any deploy where new code needs something the running environment doesn't have yet will surface as silence, not an error, unless something is written to make it loud.** + +- **Check `git branch --contains ` before assuming a commit shipped.** A documentation commit made after the feature branch was pushed was left out of PR #24 entirely and ended up dangling — reachable only through the reflog. `git log` on the branch looked normal, because the missing commit simply wasn't there. If a commit was made after the push, confirm it actually made it onto the branch that got merged. + ### Standard deploy sequence, once code is confirmed on `main` ```bash @@ -128,20 +153,36 @@ sudo docker build --no-cache -t redefined-designs:latest /volume1/docker/redefin sudo docker stop redefined-designs-syn sudo docker rm redefined-designs-syn # redeploy the stack in Portainer UI +# migrations now run automatically as the container starts — no separate step +sudo docker logs redefined-designs-syn | head -20 # confirm migrations ran before the app came up ``` -### Running migrations against the live DB (via Portainer-managed container) +### Migrations against the live DB + +**Migrations apply automatically at container start**, so a normal deploy needs no manual step. `docker logs` on the app container shows the migration output ahead of the server starting; a failed migration exits the container rather than serving a half-migrated schema. + +The manual invocation still works and is useful for applying a migration without redeploying, or for inspecting state: ```bash sudo docker exec redefined-designs-syn node migrate.js up sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c "SELECT name, run_on FROM pgmigrations ORDER BY run_on;" ``` +If the storefront ever looks empty after a deploy, check the row count before assuming data loss — the far more likely cause is a schema/code mismatch: + +```bash +sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c "SELECT count(*) FROM items;" +``` + ## Conventions - **Every commit message follows [Conventional Commits](https://www.conventionalcommits.org/)**: `feat:`, `fix:`, `chore:`, `docs:`, `test:`, `ci:`, `refactor:`, etc. - **Never commit directly to `main`.** Always branch: `feature/` or `fix/`. Open a PR, merge, branch auto-deletes (repo setting is on). - Local dev/editing happens in **VS Code**, pushed via **PowerShell** `git` — the NAS-side `gitc` workflow is *only* for pulling already-merged code down to deploy, never for authoring changes. +- **Thom does the pushing.** Commit locally and stop; don't `git push` on his behalf. +- **When work comes from a Gitea issue, post every clarifying question and its answer back to that issue as a comment** — including the options considered and why the rejected ones were rejected. The issue is the durable record; decisions made in a chat session are invisible to anyone reading it later. Post each round as the answers come in rather than batching everything to the end. +- **Design specs live in `docs/superpowers/specs/YYYY-MM-DD--design.md`** and are committed before implementation starts. +- **`.superpowers/` stays gitignored, but design artifacts inside it must be lifted out before they are lost.** That directory is scratch state belonging to the brainstorming tool and contains a session token, PID files, and absolute local paths — none of which belong in the repo. The mockups it holds *are* worth keeping, so copy them into `docs/superpowers/specs/--mockups/` and wrap them as standalone pages (they are served as fragments inside a tool-provided frame, so they need its style tokens and `toggleSelect` helper inlined to open on their own). Keep the rejected options, not just the chosen one — the value is in the comparison. ## Testing @@ -150,6 +191,15 @@ sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c - **E2e (Playwright)**: needs backend running against a migrated DB; `cd frontend && npm run test:e2e` - CI (`tests.yml`) runs all three as separate jobs, each posting a pass/fail summary to Gitea's job Summary tab. The `frontend-e2e` job runs `node migrate.js up` against a Postgres service container — same migration mechanism as everywhere else, no schema duplication anywhere in the project anymore. +### E2E constraints — the local database is never reset + +Integration tests truncate between cases (`resetDb()` in `tests/integration/setup/testDb.ts` — **add any new table to that TRUNCATE list**, or state leaks between tests). Playwright specs have no such hook and run against whatever is already there, which locally accumulates across every previous run. Consequences worth knowing before writing a new spec: + +- **Name every fixture with a per-run suffix** and assert on those names only. A bare `.first()` or an unscoped `getByText` will find leftovers from an earlier run and produce a failure that looks like a product bug but isn't. This cost real debugging time. +- **`beforeAll` runs once per worker, but a worker can be handed the same spec file in more than one batch**, re-running it against the module-cached suffix. `filters.spec.ts` therefore checks whether its fixtures already exist and returns early — without that, the second pass 409s on category names and duplicates every item, which then breaks strict-mode locators. +- **Tables paginate.** With dozens of accumulated rows a freshly created record often isn't on page 1; `admin-taxonomy.spec.ts` confirms tag creation through the API rather than hunting for the row. +- **Clicking a submit button only dispatches the request.** Wait for the resulting confirmation (`'Tag added'`) before querying the API, or the read races the write. This produced a one-in-four flake until fixed. + ## Known gaps / natural next steps - **No CD** — Gitea Actions runs tests/analysis but doesn't deploy. Manual NAS rebuild is still required after every merge. @@ -159,10 +209,19 @@ sudo docker exec -it redefined-designs-db-syn psql -U redefined -d redefined -c - **Daily cart reminder emails** fire via `node-cron` inside the app process at 9am container-local time — if the container restarts frequently, reminders could silently stop firing with no alerting on that failure mode. - **Old single-item checkout route files** (`backend/src/routes/paypal.ts`, `backend/src/routes/demo.ts`) are dead code, unmounted but never deleted — safe cleanup opportunity. - **`backend/src/routes/shippingAddresses.ts` USPS OAuth token format** was implemented against the current (2026) USPS Addresses API docs at time of writing, using a JSON-body `client_credentials` request — if USPS changes their API again, this is the first place to check. +- **`TAG_COLORS` is duplicated** between `backend/src/utils.ts` and `frontend/src/admin/Tags.tsx`. The server validates against its copy, so editing one alone makes the admin colour picker offer values that get rejected with a `400`. There's no shared module between backend and frontend in this repo to put it in. +- **The local e2e database accumulates junk indefinitely** — every Playwright run seeds categories, tags, and items that are never cleaned up, so the admin tables and filter drawer fill with `Furniture fmsxb…` noise over time. Harmless, but `npm run db:test:down` + `db:test:up` + `migrate:up` resets it when the clutter starts getting in the way. CI is unaffected (fresh service container per run). +- **The storefront still lists sold items** and the price-range bounds are computed across all items regardless of status. Unchanged by the filters work — deliberately left as-is, but worth revisiting if sold stock ever outnumbers available stock. +- **No admin-side filtering.** The admin inventory table shows category and tag columns but can't filter or search on them; with 100+ items that will start to hurt. ## Where to look first for common tasks -- Add/change a DB table → new file in `backend/migrations/` via `npm run migrate:create -- descriptive-name`, fill in `pgm.sql(...)` for up/down +- Add/change a DB table → new file in `backend/migrations/` via `npm run migrate:create -- descriptive-name`, fill in `pgm.sql(...)` for up/down, **and add the table to `resetDb()`** in `backend/tests/integration/setup/testDb.ts` - Change cart/checkout behavior → `backend/src/routes/cartCheckout.ts` (the whole reserve → checkout → complete lifecycle lives here) - Change admin-configurable settings → `admin_settings` table + `backend/src/routes/adminSettings.ts` + `frontend/src/admin/Settings.tsx` +- Add a new async route → wrap the handler in `asyncRoute()` from `backend/src/asyncRoute.ts`, or a failure will hang the request instead of returning 500 +- Change what an item row returns → `backend/src/itemSelect.ts` (one place, used by both the public and admin routes) +- Change storefront filtering → `backend/src/itemFilters.ts` (parsing + SQL), `backend/src/routes/filters.ts` (`/api/filters`, the drawer's single fetch), `frontend/src/filters.ts` (state, URL round-trip, tree building), `frontend/src/components/FilterDrawer.tsx` +- Change category/tag management → `backend/src/routes/adminCategories.ts`, `backend/src/routes/adminTags.ts`, `frontend/src/admin/Categories.tsx`, `frontend/src/admin/Tags.tsx` +- Change tag colours → `TAG_COLORS` + `tagColorFor()` in `backend/src/utils.ts`; the list is **duplicated** in `frontend/src/admin/Tags.tsx` for the override picker, and the server rejects anything outside it, so the two must be changed together - NPM/authentik/DSM reverse-proxy config for this app → not in this repo; documented in the broader homelab's Claude Project knowledge base, not here \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index 7abf3e2..39a95f9 100755 --- a/Dockerfile +++ b/Dockerfile @@ -22,4 +22,12 @@ COPY --from=backend-build /app/backend/migrations ./migrations COPY --from=frontend-build /app/frontend/dist ./public ENV NODE_ENV=production EXPOSE 3000 -CMD ["node", "dist/server.js"] \ No newline at end of file +# Migrations run before the app serves, so deployed code can never be ahead of +# the database schema. Previously this was a separate, manual +# `docker exec ... node migrate.js up` that was easy to forget — and forgetting +# it meant every item query referenced tables that did not exist yet, which +# surfaced as an empty storefront rather than an error. +# +# `&&` matters: migrate.js exits non-zero on failure, so a bad migration stops +# the container instead of letting it serve against a half-migrated schema. +CMD ["sh", "-c", "node migrate.js up && node dist/server.js"] \ No newline at end of file diff --git a/README.md b/README.md index a86f672..2d02b65 100755 --- a/README.md +++ b/README.md @@ -160,4 +160,78 @@ Gitea Actions runs two workflows on every push to `main` and on pull requests: ## Production deployment -Production runs as a single Docker image (multi-stage build — the frontend is built to static files and served directly by the backend), deployed via Portainer behind Nginx Proxy Manager, with authentik forward-auth gating `/admin`. That infrastructure is homelab-specific and documented separately outside this repo. \ No newline at end of file +Production runs as a single Docker image (multi-stage build — the frontend is built to static files and served directly by the backend), deployed via Portainer behind Nginx Proxy Manager, with authentik forward-auth gating `/admin`. That infrastructure is homelab-specific and documented separately outside this repo. + +The container applies pending migrations before starting the server, so deployed code can never be ahead of the database schema. A failed migration stops the container rather than letting it serve against a schema it doesn't match — check `docker logs` on the app container if it doesn't come up. + +## QA environment + +`docker-compose.qa.yml` defines a disposable QA stack for reviewing merged-but-undeployed changes online. It runs alongside production on the same NAS with its own containers, database, volumes, and host port, and is started only when a review is needed. + +It differs from production deliberately: `DEMO_MODE=true` with no PayPal credentials (so checkout is exercisable but cannot reach live PayPal), no SMTP configuration (so it cannot email anyone), and `restart: "no"` (so a NAS reboot doesn't quietly bring it back up). + +### One-time setup on the NAS + +**Build the image before creating the stack.** `redefined-designs:qa` exists only on the NAS — it is never pushed to a registry. If the stack is deployed first, Compose finds no local image, falls back to pulling from Docker Hub, and fails with a misleading `pull access denied ... repository does not exist or may require 'docker login'`. + +```bash +cd /volume1/docker/redefined-designs +sudo docker build --no-cache -t redefined-designs:qa /volume1/docker/redefined-designs +sudo docker images | grep redefined-designs # confirm the qa tag is present +``` + +For the same reason, leave Portainer's **"Pull latest image"** toggle **off** when deploying or redeploying this stack. The compose file sets `pull_policy: never` so a missing image reports itself as missing rather than as a registry authentication problem. + +Next, create the directory structure. These paths are deliberately **not** under `/volume1/configs/redefined-designs` — sharing production's Postgres directory would mean QA writing into production's database files. + +```bash +sudo mkdir -p /volume1/configs/redefined-designs-qa/postgres +sudo mkdir -p /volume1/configs/redefined-designs-qa/uploads + +# The official postgres image runs as uid/gid 999. Without this the DB +# container exits immediately with a data-directory permissions error. +sudo chown -R 999:999 /volume1/configs/redefined-designs-qa/postgres +sudo chmod 700 /volume1/configs/redefined-designs-qa/postgres + +# Uploads are written by the app container, which runs as root. +sudo chown -R root:root /volume1/configs/redefined-designs-qa/uploads +sudo chmod 755 /volume1/configs/redefined-designs-qa/uploads + +# Confirm the two environments are separate before going further. +ls -la /volume1/configs/redefined-designs-qa/ +ls -la /volume1/configs/redefined-designs/ +``` + +Then, in Portainer, create a stack **named `redefined-designs-qa`** from `docker-compose.qa.yml`, with one environment variable `QA_DB_PASSWORD`. The stack name matters: it becomes the compose project name, and reusing production's name would make compose reconcile the two stacks against each other and remove the production containers. + +Finally, add an Nginx Proxy Manager host for `qa-redefined-designs` pointing at the NAS on port `32751`, with authentik forward-auth on `location /` — the whole site, not just `/admin`, so nothing unreleased is publicly reachable. + +### Reviewing a change + +```bash +cd /volume1/docker/redefined-designs +gitc() { sudo docker run --rm -it -v /volume1/docker/redefined-designs:/repo -v ~/.gitconfig-docker/.gitconfig:/root/.gitconfig -w /repo alpine/git "$@"; } +gitc pull origin main +gitc log --oneline -3 + +# Rebuild the QA image before restarting the stack — the stack does not build +# anything itself, and will otherwise run whatever was last tagged :qa. +sudo docker build --no-cache -t redefined-designs:qa /volume1/docker/redefined-designs +# start the redefined-designs-qa stack in Portainer + +# Migrations run as the container starts — confirm before reviewing. +sudo docker logs redefined-designs-qa-syn | head -30 +``` + +Stop the stack in Portainer when finished. + +### Resetting QA data + +QA data is disposable. To start from an empty database, stop the stack first, then: + +```bash +# Check the path before running this. It must contain -qa. +sudo rm -rf /volume1/configs/redefined-designs-qa/postgres/pgdata +``` + +Restarting the stack recreates the database and re-runs every migration from scratch. \ No newline at end of file diff --git a/backend/migrate.js b/backend/migrate.js index 539a2e4..4fd9d75 100644 --- a/backend/migrate.js +++ b/backend/migrate.js @@ -1,22 +1,54 @@ const { runner } = require('node-pg-migrate'); +const { Client } = require('pg'); const path = require('path'); const direction = process.argv[2] || 'up'; -runner({ - databaseUrl: { - host: process.env.PGHOST || 'localhost', - port: parseInt(process.env.PGPORT || '5432', 10), - user: process.env.PGUSER, - password: process.env.PGPASSWORD, - database: process.env.PGDATABASE - }, - dir: path.resolve(__dirname, 'migrations'), - direction, - migrationsTable: 'pgmigrations', - count: direction === 'down' ? 1 : Infinity, - log: (msg) => console.log(msg) -}) +const dbConfig = { + host: process.env.PGHOST || 'localhost', + port: parseInt(process.env.PGPORT || '5432', 10), + user: process.env.PGUSER, + password: process.env.PGPASSWORD, + database: process.env.PGDATABASE +}; + +// This now runs at container start, ahead of the app, so it can come up before +// Postgres is accepting connections — on the NAS the database container is +// routinely slower to be ready than the app container. Without a wait the +// migration would fail, take the app down with it, and look like a broken +// deploy rather than a startup race. +const WAIT_ATTEMPTS = 30; +const WAIT_INTERVAL_MS = 2000; + +async function waitForDb() { + for (let attempt = 1; attempt <= WAIT_ATTEMPTS; attempt++) { + const client = new Client(dbConfig); + try { + await client.connect(); + await client.end(); + return; + } catch (err) { + await client.end().catch(() => {}); + if (attempt === WAIT_ATTEMPTS) { + throw new Error(`Database unreachable after ${WAIT_ATTEMPTS} attempts: ${err.message}`); + } + console.log(`Waiting for database (attempt ${attempt}/${WAIT_ATTEMPTS})...`); + await new Promise((resolve) => setTimeout(resolve, WAIT_INTERVAL_MS)); + } + } +} + +waitForDb() + .then(() => + runner({ + databaseUrl: dbConfig, + dir: path.resolve(__dirname, 'migrations'), + direction, + migrationsTable: 'pgmigrations', + count: direction === 'down' ? 1 : Infinity, + log: (msg) => console.log(msg) + }) + ) .then((applied) => { console.log(`Migration complete — ${applied.length} migration(s) ${direction === 'down' ? 'reverted' : 'applied'}.`); process.exit(0); diff --git a/backend/src/app.ts b/backend/src/app.ts index 7d3b533..86563f1 100755 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -1,4 +1,4 @@ -import express from 'express'; +import express, { Request, Response, NextFunction } from 'express'; import cookieParser from 'cookie-parser'; import path from 'path'; import itemsRouter from './routes/items'; @@ -58,4 +58,20 @@ if (process.env.NODE_ENV !== 'test') { }); } +// Mounted last, so it sees errors from every route above. Without it, a route +// that hands an error to next() falls through to Express's default handler, +// and — worse — an async route that rejects never responds at all, leaving the +// client hanging. A hung request is indistinguishable from an empty catalogue +// in the UI, which is exactly how a schema mismatch once read as "the store +// has no items". Always answer. +// +// The four-argument signature is what marks this as error middleware; `next` +// is unused but must stay for Express to recognise it. +// eslint-disable-next-line @typescript-eslint/no-unused-vars +app.use((err: Error, _req: Request, res: Response, _next: NextFunction) => { + console.error(err); + if (res.headersSent) return; + res.status(500).json({ error: 'internal error' }); +}); + export default app; diff --git a/backend/src/asyncRoute.ts b/backend/src/asyncRoute.ts new file mode 100644 index 0000000..1901f40 --- /dev/null +++ b/backend/src/asyncRoute.ts @@ -0,0 +1,20 @@ +import { Request, Response, NextFunction, RequestHandler } from 'express'; + +// Express 4 does not forward a rejected promise from an async handler to the +// error middleware. An async route that throws therefore never responds at all +// — the request hangs until the client gives up, which reads to a user as "the +// page is empty" rather than "the server failed". Wrapping the handler routes +// the rejection into next(), so the error middleware can answer with a 500. +// +// Express 5 does this natively; drop this helper if the project ever upgrades. +export function asyncRoute( + handler: (req: Request, res: Response, next: NextFunction) => unknown +): RequestHandler { + return (req, res, next) => { + // Promise.resolve also captures a synchronous throw, so both failure modes + // reach the same place. + return Promise.resolve() + .then(() => handler(req, res, next)) + .catch(next); + }; +} diff --git a/backend/src/routes/adminCategories.ts b/backend/src/routes/adminCategories.ts index ca8c75d..3852c9e 100644 --- a/backend/src/routes/adminCategories.ts +++ b/backend/src/routes/adminCategories.ts @@ -1,5 +1,6 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; const router = Router(); @@ -36,7 +37,7 @@ async function parentExists(id: number): Promise { return rows.length > 0; } -router.get('/', async (_req: Request, res: Response) => { +router.get('/', asyncRoute(async (_req: Request, res: Response) => { const { rows } = await pool.query( `SELECT c.id, c.name, c.parent_id, c.sort_order, (SELECT COUNT(*)::int FROM items i WHERE i.category_id = c.id) AS item_count @@ -44,9 +45,9 @@ router.get('/', async (_req: Request, res: Response) => { ORDER BY c.sort_order, lower(c.name)` ); res.json(rows); -}); +})); -router.post('/', async (req: Request, res: Response) => { +router.post('/', asyncRoute(async (req: Request, res: Response) => { const name = readName(req.body.name); if (!name) { return res.status(400).json({ error: 'name is required' }); @@ -76,9 +77,9 @@ router.post('/', async (req: Request, res: Response) => { } throw err; } -}); +})); -router.put('/:id', async (req: Request, res: Response) => { +router.put('/:id', asyncRoute(async (req: Request, res: Response) => { const id = Number(req.params.id); const existing = await pool.query(`SELECT id, name, parent_id, sort_order FROM categories WHERE id = $1`, [id]); if (!existing.rows.length) { @@ -134,9 +135,9 @@ router.put('/:id', async (req: Request, res: Response) => { } throw err; } -}); +})); -router.delete('/:id', async (req: Request, res: Response) => { +router.delete('/:id', asyncRoute(async (req: Request, res: Response) => { const id = Number(req.params.id); const { rows: subtree } = await pool.query(`${SUBTREE_CTE} SELECT id FROM subtree`, [id]); if (!subtree.length) { @@ -154,6 +155,6 @@ router.delete('/:id', async (req: Request, res: Response) => { await pool.query(`DELETE FROM categories WHERE id = $1`, [id]); res.json({ deleted_categories: ids.length, uncategorized_items: affected[0].n }); -}); +})); export default router; diff --git a/backend/src/routes/adminTags.ts b/backend/src/routes/adminTags.ts index f825902..7a0130d 100644 --- a/backend/src/routes/adminTags.ts +++ b/backend/src/routes/adminTags.ts @@ -1,5 +1,6 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; import { TAG_COLORS, tagColorFor } from '../utils'; const router = Router(); @@ -20,7 +21,7 @@ function readColor(value: unknown): string | null | undefined { return value; } -router.get('/', async (_req: Request, res: Response) => { +router.get('/', asyncRoute(async (_req: Request, res: Response) => { const { rows } = await pool.query( `SELECT t.id, t.name, t.color, (SELECT COUNT(*)::int FROM item_tags it WHERE it.tag_id = t.id) AS item_count @@ -28,9 +29,9 @@ router.get('/', async (_req: Request, res: Response) => { ORDER BY lower(t.name)` ); res.json(rows); -}); +})); -router.post('/', async (req: Request, res: Response) => { +router.post('/', asyncRoute(async (req: Request, res: Response) => { const name = readName(req.body.name); if (!name) { return res.status(400).json({ error: 'name is required' }); @@ -54,9 +55,9 @@ router.post('/', async (req: Request, res: Response) => { } throw err; } -}); +})); -router.put('/:id', async (req: Request, res: Response) => { +router.put('/:id', asyncRoute(async (req: Request, res: Response) => { const id = Number(req.params.id); const existing = await pool.query(`SELECT id, name, color FROM tags WHERE id = $1`, [id]); if (!existing.rows.length) { @@ -93,12 +94,12 @@ router.put('/:id', async (req: Request, res: Response) => { } throw err; } -}); +})); -router.delete('/:id', async (req: Request, res: Response) => { +router.delete('/:id', asyncRoute(async (req: Request, res: Response) => { // item_tags cascades; the items themselves are untouched. await pool.query(`DELETE FROM tags WHERE id = $1`, [req.params.id]); res.status(204).end(); -}); +})); export default router; diff --git a/backend/src/routes/filters.ts b/backend/src/routes/filters.ts index 8a5d5cf..b6bf4ad 100644 --- a/backend/src/routes/filters.ts +++ b/backend/src/routes/filters.ts @@ -1,12 +1,13 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; const router = Router(); // Everything the storefront's filter drawer needs, in one request: the whole // category tree (flat — the frontend nests it), every tag with its colour, and // the catalogue's price bounds for the slider. -router.get('/', async (_req: Request, res: Response) => { +router.get('/', asyncRoute(async (_req: Request, res: Response) => { const [categories, tags, price] = await Promise.all([ pool.query( `SELECT id, name, parent_id, sort_order FROM categories ORDER BY sort_order, lower(name)` @@ -31,6 +32,6 @@ router.get('/', async (_req: Request, res: Response) => { tags: tags.rows, priceRange: price.rows[0] }); -}); +})); export default router; diff --git a/backend/src/routes/items.ts b/backend/src/routes/items.ts index 222d026..7f5c1d8 100755 --- a/backend/src/routes/items.ts +++ b/backend/src/routes/items.ts @@ -1,11 +1,12 @@ import { Router, Request, Response } from 'express'; import { pool } from '../db'; +import { asyncRoute } from '../asyncRoute'; import { PUBLIC_ITEM_SELECT } from '../itemSelect'; import { parseItemFilters, buildItemFilterSql, FilterError } from '../itemFilters'; const router = Router(); -router.get('/', async (req: Request, res: Response) => { +router.get('/', asyncRoute(async (req: Request, res: Response) => { let filters; try { filters = parseItemFilters(req.query as Record); @@ -22,12 +23,12 @@ router.get('/', async (req: Request, res: Response) => { const where = clauses.length ? `WHERE ${clauses.join(' AND ')}` : ''; const { rows } = await pool.query(`${PUBLIC_ITEM_SELECT} ${where} ORDER BY i.created_at DESC`, params); res.json(rows); -}); +})); -router.get('/:id', async (req: Request, res: Response) => { +router.get('/:id', asyncRoute(async (req: Request, res: Response) => { const { rows } = await pool.query(`${PUBLIC_ITEM_SELECT} WHERE i.id = $1`, [req.params.id]); if (!rows.length) return res.status(404).json({ error: 'not found' }); res.json(rows[0]); -}); +})); export default router; diff --git a/backend/tests/integration/errorHandling.integration.test.ts b/backend/tests/integration/errorHandling.integration.test.ts new file mode 100644 index 0000000..98a67e5 --- /dev/null +++ b/backend/tests/integration/errorHandling.integration.test.ts @@ -0,0 +1,34 @@ +import request from 'supertest'; +import app from '../../src/app'; +import { pool } from '../../src/db'; +import { resetDb, closeDb } from './setup/testDb'; + +beforeEach(async () => { + await resetDb(); +}); + +afterAll(async () => { + await pool.end(); + await closeDb(); +}); + +describe('unexpected route failures', () => { + it('answers with 500 instead of leaving the request hanging', async () => { + // A non-numeric id reaches Postgres as `WHERE i.id = 'not-a-number'`, which + // raises invalid-input-syntax. Express 4 does not forward a rejected async + // handler on its own, so without the asyncRoute wrapper plus the error + // middleware this request never gets a response at all — and a hung request + // renders as an empty storefront rather than a visible failure. + const res = await request(app).get('/api/items/not-a-number'); + + expect(res.status).toBe(500); + expect(res.body.error).toBe('internal error'); + }); + + it('does not leak the underlying database error to the client', async () => { + const res = await request(app).get('/api/items/not-a-number'); + + expect(JSON.stringify(res.body)).not.toContain('syntax'); + expect(JSON.stringify(res.body)).not.toContain('items'); + }); +}); diff --git a/backend/tests/unit/asyncRoute.test.ts b/backend/tests/unit/asyncRoute.test.ts new file mode 100644 index 0000000..1b8026b --- /dev/null +++ b/backend/tests/unit/asyncRoute.test.ts @@ -0,0 +1,44 @@ +import { Request, Response, NextFunction } from 'express'; +import { asyncRoute } from '../../src/asyncRoute'; + +function fakeArgs() { + const next = jest.fn() as unknown as NextFunction; + return { req: {} as Request, res: {} as Response, next }; +} + +describe('asyncRoute', () => { + it('passes a rejected handler to next so Express can answer the request', async () => { + const boom = new Error('relation "categories" does not exist'); + const { req, res, next } = fakeArgs(); + + await asyncRoute(async () => { throw boom; })(req, res, next); + + expect(next).toHaveBeenCalledWith(boom); + }); + + it('passes a synchronous throw to next as well', async () => { + const boom = new Error('sync failure'); + const { req, res, next } = fakeArgs(); + + await asyncRoute(() => { throw boom; })(req, res, next); + + expect(next).toHaveBeenCalledWith(boom); + }); + + it('leaves a successful handler alone', async () => { + const { req, res, next } = fakeArgs(); + + await asyncRoute(async () => 'fine')(req, res, next); + + expect(next).not.toHaveBeenCalled(); + }); + + it('forwards the same arguments it was given', async () => { + const handler = jest.fn(async () => undefined); + const { req, res, next } = fakeArgs(); + + await asyncRoute(handler)(req, res, next); + + expect(handler).toHaveBeenCalledWith(req, res, next); + }); +}); diff --git a/docker-compose.qa.yml b/docker-compose.qa.yml new file mode 100644 index 0000000..0ae7831 --- /dev/null +++ b/docker-compose.qa.yml @@ -0,0 +1,94 @@ +# QA stack — a disposable copy of the app for reviewing merged-but-undeployed +# changes online. See issue #25. +# +# Deployed as its own Portainer stack, separate from production. Every value +# that could collide with production has been changed: container names, host +# port, volume paths, database name, and image tag. Do not copy a path or port +# back from the production stack — a shared Postgres data directory would mean +# QA writing into production's database files. +# +# Name the Portainer stack `redefined-designs-qa`, NOT `redefined-designs`. +# The stack name becomes the compose project name. Reusing production's name +# would make compose treat this as the same project and reconcile the two +# against each other — it would happily remove the production containers +# because they are not declared in this file. +# +# Required stack environment variable: +# QA_DB_PASSWORD — deliberately not named DB_PASSWORD, so pasting the +# production stack's variables here does nothing silently. +# +# BUILD THE IMAGE BEFORE DEPLOYING THIS STACK. redefined-designs:qa exists only +# on the NAS and is never pushed to a registry, so deploying first makes Compose +# fall back to pulling from Docker Hub and fail with a misleading +# "pull access denied ... repository does not exist or may require docker login". +# +# sudo docker build --no-cache -t redefined-designs:qa /volume1/docker/redefined-designs +# +# For the same reason, leave Portainer's "Pull latest image" toggle off. +# `pull_policy: never` below makes a missing image report itself as missing +# rather than as a registry authentication problem. If the Docker Compose +# version on the NAS ever rejects that key, it is safe to delete the line — it +# only improves the error message. + +services: + redefined-designs-qa: + image: redefined-designs:qa + pull_policy: never + container_name: redefined-designs-qa-syn + environment: + - TZ=America/Chicago + - PORT=3000 + - PGHOST=redefined-designs-qa-db-syn + - PGPORT=5432 + - PGUSER=redefined_qa + - PGPASSWORD=${QA_DB_PASSWORD} + - PGDATABASE=redefined_qa + + # No PayPal credentials at all. DEMO_MODE lets the full cart and checkout + # flow run without them, so QA can exercise the whole purchase path with + # no way to reach live PayPal. Never set PAYPAL_ENV=live here. To test a + # real PayPal integration change, add sandbox credentials and set + # PAYPAL_ENV=sandbox — never the live ones. + - DEMO_MODE=true + + # No SMTP configuration either. The mailer degrades gracefully when + # unconfigured: it logs a warning and skips sending. That is the desired + # behaviour here — a QA run must not be able to email real customers if + # a fixture ever contains a real address. + - SITE_CURRENCY=USD + - RESERVATION_MINUTES=15 + - PUBLIC_URL=https://qa-redefined-designs.bermudalamb.synology.me + volumes: + # Separate uploads directory. Sharing production's would let a QA run + # write into, and a QA teardown delete, real product images. + - /volume1/configs/redefined-designs-qa/uploads:/app/uploads + ports: + # 32751, not production's 32750. + - 32751:3000 + depends_on: + redefined-designs-qa-db-syn: + condition: service_healthy + # Not `unless-stopped`: QA is meant to be up only while a review is + # happening. `unless-stopped` would silently bring it back after every NAS + # reboot and leave it running indefinitely. + restart: "no" + + redefined-designs-qa-db-syn: + image: postgres:16 + container_name: redefined-designs-qa-db-syn + environment: + - POSTGRES_USER=redefined_qa + - POSTGRES_PASSWORD=${QA_DB_PASSWORD} + - POSTGRES_DB=redefined_qa + - PGDATA=/var/lib/postgresql/data/pgdata + volumes: + # Distinct data directory from production's + # /volume1/configs/redefined-designs/postgres. This is the single most + # important difference in this file. + - /volume1/configs/redefined-designs-qa/postgres:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U redefined_qa -d redefined_qa"] + interval: 10s + timeout: 5s + retries: 10 + restart: "no" diff --git a/docs/superpowers/specs/2026-08-17-categories-and-tags-design.md b/docs/superpowers/specs/2026-08-17-categories-and-tags-design.md index 97e8fbc..8a2e77c 100644 --- a/docs/superpowers/specs/2026-08-17-categories-and-tags-design.md +++ b/docs/superpowers/specs/2026-08-17-categories-and-tags-design.md @@ -40,6 +40,10 @@ The issue's phrase "rules that dictate how the app automatically organizes items resolved to mean manual tree assignment, matching its own follow-on sentence that categories are "only metadata for organizing the items into a tree-like structure." +The layout decisions were made against wireframes, archived alongside this spec in +[`2026-08-17-categories-and-tags-mockups/`](2026-08-17-categories-and-tags-mockups/) — they record +the rejected options (sidebar rail, dropdown row, bottom sheet) as well as the chosen one. + ## Schema One new migration, created with `npm run migrate:create -- add-categories-and-tags`. diff --git a/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/README.md b/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/README.md new file mode 100644 index 0000000..86d45c0 --- /dev/null +++ b/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/README.md @@ -0,0 +1,27 @@ +# Categories and tags — design mockups + +Wireframes produced while designing [issue #23](https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs/issues/23), +kept because they record the options that were rejected, not just the one that shipped. + +Open either file directly in a browser — they are self-contained, with no server, +build step, or network access required. + +| File | Question it answered | Outcome | +| --- | --- | --- | +| `filter-layout.html` | Where do the storefront's category / tag / price filters live? | **C — drawer plus removable chips.** A was a permanent sidebar rail (most discoverable, but costs ~25% of grid width); B was a row of dropdowns (full width, but hides the category tree until opened). | +| `filter-layout-mobile.html` | How does that drawer arrive on a phone? | **C1 — side drawer**, entering from the right at near full-screen. C2 was a bottom sheet: better thumb reach, but a deep category tree gets cramped. | + +The reasoning behind each choice is in +[the design spec](../2026-08-17-categories-and-tags-design.md), and the +question-and-answer trail is on the issue itself. + +## Why these are copies + +These began as content fragments inside `.superpowers/brainstorm/`, a scratch +directory belonging to the brainstorming tool. That directory is gitignored and +should stay that way — it also holds a session token, PID files, and absolute +local paths, none of which belong in the repository. + +The mockups themselves are design artifacts, so they were lifted out and wrapped +into standalone pages: the frame's style tokens and its `toggleSelect` helper are +inlined here, which is the only difference from what was reviewed. diff --git a/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/filter-layout-mobile.html b/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/filter-layout-mobile.html new file mode 100644 index 0000000..814df68 --- /dev/null +++ b/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/filter-layout-mobile.html @@ -0,0 +1,378 @@ + + + + + +Mobile drawer variants + + + + +
+ Archived mockup — Mobile drawer variants. + Produced while designing + issue #23. + The option marked as chosen is recorded in + the design spec. +
+ + +

Option C on a phone — and one sub-choice

+

+ The closed state is the same either way. What differs is how the filter panel arrives: a + full-height side drawer or a bottom sheet. Both are one antd Drawer with a + different placement, so this is purely a feel decision. Click the drawer style you prefer. +

+ +
+ + +
+
Closed — browsing
+
Full-width button, applied filters as removable chips, two cards per row.
+
+
+
+ +
⚙ Filters 3
+
+ Coffee Tables × + vintage × + $120–$800 × + Clear all +
+
+
+
Oak coffee table$340 +
vintageoak
+
Elm slab$520 +
vintage
+
Walnut low$690 +
vintagerare
+
Teak round$275 +
vintage
+
+
+
+
+
+ + +
+
C1 — Side drawer
+
Slides in from the right, near full-screen. Reads as "a page of filters".
+
+
+
+ +
+
+
Oak coffee table$340
+
Elm slab$520
+
+
+
+
+
Filters
+
+
Category
+
+
All items
+
▾ Furniture
+
  ▾ Tables
+
    Coffee Tables
+
    Side Tables
+
▸ Decor
+
+
Tags — must have all
+
+ vintage ✓ + handmade + oak + restored +
+
Price
+
+
+
+
+
$120$800
+
+
+
+
Clear all
+
Show 12 items
+
+
+
+
+
+ + +
+
C2 — Bottom sheet
+
Rises from the bottom, ~78% height. Controls sit within thumb reach; grid stays partly visible.
+
+
+
+ +
⚙ Filters 3
+
+
+
Oak coffee table$340
+
Elm slab$520
+
+
+
+
+
+
Filters
+
+
Category
+
+
All items
+
▾ Furniture  ▸ Tables
+
  Coffee Tables
+
▸ Decor
+
+
Tags — must have all
+
+ vintage ✓ + handmade + oak +
+
Price
+
+
+
+
+
$120$800
+
+
+
+
Clear all
+
Show 12 items
+
+
+
+
+
+ +
+ +
+

What carries over from desktop

+

+ Identical component tree at both sizes — the drawer just changes placement and + width/height at the antd md breakpoint, so there is no second + implementation to keep in sync. The chip row is the only piece that behaves differently: on desktop it + sits inline next to the Filters button, on mobile it wraps onto its own line beneath it. +

+

+ Tags now confirmed as AND — an item must carry every selected tag to show up. The panel + labels this explicitly ("must have all") so a user who selects two tags and sees the grid shrink to + nothing understands why. +

+
+ + + + diff --git a/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/filter-layout.html b/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/filter-layout.html new file mode 100644 index 0000000..d9c8af4 --- /dev/null +++ b/docs/superpowers/specs/2026-08-17-categories-and-tags-mockups/filter-layout.html @@ -0,0 +1,394 @@ + + + + + +Storefront filter layout options + + + + +
+ Archived mockup — Storefront filter layout options. + Produced while designing + issue #23. + The option marked as chosen is recorded in + the design spec. +
+ + +

Storefront filter layout — which shape fits?

+

+ Three filters land on the storefront: category (a tree), tags (multi-select, color-coded), + and price range. They differ mainly in how much screen they take from the item grid, and how well + they survive on a phone. Click the one you prefer. +

+ +
+ + +
+
A Sidebar filter rail
+
+
+
+ Redefined Designs + + DarkCartSign up + +
+
+
+ +
+
All items
+
▾ Furniture
+
  ▾ Tables
+
    Coffee
+
    Side
+
▸ Decor
+
+ +
+ vintage + handmade + oak + restored +
+ +
+
+
+
+
$120$800
+
+
+
+
Oak table$340 +
+
Elm slab$520 +
+
Walnut$690 +
+
Teak low$275 +
+
Ash round$410 +
+
Pine box$180 +
+
+
+
+
+
+ Everything visible at once — the tree reads as actual folders, tag colors are on display, no clicking to + discover what's filterable. Costs ~25% of the grid width, so cards drop from 4 to 3 per row on desktop. + On mobile the rail has to collapse into an accordion above the grid. +
+
+ + +
+
B Horizontal filter bar
+
+
+
+ Redefined Designs + + DarkCartSign up + +
+
+ Furniture / Tables / Coffee + + vintageoak + + + $120 – $800 + Clear +
+
+
+
Oak table$340 +
+
Elm slab$520 +
+
Walnut$690 +
+
Teak low$275 +
+
Ash round$410 +
+
Pine box$180 +
+
Cedar$230 +
+
Maple$610 +
+
+
+
+
+
+ Three compact controls in one row — an antd Cascader for the category tree, a tag multi-select, + and a price-range dropdown. Full grid width is preserved (4 cards per row), and the same row wraps + naturally on a phone. The tree is only visible once you open the cascader, so the category structure + is less discoverable. +
+
+ + +
+
C Drawer + active-filter chips
+
+
+
+ Redefined Designs + + Filters (3)Cart + +
+
+ Coffee Tables × + vintage × + $120–$800 × +
+
+
+
Oak table$340
+
Elm slab$520
+
Walnut$690
+
Teak low$275
+
Ash round$410
+
Pine box$180
+
Cedar$230
+
Maple$610
+
+
+
+
+
Filters
+ +
+
All items
+
▾ Furniture
+
  ▾ Tables
+
    Coffee
+
▸ Decor
+
+ +
+ vintage + handmade + oak +
+ +
+
+
+
+
$120$800
+
+
+
+
+
+ A "Filters" button opens a slide-over panel holding the full tree, tag pills, and price slider; what's + currently applied stays visible as removable chips above the grid. Full grid width, and identical + behaviour on desktop and mobile. Trade-off: filtering is a two-step interaction, and nothing hints at + the category structure until you open the drawer. +
+
+ +
+ +
+

Same for all three

+

+ Filters combine with AND (category and tags and price). Multiple selected tags are the one + open sub-question — "has any of these tags" vs "has all of them" — I'll ask that next. + Filtering happens server-side via query params on /api/items, so the result set stays correct + no matter how many items exist. +

+
+ + + + diff --git a/frontend/playwright.config.ts b/frontend/playwright.config.ts index 6ac5da9..f37abfa 100755 --- a/frontend/playwright.config.ts +++ b/frontend/playwright.config.ts @@ -7,7 +7,12 @@ export default defineConfig({ reporter: [['list']], use: { baseURL: 'http://localhost:5173', - trace: 'on-first-retry' + trace: 'on-first-retry', + // The app turns off antd's transitions under this preference. Animated + // popups never settle long enough for Playwright's stability check when + // the machine is loaded, which showed up as clicks timing out on a button + // that was plainly visible and enabled. + reducedMotion: 'reduce' }, webServer: { command: 'npm run dev', diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index d34d380..6ab30bf 100755 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,5 +1,5 @@ import { useEffect, useState, useCallback, useMemo } from 'react'; -import { Layout, Typography, Switch, Row, Col, Spin, Button, theme, Badge, Empty } from 'antd'; +import { Layout, Typography, Switch, Row, Col, Spin, Button, theme, Badge, Empty, Alert } from 'antd'; import { ShoppingCartOutlined, FilterOutlined } from '@ant-design/icons'; import { Link, useSearchParams } from 'react-router-dom'; import { Item, FilterOptions, fetchItems, fetchFilterOptions } from './api'; @@ -27,6 +27,7 @@ const FILTER_DEBOUNCE_MS = 250; export default function App() { const [items, setItems] = useState([]); const [loading, setLoading] = useState(true); + const [failed, setFailed] = useState(false); const [options, setOptions] = useState(null); const [drawerOpen, setDrawerOpen] = useState(false); const [searchParams, setSearchParams] = useSearchParams(); @@ -55,7 +56,14 @@ export default function App() { const load = useCallback(() => { return fetchItems(filtersFromSearchParams(new URLSearchParams(filterKey))) - .then(setItems) + .then((loaded) => { + setItems(loaded); + setFailed(false); + }) + // A failed request must never fall through to the empty state: telling a + // customer "no items yet" when the server is broken hides the outage and + // reads as an empty shop. + .catch(() => setFailed(true)) .finally(() => setLoading(false)); }, [filterKey]); @@ -124,8 +132,16 @@ export default function App() { /> - {loading && !items.length ? : null} - {!loading && !items.length ? ( + {loading && !items.length && !failed ? : null} + {failed ? ( + { setLoading(true); load(); }}>Retry} + /> + ) : !loading && !items.length ? ( ({ - value: node.id, - title: node.name, - children: node.children.length ? toCategoryTreeData(node.children) : undefined - })); -} +import CategoryTreeSelect from './CategoryTreeSelect'; const { Header, Content } = Layout; const { Title } = Typography; @@ -47,6 +33,7 @@ function Inventory() { const [description, setDescription] = useState(''); const [categories, setCategories] = useState([]); const [tags, setTags] = useState([]); + const [saving, setSaving] = useState(false); const { mode } = useThemeMode(); const load = () => fetchAdminItems().then(setItems); @@ -94,21 +81,55 @@ function Inventory() { fd.append('category_id', values.category_id == null ? '' : String(values.category_id)); fd.append('tags', JSON.stringify(values.tags ?? [])); fileList.forEach(f => { if (f.originFileObj) fd.append('images', f.originFileObj as File); }); - await saveItem(editingItem?.id ?? null, fd); + + // Only report success, close the form, and discard the entered values once + // the server has actually accepted the write. + setSaving(true); + try { + await saveItem(editingItem?.id ?? null, fd); + } catch (err) { + message.error(`Couldn't save item — ${(err as Error).message}`); + return; + } finally { + setSaving(false); + } + message.success(editingItem ? 'Item updated' : 'Item added'); setModalOpen(false); load(); loadOptions(); } + // Status changes silently did nothing on failure — the row simply stayed put + // with no indication why. + async function handleStatusChange(action: (id: number) => Promise, id: number, label: string) { + try { + await action(id); + } catch (err) { + message.error(`Couldn't ${label} — ${(err as Error).message}`); + return; + } + load(); + } + async function handleDelete(id: number) { - await deleteItem(id); + try { + await deleteItem(id); + } catch (err) { + message.error(`Couldn't delete item — ${(err as Error).message}`); + return; + } message.success('Item deleted'); load(); } async function handleDeleteImage(itemId: number, imageId: number) { - await deleteItemImage(itemId, imageId); + try { + await deleteItemImage(itemId, imageId); + } catch (err) { + message.error(`Couldn't remove image — ${(err as Error).message}`); + return; + } message.success('Image removed'); load(); setEditingItem(prev => prev && prev.id === itemId @@ -159,8 +180,8 @@ function Inventory() { {item.status !== 'sold' - ? - : } + ? + : } ) } @@ -174,7 +195,7 @@ function Inventory() { - setModalOpen(false)} destroyOnClose width={720}> + setModalOpen(false)} destroyOnHidden width={720}>
@@ -187,13 +208,7 @@ function Inventory() { - + ({ + value: node.id, + title: node.name, + children: node.children.length ? toTreeData(node.children) : undefined + })); +} + +interface Props { + // Supplied by antd's Form.Item. `id` has to be forwarded or the field loses + // its association with the rendered