From 4ca40bf2b55cd8e643060fb5e653e5ecd31f9560 Mon Sep 17 00:00:00 2001
From: belviskhoremk <belviskhoremk@gmail.com>
Date: Wed, 12 Aug 2026 23:03:32 +0200
Subject: [PATCH] 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
---
 src/components/InquiryModal.test.tsx |  3 +--
 src/components/InquiryModal.tsx      |  7 +++++-
 src/data/availability.test.ts        |  6 +++++
 src/data/availability.ts             |  9 ++++++++
 src/hooks/useBooking.test.ts         |  6 +----
 src/hooks/useBooking.ts              | 10 +++++++--
 src/pages/Book.test.tsx              | 33 +++++++++++++++++++++++++++-
 src/pages/Book.tsx                   |  7 +++++-
 src/pages/Home.tsx                   |  4 ++++
 src/pages/admin/AdminSchedule.tsx    |  2 ++
 10 files changed, 75 insertions(+), 12 deletions(-)

diff --git a/src/components/InquiryModal.test.tsx b/src/components/InquiryModal.test.tsx
index f4a73e6..f8a7e0a 100644
--- a/src/components/InquiryModal.test.tsx
+++ b/src/components/InquiryModal.test.tsx
@@ -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) },
diff --git a/src/components/InquiryModal.tsx b/src/components/InquiryModal.tsx
index e0e4fa4..6156567 100644
--- a/src/components/InquiryModal.tsx
+++ b/src/components/InquiryModal.tsx
@@ -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>
         )}
diff --git a/src/data/availability.test.ts b/src/data/availability.test.ts
index 3c38ca6..f60792d 100644
--- a/src/data/availability.test.ts
+++ b/src/data/availability.test.ts
@@ -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)
diff --git a/src/data/availability.ts b/src/data/availability.ts
index 070eedb..4a92e2a 100644
--- a/src/data/availability.ts
+++ b/src/data/availability.ts
@@ -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(
diff --git a/src/hooks/useBooking.test.ts b/src/hooks/useBooking.test.ts
index c4d20d0..f640176 100644
--- a/src/hooks/useBooking.test.ts
+++ b/src/hooks/useBooking.test.ts
@@ -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) },
diff --git a/src/hooks/useBooking.ts b/src/hooks/useBooking.ts
index 79722c6..8e72be2 100644
--- a/src/hooks/useBooking.ts
+++ b/src/hooks/useBooking.ts
@@ -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 }
 }
diff --git a/src/pages/Book.test.tsx b/src/pages/Book.test.tsx
index a12c035..c31a89e 100644
--- a/src/pages/Book.test.tsx
+++ b/src/pages/Book.test.tsx
@@ -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()
+  })
 })
diff --git a/src/pages/Book.tsx b/src/pages/Book.tsx
index 217a82b..3b977d0 100644
--- a/src/pages/Book.tsx
+++ b/src/pages/Book.tsx
@@ -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>
diff --git a/src/pages/Home.tsx b/src/pages/Home.tsx
index 1221b64..229d66d 100644
--- a/src/pages/Home.tsx
+++ b/src/pages/Home.tsx
@@ -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>
diff --git a/src/pages/admin/AdminSchedule.tsx b/src/pages/admin/AdminSchedule.tsx
index 7d6b7dc..136e731 100644
--- a/src/pages/admin/AdminSchedule.tsx
+++ b/src/pages/admin/AdminSchedule.tsx
@@ -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) })}
             />