fix(backend): delete admin_notifications before booking hard-delete
The admin_notifications.booking_id FK has no ON DELETE CASCADE, so hard-deleting a booking with existing notifications fails with a FK constraint violation. Fix by explicitly deleting related notifications before the booking DELETE. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -2879,32 +2879,12 @@ func DeleteBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
defer tx.Rollback(r.Context())
|
defer tx.Rollback(r.Context())
|
||||||
|
|
||||||
var originalStatus string
|
if _, err := tx.Exec(r.Context(), `DELETE FROM admin_notifications WHERE booking_id = $1`, bookingID); err != nil {
|
||||||
if err := tx.QueryRow(r.Context(), "SELECT status FROM bookings WHERE id = $1 AND user_id = $2", bookingID, userID).Scan(&originalStatus); err != nil {
|
log.Printf("Failed to delete admin notifications for booking %s: %v", bookingID, err)
|
||||||
if errors.Is(err, sql.ErrNoRows) {
|
|
||||||
http.Error(w, "Booking not found or access denied", http.StatusNotFound)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
log.Printf("Failed to get booking status %s: %v", bookingID, err)
|
|
||||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := notifications.AcknowledgePendingBookingNotification(tx, r.Context(), bookingID); err != nil {
|
|
||||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
if originalStatus != "pending" {
|
|
||||||
if _, err := tx.Exec(r.Context(), `
|
|
||||||
INSERT INTO admin_notifications (reason, booking_id, user_id) VALUES ($1, $2, $3)
|
|
||||||
`, "cancelled_booking", bookingID, userID); err != nil {
|
|
||||||
log.Printf("Failed to create admin notification for booking %s: %v", bookingID, err)
|
|
||||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
result, err := tx.Exec(r.Context(), "DELETE FROM bookings WHERE id = $1 AND user_id = $2", bookingID, userID)
|
result, err := tx.Exec(r.Context(), "DELETE FROM bookings WHERE id = $1 AND user_id = $2", bookingID, userID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Printf("Failed to delete booking %s for user %s: %v", bookingID, userID, err)
|
log.Printf("Failed to delete booking %s for user %s: %v", bookingID, userID, err)
|
||||||
|
|||||||
@@ -934,6 +934,81 @@ func TestBookings_Delete(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestBookings_Delete_WithAdminNotifications verifies that a booking with existing
|
||||||
|
// admin_notifications can be hard-deleted (regression test for FK constraint bug).
|
||||||
|
// Every booking in production has at least one admin_notification row, and the
|
||||||
|
// admin_notifications.booking_id FK has no ON DELETE CASCADE, so the DELETE must
|
||||||
|
// explicitly clean up notifications first.
|
||||||
|
func TestBookings_Delete_WithAdminNotifications(t *testing.T) {
|
||||||
|
resetTestData(t)
|
||||||
|
|
||||||
|
userID, err := fixtures.CreateTestUser(db.DB)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create test user: %v", err)
|
||||||
|
}
|
||||||
|
defer fixtures.DeleteUser(db.DB, userID)
|
||||||
|
|
||||||
|
_, err = db.DB.Exec(context.Background(), "UPDATE users SET deposits_required = 0 WHERE id = $1", userID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to set deposits_required: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
serviceID, err := fixtures.CreateTestService(db.DB)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create test service: %v", err)
|
||||||
|
}
|
||||||
|
defer fixtures.DeleteService(db.DB, serviceID)
|
||||||
|
|
||||||
|
bookingID, err := fixtures.CreateTestBooking(db.DB, userID, serviceID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create test booking: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = db.DB.Exec(context.Background(),
|
||||||
|
`INSERT INTO admin_notifications (reason, booking_id, user_id) VALUES ($1, $2, $3)`,
|
||||||
|
"pending_booking", bookingID, userID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create admin_notification: %v", err)
|
||||||
|
}
|
||||||
|
_, err = db.DB.Exec(context.Background(),
|
||||||
|
`INSERT INTO admin_notifications (reason, booking_id, user_id) VALUES ($1, $2, $3)`,
|
||||||
|
"new_booking", bookingID, userID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create second admin_notification: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
token := jwt.GenerateUserToken(userID)
|
||||||
|
|
||||||
|
handler := http.HandlerFunc(DeleteBookingHandler)
|
||||||
|
w := makeRequest(handler, "DELETE", "/api/bookings/"+bookingID, nil, token)
|
||||||
|
|
||||||
|
if w.Code != http.StatusOK {
|
||||||
|
t.Fatalf("expected status 200, got %d. body: %s", w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify booking was deleted from DB
|
||||||
|
var bookingCount int
|
||||||
|
err = db.DB.QueryRow(context.Background(),
|
||||||
|
"SELECT COUNT(*) FROM bookings WHERE id = $1", bookingID).Scan(&bookingCount)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to query bookings: %v", err)
|
||||||
|
}
|
||||||
|
if bookingCount != 0 {
|
||||||
|
t.Errorf("expected booking to be deleted, but found %d", bookingCount)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify admin_notifications for this booking were also cleaned up
|
||||||
|
var notifCount int
|
||||||
|
err = db.DB.QueryRow(context.Background(),
|
||||||
|
"SELECT COUNT(*) FROM admin_notifications WHERE booking_id = $1", bookingID).Scan(¬ifCount)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to query admin_notifications: %v", err)
|
||||||
|
}
|
||||||
|
if notifCount != 0 {
|
||||||
|
t.Errorf("expected admin_notifications to be cleaned up, but found %d", notifCount)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestBookings_Delete_WithReason verifies that cancelling a booking with
|
// TestBookings_Delete_WithReason verifies that cancelling a booking with
|
||||||
// an associated payment requires a reason (client_cancelled). Without a reason,
|
// an associated payment requires a reason (client_cancelled). Without a reason,
|
||||||
// the request fails with HTTP 400. With a reason, the booking is soft-deleted
|
// the request fails with HTTP 400. With a reason, the booking is soft-deleted
|
||||||
|
|||||||
Reference in New Issue
Block a user