Files
Crussell/.sisyphus/plans/45-loyalty-discount-system-v2.md
T
2026-06-04 21:12:21 +01:00

24 KiB
Raw Blame History

Plan: #45 Loyalty & Discount System v2

Supersedes: .sisyphus/plans/45-loyalty-discount-system.md (v1)

Status

Phase: Planning — no implementation yet.


1. Stamp Earning — Clarification & Fixes

Current Behaviour (correct, no change needed)

The stamp earning logic in ProgressBookingHandler (bookings.go lines 2035-2063) is already correct:

  • Every completed booking with bookingTotal > 0 earns 1 stamp — including the very first booking
  • Max 1 stamp per calendar day — the NOT EXISTS subquery checks for other completed bookings with updated_at >= CURRENT_DATE - INTERVAL '1 day'
  • Free bookings (£0.00 total) earn no stamp — the if bookingTotal > 0 gate
  • At exactly 10 stamps → auto-create loyalty_redemptions row with status = 'pending'
  • On next paid completion → apply 10% discount, deduct 10 stamps (via GREATEST(0, stamps - 10)), then earn +1 for this completion → net stamps = 1

The "zero-total" note explained

"Zero-total bookings earn no stamps" refers to bookings where all services are £0.00 (free services, complimentary treatments). It does NOT mean "first N bookings don't earn stamps." A customer's very first paid booking earns stamp #1.

Flow summary

Booking 1 (£50)  → stamp 1
Booking 2 (£50)  → stamp 2  (if different day)
...
Booking 10 (£50) → stamp 10 → pending redemption created
Booking 11 (£50) → apply 10% discount (£5 off) → stamps reset to 0 → earn +1 → stamps = 1
Booking 12 (£50) → stamp 2
...
Booking 21 (£50) → stamp 10 → pending redemption created
Booking 22 (£50) → apply 10% discount → stamps = 1
...

No code changes needed for stamp earning

The existing logic is correct. Tests TestDiscount_Loyalty_FullCycle, TestDiscount_Loyalty_OneStampPerDay, TestDiscount_Loyalty_ZeroTotalNoStamp, and TestDiscount_Loyalty_CycleRepeats all verify this.


2. Discount Stacking — Redesign

Problem

The current system uses a strict one-discount-per-booking priority cascade:

Loyalty → blocks everything
Time-based campaign → blocks milestones
Per-user milestone → blocks global and anniversary
Global milestone → blocks anniversary

This means a customer with a full loyalty card AND an active "10% off" campaign AND a "100th booking" milestone only gets the loyalty discount. The campaign and milestone are wasted.

Desired Behaviour

All applicable discounts stack additively (not compound):

  • 10% loyalty + 10% time-based campaign = 20% off
  • 10% loyalty + 10% per-user milestone + 5% anniversary = 25% off
  • 10% loyalty + 15% time-based + 10% per-user + 5% anniversary + 20% global = 60% off

Each discount is calculated against the original booking total (not the post-discount total). Each creates its own booking_discounts row and its own payments row with payment_method = 'discount'.

Stacking Rules

Discount Source Stacking Selection
Loyalty (10%) Always stacks At most 1 per booking (from pending redemption)
Time-based campaign Stacks with loyalty and milestones Pick single best (highest %) if multiple active
Per-user milestone Stacks with everything At most 1 per user per campaign (dedup via booking_discounts)
Global milestone Stacks with everything Respects max_redemptions
Anniversary milestone Stacks with everything At most 1 per user per campaign (dedup via booking_discounts)

Multiple milestones CAN stack on the same booking. If a user hits their 100th booking (per-user milestone) on the same day as their 1-year anniversary, both apply.

Multiple time-based campaigns do NOT stack. Pick the single highest % active campaign.

Implementation Changes

File: backend/handlers/bookings/bookings.goProgressBookingHandler (lines 2004-2205)

Step 1: Loyalty Redemption (lines 2004-2032) — NO CHANGE

Keep as-is. Applies 10% if pending redemption exists. Creates booking_discounts + payments rows.

Step 2: Time-Based Campaign (lines 2065-2096) — REMOVE GUARD

Remove the hasLoyaltyDiscount check (lines 2066-2069). The time-based campaign block should execute regardless of whether loyalty was applied.

// BEFORE (line 2066-2069):
var hasLoyaltyDiscount bool
_ = db.DB.QueryRow(r.Context(), `SELECT EXISTS(SELECT 1 FROM booking_discounts WHERE booking_id = $1 AND discount_source = 'loyalty')`, bookingID).Scan(&hasLoyaltyDiscount)
if !hasLoyaltyDiscount { ... }

// AFTER:
// Remove the hasLoyaltyDiscount check entirely. Always check for time-based campaigns.

Step 3: Milestone Campaigns (lines 2098-2205) — REMOVE GUARDS

Remove the hasDiscount check (lines 2099-2102). Milestone checks should execute regardless of prior discounts.

Remove the milestoneCampaignID == "" guards (lines 2130, 2158). Global and anniversary milestones should be checked independently of per-user milestones.

Scoping note: The variables userBookingCount and milestoneCampaignID are currently declared inside the if !hasDiscount {} block. When that outer block is removed, these declarations simply become top-level within the if bookingTotal > 0 {} block. No restructuring needed. However, milestoneCampaignID will no longer be used as a guard — it becomes dead code after the per-user check and can be removed or left as-is (it's assigned but never read again).

// BEFORE (line 2099-2102):
var hasDiscount bool
_ = db.DB.QueryRow(r.Context(), `SELECT EXISTS(SELECT 1 FROM booking_discounts WHERE booking_id = $1)`, bookingID).Scan(&hasDiscount)
if !hasDiscount { ... }

// AFTER:
// Remove the hasDiscount check entirely. Always check all milestone types.
// BEFORE (line 2130):
if milestoneCampaignID == "" { /* check global */ }

// AFTER:
// Remove this guard. Always check global milestone independently.
// BEFORE (line 2158):
if milestoneCampaignID == "" { /* check anniversary */ }

// AFTER:
// Remove this guard. Always check anniversary independently.

Discount Amount Calculation

Each discount calculates against the original booking total:

discountAmount := roundTo2(bookingTotal * discountPercent / 100)

This is already how each step works — no change needed to the calculation itself. The key change is that multiple booking_discounts rows and multiple payments rows are created for the same booking.

Example: £100 booking with loyalty + time-based + anniversary

Step Source % Amount booking_discounts row payments row
1 loyalty 10% £10.00 discount_source='loyalty' method='discount', amount=10.00
2 time_based campaign 5% £5.00 discount_source='campaign', campaign_type='time_based' method='discount', amount=5.00
3 anniversary milestone 10% £10.00 discount_source='campaign', milestone_type='anniversary' method='discount', amount=10.00
Total 25% £25.00 3 rows 3 rows

Customer pays: £100 - £25 = £75 (via remaining payment methods).

Test Helper Issue

The existing getDiscountForBooking helper does a single-row scan:

SELECT discount_source, discount_amount FROM booking_discounts WHERE booking_id = $1

This will fail (or return arbitrary results) once stacking creates multiple rows per booking. The rewrite of TestDiscount_LoyaltyPriority must not use this helper, and the new stacking tests (1322) will need different query patterns.

Recommended new helpers:

// Count discount rows for a booking
func getDiscountRowCount(t *testing.T, bookingID string) int

// Get all discounts for a booking as a slice
func getAllDiscountsForBooking(t *testing.T, bookingID string) []struct{ Source string; Amount float64 }

// Sum all discount amounts for a booking
func getTotalDiscountAmount(t *testing.T, bookingID string) float64

The existing getDiscountForBooking single-row helper should be retired or renamed to avoid confusion.

Test Changes

The existing TestDiscount_LoyaltyPriority test (line 540) asserts that campaign discounts are blocked when loyalty applies. This test must be rewritten to assert the opposite: that both loyalty AND campaign discounts are applied.


3. Expanded Test Suite

Current Tests (12)

# Test What It Verifies
1 TestDiscount_Loyalty_FullCycle 10 stamps → redemption → 11th booking discount → stamps reset to 1
2 TestDiscount_Loyalty_ExistingRedemptionApplies Pre-existing pending redemption applies on completion
3 TestDiscount_Loyalty_OneStampPerDay Same-day completions only give 1 stamp
4 TestDiscount_Loyalty_ZeroTotalNoStamp £0 bookings don't earn stamps
5 TestDiscount_Loyalty_CycleRepeats After redemption, cycle restarts
6 TestDiscount_TimeBasedCampaign Time-based campaign applies correctly
7 TestDiscount_PerUserMilestone Per-user booking count milestone
8 TestDiscount_GlobalMilestone Global booking count milestone
9 TestDiscount_AnniversaryMilestone Anniversary milestone with dedup
10 TestDiscount_LoyaltyPriority Loyalty blocks campaigns (MUST CHANGE)
11 TestDiscount_NoDiscountOnZeroTotal Zero-total gets no discounts
12 TestDiscount_CampaignMaxRedemptions Campaign respects max_redemptions

New Tests Required

Stacking Tests

# Test What It Verifies
13 TestDiscount_Stacking_LoyaltyPlusTimeBased Loyalty 10% + time-based 5% = 15% off, two booking_discounts rows, two payments rows
14 TestDiscount_Stacking_LoyaltyPlusMilestone Loyalty 10% + per-user milestone 15% = 25% off
15 TestDiscount_Stacking_LoyaltyPlusAnniversary Loyalty 10% + anniversary 10% = 20% off
16 TestDiscount_Stacking_AllThreeTypes Loyalty + time-based + anniversary = sum of all three
17 TestDiscount_Stacking_MultipleMilestones Per-user + global + anniversary all apply on same booking
18 TestDiscount_Stacking_LoyaltyPlusGlobalMilestone Loyalty + global milestone stack
19 TestDiscount_Stacking_TimeBasedPlusMilestone Time-based campaign + per-user milestone stack
20 TestDiscount_Stacking_DiscountAmountsSumCorrectly Verify total discount = sum of individual amounts, each calculated against original total
21 TestDiscount_Stacking_MultiplePaymentRecords Each discount creates its own payments row with method='discount'
22 TestDiscount_Stacking_MultipleBookingDiscountRows Each discount creates its own booking_discounts row with correct source/type

Edge Case Tests

# Test What It Verifies
23 TestDiscount_ExpiredRedemptionDoesNotApply Redemption past expires_at is not applied
24 TestDiscount_MultiplePendingRedemptions_UsesOldest If somehow multiple pending redemptions exist, oldest is used first
25 TestDiscount_StampCountAboveTen If stamps somehow exceed 10 (e.g., admin manual set), redemption still created at check, GREATEST(0, stamps-10) handles reset
26 TestDiscount_MixedFreeAndPaidServices Booking with one £0 service and one £50 service → total £50 → earns stamp
27 TestDiscount_CampaignBoundaryStart Campaign start_date exactly at completion time → applies
28 TestDiscount_CampaignBoundaryEnd Campaign end_date exactly at completion time → applies
29 TestDiscount_CampaignExpiredDoesNotApply Campaign past end_date → no discount
30 TestDiscount_CampaignDraftDoesNotApply Campaign with status='draft' → no discount
31 TestDiscount_CampaignCancelledDoesNotApply Campaign with status='cancelled' → no discount
32 TestDiscount_PriceOverrideRespected Admin overrides service price → discount calculated on overridden price
33 TestDiscount_AnniversaryDedupWithStacking Anniversary dedup still works even when other discounts also apply
34 TestDiscount_PerUserMilestoneDedupWithStacking Per-user milestone dedup still works with stacking
35 TestDiscount_GlobalMilestoneMaxRedemptionsWithStacking Global max_redemptions still respected with stacking
36 TestDiscount_BestTimeBasedCampaignSelected Multiple active time-based campaigns → highest % is selected
37 TestDiscount_FirstBookingEarnsStamp Brand new user's first completed paid booking earns stamp #1
38 TestDiscount_TenStampsCreatesRedemption Completing a booking that brings stamps to exactly 10 creates pending redemption
39 TestDiscount_RedemptionAppliedBeforeStampIncrement On discounted booking: redemption applied first (stamps → 0), then +1 stamp earned

Campaign Status Tests

# Test What It Verifies
40 TestCampaign_CreateAsDraft Creating campaign without status → defaults to 'draft'
41 TestCampaign_ActivateDraft Update draft → active works
42 TestCampaign_CompleteActive Update active → completed works
43 TestCampaign_CancelActive Update active → cancelled works
44 TestCampaign_RevertToDraft Update active → draft works (for re-editing before re-activation)
45 TestCampaign_DraftDoesNotApplyDiscounts Draft campaign does not trigger discounts at completion

Total: 45 tests (12 existing + 33 new, with test #10 rewritten)


4. discount_eligible — Scrap It

Problem Analysis

The discount_eligible BOOLEAN column on bookings is completely dead code:

  • Defined in schema (init-script.sql line 241)
  • Listed in Technical Manual
  • Never read, written, or referenced by any Go code, TypeScript, or handler
  • Not in the Booking struct
  • Not in any INSERT or UPDATE query

Why Pre-Assignment Is Fundamentally Flawed

The original v1 plan proposed setting discount_eligible = TRUE at booking creation time if the user has a pending redemption or active campaign. This is architecturally wrong because:

  1. Out-of-order completion: User books A (Tuesday) then B (Monday). B completes first and gets the loyalty discount. A was marked "eligible" but the redemption is now consumed. Bad state.

  2. Campaign changes: Admin creates a campaign after a booking is made. Existing bookings aren't retroactively marked eligible. Or admin cancels a campaign — bookings still marked eligible won't get the discount.

  3. Price overrides: Admin overrides service prices after booking creation. The eligibility was calculated on original prices.

  4. Multiple pending bookings: User has 3 future bookings. All marked "eligible." Only the first to complete actually gets the discount. The other 2 show misleading eligibility.

  5. Milestone unpredictability: Per-user and global milestones depend on completion counts that change as other bookings complete. You can't predict at creation time whether a specific booking will hit the milestone.

Decision: Drop the Column

ALTER TABLE bookings DROP COLUMN discount_eligible;

What Replaces It

Nothing on the booking row. Discount eligibility is computed entirely at completion time in ProgressBookingHandler. This is the correct approach because:

  • All data is final at completion (prices, services, user history)
  • No race conditions between multiple pending bookings
  • No stale state from campaign changes
  • No misleading UI indicators

For the customer UI ("you have a discount waiting"), query the source tables directly:

  • SELECT COUNT(*) FROM loyalty_redemptions WHERE user_id = $1 AND status = 'pending' AND expires_at > NOW() → "You have a 10% loyalty discount waiting for your next completed appointment"
  • SELECT name FROM discount_campaigns WHERE status = 'active' AND campaign_type = 'time_based' AND start_date <= NOW() AND end_date >= NOW() → "Summer Sale: 5% off all services this week!"

These queries are already fast (indexed) and always reflect current truth.

File Changes

File Change
init-scripts/init-script.sql Remove discount_eligible BOOLEAN NOT NULL DEFAULT FALSE from bookings table (line 241)
obsidian/Crussell/Technical Manual.md Remove discount_eligible from bookings column list

5. Campaign Status Lifecycle — Fix

Current State (broken)

Operation DB enum allows Code allows Issue
Create draft (default), active, completed, cancelled Hardcoded "active" only Cannot create as draft
Update draft, active, completed, cancelled active, cancelled, completed only Cannot update to draft
GET filter draft, active, completed, cancelled active, paused, cancelled, expired Phantom statuses; missing draft/completed

Desired Lifecycle

draft → active → completed
  ↑         ↓
  └── cancelled
  • Create: Always creates as draft. Admin reviews and manually activates.
  • Activate: draftactive. Campaign starts applying discounts.
  • Complete: activecompleted. Campaign stops applying (past its useful life).
  • Cancel: Any → cancelled. Campaign permanently disabled.
  • Revert: activedraft. Admin wants to edit and re-activate.

Implementation Changes

File: backend/handlers/admin/discount_campaigns.go

5.1 CreateDiscountCampaign (line 310)

Change hardcoded "active" to "draft":

// BEFORE (line 310):
"active",

// AFTER:
"draft",

5.2 UpdateDiscountCampaign (lines 480-488)

Add "draft" to the whitelist:

// BEFORE:
if *req.Status != "active" && *req.Status != "cancelled" && *req.Status != "completed" {
    http.Error(w, "Status must be 'active', 'cancelled', or 'completed'", http.StatusBadRequest)
    return
}

// AFTER:
if *req.Status != "draft" && *req.Status != "active" && *req.Status != "cancelled" && *req.Status != "completed" {
    http.Error(w, "Status must be 'draft', 'active', 'cancelled', or 'completed'", http.StatusBadRequest)
    return
}

5.3 GetDiscountCampaigns filter (line 81)

Fix phantom statuses:

// BEFORE:
validStatuses := map[string]bool{"active": true, "paused": true, "cancelled": true, "expired": true}

// AFTER:
validStatuses := map[string]bool{"draft": true, "active": true, "completed": true, "cancelled": true}

5.4 Frontend Campaign Management — ALREADY DONE

The DiscountsManagement.svelte component already implements the full draft lifecycle:

  • Campaign type includes 'draft' in the status union (line 26)
  • Status badge display handles all four statuses including draft (lines 290293)
  • "Activate" button exists for c.status === 'draft' (lines 368/373, 423/428)
  • "Complete" button exists for c.status === 'active' (lines 378/383, 433/438)
  • "Cancel" button exists for non-cancelled campaigns (lines 388/393, 443/448)
  • Conditional at c.status === 'active' || c.status === 'completed' (lines 398, 453) — likely "Revert to Draft"

No frontend changes needed. The frontend is already aligned with the desired lifecycle. Only the backend changes in Phase 2 are required.

File Changes

File Change
backend/handlers/admin/discount_campaigns.go Fix create default, update whitelist, GET filter
frontend/src/lib/components/admin/DiscountsManagement.svelte Update status buttons for draft lifecycleAlready done

6. Square Payments — Reality Check

Current State

Square is stubbed locally via build tags (//go:build !prod in internal/square/dev.go). The dev mock simulates Square Terminal API responses without hitting real Square servers.

No real-world Square integration has been tested. The prod stub (internal/square/prod.go) is a placeholder.

Impact on Loyalty/Discount System

The loyalty/discount system is independent of Square. Discounts create payments rows with payment_method = 'discount' — this is a bookkeeping entry, not a Square transaction. The actual payment collection (cash, card via Square Terminal, etc.) happens separately.

No changes needed to the loyalty/discount system for Square. When real Square is wired up, the discount payments will coexist with Square payments on the same booking.

Future Consideration (not in scope)

When Square is live:

  • Online deposit payments will create payments rows with payment_method = 'square_terminal' or 'square_web'
  • The discount payments rows reduce the remaining balance
  • The Square payment amount should be bookingTotal - sum(all discount amounts) - sum(other payments)
  • This is already how the system works — amount_due = total - sum(completed payments)

7. Implementation Plan

Phase 1: Drop discount_eligible (trivial)

Step File Change
1.1 init-scripts/init-script.sql Remove discount_eligible column from bookings table
1.2 obsidian/Crussell/Technical Manual.md Remove from column list

Phase 2: Fix Campaign Status Lifecycle

Step File Change
2.1 backend/handlers/admin/discount_campaigns.go Change create default to "draft" (line 310)
2.2 backend/handlers/admin/discount_campaigns.go Add "draft" to update whitelist (line 481)
2.3 backend/handlers/admin/discount_campaigns.go Fix GET filter to match DB enum (line 81)
2.4 frontend/src/lib/components/admin/DiscountsManagement.svelte Update status buttons for draft lifecycleAlready done

Phase 3: Implement Discount Stacking

Step File Change
3.1 backend/handlers/bookings/bookings.go Remove hasLoyaltyDiscount guard (lines 2066-2069)
3.2 backend/handlers/bookings/bookings.go Remove hasDiscount guard (lines 2099-2102)
3.3 backend/handlers/bookings/bookings.go Remove milestoneCampaignID == "" guards (lines 2130, 2158)
3.4 backend/handlers/bookings/bookings.go Rewrite TestDiscount_LoyaltyPriority to assert stacking

Phase 4: Expanded Test Suite

Step File Change
4.1 backend/handlers/bookings/discount_test.go Add stacking tests (#13-22)
4.2 backend/handlers/bookings/discount_test.go Add edge case tests (#23-39)
4.3 backend/handlers/bookings/discount_test.go Add campaign status tests (#40-45)
4.4 backend/handlers/bookings/discount_test.go Rewrite TestDiscount_LoyaltyPriority

Phase 5: Frontend Updates

Step File Change
5.1 frontend/src/lib/components/admin/DiscountsManagement.svelte Draft lifecycle buttonsAlready done
5.2 frontend/src/routes/account/+page.svelte No changes needed (already shows stamp card correctly)

8. File Change Summary

File Change Type Phase
init-scripts/init-script.sql Remove discount_eligible column 1
obsidian/Crussell/Technical Manual.md Remove discount_eligible reference 1
backend/handlers/admin/discount_campaigns.go Fix status lifecycle (3 changes) 2
frontend/src/lib/components/admin/DiscountsManagement.svelte Draft lifecycle UIAlready done 2, 5
backend/handlers/bookings/bookings.go Remove mutual-exclusivity guards (4 changes) 3
backend/handlers/bookings/discount_test.go Rewrite 1 test, add 33 new tests, add new helpers 4

9. Risks & Mitigations

Risk Mitigation
Stacking produces unexpectedly large discounts Each discount is a separate booking_discounts row — easy to audit. Consider adding a total discount cap (e.g., 50%) as a future safeguard.
Multiple payments rows with method='discount' confuse payment reconciliation Each has a distinct booking_discounts.source_id linking to the specific redemption/campaign. Clear audit trail.
Removing mutual-exclusivity guards changes behaviour for existing bookings Only affects future completions. Existing booking_discounts rows are unchanged.
Campaign status change from "active" default to "draft" breaks existing campaigns No existing campaigns in production (system is local-only). For dev DB, run migration to update any existing "active" campaigns that should be "draft".
discount_eligible column removal breaks something Verified: zero references in any Go, TypeScript, or SQL code outside the schema definition itself. Safe to drop.
Test suite expansion reveals bugs in stacking logic That's the point — tests should catch issues before production.