From 29ae1139017f82012d722b8886b0251110e30769 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Wed, 24 Jun 2026 23:50:28 +0100 Subject: [PATCH] docs: update README and obsidian docs to reflect timezone/tx/VAT/middleware changes Update README with middleware section, FOR UPDATE + closing_time in bookings, pending-payment-first VAT flow in gift cards, and bumped test counts (1,043 run / 1,190 defined). Update Overview.md with transaction pattern documentation, middleware details, and new files (closing_time, repo). Update Technical Manual.md test count (1,190). Update Testing Architecture with new test additions and timing. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- README.md | 10 +++++---- obsidian/Crussell/Overview.md | 15 +++++++++++-- obsidian/Crussell/Technical Manual.md | 2 +- .../Testing Architecture & DB Management.md | 21 ++++++++++++------- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 9752843..d6e3889 100644 --- a/README.md +++ b/README.md @@ -4,9 +4,9 @@ Nail salon booking platform — Go 1.25 backend + SvelteKit 5 SPA + PostgreSQL 1 ## Features -**Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). Guest accounts with GDPR-compliant anonymization. Service eligibility based on age + patch test validity. +**Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). Guest accounts with GDPR-compliant anonymization. Service eligibility based on age + patch test validity. Overlap checks use `FOR UPDATE` row locks inside transactions. Closing-hours validation extracted into a reusable `closing_time` helper. -**Payments**: Square Terminal (in-person) + Web Payments SDK (online). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A PostgreSQL `pg_advisory_lock` serializes payment attempts per-booking to prevent two-tab double-payment races. +**Payments**: Square Terminal (in-person) + Web Payments SDK (online). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A PostgreSQL `pg_advisory_lock` serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases now insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record. **Gift Cards**: Multi-method purchase (cash, card machine, online card, giveaway). Inventory cards for stock management. 24-month rolling expiry. Idle account cleanup (2yr/5yr thresholds). Expired balance recovery with admin audit trail. Transaction audit log. Idempotency keys for purchases. @@ -24,6 +24,8 @@ Nail salon booking platform — Go 1.25 backend + SvelteKit 5 SPA + PostgreSQL 1 **Infrastructure**: Docker Compose (postgres, backend, sabredav, nginx). Dev mock for Square payments (`//go:build dev`). RustFS dev storage, Cloudflare R2 for prod. SabreDAV CardDAV sync for profile photos. +**Middleware**: `JsonContentType` sets `Content-Type: application/json` globally, replacing ~80+ individual `w.Header().Set()` calls. `RespondJSON`/`RespondError` helpers standardise API response format. Progressive rate limiting (dual-window) on login/register with account lockout. + ## Limitations - **Single employee** — no multi-staff scheduling, no team management @@ -74,8 +76,8 @@ Default logins (password: `password`): ```bash cd backend && go build -o bin/backend ./main.go cd frontend && npm ci && npm run build -cd backend && go test -tags "test,dev" -count=1 -parallel 8 ./... # ~995 tests, 0 failures, 4 skipped (~10s) -cd backend && go test -tags "test,dev" -count=10 -parallel 8 ./... # thorough verification (~42s) +cd backend && go test -tags "test,dev" -count=1 -parallel 8 ./... # ~1,043 tests, 0 failures, 4 skipped (~11s) +cd backend && go test -tags "test,dev" -count=10 -parallel 8 ./... # thorough verification (~44s) ``` ## Full Documentation diff --git a/obsidian/Crussell/Overview.md b/obsidian/Crussell/Overview.md index 04ff013..8c4445e 100644 --- a/obsidian/Crussell/Overview.md +++ b/obsidian/Crussell/Overview.md @@ -18,6 +18,8 @@ Three booking flows, each with its own entry point and reservation TTL: Slot reservations stored as `time_blocker` entries with `RESERVATION:*` descriptions — no separate reservation table. They automatically participate in availability calculations. Anonymous reservation cap: 50 per 10-minute rolling window (429 if exceeded). +Overlap checks now use `FOR UPDATE` row locks inside transactions — the overlap query runs inside `Begin`/`Commit` to prevent race conditions. Closing-hours validation extracted into a reusable `checkClosingHours()` helper and `closing_time.go`. A shared `repo.go` provides common DB query helpers across booking handlers. + **Deposit system:** Bookings have a 24h deposit deadline. Unpaid bookings enter `pending_release` — the slot becomes vulnerable to eviction by overlapping new bookings. Payment of >= 20% of the total at any point promotes back to `confirmed`. Evicted bookings enter `deposit_lapsed`. Admin forgiveness (`forgive_fees`/`forgive_noshow`) on cancel and reschedule. Refund calculation uses notice-period tiers (72h/24h full/partial/none) with deposit protection capping retention at 50% of total. Service eligibility filters by age (min age on service) and patch test validity (6-month expiry, 24-hour notice period). Guest accounts are disposable — no identity tracking across bookings, PII scrubbed 6 months after appointment. @@ -51,7 +53,7 @@ Expiry is 24 months from last use (not from purchase). Each use resets the timer Accounts idle 2+ years (no balance) or 5+ years (with balance) are anonymized. Balances before deletion move to `gift_card_expired_balances`. `CleanupIdleAccounts()` runs on availability fetch. -VAT treatment: gift cards are Single-Purpose Vouchers (SPVs) by default — VAT charged at purchase, not redemption. Configurable to Multi-Purpose Voucher (MPV) in business settings. +VAT treatment: gift cards are Single-Purpose Vouchers (SPVs) by default — VAT charged at purchase, not redemption. Configurable to Multi-Purpose Voucher (MPV) in business settings. Gift card purchases now insert a pending payment record with VAT applied before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record rather than losing the payment. ### Scheduling @@ -105,6 +107,15 @@ Campaign lifecycle: `draft → active → completed` (or any → `cancelled`, `a **Storage**: S3/R2 abstraction with build tags — RustFS in dev (local filesystem), Cloudflare R2 in prod (requires credentials). Portfolio: multi-format pipeline (AVIF/WebP/JPEG/JXL) with client-side WASM encoding via `@jsquash/*` and `@discourse/jxl`. +**Middleware**: `JsonContentType` middleware (in `mw/contenttype.go`) sets `Content-Type: application/json` globally, replacing ~80+ individual `w.Header().Set()` calls across all handlers. `RespondJSON` and `RespondError` helpers (in `mw/response.go`) provide consistent JSON response formatting. Global middleware stack in `main.go` also includes CSP headers, CORS, and rate limiting. + +**All backend handlers now use explicit `Begin`/`defer Rollback`/`Commit` transactions** for DB writes. The old pattern of `db.Conn.Exec(ctx, ...)` for multi-step operations has been replaced with explicit transaction management — every handler that writes to the DB starts a transaction, defers a rollback, and explicitly commits only after all writes succeed. This was applied across auth (JWT revocation, refresh token storage), bookings (overlap checks, reservation creation), payments (gift card purchase, refunds, balance claims), and all admin handlers. + +**`FOR UPDATE` row locking** added to: +- Booking overlap checks (`AdminReserveSlotHandler`, `AdminCreateBookingForUserHandler`) +- Booking status transitions (`ProgressBookingHandler`, `AdminCancelBookingHandler`) +- Expired balance claims (`ClaimExpiredBalance`) + **Build tags**: `dev` vs `!dev` for Square client, S3 storage, DAV service. `test` for test files. ## Limitations @@ -204,7 +215,7 @@ npm run dev # Dev server with HMR ```bash cd backend -go test -tags "test,dev" ./... # ~995 tests, 0 failures, 4 skipped (~1,142 functions defined) +go test -tags "test,dev" ./... # ~1,043 tests, 0 failures, 4 skipped (~1,190 functions defined) go test -tags "test,dev" -v -run TestName ./... # Single test ``` diff --git a/obsidian/Crussell/Technical Manual.md b/obsidian/Crussell/Technical Manual.md index ad24105..ca70508 100644 --- a/obsidian/Crussell/Technical Manual.md +++ b/obsidian/Crussell/Technical Manual.md @@ -1242,7 +1242,7 @@ Files with this pattern: `bookings.go` (4 handlers), `custom_services.go`, `user ### Test Coverage -**~1,142 test functions defined** across all packages (up from 981). Booking integration tests expanded significantly: duplicate completion guard, daily stamp cap (handler + SQL subquery), invalid status transitions, sequential edit, timezone independence, and past-booking no-show guard. +**~1,190 test functions defined** across all packages (up from 1,142). Recent additions: closing_time tests (3), content-type middleware tests (2), new booking handler tests (FOR UPDATE overlap checks, admin reserve with closing_time, gift card buy with VAT). Booking integration tests continue to expand: duplicate completion guard, daily stamp cap (handler + SQL subquery), invalid status transitions, sequential edit, timezone independence, and past-booking no-show guard. The `clock` package itself has tests (2) for Now() and clock interface correctness. | Package | Coverage Area | |---------|--------------| diff --git a/obsidian/Crussell/Testing Architecture & DB Management.md b/obsidian/Crussell/Testing Architecture & DB Management.md index f30cbe9..845f457 100644 --- a/obsidian/Crussell/Testing Architecture & DB Management.md +++ b/obsidian/Crussell/Testing Architecture & DB Management.md @@ -74,10 +74,10 @@ Tests must pass reliably at both verification levels: ``` # Development quick-check (fast): -go test -tags "test,dev" -count=1 -parallel 8 ./... # ~9s +go test -tags "test,dev" -count=1 -parallel 8 ./... # ~11s # Thorough completion verification (catches flakiness): -go test -tags "test,dev" -count=10 -parallel 8 ./... # ~39s +go test -tags "test,dev" -count=10 -parallel 8 ./... # ~44s ``` Features and their tests should pass `-count=1` for iterative development, but always confirm with `-count=10` before considering a feature complete. This catches race conditions in shared globals, goroutine-unsafe library types (e.g., `golang.org/x/text/cases.Caser`), and timing-dependent failures. @@ -499,11 +499,11 @@ This appears in `TestAccount_DeleteGuest` and `TestLoyalty_Get`. The `dav.Servic | Metric | Value | |--------|-------| -| Quick check (`-count=1`) | **~10s** | -| Thorough (`-count=10`) | **~42s** | +| Quick check (`-count=1`) | **~11s** | +| Thorough (`-count=10`) | **~44s** | | Strict serial (`-p 1`) | ~95s | | Packages | 19 tested, 0 failures | -| Tests | ~995 run, 4 skipped, 0 failing (~1,142 defined; ~147 excluded by build tags in non-dev mode) | +| Tests | ~1,043 run, 4 skipped, 0 failing (~1,190 defined; ~147 excluded by build tags in non-dev mode) | New test additions in this batch: | Test | Coverage | @@ -514,6 +514,12 @@ New test additions in this batch: | `TestBookings_Edit_SequentialEdit` | Calls EditBookingHandler twice with different start times — verifies both edits take effect (no stale-duration bug) | | `TestBookings_TimezoneIndependence` | Creates a booking with a UTC time, verifies the stored and retrieved times match exactly with no timezone shift | | `TestDeleteBooking_PastConfirmed_NoNoShow` | Cancels a past confirmed booking — verifies no retroactive no-show penalty via `startTime.After(clock.Now())` guard | +| `TestClosingTime_*` | 3 tests for closing hours validation (edge cases, error handling) | +| `TestContentType_*` | 2 tests for the new `JsonContentType` middleware | +| `TestAdminReserveSlot_*` (expanded) | New overlap coverage using `FOR UPDATE` inside transactions | +| `TestBuyGiftCard_*` (expanded) | Tests for pending-payment-first flow with VAT integration | + +**Test definition count:** 1,190 total (up from 1,142). Growth driven by new `clock` package tests (2), `closing_time` tests (3), `contenttype` middleware tests (2), and expanded booking/payment handler test coverage. ### What Drives Test Time @@ -530,6 +536,7 @@ New test additions in this batch: |--------|--------|-------|--------| | **Per-package databases** | ~90s serial | ~40s parallel | 55% | | **`t.Parallel()` within packages** | ~40s parallel | ~25s parallel | 37% | +| **New tests (closing_time, middleware, expanded bookings)** | ~25s parallel | ~28s parallel | — (added coverage) | | **Per-test transaction rollback by `SetupTestTx`** | — | Eliminates truncation overhead (~5-10s) | Included above | | **`PreferSimpleProtocol` on test pool** | — | Eliminates prepared statement "conn busy" | Required for parallelism | @@ -629,9 +636,9 @@ This shouldn't appear anymore — the auth package's TestMain was updated to use ### Q: What's the total test count? -~1,142 test functions defined across all `_test.go` files. `go test -tags "test,dev" -count=1` reports ~995 run + 4 skipped (~147 are excluded by build tag combinations — some dev-only tests have `//go:build test && dev` and may not match every tag set). 0 failures across 19 packages. +~1,190 test functions defined across all `_test.go` files. `go test -tags "test,dev" -count=1` reports ~1,043 run + 4 skipped (~147 are excluded by build tag combinations — some dev-only tests have `//go:build test && dev` and may not match every tag set). 0 failures across 19 packages. -**Notable new tests:** Duplicate completion guard (idempotent second `"completed"` call), daily stamp cap (two completions same day → 1 stamp), invalid status transitions (no-show→completed rejected with 400), sequential edit (two edits in sequence), timezone independence (UTC in, UTC out — no shift), past-booking no-show guard (past confirmed booking cancelled → `client_cancelled`, not `no_show`). +**Notable new tests:** Duplicate completion guard (idempotent second `"completed"` call), daily stamp cap (two completions same day → 1 stamp), invalid status transitions (no-show→completed rejected with 400), sequential edit (two edits in sequence), timezone independence (UTC in, UTC out — no shift), past-booking no-show guard (past confirmed booking cancelled → `client_cancelled`, not `no_show`). New closing_time tests (3), content-type middleware tests (2), clock package tests (2), expanded admin reserve overlap tests, and expanded gift card buy flow tests with VAT. ### Q: Why use `-count=10` for thorough verification?