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
65 lines
2.8 KiB
PL/PgSQL
65 lines
2.8 KiB
PL/PgSQL
-- Follow-up to the final whole-branch review before merge (deferred, fast
|
|
-- follow-up task): public.bookings had no uniqueness constraint on
|
|
-- (date, time), so two concurrent bookings for the same slot could both
|
|
-- insert successfully -- the second one would silently no-op on the
|
|
-- booked_slots mirror (sync_booked_slot()'s `on conflict do nothing`)
|
|
-- instead of failing, leaving the app's slot-availability check unreliable
|
|
-- under concurrency.
|
|
--
|
|
-- A partial unique index on confirmed bookings only (not a plain unique
|
|
-- constraint on the whole table) is required because status can be
|
|
-- 'cancelled': a cancelled booking must not permanently block the slot it
|
|
-- once held, and multiple cancelled bookings may legitimately share a
|
|
-- (date, time) (e.g. several people asked for the same slot, all declined).
|
|
--
|
|
-- SELF-HEALING: on `supabase db push` against the live project, this
|
|
-- `create unique index` could fail outright if any existing confirmed
|
|
-- booking already shares a (date, time) with another -- exactly the bug
|
|
-- being fixed here, so entirely plausible in already-live data. Rather than
|
|
-- have the migration fail and block deploy, dedupe_confirmed_booking_slots()
|
|
-- below collapses each duplicate group down to the earliest-created row
|
|
-- (keeping it 'confirmed') and cancels the rest before the index is
|
|
-- created, so index creation is guaranteed to succeed either way. It's
|
|
-- defined as a standalone function (called once below, but also directly
|
|
-- callable) so the pgTAP test can exercise the exact same logic the
|
|
-- migration runs, rather than a copy that could drift from it.
|
|
create or replace function public.dedupe_confirmed_booking_slots()
|
|
returns void
|
|
language plpgsql
|
|
security definer
|
|
set search_path = ''
|
|
as $$
|
|
declare
|
|
dup record;
|
|
begin
|
|
for dup in
|
|
select date, time, count(*) as cnt
|
|
from public.bookings
|
|
where status = 'confirmed'
|
|
group by date, time
|
|
having count(*) > 1
|
|
loop
|
|
update public.bookings
|
|
set status = 'cancelled'
|
|
where date = dup.date and time = dup.time and status = 'confirmed'
|
|
and id <> (
|
|
select id from public.bookings
|
|
where date = dup.date and time = dup.time and status = 'confirmed'
|
|
order by created_at asc, id asc
|
|
limit 1
|
|
);
|
|
raise notice 'dedupe_confirmed_booking_slots: cancelled % duplicate confirmed booking(s) for slot % %, keeping the earliest-created one',
|
|
dup.cnt - 1, dup.date, dup.time;
|
|
end loop;
|
|
end;
|
|
$$;
|
|
|
|
revoke execute on function public.dedupe_confirmed_booking_slots() from public, anon, authenticated;
|
|
grant execute on function public.dedupe_confirmed_booking_slots() to service_role;
|
|
|
|
select public.dedupe_confirmed_booking_slots();
|
|
|
|
create unique index bookings_active_slot_idx
|
|
on public.bookings (date, time)
|
|
where status = 'confirmed';
|