diff --git a/.gitea/workflows/backend-tests.yaml b/.gitea/workflows/backend-tests.yaml new file mode 100644 index 0000000..a9c2877 --- /dev/null +++ b/.gitea/workflows/backend-tests.yaml @@ -0,0 +1,63 @@ +name: Backend Tests +description: Runs Go tests with PostgreSQL for all backend packages. +on: + push: + branches: + - main + - develop + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: myuser + POSTGRES_PASSWORD: mypassword + POSTGRES_DB: mydb + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: + - 5432:5432 + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version: "1.25" + cache-dependency-path: backend/go.sum + + - name: Install system deps + run: sudo apt-get update && sudo apt-get install -y postgresql-client + + - name: Wait for PostgreSQL + run: | + for i in $(seq 1 30); do + pg_isready -h 127.0.0.1 -U myuser && break + sleep 1 + done + + - name: Build + working-directory: backend + run: go build ./... + + - name: Vet + working-directory: backend + run: go vet ./... + + - name: Test (all packages) + working-directory: backend + run: go test -tags "test,dev" -count=1 -v -timeout 120s ./handlers/... ./auth/... ./mw/... ./internal/... ./db/... + env: + PGHOST: 127.0.0.1 + PGUSER: myuser + PGPASSWORD: mypassword + PGDATABASE: mydb diff --git a/.sisyphus/plans/timezone-tx-migration-completion.md b/.sisyphus/plans/timezone-tx-migration-completion.md new file mode 100644 index 0000000..66747d3 --- /dev/null +++ b/.sisyphus/plans/timezone-tx-migration-completion.md @@ -0,0 +1,486 @@ +# Timezone + Transaction Migration — Completion Plan + +## Workflow per Issue (MANDATORY) + +For EVERY issue in this plan, follow this exact sequence: + +``` +Step 1: IMPLEMENT the fix (delegate to subagent) +Step 2: WAIT for subagent to complete +Step 3: VERIFY — Read the changed files, run lsp_diagnostics, run go build +Step 4: CODE REVIEW — Manually inspect the diff. Check: + - Does the fix match the intended pattern? + - Are there edge cases the fix misses? + - Does it follow existing codebase conventions? +Step 5: ADD BACKEND TESTS for the fix (delegate to subagent) +Step 6: WAIT for test subagent to complete +Step 7: VERIFY tests — lsp_diagnostics, go build, go test on the relevant package +Step 8: UPDATE todo — mark issue complete +Step 9: MOVE TO next issue +``` + +**NO parallel execution across issues.** Each issue is a lock-step sequence. + +## Ground Rules + +- **NO `git checkout`**, `git stash`, `git reset`, `git restore`, or any destructive git commands. +- **NO `rm -rf`**, mass file deletion, or destructive filesystem operations. +- **Do NOT commit** unless explicitly asked. +- **Do NOT create new files** unless the plan explicitly says to. Prefer editing existing files. +- **Stage only intended files** if you do stage anything. +- **Every delegation prompt** must include the 6-section structure: + 1. TASK — atomic, specific goal + 2. EXPECTED OUTCOME — concrete deliverables + 3. REQUIRED TOOLS — explicit whitelist + 4. MUST DO — exhaustive requirements + 5. MUST NOT DO — forbidden actions + 6. CONTEXT — file paths, existing patterns, constraints + +--- + +## Issue 1: Fix `calculateAge()` in format.ts + +**File:** `/home/popertots/Crussell/frontend/src/lib/utils/format.ts` +**Lines:** ~92-101 +**Bug:** Uses browser-local `getFullYear()/getMonth()/getDate()` for age calculation. + +### Step 1 — Implement fix +Delegate to a `deep` or `unspecified-high` agent. Use Read + Edit tools only. + +**Prompt:** +``` +TASK: Fix calculateAge() function in /home/popertots/Crussell/frontend/src/lib/utils/format.ts to use Europe/London timezone. + +EXPECTED OUTCOME: calculateAge() returns correct age for all UK users regardless of browser timezone. Function signature, return type, and all callers remain unchanged. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- Read lines 85-105 of the file to see the current implementation +- Parse the DOB string directly (split by '-') to get year/month/day +- Get today's London date using toLocaleDateString('en-CA', { timeZone: 'Europe/London' }) +- Compute age: difference in years, subtract 1 if birthday hasn't occurred yet in London time +- Return integer (years) + +MUST NOT DO: +- Do not change function signature or how callers use it +- Do not import external date libraries +- Do not modify any other function in format.ts + +CONTEXT: Used during registration to validate minimum age (16+). DOB comes from form as "YYYY-MM-DD" string. UK-only app. +``` + +### Step 3 — Verify +Read the changed function. Confirm: +- DOB is parsed by string split (not via `new Date(dob)`) +- London date uses `toLocaleDateString('en-CA', { timeZone: 'Europe/London' })` +- Age computation handles month/day comparison correctly +- Run `lsp_diagnostics` on format.ts + +### Step 5 — Add backend tests (skip — this is frontend-only code) + +--- + +## Issue 2: Fix 4 display components missing `timeZone: 'Europe/London'` + +### Step 1 — Implement fix +Delegate to a `visual-engineering` or `deep` agent. Read each file, then edit. + +**Files to fix:** +1. `/home/popertots/Crussell/frontend/src/lib/components/admin/CustomServicesManagement.svelte` — line ~289 +2. `/home/popertots/Crussell/frontend/src/lib/components/admin/GiftCardsManagement.svelte` — line ~752 +3. `/home/popertots/Crussell/frontend/src/routes/gdpr/+page.svelte` — lines ~262 and ~268 + +**Prompt:** +``` +TASK: Add timeZone: 'Europe/London' to toLocaleDateString/toLocaleString calls in 4 locations across 3 frontend files. + +EXPECTED OUTCOME: All date displays use Europe/London timezone so that UTC timestamps near midnight display the correct London date. + +REQUIRED TOOLS: Read, Edit, Grep + +MUST DO: +- Read each file at the specified line to understand the context +- Determine if the value being formatted is a datetime (needs timezone) or date-only (safe as-is) +- Add timeZone: 'Europe/London' to the options object where needed +- The call sites are: + 1. CustomServicesManagement.svelte:~289 — likely a toLocaleDateString for expiry/schedule display + 2. GiftCardsManagement.svelte:~752 — gift card date display + 3. gdpr/+page.svelte:~262 — some GDPR-related date + 4. gdpr/+page.svelte:~268 — some GDPR-related date + +MUST NOT DO: +- Do not change any logic other than adding the timeZone option +- Do not modify any CSS or HTML structure + +CONTEXT: UK-only booking platform. UTC timestamps near midnight display wrong London date without explicit timeZone option. +``` + +### Step 3 — Verify +Read each changed file. Confirm the `timeZone: 'Europe/London'` was added correctly. +Run `lsp_diagnostics` on each changed file (if the LSP supports Svelte). + +### Step 5 — Add backend tests (skip — these are frontend-only changes) + +--- + +## Issue 3: Fix remaining `toISOString().split('T')[0]` calls + +### Step 1 — Implement fix +Delegate to a `deep` or `visual-engineering` agent. + +**Files:** +1. `/home/popertots/Crussell/frontend/src/routes/login/+page.svelte` — line ~491 +2. `/home/popertots/Crussell/frontend/src/routes/gdpr/+page.svelte` — line ~467 + +**Prompt:** +``` +TASK: Replace 2 remaining toISOString().split('T')[0] calls with toLocaleDateString('en-CA', { timeZone: 'Europe/London' }) in the frontend. + +EXPECTED OUTCOME: No more toISOString().split('T')[0] patterns remain in the frontend. Date-only strings are produced via London-timezone-aware formatting. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- Read login/+page.svelte around line 491 — replace toISOString().split('T')[0] with toLocaleDateString('en-CA', { timeZone: 'Europe/London' }) +- Read gdpr/+page.svelte around line 467 — same replacement +- Verify the context: login uses the value as a max date attribute; gdpr uses it in a JSON filename + +MUST NOT DO: +- Do not change any surrounding logic +- Do not remove the +00:00 or any other API communication toISOString() calls that were confirmed safe + +CONTEXT: Both are cosmetic — login date input max value and GDPR JSON filename. But keeping them consistent prevents future timezone bugs. +``` + +### Step 3 — Verify +Read both changed files. Confirm the replacement intent (date-only, not API communication). + +### Step 5 — Add backend tests (skip — frontend-only changes) + +--- + +## Issue 4: Fix 3 genuine payment bugs + +**Background:** The reference pattern is `ProcessPendingSquareRefunds` in `refunds.go`. It inserts records with `status='pending'` inside the transaction, commits, THEN calls the Square API. The schema already supports this: `payment_status` ENUM has `'pending'`, and all tables default to it. + +### Step 1 — Fix Issue 4a: CreateTipPayment Square-BEFORE-tx + +**File:** `/home/popertots/Crussell/backend/handlers/payments/handlers.go` +**Lines:** ~1700-1752 + +**Prompt:** (delegate to `unspecified-high` agent) +``` +TASK: Fix CreateTipPayment handler in /home/popertots/Crussell/backend/handlers/payments/handlers.go so the Square API call happens AFTER the DB transaction commit, not before. + +EXPECTED OUTCOME: Square CreatePayment is called only after the DB transaction commits successfully. If the transaction fails, Square is never called. No money lost. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- Read lines 1690-1760 to understand the current flow +- Restructure so: start tx → INSERT payment with status='pending' and square_payment_id=NULL → tx.Commit() → call SquareClient.CreatePayment() → if success, UPDATE payment SET status='completed', square_payment_id=$1 +- Keep the same idempotency key pattern +- Keep the same error logging +- The payment record already has a `status` column (payment_status ENUM, defaults to 'pending') + +MUST NOT DO: +- Do not change the function signature +- Do not change any other handler in the file +- Do not remove the advisory unlock or any existing cleanup + +CONTEXT: Currently Square is called at line ~1711 BEFORE the tx starts at ~1732. If tx.Commit() fails, the Square charge is orphaned with no recovery path. +``` + +### Step 3 — Code Review for 4a +Read the changed handler. Verify: +- tx begins FIRST +- Payment record inserted BEFORE Square call with `status='pending'` +- `tx.Commit()` happens BEFORE `SquareClient.CreatePayment()` +- Square result updates the record to `'completed'` +- Error handling covers: tx.Commit() fails (return 500, no Square charge), Square fails (leave as 'pending' for manual retry) +- `go build ./handlers/payments/` passes + +### Step 5 — Add backend tests for 4a +Delegate to subagent: +``` +TASK: Add test coverage for the CreateTipPayment fix in handlers/payments/payments_test.go. + +EXPECTED OUTCOME: A test that verifies when CreateTipPayment's tx.Commit() fails, the Square API is never called, and when tx.Commit() succeeds, Square IS called. + +REQUIRED TOOLS: Read, Edit, Grep + +MUST DO: +- Read the existing test patterns in payments_test.go (mock Square client, test HTTP handlers) +- Create a test that: + 1. Sets up a booking with a tip amount + 2. Mocks SquareClient.CreatePayment to succeed + 3. Calls CreateTipPayment handler + 4. Asserts the payment record exists with status='completed' +- Create another test that: + 1. Sets up a booking with a tip amount + 2. Mocks the DB to fail on commit (use test context cancellation or tx rollback trigger) + 3. Verifies Square.CreatePayment was NOT called + 4. Verifies 500 status is returned + +MUST NOT DO: +- Do not modify production code (only add tests) +- Do not use external mocking frameworks (use existing mock patterns) +- Follow existing test file conventions + +CONTEXT: Payment tests use mock square client. Test fixtures create bookings. Test transactions use testutils.SetupTestTx(t). +``` + +### Step 7 — Verify tests for 4a +Run `go test ./handlers/payments/ -run TestCreateTipPayment -v -count=1` — confirm it passes. + +--- + +### Step 1 — Fix Issue 4b: BuyGiftCard Square-BEFORE-tx + +**File:** `/home/popertots/Crussell/backend/handlers/payments/giftcards.go` +**Lines:** ~860-1044 + +**Prompt:** (delegate to `unspecified-high` agent) +``` +TASK: Fix BuyGiftCard handler in giftcards.go so the Square API call happens AFTER the DB transaction commit. + +EXPECTED OUTCOME: Square CreatePayment is called only after DB transaction commits successfully. If commit fails, Square is never called. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- Read lines 840-1060 of giftcards.go +- Restructure: start tx → INSERT gift_card with status='pending' → INSERT payment with status='pending' → tx.Commit() → squareClient.CreatePayment() → if success, UPDATE gift_card SET status='active' AND UPDATE payment SET status='completed' +- Remove the Square API calls from before the tx (lines ~860-906) +- Keep the idempotency check at lines ~846-855 +- The gift_cards table uses the same payment_status ENUM pattern + +MUST NOT DO: +- Do not change the handler signature +- Do not change gift card value calculations or VAT +- Do not modify the CreateCardOnFileRaw call (it's for card tokenization, not charging) + +CONTEXT: Currently Square CreatePayment is called at line ~901 before the tx starts at line ~908. Schema supports pending→completed pattern. +``` + +### Step 3 — Code Review for 4b +Read the changed handler. Verify the pending→completed pattern is correct. +`go build ./handlers/payments/` passes. + +### Step 5 — Add backend tests for 4b +Delegate to subagent following existing gift card test patterns. + +### Step 7 — Verify tests for 4b +`go test ./handlers/payments/ -run TestBuyGiftCard -v -count=1` passes. + +--- + +### Step 1 — Fix Issue 4c: CreateTillSale saved_card/online_square inside-tx + +**File:** `/home/popertots/Crussell/backend/handlers/payments/till.go` +**Lines:** ~112-420 + +**Prompt:** (delegate to `unspecified-high` agent) +``` +TASK: Fix CreateTillSale handler in till.go so Square API calls for saved_card and online_square payment methods happen AFTER the DB transaction commit. + +EXPECTED OUTCOME: Square CreatePayment for saved_card and online_square methods is called only after tx.Commit(). If commit fails, Square is never called. Card_machine, cash, and on_the_house methods remain unchanged. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- Read till.go lines 100-430 fully +- Understand the 4 payment methods: cash (no Square), card_machine (checkout only — keep as is), saved_card (Square Payment), online_square (Square card tokenize + Payment), on_the_house (no Square) +- For saved_card and online_square: + - Insert the till_sale INSIDE the tx with status='pending' (saleStatus = "pending") + - Insert/update gift_card records INSIDE the tx as currently done + - Commit the tx + - THEN call SquareClient.CreatePayment() + - On Square success, update the till_sale status to 'completed' with the square_payment_id + - On Square failure, leave till_sale as 'pending' for manual retry +- For card_machine: keep the existing pattern (checkout created inside tx, reconciled by GetTillCheckoutStatus) +- For cash/on_the_house: keep as-is (no Square call) + +MUST NOT DO: +- Do not change the function signature or request/response types +- Do not change the VAT calculation logic +- Do not modify card_machine, cash, or on_the_house flows +- Do not remove the gift card creation/topup logic from inside the tx (it should stay there) + +CONTEXT: The till_sales table has a `status payment_status DEFAULT 'pending'` column. The gift_card_transactions table tracks gift card operations. The existing card_machine flow already uses saleStatus = "pending". +``` + +### Step 3 — Code Review for 4c +Read the changed handler. Verify: +- saved_card flow uses pending→commit→Square→completed +- online_square flow uses pending→commit→Square→completed +- card_machine flow unchanged +- cash/on_the_house flows unchanged +- `go build ./handlers/payments/` passes + +### Step 5 — Add backend tests for 4c +Delegate to subagent to add tests for the till sale flows. + +### Step 7 — Verify tests for 4c +`go test ./handlers/payments/ -run TestCreateTillSale -v -count=1` passes. + +--- + +## Issue 5: Migrate all `time.Now()` → `clock.Now()` in test files + +**Strategy:** Process by Go package directory, one directory at a time. + +**Order:** +1. `handlers/today/` (already clean — 0 replacements, just verify) +2. `handlers/admin/` (4 files, ~67 replacements) +3. `handlers/scheduling/` (2 files, ~54 replacements) +4. `handlers/payments/` (4 files, ~25 replacements) +5. `handlers/auth/` + `auth/` (2 files, ~12 replacements) +6. `handlers/user/` (1 file, ~5 replacements) +7. `handlers/notifications/` (1 file, ~4 replacements) +8. `mw/` (1 file, ~4 replacements) +9. `handlers/bookings/` (7 files, ~232 replacements — save the largest for last) + +**For EACH directory:** + +### Step 1 — Implement migrate +Delegate to an `unspecified-high` agent. + +**Prompt template:** +``` +TASK: Replace all time.Now() calls with clock.Now() in test files in /home/popertots/Crussell/backend/handlers/[PACKAGE]/. + +EXPECTED OUTCOME: Every test file in the package uses clock.Now() instead of time.Now(). Import of "crussell/clock" is added where missing. Build passes. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- For EACH *_test.go file in the directory: + 1. Read the file to find all time.Now() calls + 2. If the file doesn't already import "crussell/clock", add it to the imports + 3. Replace EVERY occurrence of time.Now() with clock.Now() + 4. Handle special cases: + - time.Now().UTC() → clock.Now() (clock.Now() already returns UTC) + - time.Now().In(londonLocation) → clock.Now().In(londonLocation) (time.Now().In() → clock.Now().In()) + - time.Now().Add(...) → clock.Now().Add(...) (bulk replace works) + - time.Now().UnixNano() → clock.Now().UnixNano() (same) + - time.Now().Truncate(...) → clock.Now().Truncate(...) (same) + 5. Save each file after editing +- Run go build ./handlers/[PACKAGE]/... to verify it compiles + +SPECIAL CASE for handlers/bookings/bookings_test.go: +- The nextWorkingHour() helper function at ~line 48 uses time.Now() with .Hour() for wall-clock logic +- Fix: replace time.Now() with clock.Now().In(londonLocation) so the wall-clock check uses London time +- Then change the return statement to use .UTC() before returning + +MUST NOT DO: +- Do not modify production code files (only *_test.go files) +- Do not modify files outside the specified directory +- Do not add "crussell/clock" to production code files +- Do not remove any existing imports + +CONTEXT: clock.Now() returns time.Now().UTC(). All production code already uses clock.Now(). The test files were left behind in the previous migration. +``` + +### Step 3 — Verify +Check each changed file: +- Import of "crussell/clock" is present +- `time.Now()` is fully replaced (no lingering occurrences) +- `go build ./handlers/[PACKAGE]/...` passes +- `go vet ./handlers/[PACKAGE]/...` passes + +### Step 5 — Add backend tests (not needed for this issue — it IS test migration) + +### Step 9 — Move to next package directory + +--- + +## Issue 6: Add autumn DST regression tests + +### Step 1 — Add tests (one file at a time) + +**Test file 1:** `/home/popertots/Crussell/backend/handlers/scheduling/scheduling_test.go` + +**Tests to add:** +- `TestScheduling_DST_AutumnBack_BookingAt0130BST` — booking at 01:30 BST (00:30 UTC) on Oct 25 spans transition +- `TestScheduling_DST_AutumnBack_BookingAt0130GMT` — booking at 01:30 GMT (01:30 UTC) on Oct 25 + +Delegate to subagent: +``` +TASK: Add autumn DST regression tests to scheduling_test.go. + +EXPECTED OUTCOME: Two new test functions covering bookings at the repeated hour on autumn DST day. + +REQUIRED TOOLS: Read, Edit + +MUST DO: +- Read the existing test TestScheduling_DST_BlockerOnSpringForward (~line 2580) for pattern reference +- Read scheduling_test.go helpers: resetTestData, makeAdminAvailableHoursRequest, findDayByDate, slotExists +- Add TestScheduling_DST_AutumnBack_BookingAt0130BST: + - Date: 2026-10-25 (Oct 25, 2026 = last Sunday of October, BST→GMT transition) + - Create exceptional hours for Sunday (weekday 6) 09:00-17:00 + - Create a booking at 01:30 BST = 00:30 UTC (spans DST transition) + - Query available hours for 2026-10-25 + - Assert: the 00:30 slot is blocked (booking starts) +- Add TestScheduling_DST_AutumnBack_BookingAt0130GMT: + - Same setup, but booking at 01:30 GMT = 01:30 UTC + - Query available hours + - Assert: the 01:30 slot is blocked + +MUST NOT DO: +- Do not modify production code +- Do not modify existing tests +- Use time.Date with londonLocation for explicit timezone construction + +CONTEXT: The londonLocation variable is available in scheduling_test.go (package-level). Oct 25, 2026 is the autumn DST date. BST ends at 02:00 BST → 01:00 GMT. The duplicated hour is 01:00-02:00 wall-clock time (occurs twice: first as BST, then as GMT). +``` + +### Step 3 — Verify +Read the added test functions. `go build ./handlers/scheduling/` passes. + +### Step 5 — Add more tests (next file) + +**Test file 2:** `/home/popertots/Crussell/backend/handlers/today/today_test.go` + +**Test to add:** +- `TestFindWeekSummaryRange_AutumnDST` — London-aligned boundaries for Oct 25 + +**Test file 3:** `/home/popertots/Crussell/backend/handlers/bookings/admin_reserve_test.go` + +**Test to add:** +- `TestAdminReserveSlot_AutumnDST_ClosingBoundary` — closing time on autumn DST day + +**Test file 4:** `/home/popertots/Crussell/backend/handlers/admin/bookings_test.go` + +**Test to add:** +- `TestAdminBookings_CountMatchesData_AutumnDST` — count query at autumn boundary + +Each gets its own subagent delegation, implemented and verified sequentially. + +--- + +## Issue 7: Align `timeSlots.ts` buffer to 60 minutes + +**File:** `/home/popertots/Crussell/frontend/src/lib/utils/timeSlots.ts` +**Line:** ~201 + +### Step 1 — Implement fix +Simple one-line change. Can do directly or delegate. + +**Fix:** Change `Math.ceil((currentMinutes + 15) / 15) * 15` to `Math.ceil((currentMinutes + 60) / 15) * 15`. + +### Step 3 — Verify +Read the changed line. Confirm the correct line was changed. + +### Step 5 — Add backend tests (skip — frontend-only) + +--- + +## Completion Verification + +After all 7 issues are complete: +1. `cd /home/popertots/Crussell/backend && go build ./...` — must pass with zero errors +2. `cd /home/popertots/Crussell/backend && go vet ./...` — must pass with zero errors +3. `cd /home/popertots/Crussell/backend && go test ./...` — must pass (or note pre-existing failures) +4. Report on each issue: completed, any deviations, any test failures