fix(bookings): add error handling for unchecked QueryRow and improve overlap detection
- admin_reserve.go, reserve.go: wrap unchecked QueryRow.Scan with error handling - bookings.go: replace next-booking overlap check with COUNT(*) in UpdateBookingServicesHandler - bookings.go: wrap EditBookingHandler in a transaction with EvictPendingReleaseOverlapping - bookings.go: add error handling for QueryRow and Evict in ConfirmBookingHandler - bookings.go: add error handling and eviction reorder in AdminRescheduleBookingHandler Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -129,7 +129,7 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
var cnt int
|
var cnt int
|
||||||
db.Conn.QueryRow(r.Context(), `
|
if err := db.Conn.QueryRow(r.Context(), `
|
||||||
SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed')
|
SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed')
|
||||||
AND start_time < $2
|
AND start_time < $2
|
||||||
AND start_time + (INTERVAL '1 minute' * (
|
AND start_time + (INTERVAL '1 minute' * (
|
||||||
@@ -141,7 +141,11 @@ func AdminReserveSlotHandler(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
|
FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id=cs.id WHERE bcs.booking_id=bookings.id
|
||||||
) sub
|
) sub
|
||||||
)) > $1
|
)) > $1
|
||||||
`, req.StartTime, endTime).Scan(&cnt)
|
`, req.StartTime, endTime).Scan(&cnt); err != nil {
|
||||||
|
log.Printf("Failed to check overlap: %v", err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
if cnt > 0 {
|
if cnt > 0 {
|
||||||
http.Error(w, "Cannot book this time - slot overlaps with an existing booking", http.StatusConflict)
|
http.Error(w, "Cannot book this time - slot overlaps with an existing booking", http.StatusConflict)
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -1434,22 +1434,29 @@ func UpdateBookingServicesHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
newEndTime := startTime.Add(time.Duration(newTotalDuration) * time.Minute)
|
newEndTime := startTime.Add(time.Duration(newTotalDuration) * time.Minute)
|
||||||
|
|
||||||
var nextBookingStart *time.Time
|
var overlapCount int
|
||||||
err := db.Conn.QueryRow(r.Context(), `
|
err := db.Conn.QueryRow(r.Context(), `
|
||||||
SELECT start_time FROM bookings
|
SELECT COUNT(*) FROM bookings
|
||||||
WHERE start_time > $1
|
WHERE id != $1
|
||||||
AND status IN ('confirmed', 'pending', 'in_progress')
|
AND status IN ('confirmed', 'pending', 'in_progress', 'completed')
|
||||||
ORDER BY start_time ASC
|
AND start_time < $3
|
||||||
LIMIT 1
|
AND start_time + (INTERVAL '1 minute' * (
|
||||||
`, startTime).Scan(&nextBookingStart)
|
SELECT COALESCE(SUM(dur), 60) FROM (
|
||||||
if err != nil && !errors.Is(err, pgx.ErrNoRows) {
|
SELECT COALESCE(bs.override_duration_minutes, s.duration_minutes) AS dur
|
||||||
log.Printf("Failed to check next booking: %v", err)
|
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, startTime, newEndTime).Scan(&overlapCount)
|
||||||
|
if err != nil {
|
||||||
|
log.Printf("Failed to check overlap: %v", err)
|
||||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
if overlapCount > 0 {
|
||||||
if nextBookingStart != nil && newEndTime.After(*nextBookingStart) {
|
http.Error(w, "New booking duration overlaps with an existing booking", http.StatusConflict)
|
||||||
http.Error(w, fmt.Sprintf("New booking duration overlaps with next appointment starting at %s", nextBookingStart.Format(time.RFC3339)), http.StatusConflict)
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2416,8 +2423,24 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
newEndTime := req.StartTime.Add(time.Duration(durationMinutes) * time.Minute)
|
newEndTime := req.StartTime.Add(time.Duration(durationMinutes) * time.Minute)
|
||||||
|
|
||||||
|
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())
|
||||||
|
|
||||||
|
// Evict any pending_release bookings that overlap this slot.
|
||||||
|
if _, evictErr := EvictPendingReleaseOverlapping(r.Context(), tx, req.StartTime, newEndTime); evictErr != nil {
|
||||||
|
log.Printf("Failed to evict pending_release bookings on edit: %v", evictErr)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
var overlapCount int
|
var overlapCount int
|
||||||
if err := db.Conn.QueryRow(r.Context(), `
|
if err := tx.QueryRow(r.Context(), `
|
||||||
SELECT COUNT(*) FROM bookings
|
SELECT COUNT(*) FROM bookings
|
||||||
WHERE id != $1
|
WHERE id != $1
|
||||||
AND status NOT IN ('completed', 'client_cancelled', 'we_cancelled', 'no_show', 'deposit_lapsed')
|
AND status NOT IN ('completed', 'client_cancelled', 'we_cancelled', 'no_show', 'deposit_lapsed')
|
||||||
@@ -2433,6 +2456,8 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
)) > $2
|
)) > $2
|
||||||
`, bookingID, req.StartTime, newEndTime).Scan(&overlapCount); err != nil {
|
`, bookingID, req.StartTime, newEndTime).Scan(&overlapCount); err != nil {
|
||||||
log.Printf("Failed to check overlap %s: %v", bookingID, err)
|
log.Printf("Failed to check overlap %s: %v", bookingID, err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
if overlapCount > 0 {
|
if overlapCount > 0 {
|
||||||
http.Error(w, "Cannot edit - this time slot is taken", http.StatusConflict)
|
http.Error(w, "Cannot edit - this time slot is taken", http.StatusConflict)
|
||||||
@@ -2458,7 +2483,7 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
weekStart := time.Date(tm.Year(), tm.Month(), tm.Day(), 0, 0, 0, 0, tm.Location())
|
weekStart := time.Date(tm.Year(), tm.Month(), tm.Day(), 0, 0, 0, 0, tm.Location())
|
||||||
|
|
||||||
var isClosed bool
|
var isClosed bool
|
||||||
if err := db.Conn.QueryRow(r.Context(), `
|
if err := tx.QueryRow(r.Context(), `
|
||||||
SELECT EXISTS (
|
SELECT EXISTS (
|
||||||
SELECT 1 FROM exceptional_working_hours ewh
|
SELECT 1 FROM exceptional_working_hours ewh
|
||||||
JOIN exceptional_group_applications ega ON ewh.group_id = ega.group_id
|
JOIN exceptional_group_applications ega ON ewh.group_id = ega.group_id
|
||||||
@@ -2470,6 +2495,8 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
)
|
)
|
||||||
`, weekStart, weekday, bookingTime).Scan(&isClosed); err != nil {
|
`, weekStart, weekday, bookingTime).Scan(&isClosed); err != nil {
|
||||||
log.Printf("Failed to check exceptional hours: %v", err)
|
log.Printf("Failed to check exceptional hours: %v", err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
if isClosed {
|
if isClosed {
|
||||||
http.Error(w, "Cannot book on a closed day", http.StatusBadRequest)
|
http.Error(w, "Cannot book on a closed day", http.StatusBadRequest)
|
||||||
@@ -2478,7 +2505,7 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
var booking Booking
|
var booking Booking
|
||||||
booking.User = &UserSummary{}
|
booking.User = &UserSummary{}
|
||||||
if err := db.Conn.QueryRow(r.Context(), `
|
if err := tx.QueryRow(r.Context(), `
|
||||||
UPDATE bookings
|
UPDATE bookings
|
||||||
SET start_time = $1, updated_at = NOW()
|
SET start_time = $1, updated_at = NOW()
|
||||||
WHERE id = $2 AND user_id = $3
|
WHERE id = $2 AND user_id = $3
|
||||||
@@ -2496,6 +2523,12 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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
|
||||||
|
}
|
||||||
|
|
||||||
w.Header().Set("Content-Type", "application/json")
|
w.Header().Set("Content-Type", "application/json")
|
||||||
w.WriteHeader(http.StatusOK)
|
w.WriteHeader(http.StatusOK)
|
||||||
if err := json.NewEncoder(w).Encode(booking); err != nil {
|
if err := json.NewEncoder(w).Encode(booking); err != nil {
|
||||||
@@ -2921,7 +2954,7 @@ func ConfirmBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
db.Conn.QueryRow(r.Context(), `
|
if err := db.Conn.QueryRow(r.Context(), `
|
||||||
SELECT COALESCE(SUM(dur), 60) FROM (
|
SELECT COALESCE(SUM(dur), 60) FROM (
|
||||||
SELECT COALESCE(bs.override_duration_minutes, s.duration_minutes) AS dur
|
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
|
FROM booking_services bs JOIN services s ON bs.service_id = s.id WHERE bs.booking_id = $1
|
||||||
@@ -2929,7 +2962,11 @@ func ConfirmBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
SELECT COALESCE(bcs.override_duration_minutes, cs.duration_minutes)
|
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
|
FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id = cs.id WHERE bcs.booking_id = $1
|
||||||
) sub
|
) sub
|
||||||
`, bookingID).Scan(&dur)
|
`, bookingID).Scan(&dur); err != nil {
|
||||||
|
log.Printf("Failed to calculate duration on confirm: %v", err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
endTime := bkStart.Add(time.Duration(dur) * time.Minute)
|
endTime := bkStart.Add(time.Duration(dur) * time.Minute)
|
||||||
|
|
||||||
tx, err := db.Conn.Begin(r.Context())
|
tx, err := db.Conn.Begin(r.Context())
|
||||||
@@ -2942,6 +2979,8 @@ func ConfirmBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
|
|
||||||
if _, evictErr := EvictPendingReleaseOverlapping(r.Context(), tx, bkStart, endTime); evictErr != nil {
|
if _, evictErr := EvictPendingReleaseOverlapping(r.Context(), tx, bkStart, endTime); evictErr != nil {
|
||||||
log.Printf("Failed to evict pending_release bookings on confirm: %v", evictErr)
|
log.Printf("Failed to evict pending_release bookings on confirm: %v", evictErr)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
var cnt int
|
var cnt int
|
||||||
@@ -4199,14 +4238,17 @@ func AdminRescheduleBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
)
|
)
|
||||||
`, weekStart, weekday, bookingTime).Scan(&isClosed); err != nil {
|
`, weekStart, weekday, bookingTime).Scan(&isClosed); err != nil {
|
||||||
log.Printf("Failed to check exceptional hours: %v", err)
|
log.Printf("Failed to check exceptional hours: %v", err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
if isClosed {
|
if isClosed {
|
||||||
http.Error(w, "Cannot reschedule to a closed day", http.StatusBadRequest)
|
http.Error(w, "Cannot reschedule to a closed day", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if _, evictErr := EvictPendingReleaseOverlapping(r.Context(), tx, req.StartTime, newEndTime); evictErr != nil {
|
if _, evictErr := EvictPendingReleaseOverlapping(r.Context(), tx, req.StartTime, newEndTime); evictErr != nil {
|
||||||
log.Printf("Failed to evict pending_release bookings on reschedule: %v", evictErr)
|
log.Printf("Failed to evict pending_release bookings on reschedule: %v", evictErr)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
var overlapCount int
|
var overlapCount int
|
||||||
@@ -4230,6 +4272,8 @@ func AdminRescheduleBookingHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
)) > $2
|
)) > $2
|
||||||
`, bookingID, req.StartTime, newEndTime).Scan(&overlapCount); err != nil {
|
`, bookingID, req.StartTime, newEndTime).Scan(&overlapCount); err != nil {
|
||||||
log.Printf("Failed to check overlap %s: %v", bookingID, err)
|
log.Printf("Failed to check overlap %s: %v", bookingID, err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
}
|
}
|
||||||
if overlapCount > 0 {
|
if overlapCount > 0 {
|
||||||
http.Error(w, "This time slot overlaps with an existing booking", http.StatusConflict)
|
http.Error(w, "This time slot overlaps with an existing booking", http.StatusConflict)
|
||||||
|
|||||||
@@ -129,7 +129,7 @@ func ReserveSlotHandler(w http.ResponseWriter, r *http.Request) {
|
|||||||
// g. Check existing booking overlap (same query as CreateBookingHandler)
|
// g. Check existing booking overlap (same query as CreateBookingHandler)
|
||||||
endTime := req.StartTime.Add(time.Duration(svcDuration) * time.Minute)
|
endTime := req.StartTime.Add(time.Duration(svcDuration) * time.Minute)
|
||||||
var cnt int
|
var cnt int
|
||||||
db.Conn.QueryRow(r.Context(), `
|
if err := db.Conn.QueryRow(r.Context(), `
|
||||||
SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed')
|
SELECT COUNT(*) FROM bookings WHERE status IN ('pending','confirmed','in_progress','completed')
|
||||||
AND start_time < $2
|
AND start_time < $2
|
||||||
AND start_time + (INTERVAL '1 minute' * (
|
AND start_time + (INTERVAL '1 minute' * (
|
||||||
@@ -141,7 +141,11 @@ func ReserveSlotHandler(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
|
FROM booking_custom_services bcs JOIN custom_services cs ON bcs.custom_service_id=cs.id WHERE bcs.booking_id=bookings.id
|
||||||
) sub
|
) sub
|
||||||
)) > $1
|
)) > $1
|
||||||
`, req.StartTime, endTime).Scan(&cnt)
|
`, req.StartTime, endTime).Scan(&cnt); err != nil {
|
||||||
|
log.Printf("Failed to check overlap: %v", err)
|
||||||
|
http.Error(w, "Internal server error", http.StatusInternalServerError)
|
||||||
|
return
|
||||||
|
}
|
||||||
if cnt > 0 {
|
if cnt > 0 {
|
||||||
http.Error(w, "Cannot book this time - slot overlaps with an existing booking", http.StatusConflict)
|
http.Error(w, "Cannot book this time - slot overlaps with an existing booking", http.StatusConflict)
|
||||||
return
|
return
|
||||||
|
|||||||
Reference in New Issue
Block a user