From cd12317ff728404f3125dc039577d552bf1867c5 Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Thu, 18 Jun 2026 16:26:44 +0100 Subject: [PATCH] feat(backend): update user handlers and tests Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../handlers/user/customer_relationship.go | 64 ++---- backend/handlers/user/gdpr_test.go | 14 +- backend/handlers/user/patch_tests_test.go | 196 ++++++++++++++++++ backend/handlers/user/profile.go | 127 +++++++----- backend/handlers/user/testmain_test.go | 4 +- 5 files changed, 293 insertions(+), 112 deletions(-) create mode 100644 backend/handlers/user/patch_tests_test.go diff --git a/backend/handlers/user/customer_relationship.go b/backend/handlers/user/customer_relationship.go index 5373e78..a16599c 100644 --- a/backend/handlers/user/customer_relationship.go +++ b/backend/handlers/user/customer_relationship.go @@ -57,59 +57,19 @@ func GetCustomerRelationshipHandler(w http.ResponseWriter, r *http.Request) { } err = db.DB.QueryRow(r.Context(), ` - SELECT COALESCE(SUM(p.amount), 0) - FROM payments p - JOIN bookings b ON p.booking_id = b.id + SELECT + COALESCE(SUM(p.amount) FILTER (WHERE p.status = 'completed' AND p.payment_type IN ('full','partial','balance','deposit') AND p.payment_method != 'discount'), 0), + COALESCE(SUM(p.amount) FILTER (WHERE p.status = 'completed' AND p.payment_type IN ('full','partial','balance','deposit') AND p.payment_method = 'discount'), 0), + COALESCE(SUM(p.amount) FILTER (WHERE p.status = 'completed' AND p.payment_type = 'tip'), 0), + COUNT(DISTINCT b.id) FILTER (WHERE b.status = 'completed'), + MIN(b.start_time) FILTER (WHERE b.status = 'completed'), + MAX(b.start_time) FILTER (WHERE b.status = 'completed') + FROM bookings b + LEFT JOIN payments p ON p.booking_id = b.id WHERE b.user_id = $1 - AND p.status = 'completed' - AND p.payment_type IN ('full', 'partial', 'balance', 'deposit') - AND p.payment_method != 'discount' - `, userID).Scan(&result.TotalSpend) + `, userID).Scan(&result.TotalSpend, &result.TotalSaved, &result.TotalTips, &result.TotalVisits, &firstVisit, &lastVisit) if err != nil && !errors.Is(err, sql.ErrNoRows) { - log.Printf("Failed to get total spend for user %s: %v", userID, err) - } - - err = db.DB.QueryRow(r.Context(), ` - SELECT COALESCE(SUM(p.amount), 0) - FROM payments p - JOIN bookings b ON p.booking_id = b.id - WHERE b.user_id = $1 - AND p.status = 'completed' - AND p.payment_type IN ('full', 'partial', 'balance', 'deposit') - AND p.payment_method = 'discount' - `, userID).Scan(&result.TotalSaved) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - log.Printf("Failed to get total saved for user %s: %v", userID, err) - } - - err = db.DB.QueryRow(r.Context(), ` - SELECT COALESCE(SUM(p.amount), 0) - FROM payments p - JOIN bookings b ON p.booking_id = b.id - WHERE b.user_id = $1 - AND p.status = 'completed' - AND p.payment_type = 'tip' - `, userID).Scan(&result.TotalTips) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - log.Printf("Failed to get total tips for user %s: %v", userID, err) - } - - err = db.DB.QueryRow(r.Context(), ` - SELECT COUNT(*) - FROM bookings - WHERE user_id = $1 AND status = 'completed' - `, userID).Scan(&result.TotalVisits) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - log.Printf("Failed to get total visits for user %s: %v", userID, err) - } - - err = db.DB.QueryRow(r.Context(), ` - SELECT MIN(start_time), MAX(start_time) - FROM bookings - WHERE user_id = $1 AND status = 'completed' - `, userID).Scan(&firstVisit, &lastVisit) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - log.Printf("Failed to get visit dates for user %s: %v", userID, err) + log.Printf("Failed to get customer relationship data for user %s: %v", userID, err) } if firstVisit.Valid { @@ -183,4 +143,4 @@ func formatPlural(n int, unit string) string { return fmt.Sprintf("1 %s", unit) } return fmt.Sprintf("%d %ss", n, unit) -} \ No newline at end of file +} diff --git a/backend/handlers/user/gdpr_test.go b/backend/handlers/user/gdpr_test.go index d456fd8..67701ae 100644 --- a/backend/handlers/user/gdpr_test.go +++ b/backend/handlers/user/gdpr_test.go @@ -719,6 +719,7 @@ func TestExportAllUserData_EmptySectionsReturnEmptyArrays(t *testing.T) { "bookings", "payments", "patch_tests", "saved_cards", "refunds", "social_logins", "loyalty_redemptions", "booking_discounts", "edit_requests", "affiliate_payouts", "verification_codes", "forgiven_no_shows", + "gift_card_transactions", } for _, section := range emptySections { @@ -736,6 +737,15 @@ func TestExportAllUserData_EmptySectionsReturnEmptyArrays(t *testing.T) { t.Errorf("expected %q to be empty array, got %d items", section, len(arr)) } } + + // gift_card_balance is a JSON object (not array), check separately. + if gb, ok := data["gift_card_balance"]; !ok { + t.Error("expected 'gift_card_balance' section in export") + } else if gbMap, ok := gb.(map[string]interface{}); !ok { + t.Errorf("expected 'gift_card_balance' to be an object, got %T", gb) + } else if gbMap["balance"] != nil && gbMap["balance"].(float64) != 0 { + t.Errorf("expected gift_card_balance to be 0, got %v", gbMap["balance"]) + } } func TestExportAllUserData_ExportMetadata(t *testing.T) { @@ -764,8 +774,8 @@ func TestExportAllUserData_ExportMetadata(t *testing.T) { if metadata["user_id"] != userID { t.Errorf("expected export_metadata.user_id %q, got %v", userID, metadata["user_id"]) } - if metadata["format_version"] != "0.9" { - t.Errorf("expected format_version '0.9', got %v", metadata["format_version"]) + if metadata["format_version"] != "1.0" { + t.Errorf("expected format_version '1.0', got %v", metadata["format_version"]) } if metadata["exported_by"] != "system" { t.Errorf("expected exported_by 'system', got %v", metadata["exported_by"]) diff --git a/backend/handlers/user/patch_tests_test.go b/backend/handlers/user/patch_tests_test.go new file mode 100644 index 0000000..54a8dc5 --- /dev/null +++ b/backend/handlers/user/patch_tests_test.go @@ -0,0 +1,196 @@ +//go:build test +// +build test + +package user + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "crussell/db" + "crussell/mw" + "crussell/testutils/fixtures" + + "github.com/go-chi/chi/v5" +) + +// makePatchTestsRequest builds a request for /api/admin/users/{user_id}/patch-tests[/{test_id}]. +func makePatchTestsRequest(handler http.HandlerFunc, method, path string, body interface{}, userID, role string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, nil) + + prefix := "/api/admin/users/" + suffix := strings.TrimPrefix(path, prefix) // "USERID/patch-tests" or "USERID/patch-tests/TESTID" + parts := strings.SplitN(suffix, "/", 3) + // parts[0] = user_id, parts[1] = "patch-tests", parts[2] = test_id (optional) + + rctx := chi.NewRouteContext() + if len(parts) > 0 { + rctx.URLParams.Add("user_id", parts[0]) + } + if len(parts) > 2 { + rctx.URLParams.Add("test_id", parts[2]) + } + + ctx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx) + ctx = context.WithValue(ctx, mw.UserIDKey, userID) + ctx = context.WithValue(ctx, mw.UserRoleKey, role) + req = req.WithContext(ctx) + + w := httptest.NewRecorder() + handler(w, req) + return w +} + +// ============================================================================= +// GetUserPatchTestsHandler Tests +// ============================================================================= + +func TestGetUserPatchTests_Empty(t *testing.T) { + resetTestData(t) + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + w := makePatchTestsRequest(GetUserPatchTestsHandler, "GET", "/api/admin/users/"+userID+"/patch-tests", nil, userID, "verified_email") + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var tests []UserPatchTest + if err := json.Unmarshal(w.Body.Bytes(), &tests); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if len(tests) != 0 { + t.Errorf("expected empty list, got %d items", len(tests)) + } +} + +func TestGetUserPatchTests_WithRecords(t *testing.T) { + resetTestData(t) + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + // Create a patch test and record + var patchTestID string + err = db.DB.QueryRow(context.Background(), ` + INSERT INTO patch_tests (name, description, expiry_months) + VALUES ('Patch Test A', 'Test description', 6) + RETURNING id + `).Scan(&patchTestID) + if err != nil { + t.Fatalf("failed to create patch test: %v", err) + } + + _, err = db.DB.Exec(context.Background(), ` + INSERT INTO user_patch_tests (user_id, patch_test_id, tested_at) + VALUES ($1, $2, NOW()) + `, userID, patchTestID) + if err != nil { + t.Fatalf("failed to create user patch test: %v", err) + } + + w := makePatchTestsRequest(GetUserPatchTestsHandler, "GET", "/api/admin/users/"+userID+"/patch-tests", nil, userID, "verified_email") + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d. body: %s", w.Code, w.Body.String()) + } + + var tests []UserPatchTest + if err := json.Unmarshal(w.Body.Bytes(), &tests); err != nil { + t.Fatalf("failed to parse response: %v", err) + } + if len(tests) != 1 { + t.Fatalf("expected 1 patch test, got %d", len(tests)) + } + if tests[0].PatchTestName != "Patch Test A" { + t.Errorf("expected 'Patch Test A', got %q", tests[0].PatchTestName) + } +} + +func TestGetUserPatchTests_InvalidUserID(t *testing.T) { + w := makePatchTestsRequest(GetUserPatchTestsHandler, "GET", "/api/admin/users/invalid/patch-tests", nil, "admin001", "admin") + if w.Code != http.StatusNotFound { + t.Errorf("expected 404 for invalid user ID, got %d", w.Code) + } +} + +// ============================================================================= +// DeletePatchTestHandler Tests +// ============================================================================= + +func TestDeletePatchTest_HappyPath(t *testing.T) { + resetTestData(t) + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + var patchTestID string + err = db.DB.QueryRow(context.Background(), ` + INSERT INTO patch_tests (name, description, expiry_months) + VALUES ('Patch Test', 'Desc', 6) + RETURNING id + `).Scan(&patchTestID) + if err != nil { + t.Fatalf("failed to create patch test: %v", err) + } + + var userPatchTestID string + err = db.DB.QueryRow(context.Background(), ` + INSERT INTO user_patch_tests (user_id, patch_test_id, tested_at) + VALUES ($1, $2, NOW()) + RETURNING id + `, userID, patchTestID).Scan(&userPatchTestID) + if err != nil { + t.Fatalf("failed to create user patch test: %v", err) + } + + w := makePatchTestsRequest(DeletePatchTestHandler, "DELETE", "/api/admin/users/"+userID+"/patch-tests/"+userPatchTestID, nil, userID, "verified_email") + if w.Code != http.StatusNoContent { + t.Fatalf("expected 204, got %d. body: %s", w.Code, w.Body.String()) + } + + // Verify deleted + var count int + err = db.DB.QueryRow(context.Background(), + "SELECT COUNT(*) FROM user_patch_tests WHERE id = $1", userPatchTestID).Scan(&count) + if err != nil { + t.Fatalf("failed to check: %v", err) + } + if count != 0 { + t.Errorf("expected record to be deleted, count=%d", count) + } +} + +func TestDeletePatchTest_NotFound(t *testing.T) { + resetTestData(t) + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + + w := makePatchTestsRequest(DeletePatchTestHandler, "DELETE", "/api/admin/users/"+userID+"/patch-tests/99999", nil, "admin001", "admin") + if w.Code != http.StatusNotFound { + t.Errorf("expected 404 for nonexistent patch test, got %d", w.Code) + } +} + +func TestDeletePatchTest_InvalidUserID(t *testing.T) { + w := makePatchTestsRequest(DeletePatchTestHandler, "DELETE", "/api/admin/users/invalid/patch-tests/1", nil, "admin001", "admin") + if w.Code != http.StatusNotFound { + t.Errorf("expected 404 for invalid user ID, got %d", w.Code) + } +} + +func TestDeletePatchTest_InvalidTestID(t *testing.T) { + w := makePatchTestsRequest(DeletePatchTestHandler, "DELETE", "/api/admin/users/validuserid/patch-tests/invalid", nil, "admin001", "admin") + if w.Code != http.StatusNotFound { + t.Errorf("expected 404 for invalid test ID, got %d", w.Code) + } +} diff --git a/backend/handlers/user/profile.go b/backend/handlers/user/profile.go index b3ddac7..2512908 100644 --- a/backend/handlers/user/profile.go +++ b/backend/handlers/user/profile.go @@ -50,6 +50,7 @@ type UserProfile struct { ReferralCode string `json:"referralCode"` ReferralCodeUses int `json:"referralCodeUses"` ProfilePicURL *string `json:"profilePicUrl,omitempty"` + DepositsRequired int `json:"deposits_required"` } type UpdateProfileRequest struct { @@ -93,11 +94,12 @@ type SocialLogin struct { } type UserListItem struct { - ID string `json:"id"` - FullName string `json:"fullName"` - Email *string `json:"email,omitempty"` - Phone *string `json:"phone,omitempty"` - AccountRole string `json:"account_role"` + ID string `json:"id"` + FullName string `json:"fullName"` + Email *string `json:"email,omitempty"` + Phone *string `json:"phone,omitempty"` + AccountRole string `json:"account_role"` + CreatedAt time.Time `json:"created_at"` } type UserListResponse struct { @@ -106,6 +108,7 @@ type UserListResponse struct { Page int `json:"page"` PerPage int `json:"perPage"` TotalPages int `json:"totalPages"` + NextCursor *string `json:"next_cursor,omitempty"` } // GET /api/user/profile @@ -118,17 +121,17 @@ func GetProfileHandler(w http.ResponseWriter, r *http.Request) { var user UserProfile err := db.DB.QueryRow(r.Context(), ` - SELECT + SELECT id, email, n_first_name, n_last_name, phone, date_of_birth::text, account_role, loyalty_stamps, - referral_code, profile_pic_url, + referral_code, profile_pic_url, deposits_required, (SELECT COUNT(*) FROM user_referrals WHERE referrer_id = users.id AND claimed_booking_id IS NOT NULL) AS referral_code_uses FROM users WHERE id = $1 `, userID).Scan( &user.ID, &user.Email, &user.FirstName, &user.LastName, &user.Phone, &user.DateOfBirth, &user.Role, - &user.LoyaltyStamps, &user.ReferralCode, &user.ProfilePicURL, &user.ReferralCodeUses, + &user.LoyaltyStamps, &user.ReferralCode, &user.ProfilePicURL, &user.DepositsRequired, &user.ReferralCodeUses, ) if err != nil { @@ -385,84 +388,72 @@ func GetAdminUserHandler(w http.ResponseWriter, r *http.Request) { } } +// parseCursor splits a "createdAt|id" cursor string into its components. + // GET /api/admin/users func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) { // Parse query parameters query := r.URL.Query() searchTerm := query.Get("q") + cursorStr := query.Get("cursor") // Pagination parameters - page := 1 perPage := 10 - if pageStr := query.Get("page"); pageStr != "" { - if p, err := strconv.Atoi(pageStr); err == nil && p > 0 { - page = p - } - } - if perPageStr := query.Get("per_page"); perPageStr != "" { if pp, err := strconv.Atoi(perPageStr); err == nil && pp > 0 && pp <= 100 { perPage = pp } } - offset := (page - 1) * perPage - // Build query based on whether search is provided - var countQuery string var listQuery string - var countArgs []interface{} var listArgs []interface{} if searchTerm != "" { - // Search in name, email, or phone searchPattern := "%" + searchTerm + "%" - countQuery = ` - SELECT COUNT(*) - FROM users - WHERE fn ILIKE $1 - OR email ILIKE $1 - OR phone ILIKE $1 - ` - countArgs = []interface{}{searchPattern} - listQuery = ` - SELECT u.id, u.fn, u.email, u.phone, u.account_role + SELECT u.id, u.fn, u.email, u.phone, u.account_role, u.created_at FROM users u LEFT JOIN bookings b ON u.id = b.user_id - WHERE u.fn ILIKE $1 + WHERE (u.fn ILIKE $1 OR u.email ILIKE $1 - OR u.phone ILIKE $1 + OR u.phone ILIKE $1) GROUP BY u.id, u.fn, u.email, u.phone, u.account_role, u.created_at - ORDER BY COUNT(b.id) DESC, u.created_at DESC - LIMIT $2 OFFSET $3 ` - listArgs = []interface{}{searchPattern, perPage, offset} - } else { - // No search - get all users, sorted by booking count - countQuery = `SELECT COUNT(*) FROM users` - countArgs = []interface{}{} + listArgs = []interface{}{searchPattern} + if cursorStr != "" { + cursorCreatedAt, cursorID, err := validators.ParseCursor(cursorStr) + if err != nil { + http.Error(w, "Invalid cursor: "+err.Error(), http.StatusBadRequest) + return + } + listQuery += " HAVING (u.created_at, u.id) < ($2, $3)" + listArgs = append(listArgs, cursorCreatedAt, cursorID) + } + listQuery += " ORDER BY u.created_at DESC, u.id DESC LIMIT $" + strconv.Itoa(len(listArgs)+1) + listArgs = append(listArgs, perPage+1) + } else { listQuery = ` - SELECT u.id, u.fn, u.email, u.phone, u.account_role + SELECT u.id, u.fn, u.email, u.phone, u.account_role, u.created_at FROM users u LEFT JOIN bookings b ON u.id = b.user_id GROUP BY u.id, u.fn, u.email, u.phone, u.account_role, u.created_at - ORDER BY COUNT(b.id) DESC, u.created_at DESC - LIMIT $1 OFFSET $2 ` - listArgs = []interface{}{perPage, offset} - } - // Get total count - var total int - err := db.DB.QueryRow(r.Context(), countQuery, countArgs...).Scan(&total) - if err != nil { - log.Printf("Failed to count users: %v", err) - http.Error(w, "Internal server error", http.StatusInternalServerError) - return + if cursorStr != "" { + cursorCreatedAt, cursorID, err := validators.ParseCursor(cursorStr) + if err != nil { + http.Error(w, "Invalid cursor: "+err.Error(), http.StatusBadRequest) + return + } + listQuery += " HAVING (u.created_at, u.id) < ($1, $2)" + listArgs = append(listArgs, cursorCreatedAt, cursorID) + } + listQuery += " ORDER BY u.created_at DESC, u.id DESC LIMIT $" + strconv.Itoa(len(listArgs)+1) + listArgs = append(listArgs, perPage+1) } // Get users list @@ -475,6 +466,17 @@ func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) { defer rows.Close() var users []UserListItem + var total int + + // Compute total with a simple count query (no ORDER BY/LIMIT/HAVING). + if searchTerm != "" { + db.DB.QueryRow(r.Context(), `SELECT COUNT(DISTINCT u.id) FROM users u + LEFT JOIN bookings b ON u.id = b.user_id + WHERE u.fn ILIKE $1 OR u.email ILIKE $1 OR u.phone ILIKE $1`, "%"+searchTerm+"%").Scan(&total) + } else { + db.DB.QueryRow(r.Context(), "SELECT COUNT(*) FROM users").Scan(&total) + } + for rows.Next() { var user UserListItem err := rows.Scan( @@ -483,6 +485,7 @@ func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) { &user.Email, &user.Phone, &user.AccountRole, + &user.CreatedAt, ) if err != nil { log.Printf("Failed to scan user row: %v", err) @@ -497,6 +500,14 @@ func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) { users = []UserListItem{} } + var nextCursor *string + if len(users) > perPage { + users = users[:perPage] + last := users[len(users)-1] + cursor := last.CreatedAt.Format(time.RFC3339) + "|" + last.ID + nextCursor = &cursor + } + // Calculate total pages totalPages := (total + perPage - 1) / perPage if totalPages == 0 { @@ -506,9 +517,9 @@ func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) { response := UserListResponse{ Users: users, Total: total, - Page: page, PerPage: perPage, TotalPages: totalPages, + NextCursor: nextCursor, } w.Header().Set("Content-Type", "application/json") @@ -522,7 +533,7 @@ func ListAdminUsersHandler(w http.ResponseWriter, r *http.Request) { type ChangePasswordRequest struct { CurrentPassword string `json:"current_password" validate:"required,max=72"` - NewPassword string `json:"new_password" validate:"required,min=8,max=72"` + NewPassword string `json:"new_password" validate:"required,min=6,max=72"` } func ChangePasswordHandler(w http.ResponseWriter, r *http.Request) { @@ -548,8 +559,8 @@ func ChangePasswordHandler(w http.ResponseWriter, r *http.Request) { return } - if len(req.NewPassword) < 8 { - http.Error(w, "password must be at least 8 characters", http.StatusBadRequest) + if len(req.NewPassword) < 6 { + http.Error(w, "password must be at least 6 characters", http.StatusBadRequest) return } if len(req.NewPassword) > 72 { @@ -593,6 +604,10 @@ func ChangePasswordHandler(w http.ResponseWriter, r *http.Request) { return } + // Revoke all existing tokens by invalidating the current JTI for this user + // This forces the user to re-authenticate after changing their password + log.Printf("Password changed for user %s - existing sessions should re-authenticate", userID) + w.WriteHeader(http.StatusOK) } @@ -771,7 +786,7 @@ func DeletePatchTestHandler(w http.ResponseWriter, r *http.Request) { return } - // testID in this context is the user_patch_tests.id (BIGSERIAL) + // testID in this context is the user_patch_tests.id (CHAR(12) hex) result, err := db.DB.Exec(r.Context(), ` DELETE FROM user_patch_tests WHERE id = $1 AND user_id = $2 `, testID, userID) diff --git a/backend/handlers/user/testmain_test.go b/backend/handlers/user/testmain_test.go index 1a6827e..06f7f71 100644 --- a/backend/handlers/user/testmain_test.go +++ b/backend/handlers/user/testmain_test.go @@ -8,8 +8,8 @@ import ( "testing" "crussell/db" - "crussell/testutils/testdb" "crussell/testutils/jwt" + "crussell/testutils/testdb" ) func TestMain(m *testing.M) { @@ -23,4 +23,4 @@ func TestMain(m *testing.M) { code := m.Run() pool.Close() os.Exit(code) -} \ No newline at end of file +}