popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(bookings): remove AdminEditBookingHandler and streamline booking creation/edit overlap checks

Browse Source 
- manage.go: remove AdminEditBookingHandler (superseded by EditBookingHandler with overlap detection)

- manage.go: move overlap check inside transaction in AdminCreateBookingForUserHandler

- manage.go: add error handling to unchecked QueryRow calls in RequestEditHandler

- manage.go: reorder time_blocker deletion before overlap check in AdminApproveEditRequestHandler

- admin/bookings_test.go: remove tests for removed AdminEditBookingHandler

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
popertots
2026-06-21 21:01:27 +01:00
co-authored by Sisyphus
parent ed14df9a99
commit 96dd9e191c
2 changed files with 56 additions and 483 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/handlers/bookings/manage.go
+56 -212
View File
@@ -344,195 +344,6 @@ func AdminGetInProgressBookingHandler(w http.ResponseWriter, r *http.Request) {
}
}
// AdminEditBookingHandler allows an admin to modify the start time of any booking.
// Admin can edit any booking EXCEPT completed or cancelled bookings.
// Admin can create/edit bookings outside working hours (with warning).
// Admin can create/edit bookings that overlap with existing bookings (with warning).
func AdminEditBookingHandler(w http.ResponseWriter, r *http.Request) {
bookingID := chi.URLParam(r, "id")
if bookingID == "" || !validators.IsValidID(bookingID) {
http.Error(w, "Booking not found", http.StatusNotFound)
return
}
var req EditBookingRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
http.Error(w, "Invalid request body", http.StatusBadRequest)
return
}
// Basic validation: ensure the new time is not in the past
if time.Now().After(req.StartTime) {
http.Error(w, "Start time must be in the future", http.StatusBadRequest)
return
}
// Check if booking exists and is not completed/cancelled
var currentStatus string
err := db.Conn.QueryRow(r.Context(), "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&currentStatus)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
http.Error(w, "Booking not found", http.StatusNotFound)
return
}
log.Printf("Failed to get booking status %s: %v", bookingID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
// Block edits on completed or cancelled bookings
if currentStatus == "completed" || currentStatus == "client_cancelled" || currentStatus == "we_cancelled" {
http.Error(w, "Cannot edit a completed or cancelled booking", http.StatusForbidden)
return
}
// Get booking duration for overlap check
var durationMinutes int
err = db.Conn.QueryRow(r.Context(), `
SELECT COALESCE(SUM(dur), 60) FROM (
SELECT COALESCE(bs.override_duration_minutes, s.duration_minutes) AS dur
FROM booking_services bs JOIN services s ON bs.service_id = s.id WHERE bs.booking_id = $1
UNION ALL
SELECT COALESCE(bcs.override_duration_minutes, cs.duration_minutes)
FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id = cs.id WHERE bcs.booking_id = $1
) sub
`, bookingID).Scan(&durationMinutes)
if err != nil {
log.Printf("Failed to get booking duration %s: %v", bookingID, err)
durationMinutes = 60 // fallback
}
// Check for overlapping bookings (excluding the current booking)
var overlapCount int
newEndTime := req.StartTime.Add(time.Duration(durationMinutes) * time.Minute)
err = db.Conn.QueryRow(r.Context(), `
SELECT COUNT(*) FROM bookings
WHERE id != $1
AND status NOT IN ('completed', 'client_cancelled', 'we_cancelled', 'no_show', 'deposit_lapsed')
AND start_time < $3
AND start_time + (INTERVAL '1 minute' * (
SELECT COALESCE(SUM(dur), 60) FROM (
SELECT COALESCE(bs.override_duration_minutes, s.duration_minutes) AS dur
FROM booking_services bs
JOIN services s ON bs.service_id = s.id
WHERE bs.booking_id = bookings.id
UNION ALL
SELECT COALESCE(bcs.override_duration_minutes, cs.duration_minutes)
FROM booking_custom_services bcs
JOIN custom_services cs ON bcs.custom_service_id = cs.id
WHERE bcs.booking_id = bookings.id
) sub
)) > $2
`, bookingID, req.StartTime, newEndTime).Scan(&overlapCount)
if err != nil {
log.Printf("Failed to check overlap %s: %v", bookingID, err)
}
// Check if salon is closed (exceptional hours) - admin gets warning but can proceed
// DB uses 0=Monday..6=Sunday; Go uses 0=Sunday..6=Saturday. Convert.
weekday := int((req.StartTime.Weekday() + 6) % 7)
bookingTime := req.StartTime.Format("15:04:05")
daysToMonday := int(req.StartTime.Weekday())
if daysToMonday == 0 {
daysToMonday = 7
}
weekStart := req.StartTime.AddDate(0, 0, -daysToMonday+1).Truncate(24 * time.Hour)
// Check if salon is closed (exceptional hours)
var isClosed bool
err = db.Conn.QueryRow(r.Context(), `
SELECT EXISTS (
SELECT 1 FROM exceptional_working_hours ewh
JOIN exceptional_group_applications ega ON ewh.group_id = ega.group_id
WHERE ega.week_start = $1
AND ewh.weekday = $2
AND ewh.is_open = false
AND ewh.start_time <= $3
AND ewh.end_time >= $3
)
`, weekStart, weekday, bookingTime).Scan(&isClosed)
if err != nil {
log.Printf("Failed to check exceptional hours: %v", err)
}
isOutsideWorkingHours := isClosed
// Prevent overlap - block admin
if overlapCount > 0 {
http.Error(w, "This booking overlaps with an existing booking", http.StatusConflict)
return
}
// Build warning for outside working hours (admin can proceed with warning)
var warnings []string
if isOutsideWorkingHours {
warnings = append(warnings, "Warning: This booking is outside standard working hours")
}
// Check for time blocker overlap - admin can proceed with warning
blockerOverlap, blockerDesc, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEndTime)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
}
if blockerOverlap {
warnings = append(warnings, fmt.Sprintf("Warning: This booking overlaps with a time blocker: %s", blockerDesc))
}
// Perform the update
tx, err := db.Conn.Begin(r.Context())
if err != nil {
log.Printf("Failed to start transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
defer tx.Rollback(r.Context())
res, err := tx.Exec(r.Context(), `
UPDATE bookings
SET start_time = $1, updated_at = $2
WHERE id = $3
`, req.StartTime, time.Now(), bookingID)
if err != nil {
log.Printf("Failed to edit booking %s: %v", bookingID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
rowsAffected := res.RowsAffected()
if rowsAffected == 0 {
http.Error(w, "Booking not found", http.StatusNotFound)
return
}
// Clear any pending edit requests for this booking (admin edit takes priority)
_, err = tx.Exec(r.Context(), `
DELETE FROM booking_edit_requests
WHERE booking_id = $1
`, bookingID)
if err != nil {
log.Printf("Failed to clear edit requests for booking %s: %v", bookingID, err)
// Don't fail the request, just log the error
}
if err := tx.Commit(r.Context()); err != nil {
log.Printf("Failed to commit transaction: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
// TODO: Notify user that their edit request was superseded by admin direct edit (blocked on E5 SMTP)
// Return warnings if any
if len(warnings) > 0 {
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]interface{}{
"message": "Booking updated",
"warnings": warnings,
})
return
}
w.WriteHeader(http.StatusNoContent)
}
type AdminCreateBookingForUserRequest struct {
UserID string `json:"user_id" validate:"required"`
StartTime time.Time `json:"start_time" validate:"required"`
@@ -830,23 +641,19 @@ func AdminCreateBookingForUserHandler(w http.ResponseWriter, r *http.Request) {
// Check for overlapping confirmed/in_progress/completed bookings
allIDs := append(req.ServiceIDs, req.CustomServiceIDs...)
var dur int
db.Conn.QueryRow(r.Context(), `
err := db.Conn.QueryRow(r.Context(), `
SELECT COALESCE(SUM(dur), 0) FROM (
SELECT duration_minutes AS dur FROM services WHERE id = ANY($1)
UNION ALL
SELECT duration_minutes FROM custom_services WHERE id = ANY($1)
) combined
`, allIDs).Scan(&dur)
newEnd := req.StartTime.Add(time.Duration(dur) * time.Minute)
var cnt int
db.Conn.QueryRow(r.Context(), `
SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed') AND start_time < $2 AND start_time + (INTERVAL '1 minute' * (SELECT COALESCE(SUM(dur),60) FROM (SELECT COALESCE(bs.override_duration_minutes,s.duration_minutes) AS dur FROM booking_services bs JOIN services s ON bs.service_id=s.id WHERE bs.booking_id=bookings.id UNION ALL SELECT COALESCE(bcs.override_duration_minutes,cs.duration_minutes) FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id=cs.id WHERE bcs.booking_id=bookings.id) sub)) > $1
`, req.StartTime, newEnd).Scan(&cnt)
if cnt > 0 {
http.Error(w, "Cannot create booking - time slot overlaps with existing booking", http.StatusConflict)
if err != nil {
log.Printf("Failed to calculate duration: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
newEnd := req.StartTime.Add(time.Duration(dur) * time.Minute)
// Check for time blocker overlap - admin can proceed with warning
blockerOverlap, blockerDesc, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEnd)
@@ -862,8 +669,25 @@ func AdminCreateBookingForUserHandler(w http.ResponseWriter, r *http.Request) {
}
defer tx.Rollback(r.Context())
// Check for overlapping confirmed/in_progress/completed bookings (inside transaction)
var cnt int
err = tx.QueryRow(r.Context(), `
SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed') AND start_time < $2 AND start_time + (INTERVAL '1 minute' * (SELECT COALESCE(SUM(dur),60) FROM (SELECT COALESCE(bs.override_duration_minutes,s.duration_minutes) AS dur FROM booking_services bs JOIN services s ON bs.service_id=s.id WHERE bs.booking_id=bookings.id UNION ALL SELECT COALESCE(bcs.override_duration_minutes,cs.duration_minutes) FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id=cs.id WHERE bcs.booking_id=bookings.id) sub)) > $1
`, req.StartTime, newEnd).Scan(&cnt)
if err != nil {
log.Printf("Failed to check overlap: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if cnt > 0 {
http.Error(w, "Cannot create booking - time slot overlaps with existing booking", http.StatusConflict)
return
}
if _, evictErr := EvictPendingReleaseOverlapping(r.Context(), tx, req.StartTime, newEnd); evictErr != nil {
log.Printf("Failed to evict pending_release bookings (admin create): %v", evictErr)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
// Create booking directly as confirmed
@@ -1587,12 +1411,15 @@ func RequestEditHandler(w http.ResponseWriter, r *http.Request) {
// Calculate duration for the new time
var durMinutes int
if len(req.NewServices) > 0 {
_ = tx.QueryRow(r.Context(), `
if err := tx.QueryRow(r.Context(), `
SELECT COALESCE(SUM(s.duration_minutes), 60)
FROM services s WHERE s.id = ANY($1)
`, req.NewServices).Scan(&durMinutes)
`, req.NewServices).Scan(&durMinutes); err != nil {
log.Printf("Failed to get duration for new services: %v", err)
durMinutes = 60
}
} else {
_ = tx.QueryRow(r.Context(), `
if err := tx.QueryRow(r.Context(), `
SELECT COALESCE(SUM(dur), 60) FROM (
SELECT COALESCE(bs.override_duration_minutes, s.duration_minutes) AS dur
FROM booking_services bs JOIN services s ON bs.service_id = s.id WHERE bs.booking_id = $1
@@ -1600,7 +1427,10 @@ func RequestEditHandler(w http.ResponseWriter, r *http.Request) {
SELECT COALESCE(bcs.override_duration_minutes, cs.duration_minutes)
FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id = cs.id WHERE bcs.booking_id = $1
) sub
`, bookingID).Scan(&durMinutes)
`, bookingID).Scan(&durMinutes); err != nil {
log.Printf("Failed to get duration for existing services: %v", err)
durMinutes = 60
}
}
if durMinutes <= 0 {
durMinutes = 60
@@ -1609,7 +1439,7 @@ func RequestEditHandler(w http.ResponseWriter, r *http.Request) {
// Quick overlap check — block if slot is taken
newEnd := req.NewStartTime.Add(time.Duration(durMinutes) * time.Minute)
var overlapCount int
tx.QueryRow(r.Context(), `
if err := tx.QueryRow(r.Context(), `
SELECT COUNT(*) FROM bookings
WHERE id != $1
AND status NOT IN ('completed','client_cancelled','we_cancelled','no_show','deposit_lapsed')
@@ -1623,7 +1453,11 @@ func RequestEditHandler(w http.ResponseWriter, r *http.Request) {
FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id = cs.id WHERE bcs.booking_id = bookings.id
) sub
)) > $2
`, bookingID, *req.NewStartTime, newEnd).Scan(&overlapCount)
`, bookingID, *req.NewStartTime, newEnd).Scan(&overlapCount); err != nil {
log.Printf("Failed to check overlap: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if overlapCount > 0 {
http.Error(w, "The requested time slot has been taken. Please choose a different time.", http.StatusConflict)
return
@@ -1915,7 +1749,9 @@ func AdminApproveEditRequestHandler(w http.ResponseWriter, r *http.Request) {
// Check for applied discounts (admin warning only — discounts remain locked in)
var discountCount int
db.Conn.QueryRow(r.Context(), `SELECT COUNT(*) FROM booking_discounts WHERE booking_id = $1`, bookingID).Scan(&discountCount)
if err := db.Conn.QueryRow(r.Context(), `SELECT COUNT(*) FROM booking_discounts WHERE booking_id = $1`, bookingID).Scan(&discountCount); err != nil {
log.Printf("ADMIN APPROVE EDIT: Failed to check discounts: %v", err)
}
if discountCount > 0 {
log.Printf("ADMIN APPROVE EDIT: Booking %s has %d discount(s) applied — discounts remain locked in after reschedule", bookingID, discountCount)
}
@@ -1981,12 +1817,25 @@ func AdminApproveEditRequestHandler(w http.ResponseWriter, r *http.Request) {
`, bookingID, *newStartTime, newEndTime).Scan(&overlapCount)
if err != nil {
log.Printf("Failed to check overlap: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if overlapCount > 0 {
http.Error(w, "This edit would cause an overlap with an existing booking", http.StatusConflict)
return
}
// Delete the edit request's reservation BEFORE checking time blockers.
// The reservation (RESERVATION:edit_request:*) was created by RequestEditHandler
// to temporarily hold the slot. If not removed first, it would show up as a
// blocker and prevent the approve from succeeding.
if _, delErr := tx.Exec(r.Context(), `
DELETE FROM time_blockers
WHERE description = $1
`, fmt.Sprintf("RESERVATION:edit_request:%s", bookingID)); delErr != nil {
log.Printf("ALERT: failed to delete time_blocker: %v", delErr)
}
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), *newStartTime, newEndTime)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
@@ -2019,6 +1868,8 @@ func AdminApproveEditRequestHandler(w http.ResponseWriter, r *http.Request) {
`, weekStart, weekday, bookingTime).Scan(&isClosed)
if err != nil {
log.Printf("Failed to check exceptional hours: %v", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if isClosed {
@@ -2098,13 +1949,6 @@ func AdminApproveEditRequestHandler(w http.ResponseWriter, r *http.Request) {
return
}
if _, err := tx.Exec(r.Context(), `
DELETE FROM time_blockers
WHERE description = $1
`, fmt.Sprintf("RESERVATION:edit_request:%s", bookingID)); err != nil {
log.Printf("ALERT: failed to delete time_blocker: %v", err)
}
// Acknowledge the admin notification for this edit request
_, err = tx.Exec(r.Context(), `
UPDATE admin_notifications