fix: address final whole-branch review findings for Phase 2C
Critical: computeAvailableSlots looped forever when slot_minutes was
0 or negative -- reachable in production, since AdminSchedule's
slot-length input has no validation and clearing it yields Number('')
=== 0. Guarded the computation, added min="5"/required to the admin
input as defense in depth, and added a regression test.
Important fixes:
- Removed the one eslint-disable that slipped into the branch
(InquiryModal.test.tsx), replacing it with the explicit vi.fn<...>
generic type pattern already used everywhere else. Also cleaned up
a similar unused-var workaround in useBooking.test.ts for
consistency.
- Added a submitting/in-flight guard to both booking and inquiry
submission, so a double-click can no longer produce a false
"slot already taken" error after a booking actually succeeded (or
a duplicate inquiry row).
- Added loading states to Home.tsx (services/products) and Book.tsx
(services list, date/slot step) -- previously only Shop.tsx had
one, though the spec named all three pages.
- Restored the full date -> slot -> contact -> confirmation ->
persisted-booking test in Book.test.tsx, which had been replaced
rather than supplemented by a narrower slot-fetching test.
Deferred to a follow-up (explicit decision, not silently dropped):
duration-aware slot conflicts. The current model only blocks an exact
(date, time) match, so a 4-hour service booked at 9:00 AM doesn't
block 11:00 AM even though they'd physically overlap. Inherited from
the Phase 2A data model; this branch is what makes it reachable by
real customers, so it needs a recorded decision before go-live rather
than silent inheritance.
Claude-Session: https://claude.ai/code/session_01NhxA4npAXAxmCDEJt6kxSH
This commit is contained in:
@@ -3,8 +3,7 @@ import userEvent from '@testing-library/user-event'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
const insertMock = vi.fn()
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const fromMock = vi.fn((_: string) => ({ insert: insertMock }))
|
||||
const fromMock = vi.fn<(table: string) => { insert: typeof insertMock }>(() => ({ insert: insertMock }))
|
||||
|
||||
vi.mock('../lib/supabase', () => ({
|
||||
supabase: { from: (table: string) => fromMock(table) },
|
||||
|
||||
@@ -23,6 +23,7 @@ function InquiryModal({ product, onClose }: InquiryModalProps) {
|
||||
const [errors, setErrors] = useState<Errors>({})
|
||||
const [submitError, setSubmitError] = useState('')
|
||||
const [submitted, setSubmitted] = useState(false)
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
const nameInputRef = useRef<HTMLInputElement>(null)
|
||||
|
||||
useEffect(() => {
|
||||
@@ -39,6 +40,8 @@ function InquiryModal({ product, onClose }: InquiryModalProps) {
|
||||
|
||||
async function handleSubmit(e: FormEvent) {
|
||||
e.preventDefault()
|
||||
if (submitting) return
|
||||
|
||||
const nextErrors: Errors = {}
|
||||
if (!name.trim()) nextErrors.name = 'Le nom est requis.'
|
||||
if (!isValidEmail(email)) nextErrors.email = 'Une adresse courriel valide est requise.'
|
||||
@@ -47,6 +50,7 @@ function InquiryModal({ product, onClose }: InquiryModalProps) {
|
||||
return
|
||||
}
|
||||
|
||||
setSubmitting(true)
|
||||
setSubmitError('')
|
||||
const { error } = await supabase.from('inquiries').insert({
|
||||
product_id: product.id,
|
||||
@@ -54,6 +58,7 @@ function InquiryModal({ product, onClose }: InquiryModalProps) {
|
||||
contact_email: email,
|
||||
contact_notes: notes,
|
||||
})
|
||||
setSubmitting(false)
|
||||
|
||||
if (error) {
|
||||
setSubmitError(error.message)
|
||||
@@ -104,7 +109,7 @@ function InquiryModal({ product, onClose }: InquiryModalProps) {
|
||||
<textarea id="inquiry-notes" value={notes} onChange={(e) => setNotes(e.target.value)} />
|
||||
</div>
|
||||
{submitError && <p className={styles.error}>{submitError}</p>}
|
||||
<button type="submit">Envoyer la demande</button>
|
||||
<button type="submit" disabled={submitting}>Envoyer la demande</button>
|
||||
<button type="button" onClick={onClose}>Fermer</button>
|
||||
</form>
|
||||
)}
|
||||
|
||||
@@ -26,6 +26,12 @@ describe('computeAvailableSlots', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('returns no slots instead of hanging when slot_minutes is zero or negative', () => {
|
||||
const date = '2026-08-11'
|
||||
expect(computeAvailableSlots(templateFor(date, { slot_minutes: 0 }), [], [], date)).toEqual([])
|
||||
expect(computeAvailableSlots(templateFor(date, { slot_minutes: -30 }), [], [], date)).toEqual([])
|
||||
})
|
||||
|
||||
it('returns no slots when the date is a closed exception', () => {
|
||||
const date = '2026-08-11'
|
||||
const template = templateFor(date)
|
||||
|
||||
@@ -57,6 +57,15 @@ export function computeAvailableSlots(
|
||||
return []
|
||||
}
|
||||
|
||||
// A zero or negative slot length would make the loop below never advance
|
||||
// past startMinutes, growing `slots` without bound. This is reachable in
|
||||
// practice: the admin schedule's slot-length field is a plain number
|
||||
// input with no validation (src/pages/admin/AdminSchedule.tsx), and
|
||||
// clearing it yields Number('') === 0.
|
||||
if (!Number.isFinite(template.slot_minutes) || template.slot_minutes <= 0) {
|
||||
return []
|
||||
}
|
||||
|
||||
const startMinutes = parseTimeToMinutes(template.start_time)
|
||||
const endMinutes = parseTimeToMinutes(template.end_time)
|
||||
const bookedTimes = new Set(
|
||||
|
||||
@@ -2,11 +2,7 @@ import { renderHook, act } from '@testing-library/react'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
const insertMock = vi.fn()
|
||||
const fromMock = vi.fn((table: string) => {
|
||||
// table parameter is used to track the call in test assertions via fromMock.toHaveBeenCalledWith('bookings')
|
||||
void table // Explicitly reference to satisfy linter
|
||||
return { insert: insertMock }
|
||||
})
|
||||
const fromMock = vi.fn<(table: string) => { insert: typeof insertMock }>(() => ({ insert: insertMock }))
|
||||
|
||||
vi.mock('../lib/supabase', () => ({
|
||||
supabase: { from: (table: string) => fromMock(table) },
|
||||
|
||||
@@ -38,6 +38,7 @@ export default function useBooking(initialServiceId?: string) {
|
||||
const [state, setState] = useState<BookingState>(() => initialState(initialServiceId))
|
||||
const [errors, setErrors] = useState<BookingErrors>({})
|
||||
const [submitError, setSubmitError] = useState('')
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
|
||||
const selectService = useCallback((serviceId: string) => {
|
||||
setState((s) => ({ ...s, serviceId, step: 'date' }))
|
||||
@@ -56,6 +57,8 @@ export default function useBooking(initialServiceId?: string) {
|
||||
}, [])
|
||||
|
||||
const submit = useCallback(async (): Promise<boolean> => {
|
||||
if (submitting) return false
|
||||
|
||||
const nextErrors: BookingErrors = {}
|
||||
if (!state.contact.name.trim()) nextErrors.name = 'Le nom est requis.'
|
||||
if (!isValidEmail(state.contact.email)) nextErrors.email = 'Une adresse courriel valide est requise.'
|
||||
@@ -68,6 +71,7 @@ export default function useBooking(initialServiceId?: string) {
|
||||
return false
|
||||
}
|
||||
|
||||
setSubmitting(true)
|
||||
setSubmitError('')
|
||||
const { error } = await supabase.from('bookings').insert({
|
||||
service_id: state.serviceId,
|
||||
@@ -78,6 +82,7 @@ export default function useBooking(initialServiceId?: string) {
|
||||
contact_email: state.contact.email,
|
||||
contact_notes: state.contact.notes,
|
||||
})
|
||||
setSubmitting(false)
|
||||
|
||||
if (error) {
|
||||
if (error.code === SLOT_CONFLICT_CODE) {
|
||||
@@ -91,13 +96,14 @@ export default function useBooking(initialServiceId?: string) {
|
||||
|
||||
setState((s) => ({ ...s, step: 'confirmed' }))
|
||||
return true
|
||||
}, [state])
|
||||
}, [state, submitting])
|
||||
|
||||
const reset = useCallback(() => {
|
||||
setState(initialState(initialServiceId))
|
||||
setErrors({})
|
||||
setSubmitError('')
|
||||
setSubmitting(false)
|
||||
}, [initialServiceId])
|
||||
|
||||
return { state, errors, submitError, selectService, selectDate, selectSlot, updateContact, submit, reset }
|
||||
return { state, errors, submitError, submitting, selectService, selectDate, selectSlot, updateContact, submit, reset }
|
||||
}
|
||||
|
||||
@@ -4,8 +4,12 @@ import { MemoryRouter, Routes, Route } from 'react-router-dom'
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
const servicesOrderMock = vi.fn()
|
||||
const fromMock = vi.fn<(table: string) => { select: () => { order: typeof servicesOrderMock } }>((table: string) => {
|
||||
const bookingsInsertMock = vi.fn()
|
||||
const fromMock = vi.fn<
|
||||
(table: string) => { select: () => { order: typeof servicesOrderMock } } | { insert: typeof bookingsInsertMock }
|
||||
>((table: string) => {
|
||||
if (table === 'services') return { select: () => ({ order: servicesOrderMock }) }
|
||||
if (table === 'bookings') return { insert: bookingsInsertMock }
|
||||
throw new Error(`unexpected table: ${table}`)
|
||||
})
|
||||
const getSlotsForDateMock = vi.fn()
|
||||
@@ -52,6 +56,7 @@ function renderBook(initialEntries: { pathname: string; state?: unknown }[]) {
|
||||
describe('Book page', () => {
|
||||
beforeEach(() => {
|
||||
servicesOrderMock.mockReset().mockResolvedValue({ data: SERVICE_ROWS, error: null })
|
||||
bookingsInsertMock.mockReset().mockResolvedValue({ error: null })
|
||||
fromMock.mockClear()
|
||||
getSlotsForDateMock.mockReset().mockResolvedValue([
|
||||
{ date: '2026-08-11', time: '9:00 AM', available: true },
|
||||
@@ -86,4 +91,30 @@ describe('Book page', () => {
|
||||
expect(await screen.findByText(/aucun créneau disponible/i)).toBeInTheDocument()
|
||||
expect(await screen.findByRole('button', { name: /essayez .* à la place/i })).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('walks through date -> slot -> contact -> confirmation and persists the booking', async () => {
|
||||
renderBook([{ pathname: '/book', state: { serviceId: 'cornrows' } }])
|
||||
await userEvent.type(screen.getByLabelText(/choisissez une date/i), '2026-08-11')
|
||||
const slotButton = await screen.findByRole('button', { name: '9:00 AM' })
|
||||
await userEvent.click(slotButton)
|
||||
|
||||
await userEvent.type(await screen.findByLabelText(/^nom$/i), 'Jane Doe')
|
||||
await userEvent.type(screen.getByLabelText(/^courriel$/i), 'jane@example.com')
|
||||
await userEvent.type(screen.getByLabelText(/^téléphone$/i), '5551234567')
|
||||
await userEvent.click(screen.getByRole('button', { name: /confirmer/i }))
|
||||
|
||||
await waitFor(() =>
|
||||
expect(bookingsInsertMock).toHaveBeenCalledWith({
|
||||
service_id: 'cornrows',
|
||||
date: '2026-08-11',
|
||||
time: '9:00 AM',
|
||||
contact_name: 'Jane Doe',
|
||||
contact_phone: '5551234567',
|
||||
contact_email: 'jane@example.com',
|
||||
contact_notes: '',
|
||||
}),
|
||||
)
|
||||
expect(await screen.findByText(/nattes collées/i)).toBeInTheDocument()
|
||||
expect(screen.getByText(/mira vous attend/i)).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -16,6 +16,7 @@ function Book() {
|
||||
const availability = useAvailability()
|
||||
const [dateInput, setDateInput] = useState('')
|
||||
const [services, setServices] = useState<Service[]>([])
|
||||
const [servicesLoading, setServicesLoading] = useState(true)
|
||||
const [servicesError, setServicesError] = useState('')
|
||||
const [slots, setSlots] = useState<TimeSlot[]>([])
|
||||
const [nextAvailable, setNextAvailable] = useState<string | null>(null)
|
||||
@@ -27,6 +28,8 @@ function Book() {
|
||||
setServices(await fetchServices())
|
||||
} catch (err) {
|
||||
setServicesError(err && typeof err === 'object' && 'message' in err ? String((err as { message: unknown }).message) : String(err))
|
||||
} finally {
|
||||
setServicesLoading(false)
|
||||
}
|
||||
}
|
||||
void load()
|
||||
@@ -84,6 +87,7 @@ function Book() {
|
||||
|
||||
{booking.state.step === 'service' && (
|
||||
<div className={styles.services}>
|
||||
{servicesLoading && <p>Chargement…</p>}
|
||||
{services.map((service) => (
|
||||
<ServiceCard key={service.id} service={service} onSelect={booking.selectService} />
|
||||
))}
|
||||
@@ -92,6 +96,7 @@ function Book() {
|
||||
|
||||
{(booking.state.step === 'date' || booking.state.step === 'slot') && (
|
||||
<>
|
||||
{availability.loading && <p>Chargement…</p>}
|
||||
{booking.submitError && <p className={styles.error}>{booking.submitError}</p>}
|
||||
<div className={styles.field}>
|
||||
<label htmlFor="booking-date">Choisissez une date</label>
|
||||
@@ -178,7 +183,7 @@ function Book() {
|
||||
onChange={(e) => booking.updateContact({ notes: e.target.value })}
|
||||
/>
|
||||
</div>
|
||||
<button type="submit">Confirmer la réservation</button>
|
||||
<button type="submit" disabled={booking.submitting}>Confirmer la réservation</button>
|
||||
</form>
|
||||
)}
|
||||
</motion.div>
|
||||
|
||||
@@ -23,6 +23,7 @@ function Home() {
|
||||
|
||||
const [services, setServices] = useState<Service[]>([])
|
||||
const [products, setProducts] = useState<Product[]>([])
|
||||
const [loading, setLoading] = useState(true)
|
||||
const [error, setError] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
@@ -33,6 +34,8 @@ function Home() {
|
||||
setProducts(productsResult)
|
||||
} catch (err) {
|
||||
setError(err && typeof err === 'object' && 'message' in err ? String((err as { message: unknown }).message) : String(err))
|
||||
} finally {
|
||||
setLoading(false)
|
||||
}
|
||||
}
|
||||
void load()
|
||||
@@ -50,6 +53,7 @@ function Home() {
|
||||
</div>
|
||||
</section>
|
||||
|
||||
{loading && <p>Chargement…</p>}
|
||||
{error && <p className={styles.error}>{error}</p>}
|
||||
|
||||
<ScrollReveal>
|
||||
|
||||
@@ -157,6 +157,8 @@ function AdminSchedule() {
|
||||
<input
|
||||
id="schedule-slot-minutes"
|
||||
type="number"
|
||||
min="5"
|
||||
required
|
||||
value={template.slot_minutes}
|
||||
onChange={(e) => setTemplate({ ...template, slot_minutes: Number(e.target.value) })}
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user