refactor(backend): replace resetTestData with SetupTestDB and add new tests

Migrate all test files from resetTestData(t) to testutils.SetupTestDB(t) for isolated per-package test databases. - Add new feature tests: name history assertions, referral discount preview, time blockers, email validation, GDPR export, loyalty manual redemption - Update existing tests to use batch queries and SetupTestDB - Remove test_helpers.go resetTestData infrastructure - Add comprehensive user profile tests (442 new lines) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-06-20 16:57:36 +01:00
parent 9c68918c20
commit b03c4f6247
36 changed files with 1735 additions and 859 deletions
@@ -17,11 +17,13 @@ package admin
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"testing"
 	"time"

 	"crussell/db"
+	"crussell/testutils"
 	"crussell/handlers/user"
 	"crussell/mw"
 )
@@ -29,20 +31,48 @@ import (
 // TestAdminUsers_List verifies that an admin can list all users in the
 // system with their details including account role and type.
 func TestAdminUsers_List(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

-	// Create test users
-	_, err := db.DB.Exec(context.Background(), `
+	// Create test users with name history
+	var ninaID, bobID string
+	err := db.DB.QueryRow(context.Background(), `
 		INSERT INTO users (n_first_name, n_last_name, email, phone, date_of_birth, password_hash, account_role, account_type)
-		VALUES
-			('Nina', 'Smith', 'nina@test.com', '+447123456789', '1990-01-01', 'hash1', 'admin', 'email'),
-			('Bob', 'Jones', 'bob@test.com', '+447123456789', '1990-01-01', 'hash2', 'verified_email', 'email'),
-			('Charlie', 'Brown', 'charlie@test.com', '+447123456789', '1990-01-01', 'hash3', 'verified_email', 'email')
-	`)
+		VALUES ('Nina', 'Smith', 'nina@test.com', '+447123456789', '1990-01-01', 'hash1', 'admin', 'email')
+		RETURNING id
+	`).Scan(&ninaID)
 	if err != nil {
-		t.Fatalf("failed to create users: %v", err)
+		t.Fatalf("failed to create nina: %v", err)
 	}

+	err = db.DB.QueryRow(context.Background(), `
+		INSERT INTO users (n_first_name, n_last_name, email, phone, date_of_birth, password_hash, account_role, account_type)
+		VALUES ('Bob', 'Jones', 'bob@test.com', '+447123456789', '1990-01-01', 'hash2', 'verified_email', 'email')
+		RETURNING id
+	`).Scan(&bobID)
+	if err != nil {
+		t.Fatalf("failed to create bob: %v", err)
+	}
+
+	// Create a completed booking for Nina so she has a completed_count
+	_, err = db.DB.Exec(context.Background(), `
+		INSERT INTO bookings (user_id, start_time, status) VALUES ($1, NOW(), 'completed')
+	`, ninaID)
+	if err != nil {
+		t.Fatalf("failed to create booking for nina: %v", err)
+	}
+
+	// Insert name history for Bob (previous name that differs from current)
+	_, err = db.DB.Exec(context.Background(), `
+		INSERT INTO name_history (user_id, previous_first_name, previous_last_name)
+		VALUES ($1, 'Bobby', 'Jones')
+	`, bobID)
+	if err != nil {
+		t.Fatalf("failed to insert name_history for bob: %v", err)
+	}
+
+	userID, _ := json.Marshal(bobID)
+	_ = userID
+
 	handler := http.HandlerFunc(user.ListAdminUsersHandler)
 	w := makeAdminRequest(handler, "GET", "/api/admin/users", nil)

@@ -55,19 +85,82 @@ func TestAdminUsers_List(t *testing.T) {
 		t.Fatalf("failed to unmarshal response: %v", err)
 	}

-	if response.Total != 3 {
-		t.Errorf("expected 3 users, got %d", response.Total)
+	if response.Total != 2 {
+		t.Errorf("expected 2 users, got %d", response.Total)
 	}

-	if len(response.Users) != 3 {
-		t.Errorf("expected 3 users in list, got %d", len(response.Users))
+	if len(response.Users) != 2 {
+		t.Errorf("expected 2 users in list, got %d", len(response.Users))
+	}
+
+	// Verify name history and completed_count in user list
+	var bobFound bool
+	for _, u := range response.Users {
+		if u.ID == bobID {
+			bobFound = true
+			if u.PreviousFirstName == nil || *u.PreviousFirstName != "Bobby" {
+				t.Errorf("expected bob previousFirstName 'Bobby', got %v", u.PreviousFirstName)
+			}
+			if u.PreviousLastName == nil || *u.PreviousLastName != "Jones" {
+				t.Errorf("expected bob previousLastName 'Jones', got %v", u.PreviousLastName)
+			}
+		}
+		if u.ID == ninaID {
+			if u.CompletedCount != 1 {
+				t.Errorf("expected nina completed_count 1, got %d", u.CompletedCount)
+			}
+			// Nina has no name history — should be nil
+			if u.PreviousFirstName != nil {
+				t.Errorf("expected nina previousFirstName nil, got %v", *u.PreviousFirstName)
+			}
+		}
+	}
+	if !bobFound {
+		t.Error("expected bob in user list")
+	}
+}
+
+// TestAdminUsers_List_Page verifies the page parameter is echoed in the response.
+// Note: The user list uses cursor-based pagination, not offset-based, so page
+// is metadata only — actual page navigation is driven by the next_cursor field.
+func TestAdminUsers_List_Page(t *testing.T) {
+	testutils.SetupTestDB(t)
+
+	for i := 0; i < 5; i++ {
+		_, err := db.DB.Exec(context.Background(), `
+			INSERT INTO users (n_first_name, n_last_name, email, phone, date_of_birth, password_hash, account_role, account_type)
+			VALUES ('User', $1, $2, '+447123456789', '1990-01-01', 'hash', 'verified_email', 'email')
+		`, fmt.Sprintf("LastName_%d", i), fmt.Sprintf("user%d@test.com", i))
+		if err != nil {
+			t.Fatalf("failed to create user %d: %v", i, err)
+		}
+	}
+
+	handler := http.HandlerFunc(user.ListAdminUsersHandler)
+
+	w := makeAdminRequest(handler, "GET", "/api/admin/users?page=2", nil)
+	if w.Code != http.StatusOK {
+		t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
+	}
+	var resp user.UserListResponse
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if resp.Page != 2 {
+		t.Errorf("expected page 2 echoed back, got %d", resp.Page)
+	}
+	if resp.Total != 5 {
+		t.Errorf("expected total 5, got %d", resp.Total)
+	}
+	if resp.PerPage == 0 {
+		t.Error("expected per_page to be set")
 	}
 }

 // TestAdminUsers_Get tests that an admin can retrieve detailed information
 // about a specific user including their profile and account settings.
 func TestAdminUsers_Get(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create test user
 	var userID string
@@ -104,7 +197,7 @@ func TestAdminUsers_Get(t *testing.T) {
 // TestAdminUsers_Get_NotFound verifies that requesting details for a
 // non-existent user returns HTTP 404 Not Found.
 func TestAdminUsers_Get_NotFound(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	handler := http.HandlerFunc(user.GetAdminUserHandler)
 	// Use 12-char or less ID to avoid CHAR(12) constraint error
@@ -119,7 +212,7 @@ func TestAdminUsers_Get_NotFound(t *testing.T) {
 // identifies which services require patch tests and returns only those services
 // the user is eligible for based on age requirements.
 func TestAdminUsers_PatchTests_Eligible(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create test user
 	var userID string
@@ -195,7 +288,7 @@ func TestAdminUsers_PatchTests_Eligible(t *testing.T) {
 // user already has a valid patch test on file, that service is filtered out
 // from the eligible list (since they've already completed it).
 func TestAdminUsers_PatchTests_Eligible_WithExisting(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create test user
 	var userID string
@@ -281,7 +374,7 @@ func TestAdminUsers_PatchTests_Eligible_WithExisting(t *testing.T) {
 // TestAdminUsers_AddPatchTest verifies that an admin can record a patch
 // test completion for a user, creating a user_patch_tests record.
 func TestAdminUsers_AddPatchTest(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create test user
 	var userID string
@@ -340,7 +433,7 @@ func TestAdminUsers_AddPatchTest(t *testing.T) {
 }

 func TestAdminUsers_AddPatchTest_InvalidPatchTest(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create test user
 	var userID string
@@ -367,7 +460,7 @@ func TestAdminUsers_AddPatchTest_InvalidPatchTest(t *testing.T) {
 // TestAdminUsers_NonAdmin verifies that non-admin users receive HTTP 403
 // Forbidden when attempting to list users, get user details, or manage patch tests.
 func TestAdminUsers_NonAdmin(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create regular user in DB
 	_, err := db.DB.Exec(context.Background(), `
@@ -421,7 +514,7 @@ func TestAdminUsers_NonAdmin(t *testing.T) {
 // TestAdminUsers_Get_Success is an additional test verifying admin can
 // retrieve user details including ID, name, email, and account role.
 func TestAdminUsers_Get_Success(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create a test user
 	var userID string
@@ -434,6 +527,15 @@ func TestAdminUsers_Get_Success(t *testing.T) {
 		t.Fatalf("failed to create test user: %v", err)
 	}

+	// Insert name history (simulating a previous name change)
+	_, err = db.DB.Exec(context.Background(), `
+		INSERT INTO name_history (user_id, previous_first_name, previous_last_name)
+		VALUES ($1, 'OldFirst', 'OldLast')
+	`, userID)
+	if err != nil {
+		t.Fatalf("failed to insert name_history: %v", err)
+	}
+
 	// Call admin get user endpoint
 	handler := http.HandlerFunc(user.GetAdminUserHandler)
 	w := makeAdminRequest(handler, "GET", "/api/admin/users/"+userID, nil)
@@ -464,12 +566,60 @@ func TestAdminUsers_Get_Success(t *testing.T) {
 	if resp.AccountRole != "verified_email" {
 		t.Errorf("expected account_role 'verified_email', got '%s'", resp.AccountRole)
 	}
+
+	// Verify previous name from history is returned
+	if resp.PreviousFirstName == nil || *resp.PreviousFirstName != "OldFirst" {
+		t.Errorf("expected previousFirstName 'OldFirst', got %v", resp.PreviousFirstName)
+	}
+	if resp.PreviousLastName == nil || *resp.PreviousLastName != "OldLast" {
+		t.Errorf("expected previousLastName 'OldLast', got %v", resp.PreviousLastName)
+	}
+}
+
+// TestAdminUsers_Get_ShowsPreviousNameOnlyWhenDifferent verifies that previous
+// name is omitted when the name_history entry matches the current user name.
+func TestAdminUsers_Get_ShowsPreviousNameOnlyWhenDifferent(t *testing.T) {
+	testutils.SetupTestDB(t)
+
+	var userID string
+	err := db.DB.QueryRow(context.Background(), `
+		INSERT INTO users (n_first_name, n_last_name, email, phone, date_of_birth, password_hash, account_role, account_type)
+		VALUES ('Alice', 'Smith', 'alice@test.com', '+447123456789', '1990-01-01', 'hash', 'verified_email', 'email')
+		RETURNING id
+	`).Scan(&userID)
+	if err != nil {
+		t.Fatalf("failed to create user: %v", err)
+	}
+
+	// Insert name_history with the SAME name as current — should be omitted
+	_, err = db.DB.Exec(context.Background(), `
+		INSERT INTO name_history (user_id, previous_first_name, previous_last_name)
+		VALUES ($1, 'Alice', 'Smith')
+	`, userID)
+	if err != nil {
+		t.Fatalf("failed to insert name_history: %v", err)
+	}
+
+	handler := http.HandlerFunc(user.GetAdminUserHandler)
+	w := makeAdminRequest(handler, "GET", "/api/admin/users/"+userID, nil)
+
+	var resp user.AdminUserDetail
+	if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
+		t.Fatalf("failed to unmarshal: %v", err)
+	}
+
+	if resp.PreviousFirstName != nil {
+		t.Errorf("expected previousFirstName nil (same as current), got %v", *resp.PreviousFirstName)
+	}
+	if resp.PreviousLastName != nil {
+		t.Errorf("expected previousLastName nil (same as current), got %v", *resp.PreviousLastName)
+	}
 }

 // TestAdminUsers_AddPatchTest_Duplicate verifies that recording the same patch test
 // twice updates the tested_at timestamp (upsert behavior).
 func TestAdminUsers_AddPatchTest_Duplicate(t *testing.T) {
-	resetTestData(t)
+	testutils.SetupTestDB(t)

 	// Create test user
 	var userID string