The alias check was a negative match on an allowlist of English error strings, which returned True for empty output, for $null, and for "exit status 1: Access is denied." — so an alias switch producing nothing, or failing on the symlink permission error this file's own header warns about, was reported as success while the old version kept running. That is the bug #198 was filed about, narrowed rather than removed, and it also broke whenever nvm reworded an error. It is now a positive match on "Now using node v<what is actually running>". The floor check moves ahead of the switch and reads the constant rather than the result. Where it sat, $major was always whatever NODE_VERSION says, so it validated the switch it had just made instead of the pin it exists to guard, and could never fire. Use-NodeLatest is now Use-PinnedNode. In a change whose whole subject is that "latest" means something people do not expect, the name was an avoidable trap. The restore default moves beside NODE_VERSION. It deliberately is not a param default: a param block runs before the dot-source, so $script:DEFAULT_NODE_VERSION is still $null there and the restore would have quietly restored nothing — leaving the machine on the pinned version, which is the exact failure the restore exists to prevent. It is resolved after the dot-source instead, and an explicit -DefaultNodeVersion still wins. Three documents described behaviour the code no longer has: README's "both scripts run nvm use latest", run-tests.ps1's .DESCRIPTION, and project-context.md's instruction to agents. All corrected, and project-context.md now also says not to run these scripts from an agent shell, which is how this machine once ended up with no Node at all. Part 4 of the issue is partly stale: backend/package.json already declares engines >=20.9.0. frontend now matches it. The larger question — whether local should be pinned to the Node 20 that CI and the production image actually run — is a decision rather than an oversight and is left open on the issue. Verified by parsing all three scripts with the PowerShell AST parser, which does not execute them. They are deliberately never run from an agent shell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
318 lines
44 KiB
Markdown
318 lines
44 KiB
Markdown
# Redefined Designs — Project Context
|
|
|
|
## Environment
|
|
|
|
This is a **self-hosted** application running on a **Synology NAS**, deployed and managed via **Portainer** (Docker Compose stacks through Portainer's UI, not raw `docker compose` CLI on the host). This is a homelab project, not a cloud-hosted SaaS — deployment is manual, there's no CI/CD pipeline to production, and infrastructure decisions are shaped by NAS constraints (Synology's permission model, limited CPU/RAM, no exposed Postgres port, etc.).
|
|
|
|
There are **two environments**, both on the NAS:
|
|
|
|
| | Production | QA |
|
|
| --- | --- | --- |
|
|
| Hostname | `redefined-designs.bermudalamb.synology.me` | `qa-redefined-designs.bermudalamb.synology.me` |
|
|
| App / DB container | `redefined-designs-syn` / `redefined-designs-db-syn` | `redefined-designs-qa-syn` / `redefined-designs-qa-db-syn` |
|
|
| Database | `redefined` | `redefined_qa` |
|
|
| Image tag | `redefined-designs:latest` | `redefined-designs:qa` |
|
|
| Host port | 32750 | 32751 |
|
|
| Config paths | `/volume1/configs/redefined-designs/` | `/volume1/configs/redefined-designs-qa/` |
|
|
| Portainer stack | `redefined-designs` | `redefined-designs-qa` |
|
|
| authentik gating | `^/(admin\|api/admin)` only | the whole site (`location /`) |
|
|
| Data | Real | Disposable fixtures; no production data is ever copied |
|
|
| Restart policy | `unless-stopped` | `"no"` — up only during a review |
|
|
|
|
QA's stack lives in `docker-compose.qa.yml`; production's is managed in Portainer's UI and is not in this repo. QA runs `DEMO_MODE=true` with no PayPal credentials and no SMTP configuration, so it cannot reach live PayPal or email anyone.
|
|
|
|
**Repo**: `https://gitea.bermudalamb.synology.me/bermudalamb/redefined-designs` (self-hosted Gitea, also on the NAS)
|
|
**Production URL**: `https://redefined-designs.bermudalamb.synology.me`
|
|
**Container names**: `redefined-designs-syn` (app), `redefined-designs-db-syn` (Postgres)
|
|
**NAS repo path**: `/volume1/docker/redefined-designs`
|
|
|
|
## 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, category/tag taxonomy, customer, and settings management. The storefront filters items by category, tags, and price range.
|
|
|
|
## Tech stack
|
|
|
|
- **Backend**: Express + TypeScript, Postgres (`pg`), single Docker image
|
|
- **Frontend**: React + TypeScript + antd (Vite build), served as static files by the backend in production
|
|
- **Auth**: two separate systems —
|
|
- **Customers**: email/password, bcrypt, session cookie (`rd_session`), custom-built (not authentik)
|
|
- **Admin**: authentik SSO via Nginx Proxy Manager forward-auth, gated only on `/admin` and `/api/admin/*` — the storefront itself is public
|
|
- **Payments**: PayPal Orders v2 API — real multi-item cart checkout, single transaction, itemized breakdown
|
|
- **Shipping validation**: USPS Addresses API (OAuth2/REST — `apis.usps.com`, not the deprecated Web Tools XML API), US-only, optional/graceful if unconfigured
|
|
- **Email**: nodemailer via Gmail SMTP — optional, degrades gracefully if unconfigured (logs a warning, skips sending)
|
|
- **Scheduled jobs**: `node-cron` in-process (cart expiry sweep every 5 min via `setInterval`, daily cart-reminder emails at 9am via cron)
|
|
- **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`. `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
|
|
- `shipping_addresses` — per-customer, USPS-validated when USPS creds are configured
|
|
- `checkouts` / `checkout_items` — one row per multi-item PayPal/demo transaction, snapshotting item+price at purchase time
|
|
- `orders` — per-item purchase records, linked to `checkout_id` and `customer_id`
|
|
|
|
## Key architectural decisions
|
|
|
|
1. **Cart replaced the old single-item "Buy Now" flow entirely.** Items are added to cart (immediately flips `item.status` to `reserved` inside a DB transaction with `FOR UPDATE` locking — no double-sell race), then checked out together in one real multi-item PayPal transaction (one `purchase_unit`, itemized `items[]` breakdown, single capture). Old single-item route files (`routes/paypal.ts`, `routes/demo.ts`) still exist on disk but are unmounted from `app.ts` — safe to delete, never got cleaned up.
|
|
2. **Demo mode** (`DEMO_MODE=true` env var): lets the whole site — including full cart checkout — work with zero PayPal/USPS credentials configured. Same "degrade gracefully when a third-party integration isn't configured" pattern applies to PayPal, USPS, and SMTP — none of them crash the app if unset, they just no-op or skip.
|
|
3. **Account creation is prompted at add-to-cart**, not gated earlier — clicking "Add to Cart" while logged out opens an inline modal (register or login tabs), and on success the item is added automatically.
|
|
4. **GDPR/consent handling**: marketing consent is never pre-checked, is timestamped with the exact consent text shown, and customers have working self-service data export + account deletion endpoints.
|
|
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. As of #59 this holds everywhere — every route file, the second `webhookRouter` in `cartCheckout.ts`, and the globally-mounted `attachCustomer` middleware — and `backend/tests/unit/routesAreWrapped.test.ts` fails the build if a new handler is added bare, so it is enforced rather than remembered. Delete that test along with `asyncRoute` on any Express 5 upgrade.
|
|
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.
|
|
|
|
## Shipping a change — the standing checklist
|
|
|
|
**Every change goes through QA before production. This is not optional and not a judgement call — treat it as an action item on every single change, and surface it as one without being asked.** Production is not where a bad deploy should be discovered; that already happened once (see the 2026-08-17 incident below) and it is what the QA stack exists to prevent.
|
|
|
|
| # | Step | Gate before moving on |
|
|
| --- | --- | --- |
|
|
| 1 | Implement on a branch, verify locally | `npm run lint`, unit, integration and e2e all pass; `tsc --noEmit` and `npm run build` clean |
|
|
| 2 | Commit and push | `git branch -a --contains <sha>` lists the pushed branch |
|
|
| 3 | Open a PR and merge to `main` | The merge actually contains the expected commits |
|
|
| 4 | **Build and deploy to QA, and review it in a browser** | The change does what it claims, behind authentik |
|
|
| 5 | Promote the reviewed image to production | Migrations recorded, data intact, deployed bundle is the new one |
|
|
| 6 | Stop the QA stack | — |
|
|
|
|
Step 5 promotes the **same image** QA reviewed (`docker tag redefined-designs:qa redefined-designs:latest`) rather than rebuilding, so what ships is exactly what was tested. Full commands for steps 4 and 5 are in the README.
|
|
|
|
Two gates worth stating separately, because both have already gone wrong here:
|
|
|
|
- **Step 2 is not a formality.** A commit made *after* a branch was pushed was silently left out of PR #24 and ended up dangling, recoverable only through the reflog. `git log` on the branch looked normal because the commit simply was not on it.
|
|
- **Step 4 catches the class of bug that only appears when built and deployed.** The categories/tags release passed every local suite and still took production down, because the failure was a schema/code ordering problem that no local run could expose.
|
|
|
|
## Deployment workflow — read this before touching the NAS
|
|
|
|
This is where nearly every real time-sink in this project's history happened. The process:
|
|
|
|
1. Edit files locally in **VS Code**, on a branch, using **PowerShell** `git` (not SSH).
|
|
2. Push, open a PR in Gitea, merge into `main`.
|
|
3. SSH into the NAS, pull the repo, rebuild the Docker image, redeploy the Portainer stack.
|
|
|
|
### Hard-won lessons from this project's history — internalize these
|
|
|
|
- **The NAS's local git clone does NOT have a real `git` binary.** DSM doesn't ship one. Git operations on the NAS go through a throwaway `alpine/git` Docker container, wrapped in a shell function:
|
|
```bash
|
|
gitc() {
|
|
sudo docker run --rm -it \
|
|
-v /volume1/docker/redefined-designs:/repo \
|
|
-v ~/.gitconfig-docker/.gitconfig:/root/.gitconfig \
|
|
-w /repo \
|
|
alpine/git "$@"
|
|
}
|
|
```
|
|
**This function must be redefined every new SSH session** — it only lives in shell memory, doesn't persist to `~/.bashrc` (never got added there). Always redefine it first thing after SSH'ing in, before running any `gitc` command.
|
|
|
|
- **Git identity and `safe.directory` are NOT in the container image** — they're mounted from `~/.gitconfig-docker/.gitconfig` on the host, which is what makes them persist across `gitc` invocations (each `gitc` call is a brand-new container). If that file is ever missing, recreate it:
|
|
|
|
[safe]
|
|
directory = /repo
|
|
[user]
|
|
email = thomlamb@gmail.com
|
|
name = Thom Lamb
|
|
|
|
- **The NAS git working directory will show every file as "modified" even right after a clean pull, if `core.fileMode` isn't disabled.** Synology forces broad `-rwxrwxrwx+` permissions on all files regardless of what git committed, and git tracks the executable bit as part of a file's tracked "mode" by default — so every file looks dirty even when content is byte-identical. Fix once per repo:
|
|
```bash
|
|
gitc config core.fileMode false
|
|
```
|
|
This is a real trap: `git status` showing a long "modified" list after a pull does NOT necessarily mean there are real local edits — check `core.fileMode` before assuming a merge conflict or lost work.
|
|
|
|
- **If `gitc pull` refuses with "local changes would be overwritten,"** don't try to cherry-pick which files to discard — on this project, that approach has repeatedly failed to fully resolve since the modified-file list is often long and partially spurious (see `core.fileMode` above). Just reset the NAS clone to match origin exactly, since the NAS is never the source of truth for code:
|
|
```bash
|
|
gitc fetch origin
|
|
gitc reset --hard origin/main
|
|
```
|
|
|
|
- **Docker layer caching has caused multiple "the fix didn't work" false alarms.** After any code change, rebuild with `--no-cache` and actually watch the output — don't assume `docker build` picked up new files just because the command succeeded:
|
|
```bash
|
|
sudo docker build --no-cache -t redefined-designs:latest /volume1/docker/redefined-designs
|
|
```
|
|
Confirm it reaches `Successfully tagged redefined-designs:latest`. A build that errors partway through (e.g. a TypeScript compile error) leaves the OLD image running untouched — Docker doesn't warn loudly about this, it just silently keeps serving stale code.
|
|
|
|
- **`docker exec` does not expand shell globs** (`*.js`, `*.sql`) unless you explicitly invoke a shell:
|
|
```bash
|
|
# WRONG — "No such file or directory"
|
|
sudo docker exec redefined-designs-syn grep -l "text" /app/public/assets/*.js
|
|
# RIGHT
|
|
sudo docker exec redefined-designs-syn sh -c "grep -l 'text' /app/public/assets/*.js"
|
|
```
|
|
|
|
- **Verify a fix actually deployed before declaring victory** — the recurring failure pattern this project has hit repeatedly is: code is correct in the repo → but the branch was never actually merged, OR the merge happened but the NAS never pulled, OR the pull happened but the Docker image was never rebuilt, OR the rebuild happened but used a stale layer cache. Each of these produces the identical symptom ("I made the fix but the UI/behavior didn't change"). The reliable way to root-cause it, in order:
|
|
1. `git log main --oneline` (local/PowerShell) — is the commit actually merged?
|
|
2. `gitc log --oneline` (NAS) — did the NAS actually pull it?
|
|
3. `grep` the actual source file on the NAS disk for a distinguishing string from the fix
|
|
4. Rebuild with `--no-cache`, watch it complete
|
|
5. `docker exec` into the container and `grep` the **built output** (`/app/public/assets/*.js` for frontend, `/app/dist/*.js` for backend) for the same string — this is the only step that confirms the deployed artifact, not just the source, has the fix
|
|
6. Hard-refresh / incognito window in the browser (aggressive JS bundle caching on both desktop and mobile browsers has caused false "still broken" reports even after a correct deploy)
|
|
|
|
- **Files must be at the repo root where the Dockerfile expects them.** The `Dockerfile` lives at the repo root (not `backend/Dockerfile` — that was a real, repeat point of confusion this project hit multiple times). `docker build ... /volume1/docker/redefined-designs` uses whatever `Dockerfile` sits directly in that directory.
|
|
|
|
- **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 <sha>` 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
|
|
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 # confirm the expected commit is at the top
|
|
sudo docker build --no-cache -t redefined-designs:latest /volume1/docker/redefined-designs
|
|
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
|
|
```
|
|
|
|
### 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
|
|
|
|
- **Never commit directly to `main`.** Always branch, open a PR, merge; the branch auto-deletes (repo setting is on).
|
|
- **Branches follow [Conventional Branch](https://conventional-branch.github.io/), with the issue number carried for Gitea:** `<type>/<issue-number>-<short-slug>`, e.g. `feature/48-my-account-modal`, `bugfix/57-cart-total-wrong`. Types are `feature`, `bugfix`, `hotfix`, `release`, `chore` — the same `feature/` prefix this repo has always used, so nothing in the existing history is wrong. **Every branch and every PR has an issue behind it — no exceptions.** When work arrives through conversation rather than the tracker, file the issue first and branch from it; do not start a branch meaning to retrofit an issue later. The escape hatch that used to sit here turned "no issue yet" into "no issue ever", and two branches went that way before it was removed (#70 and #76). The type should agree with the Conventional Commit type of the work it carries.
|
|
- **Commits follow [Conventional Commits](https://www.conventionalcommits.org/)** — `feat:`, `fix:`, `chore:`, `docs:`, `test:`, `ci:`, `refactor:` — with the issue number appended to the subject: `feat(account): open My Account as a modal (#48)`.
|
|
- **Put `Closes #48` in the commit body**, on its own line, for the commit that completes the issue (`Refs #48` when it only contributes). This is what actually closes the issue on merge, independently of whether the PR description repeats it.
|
|
- **Be clear about which part does the linking.** Gitea creates the reference from a `#48` appearing in a *commit message or PR* — never from the branch name. The branch name is for humans reading `git branch`; the reference is what ties the work to the issue. Both are wanted, but only one of them links.
|
|
- **Commit bodies are unwrapped paragraphs** — no hard line breaks inside a paragraph.
|
|
- 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.
|
|
- **Every change is deployed to QA and reviewed there before it goes to production** — raise it as an action item on every change, unprompted. See the standing checklist above.
|
|
- **Design specs live in `docs/superpowers/specs/YYYY-MM-DD-<topic>-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/<date>-<topic>-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.
|
|
|
|
## Linting
|
|
|
|
`npm run lint` in each workspace (`backend/eslint.config.mjs`, `frontend/eslint.config.mjs`, flat config, `.mjs` because neither package is `"type": "module"`). Added in #60, with a `lint` job in `tests.yml`.
|
|
|
|
The severity split is deliberate and is the whole design: every preset is downgraded to a warning, and only the rules that catch real defects are errors — `no-floating-promises`, `no-misused-promises`, `rules-of-hooks`, `exhaustive-deps`, `jsx-a11y/alt-text`. They are listed explicitly at the bottom of each config, so the CI gate is readable in one place. No `--max-warnings` flag is needed: ESLint exits non-zero on errors and zero on warnings by itself. Expect roughly 10 warnings on the backend and 34 on the frontend — that is the intended state, not a backlog someone forgot.
|
|
|
|
**`recommendedTypeChecked` is deliberately not enabled.** Its `no-unsafe-*` family reports ~325 violations, all of them downstream of `pool.query()` returning `any` rows and untyped `fetch(...).json()`. That is #65's work, and turning the rules on before that work is done buries the ~44 warnings worth reading under a backlog belonging to another issue. #65 enables them as it types those boundaries.
|
|
|
|
`@typescript-eslint/no-misused-promises` runs with `checksVoidReturn: { attributes: false }`, because `onClick={async () => ...}` is idiomatic React and safe when the handler catches its own errors — left at the default the rule flags every antd button in the admin screens.
|
|
|
|
## SonarQube
|
|
|
|
Server is **SonarQube 9.9.8 LTA, Community edition**, at the URL in `SONARQUBE_URL`. Scan settings live in `sonar-project.properties` at the repo root, not as inline `-D` args, so a local scan and the CI scan analyse the same thing; only the host and token come from Gitea secrets.
|
|
|
|
**Community edition has no branch analysis.** Every scan overwrites the single `main` analysis of whatever project key it is given, so scanning a feature branch under the real key silently replaces CI's picture of main with your working tree. `scripts/scan-local.sh` therefore defaults to the scratch key `redefined-designs-local`; pass `redefined-designs` explicitly to publish for real. The scanner runs in Docker because it needs Java 11+ and the dev machine's Java is 8.
|
|
|
|
**`frontend/tsconfig.sonar.json` is load-bearing, and its failure mode is silent.** SonarQube 9.9's bundled TypeScript predates 5.0 and rejects `"moduleResolution": "bundler"`, which `frontend/tsconfig.json` needs for Vite. Without the shim the frontend program fails to build, all 34 frontend files are skipped, and **the scan still exits `EXECUTION SUCCESS`** — the state #67 found, where the gate had been reporting on a third of the codebase while looking complete. The shim cannot use `extends`: the old compiler validates the base file while reading it, so the error fires before any override applies. `scripts/check-sonar-tsconfig.js` runs before the scan in CI and fails if the copy drifts from the real tsconfig in anything but `moduleResolution`. Delete the shim, the check, and the `sonar.typescript.tsconfigPaths` line together once the server is new enough to parse `bundler`.
|
|
|
|
Take a green SonarQube job as weak evidence. It exits success on a partially-failed analysis, so the number worth checking after a scan is `ncloc` — if it drops sharply, something is being skipped.
|
|
|
|
## Testing
|
|
|
|
- **Unit tests**: `cd backend && npm run test:unit` — no DB required
|
|
- **Integration tests**: `npm run db:test:up` (disposable tmpfs Postgres via `docker-compose.test.yml`) → `npm run migrate:up` → `npm run test:integration` → `npm run db:test:down`
|
|
- **E2e (Playwright)**: needs backend running against a migrated DB; `cd frontend && npm run test:e2e`
|
|
- **Coverage**: `npm run test:unit:cov` and `npm run test:integration:cov` in `backend` write `coverage/unit/lcov.info` and `coverage/integration/lcov.info` — separate directories because jest writes `coverage/lcov.info` by default and the second run would otherwise overwrite the first. Frontend coverage is `npm run test:e2e:cov` then `npm run coverage:report`. Added in #61.
|
|
- **Frontend coverage comes from Playwright through an istanbul-instrumented dev server**, so read it with suspicion: istanbul marks a line covered when the browser ran it, meaning a component rendered during an end-to-end test reports as covered with nothing asserting anything about it. Backend coverage, coming from tests that assert on responses, means considerably more per percentage point. The 80% gate on new code is correspondingly easier to clear on frontend changes.
|
|
- **Instrumentation is gated behind `COVERAGE=true`** and must stay that way — an instrumented bundle is larger, slower, and publishes the source structure through `window.__coverage__`. The Dockerfile runs a plain `npm run build`, which never sets it. `vite-plugin-istanbul` is also loaded by dynamic import because it is ESM-only while `vite.config.ts` evaluates as CommonJS; a static import fails the build outright.
|
|
- **`npm run coverage:report` fails when nothing was collected** rather than writing an empty report. If Playwright reuses an uninstrumented dev server every test still passes while gathering nothing, and the resulting 0% reads as "the tests stopped covering things" rather than "collection was never switched on".
|
|
- 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.
|
|
|
|
### The local Node version will not run the integration or e2e suites
|
|
|
|
`nvm4w` has both **18.16.1 (active by default)** and **24.13.1** installed, and the active one is too old for two of the three suites:
|
|
|
|
- Integration tests fail in `globalSetup` with `(0 , U.tracingChannel) is not a function` — a transitive `lru-cache` needs `diagnostics_channel.tracingChannel`, added in Node 20.2.
|
|
- Playwright refuses outright: "Playwright requires Node.js 20 or higher."
|
|
|
|
Neither failure mentions the Node version as the cause, and the first one reads like a broken dependency. Rather than switching the user's active version, prepend the newer one for the single command:
|
|
|
|
```bash
|
|
export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH"
|
|
```
|
|
|
|
Thom is fine with the scripts switching the active version for a test run: they use the pinned `NODE_VERSION` (26.7.0) in `scripts/NodeVersion.ps1` and restore 18.16.1 when finished, including on failure. Do not run those scripts from an agent shell — they prompt for elevation and can leave the machine with no Node at all.
|
|
|
|
Unit tests and `tsc` run fine on 18, so a green `npm test` says nothing about whether the other two suites can even start.
|
|
|
|
### 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.
|
|
- **`isVisible()` does not wait.** It answers about *this instant*, so guarding an optional dialog with `if (await x.isVisible())` loses the race whenever the dialog is still on its way — and an antd modal left open then intercepts every later click, which surfaces as an unrelated element "not found" thirty seconds later. If the dialog is deterministic, click it unconditionally and let the locator auto-wait; only use `isVisible()` when it genuinely may never appear, and even then give it something to wait on first.
|
|
- **The 5s default `expect` timeout is too tight for anything waiting on a round-trip.** Registration is a bcrypt hash — about half a second unloaded, and well past 5s when the suite's workers all register at once. The failure surfaces on whichever test lost the race, so it looks like an unrelated flake that moves between runs. Give such assertions an explicit generous timeout; they are asserting *that* the server answered, not how fast.
|
|
- **The local database's accumulated junk eventually shows up as flake, not just clutter.** At ~450 items and ~250 customers the storefront render and the parallel bcrypt load together push these round-trips past their timeouts, and the repeated runs also trip the password-reset rate limiter. When failures start rotating between unrelated specs on each run, reset the database before debugging any of them.
|
|
- **`fullyParallel: true` means a test that mutates a shared fixture races every other test in the file.** A spec that marked an item sold broke the sibling tests reading that same item. Give any test that changes an item's state its own fixture.
|
|
|
|
## 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.
|
|
- **SonarQube CI still uses the `admin` token**, not a dedicated `gitea-ci` user (flagged early on, never circled back to).
|
|
- **USPS/PayPal credentials**: verify these are actually populated in the Portainer stack env vars before assuming checkout works end-to-end in production — `DEMO_MODE` may still be the only thing actually exercised live.
|
|
- **Double opt-in for marketing email** was deliberately deferred (single opt-in + easy unsubscribe judged sufficient for now).
|
|
- **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.
|
|
- **The marketing consent wording is duplicated** between `MARKETING_CONSENT_TEXT` in `backend/src/utils.ts` and the constant of the same name in `frontend/src/customer/AuthForm.tsx`. The server stores its copy verbatim against the customer's consent record, so the whole point is that the record says what the customer actually saw — a label that drifts from the stored string quietly defeats that. This is not hypothetical: before the form was shared there were three wordings in play (the register page's, a shorter one in the cart prompt, and the stored string) and none matched. An e2e test now asserts the rendered label equals the stored wording, which is the only thing spanning the two sides.
|
|
- **`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. Deliberately left as-is, and now load-bearing: the favorites filter (#35) shows sold favorites on purpose, since an item that just sold is often what the customer came back to look at after the #34 email. Anyone wanting only purchasable stock combines the favorites toggle with the status filter. Worth revisiting if sold stock ever outnumbers available stock — but changing the default would change what a favorites view means.
|
|
- **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, **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 how a customer route is framed (modal vs page) → `frontend/src/main.tsx`. `AppRoutes` renders the route table against a *backdrop* location rather than the real one: everything in `MODAL_ROUTES` (`/account`, `/login`, `/register`, `/forgot-password`, `/reset-password`) is a modal over the page named in `location.state.background`, falling back to the storefront when there is none (a bookmark, an email link). The link that opens one must pass `state={{ background: location }}`, or closing goes to the fallback instead of where the customer was; steps *within* a flow navigate with `replace` so the whole detour stays one history entry. This is the pattern to copy for the remaining dead-end pages (#52) — it came out of #51 — it keeps the URL real and linkable while making sure closing always lands somewhere. The link that opens it must pass `state={{ background: location }}`, or closing goes to the fallback instead of where the customer was.
|
|
- Change sign-in or registration → `frontend/src/customer/AuthForm.tsx`, which is the single implementation. It is rendered both by `AuthRouteModal` (the `/login` and `/register` routes) and by `AuthPromptModal` (the prompt shown when a signed-out visitor adds to the cart, favorites, or filters by favorites). Changing one caller's copy or validation without the other is the drift this deliberately removed.
|
|
- 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`, `frontend/src/components/ActiveFilterChips.tsx`
|
|
- Add a filter dimension that depends on who is asking → follow the favorites filter (#35). The identity comes from `req.customerId` (`attachCustomer` runs globally, so it is available on the public `/api/items` too) and is passed into `buildItemFilterSql` as an explicit argument — never parsed from the query string, or a hand-edited URL could name another customer. Each route decides what to do when it cannot satisfy the filter: the storefront answers 401, the admin inventory 400, and the builder throws rather than silently dropping the clause and returning everything.
|
|
- 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 |