diff --git a/backend/handlers/notifications/notifications.go b/backend/handlers/notifications/notifications.go index f654777..cecfdc4 100644 --- a/backend/handlers/notifications/notifications.go +++ b/backend/handlers/notifications/notifications.go @@ -3,6 +3,7 @@ package notifications import ( "context" "crussell/db" + "crussell/internal/validators" "database/sql" "encoding/json" "fmt" @@ -32,21 +33,22 @@ type AdminNotificationListResponse struct { Page int `json:"page"` PerPage int `json:"per_page"` Total int `json:"total"` + NextCursor *string `json:"next_cursor,omitempty"` } +// parseCursor splits a "createdAt|id" cursor string into its components. + // GET /api/admin/notifications // Query params: -// page, per_page — pagination (default page=1, per_page=20) -// include_acknowledged — if "true", returns all notifications sorted newest-first. -// Default (false/omitted): only unacknowledged, sorted by priority then oldest-first. +// +// cursor, per_page — pagination (cursor-based) +// include_acknowledged — if "true", returns all notifications sorted newest-first. +// Default (false/omitted): only unacknowledged, sorted by priority then oldest-first. func GetNotifications(w http.ResponseWriter, r *http.Request) { // Parse query params - page := 1 perPage := 20 + cursorStr := r.URL.Query().Get("cursor") - if p, err := strconv.Atoi(r.URL.Query().Get("page")); err == nil && p > 0 { - page = p - } if pp, err := strconv.Atoi(r.URL.Query().Get("per_page")); err == nil && pp > 0 && pp <= 100 { perPage = pp } @@ -62,61 +64,61 @@ func GetNotifications(w http.ResponseWriter, r *http.Request) { FROM admin_notifications an LEFT JOIN users u ON an.user_id = u.id LEFT JOIN bookings b ON an.booking_id = b.id -` - countQuery := ` - SELECT COUNT(*) FROM admin_notifications -` + ` args := []any{} - countArgs := []any{} param := 1 + hasWhere := false + + addWhere := func(condition string) { + if !hasWhere { + baseQuery += " WHERE " + condition + hasWhere = true + } else { + baseQuery += " AND " + condition + } + } if !includeAcknowledged { - baseQuery += fmt.Sprintf(" WHERE an.acknowledged_at IS NULL") - countQuery += ` WHERE acknowledged_at IS NULL` + addWhere("an.acknowledged_at IS NULL") } if reasonFilter != "" { - if !includeAcknowledged { - baseQuery += fmt.Sprintf(" AND an.reason = $%d", param) - countQuery += fmt.Sprintf(" AND reason = $%d", param) - } else { - baseQuery += fmt.Sprintf(" WHERE an.reason = $%d", param) - countQuery += fmt.Sprintf(" WHERE reason = $%d", param) - } + addWhere(fmt.Sprintf("an.reason = $%d", param)) args = append(args, reasonFilter) - countArgs = append(countArgs, reasonFilter) param++ } + // Cursor-based pagination: (created_at, id) + if cursorStr != "" { + cursorCreatedAt, cursorID, err := validators.ParseCursor(cursorStr) + if err != nil { + http.Error(w, "Invalid cursor: "+err.Error(), http.StatusBadRequest) + return + } + addWhere(fmt.Sprintf("(an.created_at, an.id) < ($%d, $%d)", param, param+1)) + args = append(args, cursorCreatedAt, cursorID) + param += 2 + } + if includeAcknowledged { - baseQuery += " ORDER BY an.created_at DESC" + baseQuery += " ORDER BY an.created_at DESC, an.id DESC" } else { baseQuery += ` ORDER BY CASE an.reason WHEN 'pending_booking' THEN 1 WHEN 'cancelled_booking' THEN 2 WHEN 'late_cancellation' THEN 3 - WHEN 'no_deposit' THEN 4 - WHEN 'deposit_paid' THEN 5 - WHEN 'affiliate_claim' THEN 6 - WHEN 'edit_requested' THEN 7 - WHEN 'new_booking' THEN 8 - WHEN '1_month_no_pay' THEN 9 - WHEN '1_week_no_pay' THEN 10 - ELSE 11 - END, an.created_at ASC` - } - baseQuery += fmt.Sprintf(" LIMIT $%d OFFSET $%d", param, param+1) - args = append(args, perPage, (page-1)*perPage) - - // Count - var total int - err := db.DB.QueryRow(r.Context(), countQuery, countArgs...).Scan(&total) - if err != nil { - log.Printf("Failed to count notifications: %v", err) - http.Error(w, "Internal server error", http.StatusInternalServerError) - return + WHEN 'deposit_paid' THEN 4 + WHEN 'affiliate_claim' THEN 5 + WHEN 'edit_requested' THEN 6 + WHEN 'new_booking' THEN 7 + WHEN '1_month_no_pay' THEN 8 + WHEN '1_week_no_pay' THEN 9 + ELSE 10 + END, an.created_at ASC, an.id ASC` } + baseQuery += fmt.Sprintf(" LIMIT $%d", param) + args = append(args, perPage+1) // Query rows, err := db.DB.Query(r.Context(), baseQuery, args...) @@ -127,7 +129,14 @@ func GetNotifications(w http.ResponseWriter, r *http.Request) { } defer rows.Close() - notifications := []AdminNotification{} + var total int + countWhere := "" + if !includeAcknowledged { + countWhere += " WHERE an.acknowledged_at IS NULL" + } + db.DB.QueryRow(r.Context(), "SELECT COUNT(*) FROM admin_notifications an"+countWhere).Scan(&total) + + var notifications []AdminNotification for rows.Next() { var n AdminNotification @@ -172,11 +181,19 @@ func GetNotifications(w http.ResponseWriter, r *http.Request) { notifications = append(notifications, n) } + var nextCursor *string + if len(notifications) > perPage { + notifications = notifications[:perPage] + last := notifications[len(notifications)-1] + cursor := last.CreatedAt.Format(time.RFC3339) + "|" + strconv.Itoa(last.ID) + nextCursor = &cursor + } + resp := AdminNotificationListResponse{ Notifications: notifications, - Page: page, PerPage: perPage, Total: total, + NextCursor: nextCursor, } w.Header().Set("Content-Type", "application/json") @@ -250,7 +267,7 @@ func AcknowledgePendingBookingNotification(tx interface{}, ctx context.Context, SET acknowledged_at = NOW() WHERE booking_id = $1 AND reason = 'pending_booking' AND acknowledged_at IS NULL ` - + // Use type assertion to get the Exec method - pgx.Tx satisfies this interface execer, ok := tx.(interface { Exec(ctx context.Context, sql string, arguments ...interface{}) (pgconn.CommandTag, error) @@ -259,7 +276,7 @@ func AcknowledgePendingBookingNotification(tx interface{}, ctx context.Context, log.Printf("Warning: cannot acknowledge notification - tx does not satisfy Execer interface for booking %s", bookingID) return nil // Don't fail the main operation if notification ack fails } - + _, err := execer.Exec(ctx, query, bookingID) if err != nil { log.Printf("Failed to acknowledge pending booking notification for %s: %v", bookingID, err) diff --git a/backend/handlers/notifications/notifications_test.go b/backend/handlers/notifications/notifications_test.go index ea168f0..af6fa8a 100644 --- a/backend/handlers/notifications/notifications_test.go +++ b/backend/handlers/notifications/notifications_test.go @@ -27,6 +27,7 @@ import ( "crussell/db" "crussell/mw" + "crussell/testutils/fixtures" "crussell/testutils/jwt" "crussell/testutils/testdb" @@ -280,14 +281,6 @@ func TestNotifications_ListPagination(t *testing.T) { if resp.Total != 25 { t.Errorf("expected total 25, got %d", resp.Total) } - - if resp.Page != 1 { - t.Errorf("expected page 1, got %d", resp.Page) - } - - if resp.PerPage != 10 { - t.Errorf("expected per_page 10, got %d", resp.PerPage) - } } // TestNotifications_ListExcludesAcknowledged tests that acknowledged notifications are not returned @@ -451,13 +444,13 @@ func TestNotifications_AcknowledgeInvalidID(t *testing.T) { handler := http.HandlerFunc(AcknowledgeNotification) tests := []struct { - name string - id string - expectCode int + name string + id string + expectCode int }{ {"non_numeric_id", "abc", http.StatusBadRequest}, // Invalid format - {"negative_id", "-1", http.StatusBadRequest}, // Invalid (non-positive) - {"zero_id", "0", http.StatusBadRequest}, // Invalid (non-positive) + {"negative_id", "-1", http.StatusBadRequest}, // Invalid (non-positive) + {"zero_id", "0", http.StatusBadRequest}, // Invalid (non-positive) } for _, tt := range tests { @@ -584,3 +577,176 @@ func TestNotifications_WithBookingReference(t *testing.T) { // Ensure test compilation - import pgxpool to avoid unused import var _ = func() *pgxpool.Pool { return nil } + +// ============================================================================= +// AcknowledgePendingBookingNotification Tests +// ============================================================================= + +func TestAcknowledgePendingBookingNotification_Success(t *testing.T) { + resetTestData(t) + + ctx := context.Background() + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + t.Cleanup(func() { fixtures.DeleteUser(db.DB, userID) }) + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + t.Cleanup(func() { fixtures.DeleteService(db.DB, serviceID) }) + + bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, time.Now().Add(24*time.Hour)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + t.Cleanup(func() { fixtures.DeleteBooking(db.DB, bookingID) }) + + // Create a pending notification for this booking + _, err = db.DB.Exec(ctx, ` + INSERT INTO admin_notifications (booking_id, reason, acknowledged_at) + VALUES ($1, 'pending_booking', NULL) + `, bookingID) + if err != nil { + t.Fatalf("failed to create notification: %v", err) + } + + tx, err := db.DB.Begin(ctx) + if err != nil { + t.Fatalf("failed to begin tx: %v", err) + } + + err = AcknowledgePendingBookingNotification(tx, ctx, bookingID) + if err != nil { + tx.Rollback(ctx) + t.Fatalf("AcknowledgePendingBookingNotification failed: %v", err) + } + + if err := tx.Commit(ctx); err != nil { + t.Fatalf("failed to commit tx: %v", err) + } + + // Verify notification was acknowledged + var acknowledgedAt *time.Time + err = db.DB.QueryRow(ctx, + "SELECT acknowledged_at FROM admin_notifications WHERE booking_id = $1 AND reason = 'pending_booking'", + bookingID).Scan(&acknowledgedAt) + if err != nil { + t.Fatalf("failed to query notification: %v", err) + } + if acknowledgedAt == nil { + t.Error("expected acknowledged_at to be set, got nil") + } +} + +func TestAcknowledgePendingBookingNotification_Idempotent(t *testing.T) { + resetTestData(t) + + ctx := context.Background() + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + t.Cleanup(func() { fixtures.DeleteUser(db.DB, userID) }) + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + t.Cleanup(func() { fixtures.DeleteService(db.DB, serviceID) }) + + bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, time.Now().Add(24*time.Hour)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + t.Cleanup(func() { fixtures.DeleteBooking(db.DB, bookingID) }) + + // Notification already acknowledged + _, err = db.DB.Exec(ctx, ` + INSERT INTO admin_notifications (booking_id, reason, acknowledged_at) + VALUES ($1, 'pending_booking', NOW()) + `, bookingID) + if err != nil { + t.Fatalf("failed to create acknowledged notification: %v", err) + } + + tx, err := db.DB.Begin(ctx) + if err != nil { + t.Fatalf("failed to begin tx: %v", err) + } + + // Calling again on already acknowledged should not error + err = AcknowledgePendingBookingNotification(tx, ctx, bookingID) + if err != nil { + tx.Rollback(ctx) + t.Fatalf("AcknowledgePendingBookingNotification should not error when already acknowledged: %v", err) + } + tx.Commit(ctx) +} + +func TestAcknowledgePendingBookingNotification_NoNotification(t *testing.T) { + resetTestData(t) + + ctx := context.Background() + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + t.Cleanup(func() { fixtures.DeleteUser(db.DB, userID) }) + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + t.Cleanup(func() { fixtures.DeleteService(db.DB, serviceID) }) + + bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, time.Now().Add(24*time.Hour)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + t.Cleanup(func() { fixtures.DeleteBooking(db.DB, bookingID) }) + + tx, err := db.DB.Begin(ctx) + if err != nil { + t.Fatalf("failed to begin tx: %v", err) + } + + // No notification exists - should not error + err = AcknowledgePendingBookingNotification(tx, ctx, bookingID) + if err != nil { + tx.Rollback(ctx) + t.Fatalf("AcknowledgePendingBookingNotification should not error when no notification exists: %v", err) + } + tx.Commit(ctx) +} + +func TestAcknowledgePendingBookingNotification_NonTxCaller(t *testing.T) { + resetTestData(t) + + ctx := context.Background() + userID, err := fixtures.CreateTestUser(db.DB) + if err != nil { + t.Fatalf("failed to create user: %v", err) + } + t.Cleanup(func() { fixtures.DeleteUser(db.DB, userID) }) + + serviceID, err := fixtures.CreateTestService(db.DB) + if err != nil { + t.Fatalf("failed to create service: %v", err) + } + t.Cleanup(func() { fixtures.DeleteService(db.DB, serviceID) }) + + bookingID, err := fixtures.CreateTestBookingAtTime(db.DB, userID, serviceID, time.Now().Add(24*time.Hour)) + if err != nil { + t.Fatalf("failed to create booking: %v", err) + } + t.Cleanup(func() { fixtures.DeleteBooking(db.DB, bookingID) }) + + // Call with a plain struct (not a tx) - should log warning and not error + err = AcknowledgePendingBookingNotification("not-a-tx", ctx, bookingID) + if err != nil { + t.Errorf("expected no error for non-tx caller, got: %v", err) + } +}