diff --git a/backend/clock/clock_test.go b/backend/clock/clock_test.go index 5e24542..2c051bf 100644 --- a/backend/clock/clock_test.go +++ b/backend/clock/clock_test.go @@ -11,6 +11,77 @@ func TestNow_ReturnsUTC(t *testing.T) { if now.Location() != time.UTC { t.Errorf("clock.Now() returned time in %v, expected UTC", now.Location()) } + // UTC is a fixed-offset zone: the zone abbreviation and offset must both be + // the canonical UTC values so SQL TIMESTAMPTZ comparisons and duration math + // never drift. + name, off := now.Zone() + if name != "UTC" || off != 0 { + t.Errorf("clock.Now() returned zone %q offset %d, expected UTC offset 0", name, off) + } +} + +// TestLondon_Location_IsEuropeLondon proves the single London location used for +// wall-clock business decisions is the IANA Europe/London zone (which carries +// the full DST rule table for GMT<->BST). +func TestLondon_Location_IsEuropeLondon(t *testing.T) { + t.Parallel() + if got := London.String(); got != "Europe/London" { + t.Errorf("clock.London.String() = %q, expected %q", got, "Europe/London") + } +} + +// TestLondon_DSTTransitions pins the two 2026 Europe/London transitions: +// +// Spring forward (2026-03-29 01:00 GMT -> 02:00 BST): +// - 2026-03-29 00:30 UTC = 01:30 GMT, offset +0 (still winter time) +// - 2026-03-29 01:00 UTC = 02:00 BST, offset +1 (the transition instant) +// Fall back (2026-10-25 02:00 BST -> 01:00 GMT): +// - 2026-10-25 00:30 UTC = 01:30 BST, offset +1 (still summer time) +// - 2026-10-25 01:00 UTC = 01:00 GMT, offset +0 (the ambiguous hour, which +// Go resolves to the SECOND occurrence) +// +// These instants are the boundary of every London wall-clock decision the app +// makes (closing hours, default-hours effective dates, today summaries), so +// clock.London must get them exactly right. +func TestLondon_DSTTransitions(t *testing.T) { + t.Parallel() + cases := []struct { + name string + utc time.Time + wantWall string // London wall-clock, "2006-01-02 15:04" + wantZone string // "GMT" or "BST" + wantOffsetH int + }{ + {"spring before", time.Date(2026, 3, 29, 0, 30, 0, 0, time.UTC), "2026-03-29 00:30", "GMT", 0}, + {"spring transition", time.Date(2026, 3, 29, 1, 0, 0, 0, time.UTC), "2026-03-29 02:00", "BST", 1}, + {"autumn before", time.Date(2026, 10, 25, 0, 30, 0, 0, time.UTC), "2026-10-25 01:30", "BST", 1}, + {"autumn ambiguous->GMT", time.Date(2026, 10, 25, 1, 0, 0, 0, time.UTC), "2026-10-25 01:00", "GMT", 0}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + l := tc.utc.In(London) + if got := l.Format("2006-01-02 15:04"); got != tc.wantWall { + t.Errorf("%s UTC in London = %q, expected wall-clock %q", tc.utc.Format(time.RFC3339), got, tc.wantWall) + } + zone, off := l.Zone() + if zone != tc.wantZone { + t.Errorf("%s UTC in London zone = %q, expected %q", tc.utc.Format(time.RFC3339), zone, tc.wantZone) + } + if off != tc.wantOffsetH*3600 { + t.Errorf("%s UTC in London offset = %ds, expected %dh", tc.utc.Format(time.RFC3339), off, tc.wantOffsetH) + } + }) + } + + // Sanity: the two wall-clock times on the autumn transition day share the + // same 01:xx wall hour (the ambiguous hour) but at different UTC instants — + // proving Go resolves the fold to the second occurrence when converting the + // UTC instant back to London. + fold1 := time.Date(2026, 10, 25, 0, 30, 0, 0, time.UTC).In(London).Format("15:04") // 01:30 BST + fold2 := time.Date(2026, 10, 25, 1, 30, 0, 0, time.UTC).In(London).Format("15:04") // 01:30 GMT + if fold1 != "01:30" || fold2 != "01:30" { + t.Errorf("expected both sides of the autumn fold to show 01:30, got %q and %q", fold1, fold2) + } } func TestNow_IsReasonable(t *testing.T) { diff --git a/backend/db/db.go b/backend/db/db.go index 468f775..c106217 100644 --- a/backend/db/db.go +++ b/backend/db/db.go @@ -6,7 +6,10 @@ import ( "context" "fmt" "os" + "time" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" ) @@ -27,6 +30,27 @@ func Connect() error { return err } poolCfg.ConnConfig.RuntimeParams["timezone"] = "UTC" + // Scan TIMESTAMPTZ into time.Time in UTC, never the host's local timezone. + // Without this, pgx scans into time.Local, so the JSON offset in API + // responses silently depends on the deployment host's TZ (e.g. +01:00 on a + // London host, Z on a UTC Docker host) — the instant is the same but the + // documented "backend emits UTC" invariant would be violated. + // The codec is registered as a NEW *Type rather than mutating the Type + // returned by TypeForOID: every connection's type map shares the same + // *Type pointers with the package default map, so mutating .Codec on the + // shared Type would be a data race when concurrent connections establish. + poolCfg.AfterConnect = func(_ context.Context, conn *pgx.Conn) error { + tzType, ok := conn.TypeMap().TypeForOID(pgtype.TimestamptzOID) + if !ok { + return nil + } + conn.TypeMap().RegisterType(&pgtype.Type{ + Codec: &pgtype.TimestamptzCodec{ScanLocation: time.UTC}, + Name: tzType.Name, + OID: tzType.OID, + }) + return nil + } pool, err := pgxpool.NewWithConfig(context.Background(), poolCfg) if err != nil { return err diff --git a/backend/db/db_dev.go b/backend/db/db_dev.go index d32ed45..98e4ebe 100644 --- a/backend/db/db_dev.go +++ b/backend/db/db_dev.go @@ -6,7 +6,10 @@ import ( "context" "fmt" "os" + "time" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" "github.com/jackc/pgx/v5/pgxpool" ) @@ -27,6 +30,27 @@ func Connect() error { return err } poolCfg.ConnConfig.RuntimeParams["timezone"] = "UTC" + // Scan TIMESTAMPTZ into time.Time in UTC, never the host's local timezone. + // Without this, pgx scans into time.Local, so the JSON offset in API + // responses silently depends on the deployment host's TZ (e.g. +01:00 on a + // London host, Z on a UTC Docker host) — the instant is the same but the + // documented "backend emits UTC" invariant would be violated. + // The codec is registered as a NEW *Type rather than mutating the Type + // returned by TypeForOID: every connection's type map shares the same + // *Type pointers with the package default map, so mutating .Codec on the + // shared Type would be a data race when concurrent connections establish. + poolCfg.AfterConnect = func(_ context.Context, conn *pgx.Conn) error { + tzType, ok := conn.TypeMap().TypeForOID(pgtype.TimestamptzOID) + if !ok { + return nil + } + conn.TypeMap().RegisterType(&pgtype.Type{ + Codec: &pgtype.TimestamptzCodec{ScanLocation: time.UTC}, + Name: tzType.Name, + OID: tzType.OID, + }) + return nil + } pool, err := pgxpool.NewWithConfig(context.Background(), poolCfg) if err != nil { return err diff --git a/backend/db/db_test.go b/backend/db/db_test.go index 9a70270..feccc85 100644 --- a/backend/db/db_test.go +++ b/backend/db/db_test.go @@ -8,6 +8,7 @@ import ( "os" "sync" "testing" + "time" ) func resetEnv() { @@ -71,6 +72,42 @@ func TestConnect_PingViaTestDB(t *testing.T) { } } +// TestScanLocation_UTC proves the AfterConnect hook: TIMESTAMPTZ values are +// scanned into time.Time in UTC, never the host's local timezone. Without the +// hook, pgx scans into time.Local, so a London-host dev server would emit +// +01:00 JSON offsets while a UTC Docker host emits Z — same instant, but the +// documented "backend emits UTC" invariant would silently depend on the +// deployment host's TZ. +func TestScanLocation_UTC(t *testing.T) { + closePool() + resetEnv() + + err := Connect() + if err != nil { + t.Fatalf("Connect() failed: %v", err) + } + defer closePool() + + poolConn, err := Conn.Acquire(context.Background()) + if err != nil { + t.Fatalf("Acquire failed: %v", err) + } + defer poolConn.Release() + + // A known instant: 2026-06-15 00:30 BST = 2026-06-14 23:30 UTC. + instant := time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC) + var scanned time.Time + if err := poolConn.QueryRow(context.Background(), "SELECT $1::timestamptz", instant).Scan(&scanned); err != nil { + t.Fatalf("timestamptz scan failed: %v", err) + } + if !scanned.Equal(instant) { + t.Errorf("scanned instant = %s, expected %s", scanned.Format(time.RFC3339Nano), instant.Format(time.RFC3339Nano)) + } + if scanned.Location() != time.UTC { + t.Errorf("scanned time.Time location = %v, expected time.UTC (host TZ is %v)", scanned.Location(), time.Local) + } +} + // ============================================================================= // Connection failure scenarios // ============================================================================= diff --git a/backend/handlers/admin/settings.go b/backend/handlers/admin/settings.go index 1662263..83714e7 100644 --- a/backend/handlers/admin/settings.go +++ b/backend/handlers/admin/settings.go @@ -169,8 +169,12 @@ func UpdateBusinessSettings(w http.ResponseWriter, r *http.Request) { http.Error(w, "voucher_type must be 'SPV' or 'MPV'", http.StatusBadRequest) return } - if req.GiftCardExpiryMonths != nil && *req.GiftCardExpiryMonths < 1 { - http.Error(w, "gift_card_expiry_months must be at least 1", http.StatusBadRequest) + if req.GiftCardExpiryMonths != nil && *req.GiftCardExpiryMonths < 12 { + // Legal floor, not arbitrary: UK Consumer Rights Act 2015 requires + // expiry terms to be "fair and transparent", and CMA guidance flags + // sub-12-month expiry windows as at risk of being an unfair contract + // term. 24 months is the documented default (matches John Lewis, M&S). + http.Error(w, "gift_card_expiry_months must be at least 12 (CMA guidance flags sub-12-month expiry as an unfair contract term; 24 is recommended)", http.StatusBadRequest) return } if req.DefaultVATRate != nil && (*req.DefaultVATRate < 0 || *req.DefaultVATRate > 100) { diff --git a/backend/handlers/admin/settings_test.go b/backend/handlers/admin/settings_test.go index 0503f3e..abfebb8 100644 --- a/backend/handlers/admin/settings_test.go +++ b/backend/handlers/admin/settings_test.go @@ -725,6 +725,65 @@ func TestUpdateBusinessSettings_VoucherType_SPV(t *testing.T) { } } +// TestUpdateBusinessSettings_Expiry_RejectsSub12 rejects gift_card_expiry_months +// below the legal floor. The setting now actually drives gift-card expiry (the +// cleanup job + every expiry_date write), so a sub-12-month window — flagged by +// CMA guidance as an unfair contract term under the Consumer Rights Act 2015 — +// must not be settable. +func TestUpdateBusinessSettings_Expiry_RejectsSub12(t *testing.T) { + ctx, _ := testutils.SetupTestTx(t) + + handler := http.HandlerFunc(UpdateBusinessSettings) + body := UpdateBusinessSettingsRequest{ + GiftCardExpiryMonths: intPtr(6), + } + w := makeAdminRequest(handler, "PUT", "/api/admin/settings", body, ctx) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status 400 for 6-month expiry, got %d. body: %s", w.Code, w.Body.String()) + } +} + +// TestUpdateBusinessSettings_Expiry_RejectsZero rejects gift_card_expiry_months +// of 0 (previously the only invalid value, now the legal floor subsumes it). +func TestUpdateBusinessSettings_Expiry_RejectsZero(t *testing.T) { + ctx, _ := testutils.SetupTestTx(t) + + handler := http.HandlerFunc(UpdateBusinessSettings) + body := UpdateBusinessSettingsRequest{ + GiftCardExpiryMonths: intPtr(0), + } + w := makeAdminRequest(handler, "PUT", "/api/admin/settings", body, ctx) + + if w.Code != http.StatusBadRequest { + t.Errorf("expected status 400 for 0-month expiry, got %d. body: %s", w.Code, w.Body.String()) + } +} + +// TestUpdateBusinessSettings_Expiry_Accepts24 verifies a 24-month window (the +// documented default) is accepted and persisted. +func TestUpdateBusinessSettings_Expiry_Accepts24(t *testing.T) { + ctx, _ := testutils.SetupTestTx(t) + + handler := http.HandlerFunc(UpdateBusinessSettings) + body := UpdateBusinessSettingsRequest{ + GiftCardExpiryMonths: intPtr(24), + } + w := makeAdminRequest(handler, "PUT", "/api/admin/settings", body, ctx) + + if w.Code != http.StatusOK { + t.Errorf("expected status 200 for 24-month expiry, got %d. body: %s", w.Code, w.Body.String()) + } + + var s BusinessSettings + if err := parseResponseBody(w, &s); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if s.GiftCardExpiryMonths != 24 { + t.Errorf("expected GiftCardExpiryMonths 24, got %d", s.GiftCardExpiryMonths) + } +} + // TestUpdateBusinessSettings_VoucherType_MPV accepts MPV. func TestUpdateBusinessSettings_VoucherType_MPV(t *testing.T) { ctx, _ := testutils.SetupTestTx(t) @@ -764,7 +823,7 @@ func TestUpdateBusinessSettings_NegativeExpiryMonths(t *testing.T) { if w.Code != http.StatusBadRequest { t.Errorf("expected status 400, got %d. body: %s", w.Code, w.Body.String()) } - if w.Body.String() != "gift_card_expiry_months must be at least 1\n" { + if !strings.Contains(w.Body.String(), "must be at least 12") { t.Errorf("unexpected error message: %s", w.Body.String()) } } @@ -782,8 +841,8 @@ func TestUpdateBusinessSettings_ExpiryMonths_Negative(t *testing.T) { if w.Code != http.StatusBadRequest { t.Errorf("expected status 400 for negative expiry, got %d. body: %s", w.Code, w.Body.String()) } - if w.Body.String() != "gift_card_expiry_months must be at least 1\n" { - t.Errorf("unexpected error message: %s", w.Body.String()) + if !strings.Contains(w.Body.String(), "must be at least 12") { + t.Errorf("expected legal-floor error message, got: %s", w.Body.String()) } } diff --git a/backend/handlers/bookings/dst_timezone_test.go b/backend/handlers/bookings/dst_timezone_test.go new file mode 100644 index 0000000..caa6f52 --- /dev/null +++ b/backend/handlers/bookings/dst_timezone_test.go @@ -0,0 +1,173 @@ +//go:build test && dev + +package bookings + +// Strict timezone/DST/midnight-boundary tests for the booking closing-hours +// pipeline and the date-window queries. All assertions use fixed time.Date +// instants (no wall-clock "now" drift) so they are deterministic on every run. + +import ( + "net/http" + "testing" + "time" + + "crussell/clock" + "crussell/testutils" + "crussell/testutils/fixtures" + "crussell/testutils/jwt" +) + +// TestCheckClosingHours_GMTvsBST proves the closing-time pipeline is +// DST-safe: the same 16:30 wall-clock end is accepted and the same 17:30 +// wall-clock end is rejected during BOTH a GMT season (2026-01-15, offset +0) +// and a BST season (2026-06-15, offset +1). The same wall-clock instant maps +// to UTC instants that differ by exactly the 1h DST offset between seasons — +// the conversion via londonLocation must absorb that shift. +func TestCheckClosingHours_GMTvsBST(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + // 2026-01-15 is a Thursday (DB weekday 3), 2026-06-15 is a Monday (DB + // weekday 0). Pin closing at 17:00 on both so the resolution goes through + // the real getClosingTimeForDate -> working_hours path. + for _, wd := range []int{0, 3} { + _, err := tx.Exec(ctx, ` + INSERT INTO working_hours (weekday, start_time, end_time, is_open) + VALUES ($1, '08:00', '17:00', true) + ON CONFLICT (weekday) DO UPDATE SET start_time = '08:00', end_time = '17:00', is_open = true + `, wd) + if err != nil { + t.Fatalf("failed to set working hours for weekday %d: %v", wd, err) + } + } + + cases := []struct { + name string + localEnd time.Time // Europe/London wall-clock booking end + wantUTC int // expected UTC hour of that same instant + }{ + {"GMT 2026-01-15", time.Date(2026, 1, 15, 16, 30, 0, 0, clock.London), 16}, + {"BST 2026-06-15", time.Date(2026, 6, 15, 16, 30, 0, 0, clock.London), 15}, + } + + // The SAME 16:30 wall-clock end is 16:30 UTC in GMT and 15:30 UTC in BST — + // a shift of exactly 1h (the DST offset). + if diff := cases[0].localEnd.UTC().Hour() - cases[1].localEnd.UTC().Hour(); diff != 1 { + t.Fatalf("expected the two 16:30-local ends to differ by exactly 1h in UTC, got %dh", diff) + } + + for _, tc := range cases { + t.Run(tc.name+" pass 16:30", func(t *testing.T) { + weekday := int((tc.localEnd.Weekday() + 6) % 7) + closeStr, err := getClosingTimeForDate(ctx, tx, weekday, tc.localEnd) + if err != nil { + t.Fatalf("getClosingTimeForDate failed: %v", err) + } + // The DB stores end_time as TIME so ::text returns "17:00:00". + if closeStr != "17:00" && closeStr != "17:00:00" { + t.Fatalf("expected closing 17:00 for %s, got %q", tc.name, closeStr) + } + if err := checkClosingHours(tc.localEnd, closeStr); err != nil { + t.Errorf("16:30 local end on %s must pass closing 17:00, got: %v", tc.name, err) + } + if got := tc.localEnd.UTC().Hour(); got != tc.wantUTC { + t.Errorf("16:30 local on %s should be %02d:30 UTC, got hour %d", tc.name, tc.wantUTC, got) + } + }) + } + + failCases := []struct { + name string + localEnd time.Time + }{ + {"GMT 2026-01-15", time.Date(2026, 1, 15, 17, 30, 0, 0, clock.London)}, + {"BST 2026-06-15", time.Date(2026, 6, 15, 17, 30, 0, 0, clock.London)}, + } + for _, tc := range failCases { + t.Run(tc.name+" fail 17:30", func(t *testing.T) { + if err := checkClosingHours(tc.localEnd, "17:00"); !IsPastClosing(err) { + t.Errorf("17:30 local end on %s must be rejected, got: %v", tc.name, err) + } + }) + } +} + +// TestBookingDateWindow_DSTMidnight proves the booking date-window queries +// (bookings.go GetAllUserBookingsHandler start_date/end_date conversion) bucket +// bookings by their LONDON calendar date, not their UTC date. A booking at +// 2026-06-14 23:30 UTC is 2026-06-15 00:30 BST — inside the 00:00-01:00 BST +// window where the UTC date differs from the London date — and must appear in +// the "2026-06-15" window, not "2026-06-14". +func TestBookingDateWindow_DSTMidnight(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + + // Precondition: 2026-06-14 23:30 UTC must be 2026-06-15 00:30 BST (the + // UTC-date != London-date window). This is the exact property under test. + boundary := time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC) + if got := boundary.In(clock.London).Format("2006-01-02"); got != "2026-06-15" { + t.Fatalf("test setup invariant: 2026-06-14 23:30 UTC must be London date 2026-06-15, got %s", got) + } + + // Booking A: the midnight-boundary booking (00:30 BST on the 15th). + bookingA, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, boundary) + if err != nil { + t.Fatalf("failed to create boundary booking: %v", err) + } + // Booking B: 22:30 UTC on the 14th = 23:30 BST on the 14th (clearly the 14th). + bookingB, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, time.Date(2026, 6, 14, 22, 30, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create day-before booking: %v", err) + } + + token := jwt.GenerateUserToken(userID) + + // Window 2026-06-15: only the boundary booking belongs (00:30 BST on the + // 15th); the 22:30-UTC booking is 23:30 BST on the 14th. + w := makeRequest(http.HandlerFunc(GetAllUserBookingsHandler), "GET", + "/api/bookings?start_date=2026-06-15&end_date=2026-06-15", nil, token, ctx) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp BookingListResponse + if err := parseResponseBody(w, &resp); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp.Total != 1 || len(resp.Bookings) != 1 || resp.Bookings[0].ID != bookingA { + t.Errorf("window 2026-06-15: expected only booking A (%s) at 00:30 BST, got total=%d ids=%v", + bookingA, resp.Total, bookingIDs(resp.Bookings)) + } + + // Window 2026-06-14: only booking B belongs; the 23:30-UTC boundary booking + // has already rolled over to London date 2026-06-15. + w2 := makeRequest(http.HandlerFunc(GetAllUserBookingsHandler), "GET", + "/api/bookings?start_date=2026-06-14&end_date=2026-06-14", nil, token, ctx) + if w2.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w2.Code, w2.Body.String()) + } + var resp2 BookingListResponse + if err := parseResponseBody(w2, &resp2); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if resp2.Total != 1 || len(resp2.Bookings) != 1 || resp2.Bookings[0].ID != bookingB { + t.Errorf("window 2026-06-14: expected only booking B (%s), got total=%d ids=%v", + bookingB, resp2.Total, bookingIDs(resp2.Bookings)) + } +} + +func bookingIDs(bookings []Booking) []string { + ids := make([]string, len(bookings)) + for i, b := range bookings { + ids[i] = b.ID + } + return ids +} diff --git a/backend/handlers/payments/dst_timezone_test.go b/backend/handlers/payments/dst_timezone_test.go new file mode 100644 index 0000000..225a0b6 --- /dev/null +++ b/backend/handlers/payments/dst_timezone_test.go @@ -0,0 +1,124 @@ +//go:build test && dev + +package payments + +// Strict timezone/DST tests for the payment refund tiers and the deposit +// protection window. All assertions use fixed time.Date instants. + +import ( + "testing" + "time" + + "crussell/clock" +) + +// TestRefundTiers_TimezoneIndependent proves refund tiers depend only on the +// ELAPSED time between cancellation and start (startTime.Sub(cancellationTime) +// .Hours()), never on wall-clock days or the London calendar date. Two bookings +// in different seasons — 2026-01-20 10:00 UTC (GMT) and 2026-06-20 10:00 UTC +// (BST) — cancelled with identical notice must produce identical results for +// every tier. +func TestRefundTiers_TimezoneIndependent(t *testing.T) { + t.Parallel() + + gmtStart := time.Date(2026, 1, 20, 10, 0, 0, 0, time.UTC) + bstStart := time.Date(2026, 6, 20, 10, 0, 0, 0, time.UTC) + + // Precondition: the two starts are genuinely in different seasons — same + // UTC instant-of-day but different London wall-clock zones. + if gz, _ := gmtStart.In(clock.London).Zone(); gz != "GMT" { + t.Fatalf("expected 2026-01-20 in London to be GMT, got %q", gz) + } + if bz, _ := bstStart.In(clock.London).Zone(); bz != "BST" { + t.Fatalf("expected 2026-06-20 in London to be BST, got %q", bz) + } + + notices := []struct { + name string + hours float64 + want string + }{ + {"over 72h -> full refund", 72.5, FullRefundTier}, + {"at 72h -> partial (strict > boundary)", 72, PartialRefundTier}, + {"48h -> partial refund", 48, PartialRefundTier}, + {"at 24h -> partial refund", 24, PartialRefundTier}, + {"under 24h -> no refund", 23.5, NoRefundTier}, + } + + for _, tc := range notices { + noticeDur := time.Duration(tc.hours * float64(time.Hour)) + cancellationGmt := gmtStart.Add(-noticeDur) + cancellationBst := bstStart.Add(-noticeDur) + + gmtRes := CalculateRefundForCancellation(100, 50, cancellationGmt, gmtStart) + bstRes := CalculateRefundForCancellation(100, 50, cancellationBst, bstStart) + + if gmtRes.Tier != tc.want || bstRes.Tier != tc.want { + t.Errorf("%s: want tier %q, got GMT=%q BST=%q", tc.name, tc.want, gmtRes.Tier, bstRes.Tier) + } + // Elapsed-hours logic is DST-safe: identical inputs across seasons + // produce byte-for-byte identical outputs. + if gmtRes != bstRes { + t.Errorf("%s: GMT and BST bookings with identical notice produced different refunds: GMT=%+v BST=%+v", + tc.name, gmtRes, bstRes) + } + if gmtRes.HoursUntilAppointment != tc.hours { + t.Errorf("%s: expected %.1f elapsed hours, got %.1f", tc.name, tc.hours, gmtRes.HoursUntilAppointment) + } + } +} + +// TestDepositProtectionWindow_UTCInstant proves buildSplitRecords decides +// "after booking starts" by an ABSOLUTE UTC-instant comparison +// (clock.Now().After(info.StartTime)), never by the London calendar date. A +// booking at 2026-06-15 00:30 BST == 2026-06-14 23:30 UTC — where the UTC date +// and the London date disagree — is classified purely by its UTC instant. +func TestDepositProtectionWindow_UTCInstant(t *testing.T) { + t.Parallel() + + // 2026-06-14 23:30 UTC = 2026-06-15 00:30 BST: the 00:00-01:00 BST window + // where the UTC date (2026-06-14) is the day before the London date. + boundaryUTC := time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC) + if got := boundaryUTC.In(clock.London).Format("2006-01-02"); got != "2026-06-15" { + t.Fatalf("test setup invariant: 2026-06-14 23:30 UTC must be London 2026-06-15 00:30 BST, got %s", got) + } + if got := boundaryUTC.Format("2006-01-02"); got != "2026-06-14" { + t.Fatalf("test setup invariant: boundary must remain UTC date 2026-06-14, got %s", got) + } + + // This instant is in the past -> the booking has started -> no deposit + // protection window -> a single unsplit record. + record := makeTestRecord("b-boundary-past", "full", 50) + info := &BookingPaymentInfo{StartTime: boundaryUTC, TotalAmount: 50, TotalPaid: 0} + records := buildSplitRecords(record, "full", info, 50) + if len(records) != 1 { + t.Fatalf("past BST-midnight booking: expected 1 record (no split), got %d", len(records)) + } + if records[0].PaymentType != "full" { + t.Errorf("past BST-midnight booking: expected single 'full' record, got %q", records[0].PaymentType) + } + + // Mirror case with the IDENTICAL UTC-date != London-date property but in the + // future (2099-06-14 23:30 UTC = 2099-06-15 00:30 BST): the booking has NOT + // started, so the deposit split must happen. Only the UTC instant differs + // from the case above — the London date plays no role. + futureBoundary := time.Date(2099, 6, 14, 23, 30, 0, 0, time.UTC) + if got := futureBoundary.In(clock.London).Format("2006-01-02"); got != "2099-06-15" { + t.Fatalf("test setup invariant: 2099-06-14 23:30 UTC must be London 2099-06-15, got %s", got) + } + + record2 := makeTestRecord("b-boundary-future", "full", 50) + info2 := &BookingPaymentInfo{StartTime: futureBoundary, TotalAmount: 50, TotalPaid: 0} + records2 := buildSplitRecords(record2, "full", info2, 50) + if len(records2) != 2 { + t.Fatalf("future BST-midnight booking: expected 2 records (deposit split), got %d", len(records2)) + } + if records2[0].PaymentType != "deposit" || records2[0].Amount != 25 { + t.Errorf("future BST-midnight booking: expected first record deposit £25, got %q £%.2f", + records2[0].PaymentType, records2[0].Amount) + } + if records2[1].PaymentType != "balance" || records2[1].Amount != 25 { + t.Errorf("future BST-midnight booking: expected second record balance £25, got %q £%.2f", + records2[1].PaymentType, records2[1].Amount) + } +} diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index c84a6ef..04985c2 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -1,6 +1,7 @@ package payments import ( + "context" "database/sql" "encoding/json" "errors" @@ -25,6 +26,31 @@ import ( // --- Types --- +// defaultGiftCardExpiryMonths is the CMA-recommended rolling expiry window used +// whenever business_settings.gift_card_expiry_months is unset or invalid (< 1). +const defaultGiftCardExpiryMonths = 24 + +// GetGiftCardExpiryMonths returns the configured gift-card expiry window in +// months — the SINGLE source of truth for rolling expiry. It reads +// business_settings.gift_card_expiry_months and falls back to +// defaultGiftCardExpiryMonths when the settings row is missing (pgx.ErrNoRows) +// or the stored value is < 1 (a sub-1-month window would make every card +// effectively expired on creation). Exported so the scheduling package's +// CleanupExpiredGiftCards job and the payment handlers share one implementation. +func GetGiftCardExpiryMonths(ctx context.Context, q db.Querier) (int, error) { + var months int + if err := q.QueryRow(ctx, `SELECT gift_card_expiry_months FROM business_settings LIMIT 1`).Scan(&months); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return defaultGiftCardExpiryMonths, nil + } + return 0, err + } + if months < 1 { + return defaultGiftCardExpiryMonths, nil + } + return months, nil +} + type GiftCard struct { ID string `json:"id"` TotalFundsAdded float64 `json:"total_funds_added"` @@ -385,11 +411,16 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) { if purchaseVoucherType == "" { purchaseVoucherType = "SPV" } + expiryMonths, err := GetGiftCardExpiryMonths(ctx, tx) + if err != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, err) + expiryMonths = defaultGiftCardExpiryMonths + } err = tx.QueryRow(ctx, ` - INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, last_used_at, voucher_type_at_purchase) - VALUES ($1, $1, $2, $3, NOW(), $4) + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, last_used_at, expiry_date, voucher_type_at_purchase) + VALUES ($1, $1, $2, $3, NOW(), NOW() + ($5 * INTERVAL '1 month'), $4) RETURNING id, total_funds_added, amount_remaining, created_by, created_at, is_inventory, last_used_at - `, req.Amount, adminID, req.IsInventory, purchaseVoucherType).Scan( + `, req.Amount, adminID, req.IsInventory, purchaseVoucherType, expiryMonths).Scan( &gc.ID, &gc.TotalFundsAdded, &gc.AmountRemaining, @@ -501,6 +532,12 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) { return } + expiryMonths, err := GetGiftCardExpiryMonths(ctx, tx) + if err != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, err) + expiryMonths = defaultGiftCardExpiryMonths + } + txType := "topup" var notes *string if isInventory && currentTotalFunds == 0 { @@ -515,10 +552,11 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) { UPDATE gift_cards SET total_funds_added = total_funds_added + $1, amount_remaining = amount_remaining + $1, - last_used_at = NOW() + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 RETURNING id, total_funds_added, amount_remaining, created_by, created_at - `, req.Amount, cardID).Scan( + `, req.Amount, cardID, expiryMonths).Scan( &gc.ID, &gc.TotalFundsAdded, &gc.AmountRemaining, @@ -665,12 +703,21 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) { return } + // A transfer is a "use" of BOTH cards per the rolling-expiry terms — each + // card's timer resets at the same moment its balance moves. + expiryMonths, err := GetGiftCardExpiryMonths(ctx, tx) + if err != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, err) + expiryMonths = defaultGiftCardExpiryMonths + } + _, err = tx.Exec(ctx, ` UPDATE gift_cards SET amount_remaining = amount_remaining - $1, - last_used_at = NOW() + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 - `, req.Amount, fromCardID) + `, req.Amount, fromCardID, expiryMonths) if err != nil { log.Printf("Failed to deduct from source: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -681,9 +728,10 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) { UPDATE gift_cards SET amount_remaining = amount_remaining + $1, total_funds_added = total_funds_added + $1, - last_used_at = NOW() + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 - `, req.Amount, req.ToCardID) + `, req.Amount, req.ToCardID, expiryMonths) if err != nil { log.Printf("Failed to add to destination: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -764,13 +812,21 @@ func RedeemGiftCard(w http.ResponseWriter, r *http.Request) { return } + expiryMonths, err := GetGiftCardExpiryMonths(ctx, tx) + if err != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, err) + expiryMonths = defaultGiftCardExpiryMonths + } + _, err = tx.Exec(ctx, ` UPDATE gift_cards SET amount_remaining = 0, redeemed_at = NOW(), - redeemed_by = $1 + redeemed_by = $1, + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 - `, userID, code) + `, userID, code, expiryMonths) if err != nil { log.Printf("Failed to update gift card: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -1165,6 +1221,12 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { var cardID string + expiryMonths, err := GetGiftCardExpiryMonths(ctx, issueTx) + if err != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, err) + expiryMonths = defaultGiftCardExpiryMonths + } + if req.RecipientType == "self" { var purchaseVoucherType string err = issueTx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) @@ -1177,10 +1239,10 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { purchaseVoucherType = "SPV" } err = issueTx.QueryRow(ctx, ` - INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_at, redeemed_by, is_inventory, voucher_type_at_purchase) - VALUES ($1, 0, $2, NOW(), $2, FALSE, $3) + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, redeemed_at, redeemed_by, is_inventory, last_used_at, expiry_date, voucher_type_at_purchase) + VALUES ($1, 0, $2, NOW(), $2, FALSE, NOW(), NOW() + ($4 * INTERVAL '1 month'), $3) RETURNING id - `, amountPounds, userID, purchaseVoucherType).Scan(&cardID) + `, amountPounds, userID, purchaseVoucherType, expiryMonths).Scan(&cardID) if err != nil { log.Printf("CRITICAL: Square payment succeeded (ID=%s) but gift card creation failed: %v — manual reconciliation required", paymentResult.SquarePayID, err) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -1221,10 +1283,10 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { purchaseVoucherType = "SPV" } err = issueTx.QueryRow(ctx, ` - INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase) - VALUES ($1, $1, $2, FALSE, $3) + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, last_used_at, expiry_date, voucher_type_at_purchase) + VALUES ($1, $1, $2, FALSE, NOW(), NOW() + ($4 * INTERVAL '1 month'), $3) RETURNING id - `, amountPounds, userID, purchaseVoucherType).Scan(&cardID) + `, amountPounds, userID, purchaseVoucherType, expiryMonths).Scan(&cardID) if err != nil { log.Printf("CRITICAL: Square payment succeeded (ID=%s) but gift card creation failed: %v — manual reconciliation required", paymentResult.SquarePayID, err) http.Error(w, "internal server error", http.StatusInternalServerError) diff --git a/backend/handlers/payments/giftcards_test.go b/backend/handlers/payments/giftcards_test.go index 1457d48..7d2eef7 100644 --- a/backend/handlers/payments/giftcards_test.go +++ b/backend/handlers/payments/giftcards_test.go @@ -1264,9 +1264,12 @@ func TestBuyGiftCard_RetryPending_ReattemptsCharge(t *testing.T) { } } -// TestAdminCreateGiftCard_ExpiryDateIsNull verifies that gift cards created via -// CreateGiftCard no longer have expiry_date set (rolling 24-month expiry via last_used_at). -func TestAdminCreateGiftCard_ExpiryDateIsNull(t *testing.T) { +// TestAdminCreateGiftCard_SetsRollingExpiry verifies that gift cards created via +// CreateGiftCard get an expiry_date of last_used_at + gift_card_expiry_months +// (the configured rolling-expiry window, default 24 months). The test DB seeds +// business_settings.gift_card_expiry_months = 24, so the expiry must be ~24 +// months in the future. +func TestAdminCreateGiftCard_SetsRollingExpiry(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) @@ -1299,14 +1302,25 @@ func TestAdminCreateGiftCard_ExpiryDateIsNull(t *testing.T) { t.Fatalf("failed to decode response: %v", err) } - // Verify expiry_date is NULL in the database + // Verify expiry_date IS set to last_used_at + the configured window (24mo). var expiryDate *time.Time err = tx.QueryRow(ctx, `SELECT expiry_date FROM gift_cards WHERE id = $1`, gc.ID).Scan(&expiryDate) if err != nil { t.Fatalf("failed to query gift card expiry_date: %v", err) } - if expiryDate != nil { - t.Error("expected expiry_date to be NULL for rolling-expiry gift cards") + if expiryDate == nil { + t.Fatal("expected expiry_date to be set for rolling-expiry gift cards") + } + // The test DB seeds gift_card_expiry_months = 24; the SQL computes + // NOW() + (24 * INTERVAL '1 month') = exactly +24 calendar months, so + // compare against AddDate(0, 24, 0) with slack for clock skew. + now := time.Now().UTC() + want := now.AddDate(0, 24, 0) + if expiryDate.Before(want.Add(-24 * time.Hour)) { + t.Errorf("expected expiry_date ~24 calendar months in the future, got %v (now %v)", expiryDate, now) + } + if expiryDate.After(want.Add(24 * time.Hour)) { + t.Errorf("expected expiry_date ~24 calendar months in the future, got %v (now %v)", expiryDate, now) } } diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index ba0140e..cbb7a21 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -423,8 +423,14 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) { cardVoucherType = "SPV" } - // Deduct directly from card remaining amount - _, err = tx.Exec(r.Context(), "UPDATE gift_cards SET amount_remaining = amount_remaining - $1, last_used_at = NOW() WHERE id = $2", amountPounds, cleanCardID) + // Deduct directly from card remaining amount. A payment is a + // "use" per the rolling-expiry terms — reset the timer. + gcExpiryMonths, expiryErr := GetGiftCardExpiryMonths(r.Context(), tx) + if expiryErr != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, expiryErr) + gcExpiryMonths = defaultGiftCardExpiryMonths + } + _, err = tx.Exec(r.Context(), "UPDATE gift_cards SET amount_remaining = amount_remaining - $1, last_used_at = NOW(), expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2", amountPounds, cleanCardID, gcExpiryMonths) if err != nil { log.Printf("Failed to deduct gift card amount: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) diff --git a/backend/handlers/payments/refunds.go b/backend/handlers/payments/refunds.go index 311b145..9e0211c 100644 --- a/backend/handlers/payments/refunds.go +++ b/backend/handlers/payments/refunds.go @@ -283,6 +283,11 @@ func ProcessCancellationRefundTx( log.Printf("Giftcard payment %s has no gift_card_id — cannot refund to card. Skipping.", paymentID) break } + // expiry_date is maintained by EVERY gift-card write that counts as + // a "use" (balance check, top-up, transfer, redeem, payment, refund + // credit), so a non-NULL expiry_date means the rolling timer is + // authoritative. NULL semantics stay fail-open: an unset expiry_date + // cannot prove the card is expired, so the refund proceeds. var expired bool if err := tx.QueryRow(ctx, ` SELECT expiry_date IS NOT NULL AND expiry_date < NOW() @@ -293,10 +298,17 @@ func ProcessCancellationRefundTx( log.Printf("Gift card %s has expired — money retained by salon, no refund due for booking %s", *giftCardID, bookingID) break } + // Refunding to the card is a "use" per the rolling-expiry terms — + // reset the timer at the same moment the balance is credited. + gcExpiryMonths, expiryErr := GetGiftCardExpiryMonths(ctx, tx) + if expiryErr != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, expiryErr) + gcExpiryMonths = defaultGiftCardExpiryMonths + } if _, err := tx.Exec(ctx, ` - UPDATE gift_cards SET amount_remaining = amount_remaining + $1, last_used_at = NOW() + UPDATE gift_cards SET amount_remaining = amount_remaining + $1, last_used_at = NOW(), expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 - `, refundThisPayment, *giftCardID); err != nil { + `, refundThisPayment, *giftCardID, gcExpiryMonths); err != nil { log.Printf("Failed to refund £%.2f to gift card %s: %v", refundThisPayment, *giftCardID, err) break } diff --git a/backend/handlers/payments/refunds_test.go b/backend/handlers/payments/refunds_test.go index bda9ceb..d84758a 100644 --- a/backend/handlers/payments/refunds_test.go +++ b/backend/handlers/payments/refunds_test.go @@ -455,6 +455,84 @@ func TestProcessCancellationRefund_GiftCardCreditsUserBalance(t *testing.T) { } } +// TestProcessCancellationRefund_ExpiredGiftCard_Retained verifies the +// refunds.go expiry guard: when the payment was made with an EXPIRED gift card +// (expiry_date in the past), the cancellation refund must NOT credit the card — +// the money is retained by the salon. Regression for the previously-dead +// `expiry_date IS NOT NULL AND expiry_date < NOW()` check, which never fired +// because no write path populated expiry_date. +func TestProcessCancellationRefund_ExpiredGiftCard_Retained(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + serviceID, err := fixtures.CreateTestService(tx) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, + time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + _, err = tx.Exec(ctx, "UPDATE bookings SET status = 'confirmed' WHERE id = $1", bookingID) + if err != nil { + t.Fatalf("failed to confirm booking: %v", err) + } + + // Expired card: balance 40, expiry_date 1 day in the past, last used 25mo ago. + var giftCardID string + if err := tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, expiry_date, last_used_at) + VALUES (100, 40, $1, false, NOW() - INTERVAL '1 day', NOW() - INTERVAL '25 months') + RETURNING id + `, userID).Scan(&giftCardID); err != nil { + t.Fatalf("failed to create expired gift card: %v", err) + } + + // Payment made WITH the expired card (this can happen for legacy cards that + // were still spendable before the expiry_date write paths were wired up). + _, err = tx.Exec(ctx, ` + INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, gift_card_id, created_at, updated_at) + VALUES ($1, 'full', 'giftcard', 'completed', 60, $2, NOW(), NOW()) + `, bookingID, giftCardID) + if err != nil { + t.Fatalf("failed to create giftcard payment: %v", err) + } + + farFuture := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC) + _, err = ProcessCancellationRefund( + ctx, bookingID, 100, 60, + farFuture, clock.Now(), "client_cancelled", &userID, + ) + if err != nil { + t.Fatalf("ProcessCancellationRefund failed: %v", err) + } + + // The expired-card guard must retain the money: balance unchanged at 40 + // (NOT credited +60), and no refund-to-card transaction recorded. + var amountRemaining float64 + if err := tx.QueryRow(ctx, + "SELECT amount_remaining FROM gift_cards WHERE id = $1", giftCardID).Scan(&amountRemaining); err != nil { + t.Fatalf("failed to query gift card balance: %v", err) + } + if amountRemaining != 40 { + t.Errorf("expired card must NOT be credited: expected amount_remaining 40, got %.2f", amountRemaining) + } + + var txCount int + if err := tx.QueryRow(ctx, + "SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND transaction_type = 'refund'", giftCardID).Scan(&txCount); err != nil { + t.Fatalf("failed to query gift card transactions: %v", err) + } + if txCount != 0 { + t.Errorf("expected NO refund-to-expired-card transaction, got %d", txCount) + } +} + func TestProcessCancellationRefund_CashCreditsUserBalance(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 5a99659..67c5abf 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -411,6 +411,11 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { if existingPendingID != "" { giftCardID = existingPendingGiftCard } else { + expiryMonths, expiryErr := GetGiftCardExpiryMonths(ctx, tx) + if expiryErr != nil { + log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, expiryErr) + expiryMonths = defaultGiftCardExpiryMonths + } if req.Action == "create" { var purchaseVoucherType string err = tx.QueryRow(ctx, `SELECT COALESCE(voucher_type, 'SPV') FROM business_settings LIMIT 1`).Scan(&purchaseVoucherType) @@ -423,10 +428,10 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { purchaseVoucherType = "SPV" } err = tx.QueryRow(ctx, ` - INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase) - VALUES ($1, $1, $2, FALSE, $3) + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, last_used_at, expiry_date, voucher_type_at_purchase) + VALUES ($1, $1, $2, FALSE, NOW(), NOW() + ($4 * INTERVAL '1 month'), $3) RETURNING id - `, req.Amount, adminID, purchaseVoucherType).Scan(&giftCardID) + `, req.Amount, adminID, purchaseVoucherType, expiryMonths).Scan(&giftCardID) if err != nil { log.Printf("Failed to create gift card: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -454,9 +459,10 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { SET amount_remaining = 0, redeemed_at = NOW(), redeemed_by = $1, - last_used_at = NOW() + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 - `, *req.RedeemToUserID, giftCardID) + `, *req.RedeemToUserID, giftCardID, expiryMonths) if err != nil { log.Printf("Failed to redeem gift card to user account: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) @@ -507,9 +513,10 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { UPDATE gift_cards SET total_funds_added = total_funds_added + $1, amount_remaining = amount_remaining + $1, - last_used_at = NOW() + last_used_at = NOW(), + expiry_date = NOW() + ($3 * INTERVAL '1 month') WHERE id = $2 - `, req.Amount, cardID) + `, req.Amount, cardID, expiryMonths) if err != nil { log.Printf("Failed to top up gift card: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) diff --git a/backend/handlers/scheduling/dst_timezone_test.go b/backend/handlers/scheduling/dst_timezone_test.go new file mode 100644 index 0000000..4711bcc --- /dev/null +++ b/backend/handlers/scheduling/dst_timezone_test.go @@ -0,0 +1,56 @@ +//go:build test + +package scheduling + +// Strict timezone/DST/midnight-boundary tests for the scheduling package's +// London-date derivation. All assertions use fixed time.Date instants. + +import ( + "testing" + "time" + + "crussell/clock" +) + +// TestScheduling_DefaultHours_DST_Midnight proves ApplyScheduledDefaultHours's +// "today" date (derived via LondonDateString) is the LONDON calendar date, not +// the UTC date. During BST, London dates begin at 23:00 UTC the previous day: +// both 2026-06-14 23:30 UTC (= 2026-06-15 00:30 BST) and 2026-06-15 00:30 UTC +// (= 2026-06-15 01:30 BST) fall on London date 2026-06-15 — a naive UTC date +// would report 2026-06-14 for the first instant and misdate the effective day +// of a staged default-hours change. +func TestScheduling_DefaultHours_DST_Midnight(t *testing.T) { + t.Parallel() + cases := []struct { + name string + utc time.Time + want string + }{ + {"00:30 BST window", time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC), "2026-06-15"}, + {"01:30 BST same day", time.Date(2026, 6, 15, 0, 30, 0, 0, time.UTC), "2026-06-15"}, + {"GMT midnight is UTC midnight", time.Date(2026, 1, 14, 23, 30, 0, 0, time.UTC), "2026-01-14"}, + {"GMT next day", time.Date(2026, 1, 15, 0, 30, 0, 0, time.UTC), "2026-01-15"}, + {"autumn BST->GMT transition day", time.Date(2026, 10, 25, 0, 30, 0, 0, time.UTC), "2026-10-25"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := LondonDateString(tc.utc); got != tc.want { + t.Errorf("LondonDateString(%s UTC) = %q, expected London date %q", tc.utc.Format(time.RFC3339), got, tc.want) + } + }) + } + + // Direct proof of the DST window: the UTC date of the first instant differs + // from its London date — exactly the drift the helper must absorb. + boundary := time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC) + if boundary.Format("2006-01-02") == LondonDateString(boundary) { + t.Fatal("test setup invariant: 2026-06-14 23:30 UTC must straddle the London midnight boundary") + } + + // The helper must be consistent with clock.London's own wall-clock for the + // same instant (the production seam ApplyScheduledDefaultHours now uses). + nowish := time.Date(2026, 6, 14, 23, 45, 0, 0, time.UTC) + if LondonDateString(nowish) != nowish.In(clock.London).Format("2006-01-02") { + t.Errorf("LondonDateString disagrees with the direct London conversion for %s", nowish.Format(time.RFC3339)) + } +} diff --git a/backend/handlers/scheduling/scheduled-cleanup.go b/backend/handlers/scheduling/scheduled-cleanup.go index 51faa46..f5bc51a 100644 --- a/backend/handlers/scheduling/scheduled-cleanup.go +++ b/backend/handlers/scheduling/scheduled-cleanup.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log/slog" + "time" "crussell/clock" "crussell/db" @@ -256,12 +257,20 @@ func CleanupExpiredRefreshTokens(ctx context.Context) (int, error) { return int(result.RowsAffected()), nil } +// LondonDateString returns the Europe/London calendar date (YYYY-MM-DD) for +// the given instant. ApplyScheduledDefaultHours uses this so staged +// default-hours changes roll over at London midnight rather than UTC midnight: +// during BST a London date begins at 23:00 UTC the previous day, so using the +// UTC date would misdate the effective day inside the 00:00-01:00 BST window. +func LondonDateString(t time.Time) string { + return t.In(clock.London).Format("2006-01-02") +} + // ApplyScheduledDefaultHours applies any pending default hours changes that // have reached their effective_date. Runs daily at 00:05 to catch midnight // roll-overs even if the cron is slightly delayed. func ApplyScheduledDefaultHours(ctx context.Context) (int, error) { - londonNow := clock.Now().In(clock.London) - todayStr := londonNow.Format("2006-01-02") + todayStr := LondonDateString(clock.Now()) tx, err := db.Conn.Begin(ctx) if err != nil { diff --git a/backend/handlers/scheduling/time-blockers.go b/backend/handlers/scheduling/time-blockers.go index 33e3247..65672b9 100644 --- a/backend/handlers/scheduling/time-blockers.go +++ b/backend/handlers/scheduling/time-blockers.go @@ -907,15 +907,21 @@ func CleanupExpiredDeposits(ctx context.Context) (int, error) { return n, tx.Commit(ctx) } -// CleanupExpiredGiftCards expires gift cards unused for 24 months (rolling expiry). +// CleanupExpiredGiftCards expires gift cards unused for the configured rolling +// window (default 24 months) after last use. // // Legal basis: // - UK Consumer Rights Act 2015: Expiry terms must be "fair and transparent" // - CMA guidance: 24 months is industry standard (John Lewis, M&S, Sainsbury's) // - Under 12 months risks being challenged as unfair contract term // +// The window is read from business_settings.gift_card_expiry_months (the SINGLE +// source of truth shared with the payment handlers' expiry_date writes via +// payments.GetGiftCardExpiryMonths) so the job and the refund-time check never +// drift apart. +// // This function: -// 1. Finds unredeemed cards (redeemed_by IS NULL) unused for 24+ months +// 1. Finds unredeemed cards (redeemed_by IS NULL) unused for the window // 2. Inserts into gift_card_expired_balances for recovery claims // 3. Sets amount_remaining to 0 // 4. Records transaction in gift_card_transactions @@ -936,14 +942,18 @@ func CleanupExpiredGiftCards(ctx context.Context) (int, error) { slog.Error("failed to rollback transaction", "err", err) } }() + expiryMonths, monthsErr := payments.GetGiftCardExpiryMonths(ctx, tx) + if monthsErr != nil { + return 0, fmt.Errorf("failed to read gift card expiry months: %w", monthsErr) + } rows, err := tx.Query(ctx, ` SELECT id, amount_remaining FROM gift_cards WHERE redeemed_by IS NULL AND amount_remaining > 0 - AND last_used_at < NOW() - INTERVAL '24 months' - `) + AND last_used_at < NOW() - ($1 * INTERVAL '1 month') + `, expiryMonths) if err != nil { return 0, fmt.Errorf("failed to query expired gift cards: %w", err) } diff --git a/backend/handlers/today/dst_timezone_test.go b/backend/handlers/today/dst_timezone_test.go new file mode 100644 index 0000000..691d053 --- /dev/null +++ b/backend/handlers/today/dst_timezone_test.go @@ -0,0 +1,151 @@ +//go:build test + +package today + +// Strict timezone tests for today.go's `AT TIME ZONE 'Europe/London'` date +// math. All assertions use fixed time.Date instants. + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "crussell/clock" + "crussell/db" + "crussell/testutils" + "crussell/testutils/fixtures" +) + +// TestToday_LondonDateWindow proves the SQL used by findNewBookingServices +// (today.go) computes the London calendar date from a UTC instant. At +// 2026-06-14 23:30 UTC (= 2026-06-15 00:30 BST) the London date is 2026-06-15 +// even though the UTC date is 2026-06-14 — the 00:00-01:00 BST window where a +// naive UTC date would start the 14-day "last working day" scan a day early. +func TestToday_LondonDateWindow(t *testing.T) { + t.Parallel() + ctx, _ := testutils.SetupTestTx(t) + + londonDate := func(utc time.Time) string { + t.Helper() + var date string + err := db.Conn.QueryRow(ctx, ` + SELECT (($1::timestamptz AT TIME ZONE 'Europe/London')::date)::text + `, utc).Scan(&date) + if err != nil { + t.Fatalf("London-date query failed for %s: %v", utc.Format(time.RFC3339), err) + } + return date + } + + cases := []struct { + name string + utc time.Time + want string + }{ + {"00:30 BST window", time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC), "2026-06-15"}, + {"01:30 BST same London day", time.Date(2026, 6, 15, 0, 30, 0, 0, time.UTC), "2026-06-15"}, + {"GMT winter day", time.Date(2026, 1, 15, 16, 0, 0, 0, time.UTC), "2026-01-15"}, + {"spring-forward day", time.Date(2026, 3, 29, 12, 0, 0, 0, time.UTC), "2026-03-29"}, + {"autumn-transition day", time.Date(2026, 10, 25, 12, 0, 0, 0, time.UTC), "2026-10-25"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := londonDate(tc.utc); got != tc.want { + t.Errorf("London date of %s UTC = %s, expected %s", tc.utc.Format(time.RFC3339), got, tc.want) + } + }) + } +} + +// TestToday_UTCUnchanged_Boundary pins that the same boundary instant is +// stored as a TIMESTAMPTZ unchanged (UTC), so the Go-side clock.Now() and the +// SQL-side NOW()/TIMESTAMPTZ never disagree about the instant itself. +func TestToday_UTCUnchanged_Boundary(t *testing.T) { + t.Parallel() + ctx, _ := testutils.SetupTestTx(t) + + boundary := time.Date(2026, 6, 14, 23, 30, 0, 0, time.UTC) + var roundTrip time.Time + err := db.Conn.QueryRow(ctx, `SELECT $1::timestamptz`, boundary).Scan(&roundTrip) + if err != nil { + t.Fatalf("timestamptz round-trip failed: %v", err) + } + if !roundTrip.Equal(boundary) { + t.Errorf("timestamptz round-trip altered the instant: got %s, want %s", roundTrip.Format(time.RFC3339Nano), boundary.Format(time.RFC3339Nano)) + } + // The London date derived from the round-tripped value must still be the + // BST-day date — pgx may decode with a nil Go location, so only the instant + // and its London wall-clock date matter. + zone, off := roundTrip.In(londonLocation).Zone() + if zone != "BST" || off != 3600 { + t.Errorf("round-tripped instant in London = zone %q offset %ds, expected BST +3600", zone, off) + } + if got := roundTrip.In(londonLocation).Format("2006-01-02"); got != "2026-06-15" { + t.Errorf("round-tripped instant maps to London date %s, expected 2026-06-15", got) + } +} + +// TestToday_SummaryDateLabels_LondonBusinessDay is a regression test for the +// QA-flagged DST bug: the done-for-the-day summary header showed the PREVIOUS +// day during BST. todayStart is London midnight converted to UTC (e.g. +// 2026-08-04 00:00 BST = 2026-08-03 23:00 UTC), and formatting that UTC +// instant directly with "2006-01-02" yielded "2026-08-03" for the Aug 4 +// business day. The summary labels must use the London-located instant so the +// header shows the business day (and the same date as the UTC-derived London +// calendar day the rest of the pipeline uses). +func TestToday_SummaryDateLabels_LondonBusinessDay(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + svcID := createTodayService(t, ctx, tx) + + // Seed a completed booking TODAY (London) so the handler is in the + // "done for the day" branch with a summary. + now := clock.Now() + var bookingID string + err = tx.QueryRow(ctx, ` + INSERT INTO bookings (user_id, start_time, status) + VALUES ($1, $2, 'completed') + RETURNING id + `, userID, now).Scan(&bookingID) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + addBookingService(t, ctx, tx, bookingID, svcID) + + req := httptest.NewRequest(http.MethodGet, "/api/admin/today/current-next", nil) + req = req.WithContext(ctx) + rr := httptest.NewRecorder() + GetCurrentAndNextHandler(rr, req) + + if rr.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", rr.Code, rr.Body.String()) + } + + var resp CurrentNextResponse + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + if resp.Summary == nil { + t.Fatal("expected a summary (done-for-the-day branch)") + } + + londonToday := now.In(londonLocation).Format("2006-01-02") + if resp.Summary.SummaryStartDate != londonToday { + t.Errorf("summary_start_date = %s, expected %s (the London business day — the pre-fix code emitted the previous UTC day during BST)", + resp.Summary.SummaryStartDate, londonToday) + } + // End date is the London day AFTER todayStart (todayEnd = todayStart + 24h, + // which in London wall-clock is the NEXT day). + londonTomorrow := now.In(londonLocation).AddDate(0, 0, 1).Format("2006-01-02") + if resp.Summary.SummaryEndDate != londonTomorrow { + t.Errorf("summary_end_date = %s, expected %s (the London day after the business day)", + resp.Summary.SummaryEndDate, londonTomorrow) + } +} diff --git a/backend/handlers/today/today.go b/backend/handlers/today/today.go index 07cf8c0..9795021 100644 --- a/backend/handlers/today/today.go +++ b/backend/handlers/today/today.go @@ -228,11 +228,15 @@ func GetCurrentAndNextHandler(w http.ResponseWriter, r *http.Request) { response.DoneForDay = &done if todayOpen { - // Regular done-for-the-day: show daily summary + // Regular done-for-the-day: show daily summary. Format the dates in + // Europe/London: todayStart is London midnight converted to UTC (e.g. + // 2026-08-04 00:00 BST = 2026-08-03 23:00 UTC), so formatting the UTC + // instant directly yields the PREVIOUS day during BST. Use the + // London-located instant so the summary header shows the business day. summary := computeAggregateSummary(r, todayStart, todayEnd) summary.SummaryScope = "day" - summary.SummaryStartDate = todayStart.Format("2006-01-02") - summary.SummaryEndDate = todayEnd.Format("2006-01-02") + summary.SummaryStartDate = todayStart.In(londonLocation).Format("2006-01-02") + summary.SummaryEndDate = todayEnd.In(londonLocation).Format("2006-01-02") response.Summary = summary // If tomorrow is closed, also compute a week summary @@ -241,8 +245,8 @@ func GetCurrentAndNextHandler(w http.ResponseWriter, r *http.Request) { weekStart, _ := findWeekSummaryRange(r, tomorrow) ws := computeAggregateSummary(r, weekStart, todayEnd) ws.SummaryScope = "week" - ws.SummaryStartDate = weekStart.Format("2006-01-02") - ws.SummaryEndDate = todayStart.Format("2006-01-02") + ws.SummaryStartDate = weekStart.In(londonLocation).Format("2006-01-02") + ws.SummaryEndDate = todayStart.In(londonLocation).Format("2006-01-02") response.WeekSummary = ws } } else { @@ -250,8 +254,8 @@ func GetCurrentAndNextHandler(w http.ResponseWriter, r *http.Request) { weekStart, _ := findWeekSummaryRange(r, londonNow) summary := computeAggregateSummary(r, weekStart, todayEnd) summary.SummaryScope = "week" - summary.SummaryStartDate = weekStart.Format("2006-01-02") - summary.SummaryEndDate = todayStart.Format("2006-01-02") + summary.SummaryStartDate = weekStart.In(londonLocation).Format("2006-01-02") + summary.SummaryEndDate = todayStart.In(londonLocation).Format("2006-01-02") response.Summary = summary } } diff --git a/backend/testutils/testdb/testdb.go b/backend/testutils/testdb/testdb.go index ac2ca45..0993d4e 100644 --- a/backend/testutils/testdb/testdb.go +++ b/backend/testutils/testdb/testdb.go @@ -251,7 +251,7 @@ func SeedBaseline(pool *pgxpool.Pool) { // Business settings: required by admin/settings tests and booking deposit logic. _, err := pool.Exec(ctx, ` INSERT INTO business_settings (business_name, business_address, currency_code, gift_card_expiry_months, voucher_type) - VALUES ('Test Salon', '123 Test St', 'GBP', 12, 'SPV') + VALUES ('Test Salon', '123 Test St', 'GBP', 24, 'SPV') ON CONFLICT DO NOTHING `) if err != nil { @@ -292,7 +292,7 @@ func SeedBaselineScheduling(pool *pgxpool.Pool) { // Business settings same as baseline. _, err := pool.Exec(ctx, ` INSERT INTO business_settings (business_name, business_address, currency_code, gift_card_expiry_months, voucher_type) - VALUES ('Test Salon', '123 Test St', 'GBP', 12, 'SPV') + VALUES ('Test Salon', '123 Test St', 'GBP', 24, 'SPV') ON CONFLICT DO NOTHING `) if err != nil { diff --git a/frontend/src/lib/components/account/EditRequestModal.svelte b/frontend/src/lib/components/account/EditRequestModal.svelte index c139412..051ed48 100644 --- a/frontend/src/lib/components/account/EditRequestModal.svelte +++ b/frontend/src/lib/components/account/EditRequestModal.svelte @@ -1,6 +1,6 @@