fix(ui): improve reservation lifecycle in BookingFlow

- Release reservation on service change, date change, and step-back navigation
- Add synchronous double-click payment guard (isProcessingPaymentSync)
- Immutable update for confirmedBooking to prevent race-condition overcharge
- Generate fresh idempotency key per submission attempt (was reused across
  component lifetime, causing stale-booking illusion on re-submit)
- Release reservation after successful booking submission
- Extract releaseReservation() helper for DRY reservation cleanup
- Race-condition guard in selectTimeWithValidation: pass clicked time
  explicitly so stale validation can't clobber a newer selection
This commit is contained in:
2026-07-06 17:58:14 +01:00
parent 98d561e8b6
commit e831953e5b
3 changed files with 162 additions and 65 deletions
@@ -502,8 +502,16 @@
const endStr = `${endYear}-${String(endMonth).padStart(2, '0')}-${String(daysInEndMonth).padStart(2, '0')}`; const endStr = `${endYear}-${String(endMonth).padStart(2, '0')}-${String(daysInEndMonth).padStart(2, '0')}`;
const [whRes, ahRes] = await Promise.all([ const [whRes, ahRes] = await Promise.all([
fetch(`/api/scheduling/working-hours?start=${startStr}&end=${endStr}`), fetch(`/api/scheduling/working-hours?start=${startStr}&end=${endStr}`, {
fetch(`/api/scheduling/available-hours?start=${startStr}&end=${endStr}`) headers: authStore.currentToken
? { Authorization: `Bearer ${authStore.currentToken}` }
: {}
}),
fetch(`/api/scheduling/available-hours?start=${startStr}&end=${endStr}`, {
headers: authStore.currentToken
? { Authorization: `Bearer ${authStore.currentToken}` }
: {}
})
]); ]);
if (whRes.ok && ahRes.ok) { if (whRes.ok && ahRes.ok) {
@@ -570,8 +578,16 @@
const endStr = endOfMonth.toString(); const endStr = endOfMonth.toString();
const [whRes, ahRes] = await Promise.all([ const [whRes, ahRes] = await Promise.all([
fetch(`/api/scheduling/working-hours?start=${startStr}&end=${endStr}`), fetch(`/api/scheduling/working-hours?start=${startStr}&end=${endStr}`, {
fetch(`/api/scheduling/available-hours?start=${startStr}&end=${endStr}`) headers: authStore.currentToken
? { Authorization: `Bearer ${authStore.currentToken}` }
: {}
}),
fetch(`/api/scheduling/available-hours?start=${startStr}&end=${endStr}`, {
headers: authStore.currentToken
? { Authorization: `Bearer ${authStore.currentToken}` }
: {}
})
]); ]);
if (whRes.ok && ahRes.ok) { if (whRes.ok && ahRes.ok) {
@@ -4,7 +4,7 @@
import { authStore } from '$lib/stores/auth.svelte'; import { authStore } from '$lib/stores/auth.svelte';
import { CalendarDate } from '@internationalized/date'; import { CalendarDate } from '@internationalized/date';
import { SvelteDate } from 'svelte/reactivity'; import { SvelteDate } from 'svelte/reactivity';
import { onMount } from 'svelte'; import { onMount, onDestroy } from 'svelte';
import { toast } from 'svelte-sonner'; import { toast } from 'svelte-sonner';
import type { AvailableHoursDay, Service } from '$lib/types/booking'; import type { AvailableHoursDay, Service } from '$lib/types/booking';
@@ -66,6 +66,43 @@
return () => clearInterval(interval); return () => clearInterval(interval);
}); });
// Release any held reservation if the component unmounts while a slot
// is still reserved (e.g. admin navigates away from the page). Best-effort
// — TTL cleanup will eventually run if this fails.
onDestroy(() => {
if (typeof window !== 'undefined' && window.__walkInCountdownInterval) {
clearInterval(window.__walkInCountdownInterval);
}
if (_reservationId) {
releaseWalkInReservation();
}
});
async function releaseWalkInReservation() {
if (!_reservationId) return;
const idToRelease = _reservationId;
// Clear local state first so a slow DELETE doesn't block the UI.
_reservationId = null;
reservationExpiresAt = null;
_reservationCountdown = '';
reservedDuration = 0;
reservedStartTime = null;
if (window.__walkInCountdownInterval) {
clearInterval(window.__walkInCountdownInterval);
}
try {
const res = await fetch('/api/admin/bookings/reserve', {
method: 'DELETE',
headers: { Authorization: `Bearer ${authStore.currentToken}` }
});
if (!res.ok && res.status !== 404) {
console.warn('Failed to release walk-in reservation', idToRelease, res.status);
}
} catch (e) {
console.warn('Error releasing walk-in reservation', idToRelease, e);
}
}
function timeToMinutes(time: string): number { function timeToMinutes(time: string): number {
const parts = time.split(':').map(Number); const parts = time.split(':').map(Number);
return parts[0] * 60 + parts[1]; return parts[0] * 60 + parts[1];
@@ -375,14 +412,7 @@
function handleModalClose() { function handleModalClose() {
showCreateModal = false; showCreateModal = false;
_reservationId = null; releaseWalkInReservation();
reservationExpiresAt = null;
_reservationCountdown = '';
reservedDuration = 0;
reservedStartTime = null;
if (window.__walkInCountdownInterval) {
clearInterval(window.__walkInCountdownInterval);
}
} }
</script> </script>
@@ -40,6 +40,7 @@
import UserPaymentModal from '$lib/components/payments/UserPaymentModal.svelte'; import UserPaymentModal from '$lib/components/payments/UserPaymentModal.svelte';
import { extractBookedSlots, getLunchProtectionForSlots } from '$lib/lunchProtection'; import { extractBookedSlots, getLunchProtectionForSlots } from '$lib/lunchProtection';
import { formatLocalDateTime, getLondonTodayCalendarDate } from '$lib/utils/timeSlots'; import { formatLocalDateTime, getLondonTodayCalendarDate } from '$lib/utils/timeSlots';
import { generateUUID } from '$lib/utils/uuid';
import type { import type {
Service, Service,
@@ -88,6 +89,11 @@
let selectedPaymentMethod = $state<string | null>(null); let selectedPaymentMethod = $state<string | null>(null);
let showNewCardForm = $state(false); let showNewCardForm = $state(false);
let isProcessingPayment = $state(false); let isProcessingPayment = $state(false);
// Synchronous double-click guard. Svelte 5 reactivity is async (effects run
// on the next microtask), so `isProcessingPayment` may not propagate to the
// button's `disabled` binding before a fast second click fires. This non-
// reactive flag is checked synchronously at the start of processPayment.
let isProcessingPaymentSync = false;
// New card form fields // New card form fields
let newCardNumber = $state(''); let newCardNumber = $state('');
@@ -282,6 +288,11 @@
} }
async function processPayment(amount: number) { async function processPayment(amount: number) {
// Synchronous double-click guard — set BEFORE any await so a rapid second
// click is rejected immediately, even before the reactive `disabled` has
// propagated to the button.
if (isProcessingPaymentSync) return;
isProcessingPaymentSync = true;
isProcessingPayment = true; isProcessingPayment = true;
paymentAttempted = false; paymentAttempted = false;
try { try {
@@ -296,7 +307,7 @@
const body: Record<string, unknown> = { const body: Record<string, unknown> = {
payment_type: 'deposit', payment_type: 'deposit',
amount: amountCents, amount: amountCents,
idempotency_key: crypto.randomUUID?.() ?? Date.now().toString() idempotency_key: generateUUID()
}; };
if (selectedPaymentMethod) { if (selectedPaymentMethod) {
@@ -321,9 +332,16 @@
if (response.ok) { if (response.ok) {
depositPaid = true; depositPaid = true;
confirmedBooking.deposit_paid = true; // Immutable update — avoid mutating the existing object so
confirmedBooking.amount_paid = (confirmedBooking.amount_paid || 0) + amount; // concurrent renders (e.g. a stale fetch) can't observe partial
confirmedBooking.amount_due = Math.max(0, (confirmedBooking.amount_due || 0) - amount); // state. (See audit: HIGH issue #3 — confirmedBooking mutated
// in place, potential overcharge on double-click race.)
confirmedBooking = {
...confirmedBooking,
deposit_paid: true,
amount_paid: (confirmedBooking.amount_paid || 0) + amount,
amount_due: Math.max(0, (confirmedBooking.amount_due || 0) - amount)
};
toast.success('Payment successful!'); toast.success('Payment successful!');
} else { } else {
const text = await response.text(); const text = await response.text();
@@ -335,6 +353,7 @@
); );
} finally { } finally {
isProcessingPayment = false; isProcessingPayment = false;
isProcessingPaymentSync = false;
} }
} }
@@ -416,6 +435,34 @@
} }
} }
// Best-effort release of the currently held reservation. Idempotent: safe
// to call when no reservation exists. Clears all reservation state so a
// subsequent reservation must be re-acquired.
async function releaseReservation() {
if (!_reservationId) return;
const idToRelease = _reservationId;
// Clear local state first so a slow DELETE doesn't block the UI.
_reservationId = null;
reservationExpiresAt = null;
reservationCountdown = '';
reservationExpired = false;
_reservedSlotTime = null;
_reservedSlotDate = null;
if (window.__bookingFlowCountdownInterval) {
clearInterval(window.__bookingFlowCountdownInterval);
window.__bookingFlowCountdownInterval = null;
}
try {
const res = await fetch('/api/bookings/reserve', {
method: 'DELETE',
headers: authStore.currentToken ? { Authorization: `Bearer ${authStore.currentToken}` } : {}
});
if (!res.ok) console.warn('Failed to release reservation', idToRelease, res.status);
} catch (e) {
console.warn('Error releasing reservation', idToRelease, e);
}
}
function startCountdown() { function startCountdown() {
if (!reservationExpiresAt) return; if (!reservationExpiresAt) return;
@@ -1153,6 +1200,12 @@
selectedServices = [...selectedServices, service]; selectedServices = [...selectedServices, service];
} }
// Service change invalidates any held reservation (duration may have changed).
// Release before re-fetching hours so the next reservation starts fresh.
if (currentStep >= 2 && _reservationId) {
releaseReservation();
}
// Only clear if we're on the date/time selection step // Only clear if we're on the date/time selection step
if (currentStep === 2 && selectedDate) { if (currentStep === 2 && selectedDate) {
const monthKey = `${selectedDate.year}-${String(selectedDate.month).padStart(2, '0')}`; const monthKey = `${selectedDate.year}-${String(selectedDate.month).padStart(2, '0')}`;
@@ -1193,14 +1246,30 @@
// Select a time slot with server-side re-validation // Select a time slot with server-side re-validation
async function selectTimeWithValidation(time: string) { async function selectTimeWithValidation(time: string) {
// Race-condition guard: capture the user's click and pass it explicitly
// to the validator. If the user clicks a different time before this
// validation completes, the stale result must not clobber the newer
// selection.
selectedTime = time; selectedTime = time;
await refreshAndValidateSlot();
// Time change invalidates any held reservation (slot is now stale).
// Only release if the new time differs from the reserved one — if the
// user is re-clicking the same reserved time, keep the reservation.
if (_reservationId && time !== _reservedSlotTime) {
await releaseReservation();
} }
// Re-fetch available hours silently and check if selectedTime is still available await refreshAndValidateSlot(time);
}
// Re-fetch available hours silently and check if the requested time is still
// available. `validateForTime` should be the time the user clicked, NOT the
// current selectedTime — this prevents a slow validation for a stale click
// from clearing a newer selection.
// Uses skipLoadingFlags=true to prevent UI judder (loading spinners hide DatePicker/TimeSlotPicker) // Uses skipLoadingFlags=true to prevent UI judder (loading spinners hide DatePicker/TimeSlotPicker)
async function refreshAndValidateSlot() { async function refreshAndValidateSlot(validateForTime: string | null = null) {
if (!selectedDate || !selectedTime) return; const timeToCheck = validateForTime ?? selectedTime;
if (!selectedDate || !timeToCheck) return false;
const monthKey = `${selectedDate.year}-${String(selectedDate.month).padStart(2, '0')}`; const monthKey = `${selectedDate.year}-${String(selectedDate.month).padStart(2, '0')}`;
delete availableHoursCache[monthKey]; delete availableHoursCache[monthKey];
@@ -1210,12 +1279,12 @@
const dayAvailable = availableHours?.[dateStr]?.slots; const dayAvailable = availableHours?.[dateStr]?.slots;
if (!dayAvailable || dayAvailable.length === 0) { if (!dayAvailable || dayAvailable.length === 0) {
toast.error('Sorry, this slot is no longer available. Please choose a different time.'); toast.error('Sorry, this slot is no longer available. Please choose a different time.');
selectedTime = null; if (selectedTime === timeToCheck) selectedTime = null;
return false; return false;
} }
const duration = getTotalDuration(); const duration = getTotalDuration();
const [selHour, selMinute] = selectedTime.split(':').map(Number); const [selHour, selMinute] = timeToCheck.split(':').map(Number);
const selStart = selHour * 60 + selMinute; const selStart = selHour * 60 + selMinute;
const selEnd = selStart + duration; const selEnd = selStart + duration;
@@ -1227,7 +1296,7 @@
if (!stillAvailable) { if (!stillAvailable) {
toast.error('Sorry, this slot was just taken. Please choose a different time.'); toast.error('Sorry, this slot was just taken. Please choose a different time.');
selectedTime = null; if (selectedTime === timeToCheck) selectedTime = null;
return false; return false;
} }
@@ -1317,29 +1386,7 @@
currentStep = 3; currentStep = 3;
} else { } else {
// Different slot or no reservation — release old one first, then reserve new // Different slot or no reservation — release old one first, then reserve new
if (_reservationId) { await releaseReservation();
try {
const res = await fetch('/api/bookings/reserve', {
method: 'DELETE',
headers: authStore.currentToken
? { Authorization: `Bearer ${authStore.currentToken}` }
: {}
});
if (!res.ok) console.warn('Failed to release old reservation', res.status);
} catch (e) {
console.warn('Error releasing old reservation', e);
}
_reservationId = null;
reservationExpiresAt = null;
reservationCountdown = '';
reservationExpired = false;
_reservedSlotTime = null;
_reservedSlotDate = null;
if (window.__bookingFlowCountdownInterval) {
clearInterval(window.__bookingFlowCountdownInterval);
window.__bookingFlowCountdownInterval = null;
}
}
const slotStillFree = await refreshAndValidateSlot(); const slotStillFree = await refreshAndValidateSlot();
if (!slotStillFree) return; if (!slotStillFree) return;
@@ -1379,24 +1426,12 @@
async function submitAndProceed() { async function submitAndProceed() {
_isSubmitting = true; _isSubmitting = true;
try { try {
if (!idempotencyKey) { // Always generate a fresh idempotency key per submission attempt. The
// Generate UUID v4 manually for environments where crypto.randomUUID() is unavailable // previous behavior kept the key across the component lifetime, which
const array = new Uint8Array(16); // meant a second submission (after a success, back-nav, change) would
if (typeof window !== 'undefined' && window.crypto) { // hit the backend with the same key and get the ORIGINAL booking back,
window.crypto.getRandomValues(array); // making the user believe they made a new booking when they didn't.
} else { idempotencyKey = generateUUID();
for (let i = 0; i < 16; i++) array[i] = Math.floor(Math.random() * 256);
}
array[6] = (array[6] & 0x0f) | 0x40; // version 4
array[8] = (array[8] & 0x3f) | 0x80; // variant 1
idempotencyKey = [...array]
.map((b, i) => {
const hex = b.toString(16).padStart(2, '0');
if (i === 4 || i === 6 || i === 8 || i === 10) return '-' + hex;
return hex;
})
.join('');
}
if (!selectedDate || !selectedTime) { if (!selectedDate || !selectedTime) {
toast.error('Please select a date and time'); toast.error('Please select a date and time');
@@ -1479,6 +1514,12 @@
total_amount: booking.total_amount || getTotalPrice(), total_amount: booking.total_amount || getTotalPrice(),
duration_minutes: booking.duration_minutes || getTotalDuration() duration_minutes: booking.duration_minutes || getTotalDuration()
}; };
// Booking is now persisted — release the temporary reservation
// (best-effort, no UI block). Also clear the idempotency key so
// any future submission in this component lifetime gets a fresh
// key and is treated as a NEW booking attempt.
releaseReservation();
idempotencyKey = '';
currentStep = finalStep; currentStep = finalStep;
fetchDiscountPreview(); fetchDiscountPreview();
setTimeout(() => { setTimeout(() => {
@@ -1531,6 +1572,12 @@
function prevStep() { function prevStep() {
if (currentStep > 1) { if (currentStep > 1) {
// If leaving step 2 (date/time) and a reservation is still held, release
// it — the user is abandoning this selection. Going from step 3 back to
// step 2 keeps the reservation (the user is just reviewing).
if (currentStep === 2 && _reservationId) {
releaseReservation();
}
currentStep--; currentStep--;
setTimeout(() => { setTimeout(() => {
window.scrollTo({ top: 0, behavior: 'smooth' }); window.scrollTo({ top: 0, behavior: 'smooth' });
@@ -1764,6 +1811,10 @@
maxValue={maxCalendarDate} maxValue={maxCalendarDate}
{isDateUnavailable} {isDateUnavailable}
onchange={(newDate) => { onchange={(newDate) => {
// Date change invalidates any held reservation (slot is now stale).
if (_reservationId) {
releaseReservation();
}
selectedDate = newDate; selectedDate = newDate;
selectedTime = null; selectedTime = null;
}} }}