Fixes 3 Important issues from re-review of the previous fast-follow-up commit:
1. booked_slots desync on reschedule: the cleanup trigger was `after update
of status`, so changing date/time on a still-confirmed booking never fired
it -- the OLD slot stayed blocked and the NEW slot stayed shown as free.
Changed to an unqualified `after update`, and the UPDATE branch now
compares the full (date, time, status) triple instead of just status, so
it frees the old slot on any change away from it and (re)mirrors the new
one when the row is still/now confirmed. Added pgTAP coverage: reschedule
removes the old slot's mirror row and creates the new one.
2. The Edge Functions' comments/commit message claimed a non-200 response
would make pg_net "retry" -- that's not how pg_net works, it's
fire-and-forget and never inspects the response status. Corrected the
comments to state the real reasoning (nothing consumes the status code,
so a 500 wouldn't help and would misrepresent an already-successful
booking/inquiry as failed). Closed the actual gap that created -- a failed
Resend send was only visible in Edge Function logs nobody reads -- by
adding a `notification_failures` table (new migration, admin-only select
via RLS, no anon/authenticated write access, only service_role writes)
and having both handlers insert a row (booking/inquiry id + error
message) whenever the Resend call fails, the API key is missing, or an
unexpected error occurs. Insert failures are swallowed (best-effort --
already in a failure path, must not throw a louder one). Added pgTAP
tests for the table's RLS/grant shape.
3. `20260812000002_bookings_slot_uniqueness.sql` could have failed outright
on `supabase db push` against live data if any existing confirmed booking
already shared a (date, time) with another. Added
`dedupe_confirmed_booking_slots()`, called before the index is created:
it collapses each duplicate group down to the earliest-created row
(kept 'confirmed', others cancelled) and logs what it did via
`raise notice`, so index creation is guaranteed to succeed either way.
Extracted as a standalone, service_role-only-executable function (rather
than inlining the SQL) specifically so the pgTAP test can exercise the
exact same logic the migration runs. Added a test that seeds two
duplicate confirmed bookings (having first dropped the index to simulate
the pre-migration state), runs the dedupe function, and asserts exactly
one confirmed booking remains -- the earliest one -- before recreating
the index.
pgTAP suite grew from 23 to 37 tests. Verified locally: supabase db reset
(all 7 migrations apply cleanly), supabase test db (37/37 pass), vitest run
(51/51 pass), npm run build, npm run lint, and npm run test:functions
(6/6 pass) all succeed. Still nothing here touches the live Supabase
project.
Claude-Session: https://claude.ai/code/session_01NhxA4npAXAxmCDEJt6kxSH
Fast follow-up to the Phase 2A backend review (4 deferred Important items):
1. Add a partial unique index (date, time) where status = 'confirmed' on
public.bookings so two concurrent bookings for the same slot can no longer
both succeed. Previously sync_booked_slot()'s `on conflict do nothing`
masked the second insert as a silent no-op on the mirror table instead of
rejecting it.
2. Extend sync_booked_slot() (new migration, the applied 20260808000002 is
left untouched) to also fire on `update of status` and `delete`, removing
the booked_slots mirror row once no confirmed booking remains for a slot.
Also tightened the insert path to only mirror bookings that are actually
'confirmed' -- the original trigger mirrored every insert regardless of
status, so a booking inserted directly with status='cancelled' (allowed by
the public insert policy) would incorrectly block a slot nobody confirmed.
3. supabase/functions/{on-booking-created,on-inquiry-created}: guard
req.json() and payload.record with try/catch (400 + console.error on
failure), check the Resend response's .ok and log failures instead of
swallowing them, and skip the Resend call entirely (with a log) instead of
sending "Authorization: Bearer undefined" when RESEND_API_KEY is unset.
Still return 200 when only the notification leg fails, since the booking/
inquiry row itself already committed successfully by the time this handler
runs -- a non-200 would make pg_net retry, but retrying re-sends against
the same already-inserted row and won't fix a Resend outage or bad
payload, so it isn't the right signal.
4. README: document the Supabase-side dev loop (supabase start/db reset/test
db), npm run seed and its SUPABASE_URL/SUPABASE_SERVICE_ROLE_KEY env vars,
and npm run test:functions (Deno tests for the Edge Functions' shared
email builders).
pgTAP suite grew from 12 to 23 tests (11 new: 4 for the uniqueness
constraint, 7 for booked_slots cleanup on cancel/delete). Verified locally:
supabase db reset, supabase test db (23/23 pass), vitest run (51/51 pass),
npm run build, npm run lint, and npm run test:functions (Deno installed for
this session; 6/6 pass) all succeed. Nothing here touches the live Supabase
project -- deploying is a separate follow-up step.
Claude-Session: https://claude.ai/code/session_01NhxA4npAXAxmCDEJt6kxSH
