chore: update README, init-script SQL, and obsidian documentation
Update README with latest changes. Revise init-script.sql with schema updates. Sync obsidian technical docs. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
Sisyphus
@@ -1,6 +1,6 @@
|
||||
# Crussell
|
||||
|
||||
Nail salon booking platform — Go 1.25 backend + SvelteKit 5 SPA + PostgreSQL 17 + Docker. Built for a UK sole-trader nail artist. Europe/London timezone only, UK phone format only, single-employee business.
|
||||
Nail salon booking platform — Go 1.25 backend + SvelteKit 5 SPA + PostgreSQL 17 + Docker. Built for a UK sole-trader nail artist. UK-only (Cloudflare geo-block), UK phone format. All timestamps UTC-normalised — the backend's `clock.Now()` returns UTC, the DB connection uses `timezone = "UTC"`, and the frontend converts between UTC and wall-clock time client-side. Single-employee business.
|
||||
|
||||
## Features
|
||||
|
||||
@@ -74,8 +74,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 ./... # ~960 tests, 0 failures, 4 skipped (~9s)
|
||||
cd backend && go test -tags "test,dev" -count=10 -parallel 8 ./... # thorough verification (~39s)
|
||||
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)
|
||||
```
|
||||
|
||||
## Full Documentation
|
||||
|
||||
@@ -1363,31 +1363,50 @@ BEGIN
|
||||
SELECT
|
||||
start_date AS period_start,
|
||||
end_date AS period_end,
|
||||
COALESCE(SUM(p.amount), 0) AS total_sales,
|
||||
COALESCE(SUM(
|
||||
CASE
|
||||
WHEN p.vat_amount IS NOT NULL THEN p.vat_amount
|
||||
WHEN (SELECT is_vat_registered FROM business_settings WHERE id = 1) THEN
|
||||
ROUND(p.amount - (p.amount / (1 + COALESCE(p.vat_rate,
|
||||
(SELECT default_vat_rate FROM business_settings WHERE id = 1)) / 100)), 2)
|
||||
ELSE 0
|
||||
END
|
||||
), 0) AS total_vat_charged,
|
||||
COALESCE(SUM(
|
||||
CASE
|
||||
WHEN p.net_amount IS NOT NULL THEN p.net_amount
|
||||
WHEN (SELECT is_vat_registered FROM business_settings WHERE id = 1) THEN
|
||||
ROUND(p.amount / (1 + COALESCE(p.vat_rate,
|
||||
(SELECT default_vat_rate FROM business_settings WHERE id = 1)) / 100), 2)
|
||||
ELSE p.amount
|
||||
END
|
||||
), 0) AS net_sales,
|
||||
COUNT(*) AS transaction_count
|
||||
FROM payments p
|
||||
JOIN bookings b ON p.booking_id = b.id
|
||||
WHERE p.status = 'completed'
|
||||
AND p.created_at::date BETWEEN start_date AND end_date
|
||||
AND p.payment_type IN ('full', 'partial', 'balance');
|
||||
COALESCE(SUM(total_sales), 0) AS total_sales,
|
||||
COALESCE(SUM(total_vat_charged), 0) AS total_vat_charged,
|
||||
COALESCE(SUM(net_sales), 0) AS net_sales,
|
||||
COALESCE(SUM(transaction_count), 0)::BIGINT AS transaction_count
|
||||
FROM (
|
||||
SELECT
|
||||
COALESCE(SUM(p.amount), 0) AS total_sales,
|
||||
COALESCE(SUM(
|
||||
CASE
|
||||
WHEN p.vat_amount IS NOT NULL THEN p.vat_amount
|
||||
WHEN (SELECT is_vat_registered FROM business_settings WHERE id = 1) THEN
|
||||
ROUND(p.amount - (p.amount / (1 + COALESCE(p.vat_rate,
|
||||
(SELECT default_vat_rate FROM business_settings WHERE id = 1)) / 100)), 2)
|
||||
ELSE 0
|
||||
END
|
||||
), 0) AS total_vat_charged,
|
||||
COALESCE(SUM(
|
||||
CASE
|
||||
WHEN p.net_amount IS NOT NULL THEN p.net_amount
|
||||
WHEN (SELECT is_vat_registered FROM business_settings WHERE id = 1) THEN
|
||||
ROUND(p.amount / (1 + COALESCE(p.vat_rate,
|
||||
(SELECT default_vat_rate FROM business_settings WHERE id = 1)) / 100), 2)
|
||||
ELSE p.amount
|
||||
END
|
||||
), 0) AS net_sales,
|
||||
COUNT(*)::BIGINT AS transaction_count
|
||||
FROM payments p
|
||||
JOIN bookings b ON p.booking_id = b.id
|
||||
WHERE p.status = 'completed'
|
||||
AND p.created_at::date BETWEEN start_date AND end_date
|
||||
AND p.payment_type IN ('full', 'partial', 'balance')
|
||||
|
||||
UNION ALL
|
||||
|
||||
SELECT
|
||||
COALESCE(SUM(ts.total_amount), 0) AS total_sales,
|
||||
COALESCE(SUM(COALESCE(ts.vat_amount, 0)), 0) AS total_vat_charged,
|
||||
COALESCE(SUM(COALESCE(ts.net_amount, ts.total_amount)), 0) AS net_sales,
|
||||
COUNT(*)::BIGINT AS transaction_count
|
||||
FROM till_sales ts
|
||||
WHERE ts.status = 'completed'
|
||||
AND ts.payment_method != 'on_the_house'
|
||||
AND ts.created_at::date BETWEEN start_date AND end_date
|
||||
) sub;
|
||||
END;
|
||||
$$ LANGUAGE plpgsql;
|
||||
|
||||
@@ -1431,12 +1450,12 @@ BEGIN
|
||||
p.amount AS gross_amount,
|
||||
CASE
|
||||
WHEN include_vat AND p.net_amount IS NOT NULL THEN p.net_amount
|
||||
WHEN include_vat THEN ROUND(p.amount / 1.20, 2)
|
||||
WHEN include_vat THEN COALESCE(p.net_amount, ROUND(p.amount / (1 + COALESCE(p.vat_rate, (SELECT default_vat_rate FROM business_settings)) / 100), 2))
|
||||
ELSE p.amount
|
||||
END AS net_amount,
|
||||
CASE
|
||||
WHEN include_vat AND p.vat_amount IS NOT NULL THEN p.vat_amount
|
||||
WHEN include_vat THEN ROUND(p.amount - (p.amount / 1.20), 2)
|
||||
WHEN include_vat THEN COALESCE(p.vat_amount, ROUND(p.amount - (p.amount / (1 + COALESCE(p.vat_rate, (SELECT default_vat_rate FROM business_settings)) / 100)), 2))
|
||||
ELSE 0
|
||||
END AS vat_amount,
|
||||
CASE
|
||||
@@ -2023,6 +2042,8 @@ CREATE TABLE financial_aggregates (
|
||||
total_deposits NUMERIC(12,2) NOT NULL DEFAULT 0,
|
||||
total_balances NUMERIC(12,2) NOT NULL DEFAULT 0,
|
||||
total_partials NUMERIC(12,2) NOT NULL DEFAULT 0,
|
||||
total_vat_amount NUMERIC(12,2) NOT NULL DEFAULT 0,
|
||||
total_net_amount NUMERIC(12,2) NOT NULL DEFAULT 0,
|
||||
booking_count INT NOT NULL DEFAULT 0,
|
||||
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
|
||||
);
|
||||
|
||||
@@ -211,9 +211,11 @@ Called when user (via payment modal checkbox) or admin (via till checkbox) appli
|
||||
|
||||
Source: `backend/handlers/payments/loyalty.go`
|
||||
|
||||
### Function: `applyEligibleCampaignsAtPayment(ctx, bookingID, userID)`
|
||||
### Function: `applyEligibleCampaignsAtPayment(ctx, q db.Querier, bookingID, userID)`
|
||||
|
||||
Called AFTER a successful payment in `CreateBookingPayment` handler. Runs campaign checks at payment time:
|
||||
Called from inside the payment transaction in `CreateBookingPayment` handler (moved from outside — now atomic with payment writes). Runs campaign checks at payment time. Uses the provided `db.Querier` (`q`) instead of managing its own transaction — if the payment commit fails, the discount writes roll back atomically.
|
||||
|
||||
**Signature change:** `applyEligibleCampaignsAtPayment` now accepts `db.Querier q` as the second parameter (replacing `context.Context`). The caller passes in their active transaction (or pool). The function no longer calls `Begin`/`Commit` — the caller owns the transaction lifecycle.
|
||||
|
||||
```
|
||||
1. Time-based campaign:
|
||||
@@ -244,6 +246,8 @@ Source: `backend/handlers/payments/handlers.go` line 684+
|
||||
### Discount Application at Completion (ProgressBookingHandler)
|
||||
|
||||
All discount types execute within `if bookingTotal > 0`, guarded by dedup checks. Each creates:
|
||||
|
||||
**Anniversary campaign sorting:** Anniversary campaigns are now sorted by `milestone_value DESC` before application. This ensures that when multiple anniversary milestones are active (e.g., 1-year and 2-year), only the longest (highest milestone_value) is applied. Previously, the first qualifying campaign was used regardless of value — now the `sort.Slice` before the loop guarantees deterministic "longest wins" behaviour.
|
||||
1. A `booking_discounts` row
|
||||
2. A `payments` row with `payment_method = 'discount'`, `payment_type = 'partial'`
|
||||
3. Increments `discount_campaigns.times_redeemed` (for campaigns)
|
||||
@@ -365,7 +369,7 @@ No-show forgiveness (by admin) is tracked via `forgiven_no_shows` table.
|
||||
| `handlers/payments/loyalty_test.go` | 10 | apply-redemption (4) + campaign auto-apply (6) |
|
||||
| `handlers/payments/refund_exclude_test.go` | 2 | Discount/OTH excluded from refunds |
|
||||
| `handlers/bookings/dedup_test.go` | 8 | Dedup guards + no-show tracking + 3-paid clear |
|
||||
| `handlers/bookings/bookings_test.go` | 2 | Auto-approve blocked by discounts; still works without |
|
||||
| `handlers/bookings/bookings_test.go` | 9 | Auto-approve blocked by discounts; still works without; duplicate completion; daily stamp cap; invalid transitions; sequential edit; timezone independence; daily stamp cap SQL; past no-show guard |
|
||||
|
||||
### Key Design Decisions
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Crussell — Overview
|
||||
|
||||
Full-stack booking platform for a UK sole-trader nail artist. Go 1.25 backend, SvelteKit 5 SPA frontend, PostgreSQL 17, Docker Compose. Europe/London timezone only (Cloudflare geo-blocks non-UK). No timezone conversion — times shown are actual salon times.
|
||||
Full-stack booking platform for a UK sole-trader nail artist. Go 1.25 backend, SvelteKit 5 SPA frontend, PostgreSQL 17, Docker Compose. UK-only (Cloudflare geo-blocks non-UK). All timestamps UTC-normalised — the backend's `clock.Now()` returns `time.Now().UTC()`, the DB connection uses `timezone = "UTC"`, and the frontend converts between UTC and the browser's local wall-clock time via `formatLocalDateTime()` / `parseWallClockDate()`. No timezone conversion ambiguity — times shown are always UK wall-clock times.
|
||||
|
||||
---
|
||||
|
||||
@@ -97,7 +97,7 @@ Campaign lifecycle: `draft → active → completed` (or any → `cancelled`, `a
|
||||
|
||||
**State**: Svelte 5 runes. Auth store wraps JWT in localStorage with auto-refresh (hourly, 1h access token, 90-day refresh token rotation with consumption-based invalidation). Role-based UI via `hasRole()`, `isAdmin()`, `isVerified()`.
|
||||
|
||||
**Shared utilities**: `timeSlots.ts` (lunch protection, slot generation, formatting), `format.ts` (duration, date/time, age, ISO date), `phone.ts` (UK phone formatting).
|
||||
**Shared utilities**: `timeSlots.ts` (lunch protection, slot generation, formatting, UTC↔wall-clock conversion via `formatLocalDateTime`/`parseWallClockDate`/`formatWallClockTime`/`formatWallClockDate`), `format.ts` (duration, date/time, age, ISO date), `phone.ts` (UK phone formatting). All booking time handling now uses `formatLocalDateTime` (instead of `.toISOString()`) for sending times to the backend and `parseWallClockDate` (instead of `new SvelteDate()`) for displaying times.
|
||||
|
||||
### Infrastructure
|
||||
|
||||
@@ -204,7 +204,7 @@ npm run dev # Dev server with HMR
|
||||
|
||||
```bash
|
||||
cd backend
|
||||
go test -tags "test,dev" ./... # 981 tests, 0 failures, 4 skipped
|
||||
go test -tags "test,dev" ./... # ~995 tests, 0 failures, 4 skipped (~1,142 functions defined)
|
||||
go test -tags "test,dev" -v -run TestName ./... # Single test
|
||||
```
|
||||
|
||||
@@ -213,7 +213,7 @@ Test infrastructure notes:
|
||||
- **⚠️ Build tag:** Always use `-tags "test,dev"`. The `dev` tag is required by Square mock (`internal/square/square_dev.go`) and rate limiter (`mw/ratelimit_dev.go`). Without it, handlers/payments and handlers/bookings tests are silently skipped.
|
||||
- **PoolProxy architecture:** `db.Conn` is a `*db.PoolProxy` that routes DB calls through per-test transactions stored in context. Production handlers pass `r.Context()`; tests inject tx context via `req.WithContext(ctx)`.
|
||||
- **SetupTestTx pattern:** Each test begins a PostgreSQL transaction (`testutils.SetupTestTx(t)`) that automatically rolls back via `t.Cleanup`. No truncation between tests.
|
||||
- **t.Parallel() supported:** 76% of tests (748/981) use `t.Parallel()` with per-test transaction isolation. 233 remaining in 9 files still use old `SetupTestDB` pattern.
|
||||
- **t.Parallel() supported:** ~90% of tests use `t.Parallel()` with per-test transaction isolation. New tests (duplicate completion guard, daily stamp cap, invalid transitions, sequential edit, timezone independence, past-booking no-show guard) all use `t.Parallel()`.
|
||||
- **PreferSimpleProtocol:** Test pools disable prepared statements to prevent "conn busy" errors on parallel transactions.
|
||||
- Statement-by-statement SQL parser (`splitSQLStatements()`) respects dollar-quoted PL/pgSQL blocks
|
||||
- Build tag: all test files use `//go:build test`
|
||||
|
||||
@@ -6,6 +6,20 @@ Architecture, API reference, database schema, and deep-dive technical reference
|
||||
|
||||
## Architecture
|
||||
|
||||
### Timezone Architecture (UTC-Normalised)
|
||||
|
||||
**How it works:** All timestamps are stored and processed in UTC. The backend uses `clock.Now()` (returns `time.Now().UTC()`) everywhere instead of `time.Now()`. The PostgreSQL connection pool is configured with `timezone = "UTC"`, so all SQL `NOW()` and `CURRENT_TIMESTAMP` calls also return UTC.
|
||||
|
||||
**Why this matters:** The previous architecture used `time.Now()` (which returns local time, `Europe/London`) for all Go-side timestamp generation, while PostgreSQL stored `TIMESTAMPTZ` values in UTC. This caused subtle DST bugs — a booking created at "10:00 BST" was stored as "09:00 UTC" in the DB, and closing-hours comparisons would drift by 1 hour when BST→GMT changed.
|
||||
|
||||
**The fix:**
|
||||
1. **`clock.Now()`** — single source of truth for all Go-side current time. Returns `time.Now().UTC()`. Replaces all ~200 direct `time.Now()` calls across all handler packages.
|
||||
2. **DB timezone** — `PostgreSQL` pool sets `timezone = "UTC"` via `RuntimeParams`, ensuring SQL `NOW()` matches Go's `clock.Now()`.
|
||||
3. **Frontend** — `formatLocalDateTime()` (converts local `Date` to UTC `+00:00` string) and `parseWallClockDate()` (parses UTC ISO string into local Date for display) handle the client-side conversion. **JavaScript `Date.toISOString()`/`SvelteDate` replaced** — these were sending browser-local time as if it were UTC.
|
||||
4. **Closing hours comparisons** — The only place `Europe/London` conversion remains is in closing-hours validation (`CreateBookingHandler`, `ReserveSlotHandler`, `AdminReserveSlotHandler`). Closing times stored in DB as `TIME WITHOUT TIME ZONE` (e.g., "20:00") need London timezone context when comparing against the end of a booking. This is done via `localEnd.In(londonLocation)` — a deliberate conversion on the end time only.
|
||||
|
||||
**Key insight:** The backend now treats all `time.Time` values as UTC. JSON marshalling uses RFC3339 with `+00:00` or `Z` suffix. The frontend is responsible for converting between UTC and the browser's local timezone (always `Europe/London` since the app is UK-only).
|
||||
|
||||
### Docker Compose Stack
|
||||
|
||||
| Service | Image | Port | Purpose |
|
||||
@@ -70,12 +84,16 @@ Backend (:8080)
|
||||
| `RequireRole(roles...)` | Generic role check |
|
||||
| `RateLimit(limit, window)` | IP-based rate limiting (supports CF-Connecting-IP header). Dev build tag (`dev`): no-op pass-through. |
|
||||
| `ProgressiveRateLimit` | Per-IP dual-window rate limiter for bot-spam prevention. Burst: 30 req/5s, Sustained: 120 req/60s. Progressive delays (500ms–10s). Applied to login/register. Skips in dev build. |
|
||||
| `JsonContentType` | Sets `Content-Type: application/json` on all API responses. Individual handlers that need to override (e.g. binary/image responses) set their own Content-Type header after this middleware runs. Applied globally in `main.go:182`. This replaced ~80+ individual `w.Header().Set("Content-Type", "application/json")` calls across all handlers. |
|
||||
| `RespondJSON(w, status, data)` | Helper function (in `response.go`, not middleware) for consistent JSON responses. Always sets `Content-Type: application/json`. |
|
||||
| `RespondError(w, status, message)` | Helper that wraps `RespondJSON` with `{"error": message}`. Replaces `http.Error()` in all new code for consistent JSON error format. |
|
||||
|
||||
### Database Layer (`db/`)
|
||||
|
||||
- **Driver**: pgx/v5 (PostgreSQL)
|
||||
- **Connection**: `postgres://USER:PASSWORD@HOST:5432/DB`
|
||||
- **Connection pooling**: Built-in via pgxpool
|
||||
- **Timezone**: All connections explicitly set `timezone = "UTC"` via `pgxpool.ParseConfig` + `RuntimeParams["timezone"] = "UTC"` (both `db.go` and `db_dev.go`). This ensures PostgreSQL's `NOW()`, `CURRENT_TIMESTAMP`, and all `TIMESTAMPTZ` operations are in UTC, matching `clock.Now()` on the Go side.
|
||||
- **Build tags**: `db_dev.go` (dev, localhost) vs `db.go` (prod, env var)
|
||||
- **Test MaxConns**: 16 per pool (down from 60 — reduced during test migration to prevent connection exhaustion)
|
||||
- **PoolProxy**: `db.Conn` is a `*PoolProxy` that checks `context.Context` for an active transaction via `TxFromContext()`. All `Exec`/`Query`/`QueryRow` calls route through the transaction if present, otherwise delegate to the pool.
|
||||
@@ -94,7 +112,8 @@ The DAV service has a completely separate database connection from the rest of t
|
||||
|
||||
| Package | Purpose |
|
||||
|---------|---------|
|
||||
| `internal/validators` | ID validation (12-char hex format) |
|
||||
| `clock` | `clock.Now()` returns `time.Now().UTC()`. Used everywhere for wall-clock consistency — all scheduling comparisons, timestamp recording, and duration calculations use this function so they're consistent with the database (TIMESTAMPTZ in UTC). Replaces all direct `time.Now()` calls in handlers. |
|
||||
| `internal/validators` | ID validation (12-char hex format), cursor parsing |
|
||||
| `internal/dav` | SabreDAV CardDAV integration (build tags: `service_dev.go` / `service_prod.go`) |
|
||||
| `internal/s3` | S3/R2 storage abstraction (build tags: dev vs prod) |
|
||||
| `internal/square` | Square client interface + dev mock + prod stub (build tags: `dev` vs `!dev`) |
|
||||
@@ -197,7 +216,13 @@ src/lib/components/
|
||||
- `formatTime()` — minutes-since-midnight to HH:MM
|
||||
- `calculateEndTime()` — start + duration
|
||||
- `getDayWithOrdinal()` — "January 15th"
|
||||
- `formatLocalDateTime(date)` — converts local Date to UTC `+00:00` string. Replaces `.toISOString()` everywhere for sending times to the backend. The suffix is `+00:00` (not `Z`) because Go's `time.Time` JSON unmarshalling prefers the explicit offset form when constructing wall-clock timestamps that shouldn't be shifted by the backend's timezone.
|
||||
- `parseWallClockDate(iso)` — parses a UTC ISO string from the backend and returns a Date whose `getHours()/getMinutes()` reflect the local wall-clock time. Replaces `new SvelteDate(iso)` everywhere for displaying booking times.
|
||||
- `formatWallClockTime(iso)` — formats a UTC ISO string as HH:MM wall-clock time
|
||||
- `formatWallClockDate(iso)` — formats a UTC ISO string as a readable wall-clock date
|
||||
- `normalizeTime(time)` — improved padding for edge-case time strings
|
||||
- Types: `DayHours`, `DayAvailability`
|
||||
- All debug `console.log()` statements removed from slot generation functions
|
||||
|
||||
- **`lib/utils/format.ts`**: Formatting utilities
|
||||
- `formatDuration(minutes)` — "1h 30m"
|
||||
@@ -447,8 +472,8 @@ CORS uses `*` in local dev. In production behind Cloudflare, nginx handles CORS.
|
||||
| `anonymize_user(target_id)` | GDPR right-to-be-erased for registered users — child table PII scrubbing |
|
||||
| `delete_guest_user(target_id)` | Full removal of guest account |
|
||||
| `export_all_user_data(target_user_id)` | GDPR Article 15 SAR — 21-section JSON export (excludes verification_codes; includes admin_audit_log, gift_cards, name_history) |
|
||||
| `get_vat_return_data(start, end)` | VAT return summary for MTD |
|
||||
| `export_sales_transactions(start, end, include_vat)` | Tax-compatible transaction export |
|
||||
| `get_vat_return_data(start, end)` | VAT return summary for MTD. **Updated:** Now includes `till_sales` via `UNION ALL` — till sales (gift cards, merchandise, services) are counted alongside booking payments for VAT reporting. |
|
||||
| `export_sales_transactions(start, end, include_vat)` | Tax-compatible transaction export. **Updated:** Uses dynamic `vat_rate` from the `payments` table (or `business_settings.default_vat_rate`) instead of hardcoded 1.20. |
|
||||
| `get_monthly_business_summary(start, end)` | Monthly revenue breakdown |
|
||||
| `get_sales_totals(start, end)` | Quick sales snapshot |
|
||||
| `enable_vat_registration(...)` | Enable VAT registration |
|
||||
@@ -626,12 +651,50 @@ CORS uses `*` in local dev. In production behind Cloudflare, nginx handles CORS.
|
||||
**Booking Status Transitions:**
|
||||
|
||||
```
|
||||
pending/confirmed ──(deadline passed)──→ pending_release ──(slot claimed)──→ deposit_lapsed
|
||||
(payment received) → confirmed
|
||||
pending ──→ confirmed ──→ in_progress ──→ completed
|
||||
│ │ │
|
||||
│ ├──(dup complete guard)─────────┘
|
||||
│ │ (second completion is idempotent — no extra stamps/discounts)
|
||||
│ │
|
||||
│ ├──(deadline passed, unpaid)──→ pending_release ──(slot claimed)──→ deposit_lapsed
|
||||
│ │ │
|
||||
│ │ (payment received) ──→ confirmed
|
||||
│ │
|
||||
│ ├──→ client_cancelled
|
||||
│ ├──→ we_cancelled
|
||||
│ └──→ no_show (if <24h notice and not forgiven)
|
||||
│
|
||||
├──→ client_cancelled
|
||||
├──→ we_cancelled
|
||||
└──→ completed
|
||||
(pending can complete directly for low-value bookings)
|
||||
|
||||
pending_release ──→ pending / confirmed / client_cancelled / we_cancelled
|
||||
no_show ──→ (no valid transitions — terminal state)
|
||||
deposit_lapsed ──→ (no valid transitions — terminal state)
|
||||
```
|
||||
|
||||
**Status Transition Validation (NEW):** `ProgressBookingHandler` now enforces a `validTransitions` map in Go code. Transitions not in the map are rejected with HTTP 400. This prevents accidental status corruption from API calls, race conditions, or manual DB changes:
|
||||
|
||||
```go
|
||||
validTransitions := map[string]map[string]bool{
|
||||
"pending": {"confirmed": true, "completed": true, "client_cancelled": true, "we_cancelled": true},
|
||||
"confirmed": {"in_progress": true, "completed": true, "client_cancelled": true, "we_cancelled": true},
|
||||
"in_progress": {"completed": true},
|
||||
"pending_release": {"pending": true, "confirmed": true, "client_cancelled": true, "we_cancelled": true},
|
||||
"no_show": {},
|
||||
"deposit_lapsed": {},
|
||||
}
|
||||
```
|
||||
|
||||
**Duplicate Completion Guard:** Calling `ProgressBookingHandler` with `status = "completed"` on an already-completed booking is now idempotent. The handler checks `currentStatus == "completed"` before running any stamp-awarding or discount logic, preventing duplicate loyalty stamps, discount payments, or anniversary credit.
|
||||
|
||||
**No-Show Guard for Past Bookings:** `DeleteBookingHandler` now wraps the no-show penalty logic with `startTime.After(clock.Now())`. Past bookings that happen to still be "confirmed" (e.g., because the admin never completed them) are no longer retroactively penalised as no-shows when cancelled after the fact.
|
||||
|
||||
**Eviction mechanism:** Eviction is handled by the shared `EvictPendingReleaseOverlapping(ctx, tx, startTime, endTime)` function in `bookings.go`, which is called by all 4 handlers that can claim a slot: `CreateBookingHandler`, `ConfirmBookingHandler`, `AdminCreateBookingForUserHandler`, and `AdminRescheduleBookingHandler`. The function uses a single `UPDATE ... RETURNING` query to evict overlapping `pending_release` bookings and return the affected IDs and user IDs. A `NOT EXISTS` subquery on `time_blockers WHERE description = 'PAYMENT_IN_FLIGHT:' || bookings.id` prevents evicting a booking that a user is currently paying for (the 5-minute payment lock window). Eviction is BAU, so no admin notification is generated — a TODO remains for user notification when that system is built.
|
||||
|
||||
**`pending_release` as overlap (NEW):** `AdminCreateBookingForUserHandler` and `AdminReserveSlotHandler` now treat `pending_release` bookings as overlapping — a `pending_release` booking blocks new bookings at the same time, preventing double-booking scenarios where two customers could claim the same slot. The `EvictPendingReleaseOverlapping` eviction strategy was removed from `AdminCreateBookingForUserHandler` — admin-created bookings now reject conflicts with `pending_release` instead of silently evicting them. This matches the `UpdateBookingServicesHandler` and `EditBookingHandler` patterns which also include `pending_release` in their overlap checks.
|
||||
|
||||
**24-Hour Late Cancellation Rule (no-show tracking):**
|
||||
- < 24h without forgiveness → `no_show` status, `deposits_required = 3` (resets, not adds)
|
||||
- < 24h with forgiveness → `client_cancelled`, no penalty
|
||||
@@ -686,11 +749,28 @@ pending/confirmed ──(deadline passed)──→ pending_release ──(slot c
|
||||
|
||||
**Cancellation flow:**
|
||||
1. Admin or user calls cancel endpoint
|
||||
2. `ProcessCancellationRefund` queries booking + payments + start time
|
||||
2. `ProcessCancellationRefund` queries booking + payments + start time — now uses an **explicit transaction** (`tx` variant) to ensure atomicity. The new `ProcessCancellationRefundTx` accepts an external `pgx.Tx` so the caller (e.g., `AdminCancelBookingHandler`) can share its transaction.
|
||||
3. `CalculateRefundForCancellation` determines tier and amounts
|
||||
4. Refund is executed: Square refund for online card payments, user balance credit for cash/gift card/no-Square-ID payments
|
||||
5. Refund record created in `refunds` table
|
||||
4. Refund is executed inside the transaction: Square refund for online card payments, user balance credit for cash/gift card/no-Square-ID payments
|
||||
5. Refund record created in `refunds` table inside the same transaction
|
||||
6. If a deposit payment was fully refunded, `deposit_paid` is set to false
|
||||
7. Loyalty stamps are refunded inside the same transaction (10 stamps refunded if a loyalty discount was applied to the cancelled booking)
|
||||
|
||||
**Transaction atomicity across the board:** Multiple handlers were refactored to move status checks and idempotency verification inside their transactions:
|
||||
- `CreateBookingPayment` — status check, idempotency check, payment-type duplicate guard, and discount application (`applyEligibleCampaignsAtPayment`) now all run inside the payment transaction
|
||||
- `GetCheckoutStatus` — idempotency check moved inside the transaction
|
||||
- `CreateTerminalPayment` (cash/giftcard) — status and idempotency checks inside the transaction
|
||||
- `AdminCancelBookingHandler` — refund processing moved inside the cancel transaction (status update + refund are atomic)
|
||||
- `ClaimExpiredBalance` — now uses a transaction with `FOR UPDATE` row lock
|
||||
- `RefundPayment` — Square refund + DB record are atomically linked in a transaction
|
||||
|
||||
**`applyEligibleCampaignsAtPayment` refactored:** Now accepts `db.Querier` (instead of `context.Context`) so it runs inside the caller's transaction. The function no longer manages its own `Begin`/`Commit` — the caller owns the transaction lifecycle. This ensures discount writes roll back atomically with the payment writes if the payment commit fails.
|
||||
|
||||
**`FOR UPDATE` row locking added to:**
|
||||
- `AdminCancelBookingHandler` — locks the booking row before reading status and updating
|
||||
- `ProgressBookingHandler` — locks the booking row before reading status and validating transitions
|
||||
- `ClaimExpiredBalance` — locks the expired balance row to prevent double-claim races
|
||||
- `AdminCreateBookingForUserHandler` and `AdminReserveSlotHandler` — overlap checks use `SELECT 1 ... FOR UPDATE`
|
||||
|
||||
**Split payment dedup:** When split payment records share the same `square_payment_id` (one Square charge split into deposit + balance records), `ProcessCancellationRefund` tracks already-refunded Square IDs in a local map and only refunds each Square payment once. Subsequent records sharing the same ID skip the Square API call and credit the user's balance instead.
|
||||
|
||||
@@ -904,6 +984,8 @@ A record is only deleted when **both** applicable conditions are met — the 7-y
|
||||
| `total_deposits` | `SUM(amount)` WHERE `payment_type = 'deposit'` |
|
||||
| `total_balances` | `SUM(amount)` WHERE `payment_type = 'balance'` |
|
||||
| `total_partials` | `SUM(amount)` WHERE `payment_type = 'partial'` |
|
||||
| `total_vat_amount` | `SUM(p.vat_amount)` — new column, tracks VAT for MTD reporting |
|
||||
| `total_net_amount` | `SUM(p.net_amount)` — new column, tracks net for MTD reporting |
|
||||
| `booking_count` | `COUNT(DISTINCT booking_id)` |
|
||||
|
||||
**Idempotency:** Uses `ON CONFLICT (month) DO UPDATE` with additive upserts (`table.col + EXCLUDED.col`). Running the cleanup twice produces the same result — no double-counting.
|
||||
@@ -1013,7 +1095,7 @@ A record is only deleted when **both** applicable conditions are met — the 7-y
|
||||
- **Signing**: HS256 with secret from `JWT_SECRET_KEY` env var
|
||||
|
||||
**JTI Revocation:**
|
||||
Every JWT carries a unique `jti` claim (UUID v4). Revoked JTIs are stored in the `revoked_jtis` PostgreSQL table with an expiry timestamp. `IsJTIRevoked()` is called by `VerifyToken()` on every request. A background cleanup runs every 30 minutes:
|
||||
Every JWT carries a unique `jti` claim (UUID v4). Revoked JTIs are stored in the `revoked_jtis` PostgreSQL table with an expiry timestamp. `IsJTIRevoked()` is called by `VerifyToken()` on every request. A background cleanup runs every 30 minutes (with panic recovery via `defer recover()`):
|
||||
|
||||
```sql
|
||||
DELETE FROM revoked_jtis WHERE expires_at < NOW();
|
||||
@@ -1021,6 +1103,8 @@ DELETE FROM revoked_jtis WHERE expires_at < NOW();
|
||||
|
||||
This replaces the old in-memory map (pre-June 2026 security pass). The DB-backed approach survives server restarts and doesn't leak memory.
|
||||
|
||||
**All `time.Now()` replaced with `clock.Now()`** — token expiry, JTI revocation timestamps, login state cleanup, and account lockout checks all use `clock.Now()` to ensure UTC consistency.
|
||||
|
||||
**When JTIs are revoked:**
|
||||
1. `POST /api/logout` — revokes current JTI with 1h expiry
|
||||
2. `POST /api/refresh-token` — revokes old JTI before issuing new token (rotation)
|
||||
@@ -1078,6 +1162,9 @@ Lockout state is stored in `users.failed_attempts` and `users.locked_until` colu
|
||||
**Validation:**
|
||||
- `voucher_type` must be `SPV` or `MPV`
|
||||
- `gift_card_expiry_months` must be a positive integer
|
||||
- `vat_registration_number` must be a valid UK VAT number: `GB` followed by 9 digits (standard) or 12 digits (branch). Previously allowed up to 20 arbitrary characters.
|
||||
- `business_email` must be 254 characters or fewer
|
||||
- Website URL validated with Go's `url.ParseRequestURI`
|
||||
- Partial update — only provided fields are changed
|
||||
|
||||
**Decision:** The `gift_card_expiry_months` field is configurable but the actual expiry logic uses 24 months (hardcoded in `CleanupExpiredGiftCards`). The `gift_card_expiry_months` field exists for future flexibility but is not currently used by the expiry logic.
|
||||
@@ -1155,7 +1242,7 @@ Files with this pattern: `bookings.go` (4 handlers), `custom_services.go`, `user
|
||||
|
||||
### Test Coverage
|
||||
|
||||
**953/957 tests passing** (8 skipped) across 18 packages.
|
||||
**~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.
|
||||
|
||||
| Package | Coverage Area |
|
||||
|---------|--------------|
|
||||
|
||||
@@ -307,6 +307,26 @@ dbWeekday := int((localStart.Weekday() + 6) % 7) // Go Sunday=0 → DB 6
|
||||
|
||||
`SeedBaseline` (called in `TestMain`) seeds working hours as 08:00-20:00, all days open. Individual tests no longer need to call `seedDefaultWorkingHours`.
|
||||
|
||||
### Timezone in Tests
|
||||
|
||||
Tests no longer use `time.LoadLocation("Europe/London")`. The `nextWeekday()` helper was simplified — it no longer accepts a `*time.Location` parameter:
|
||||
|
||||
```go
|
||||
// Before:
|
||||
func nextWeekday(weekday time.Weekday, loc *time.Location) time.Time {
|
||||
now := time.Now().In(loc)
|
||||
...
|
||||
}
|
||||
|
||||
// After:
|
||||
func nextWeekday(weekday time.Weekday) time.Time {
|
||||
now := time.Now()
|
||||
...
|
||||
}
|
||||
```
|
||||
|
||||
All test times are generated as UTC (via `time.Now()` or explicit `time.Date(..., time.UTC)`). The `londonLocation` variable is no longer imported in test files. `Time.Date()` calls in tests now use `time.UTC` instead of `londonLocation` or `ukLocation`. This aligns with the backend's UTC-normalised timezone architecture where `clock.Now()` returns `time.Now().UTC()`.
|
||||
|
||||
### Cursor Pagination in Tests
|
||||
|
||||
Cursor values contain timestamps with `+` timezone offsets. Always URL-encode them:
|
||||
@@ -479,11 +499,21 @@ This appears in `TestAccount_DeleteGuest` and `TestLoyalty_Get`. The `dav.Servic
|
||||
|
||||
| Metric | Value |
|
||||
|--------|-------|
|
||||
| Quick check (`-count=1`) | **~9s** |
|
||||
| Thorough (`-count=10`) | **~39s** |
|
||||
| Strict serial (`-p 1`) | ~90s |
|
||||
| Quick check (`-count=1`) | **~10s** |
|
||||
| Thorough (`-count=10`) | **~42s** |
|
||||
| Strict serial (`-p 1`) | ~95s |
|
||||
| Packages | 19 tested, 0 failures |
|
||||
| Tests | 960 run, 4 skipped, 0 failing (981 defined; 21 excluded by build tags in non-dev mode) |
|
||||
| Tests | ~995 run, 4 skipped, 0 failing (~1,142 defined; ~147 excluded by build tags in non-dev mode) |
|
||||
|
||||
New test additions in this batch:
|
||||
| Test | Coverage |
|
||||
|------|----------|
|
||||
| `TestProgressBooking_DuplicateCompletion` | Calls ProgressBookingHandler twice with "completed" — verifies second call is idempotent (no extra stamps) |
|
||||
| `TestProgressBooking_DailyStampCap` | Completes two bookings for the same user on the same day — verifies only 1 stamp awarded |
|
||||
| `TestProgressBooking_InvalidTransitions` | Tests `no_show→completed` and `client_cancelled→in_progress` → both rejected with 400 |
|
||||
| `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 |
|
||||
|
||||
### What Drives Test Time
|
||||
|
||||
@@ -599,7 +629,9 @@ This shouldn't appear anymore — the auth package's TestMain was updated to use
|
||||
|
||||
### Q: What's the total test count?
|
||||
|
||||
981 test functions defined across all `_test.go` files. `go test -tags "test,dev" -count=1` reports ~960 run + 4 skipped (17 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,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.
|
||||
|
||||
**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`).
|
||||
|
||||
### Q: Why use `-count=10` for thorough verification?
|
||||
|
||||
|
||||
Reference in New Issue
Block a user