From 5f488e129aa2d8dfe5a1065bbe13b781ae33464d Mon Sep 17 00:00:00 2001 From: Arnas Donauskas Date: Fri, 17 Apr 2026 16:56:45 +0300 Subject: [PATCH] =?UTF-8?q?fix(contacts):=20allow=20deletion=20=E2=80=94?= =?UTF-8?q?=20SET=20NULL=20on=20broadcast=5Frecipients,=20deals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (error 23503): broadcast_recipients.contact_id and deals.contact_id were declared NOT NULL REFERENCES contacts(id) with no ON DELETE clause, so Postgres defaulted to NO ACTION. Any contact who'd ever received a broadcast or been attached to a deal became un-deletable with "violates foreign key constraint broadcast_recipients_contact_id_fkey". Fix (migration 004): Relax both FKs to ON DELETE SET NULL + drop NOT NULL on both columns. History rows survive with contact_id = NULL. The UI is already null-safe: - broadcast detail: contact?.name ?? 'Unknown' - deal card: contact?.name || contact?.phone || 'No contact' - CSV export: contact?.name ?? '' Why not CASCADE: CASCADE would wipe broadcast recipient rows and trigger the aggregate recount on broadcasts (migration 003), retroactively changing sent_count / delivered_count / read_count. It would also delete deal history. SET NULL preserves both counts and history. Type/code: - Deal.contact_id + BroadcastRecipient.contact_id widen to string | null - DealForm: setContactId(deal.contact_id ?? "") — the contact picker uses "" to mean unselected Co-Authored-By: Claude Opus 4.7 (1M context) --- src/components/pipelines/deal-form.tsx | 4 +- src/types/index.ts | 13 +++- .../004_contact_delete_set_null.sql | 65 +++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 supabase/migrations/004_contact_delete_set_null.sql diff --git a/src/components/pipelines/deal-form.tsx b/src/components/pipelines/deal-form.tsx index 1027a6f..8e63a80 100644 --- a/src/components/pipelines/deal-form.tsx +++ b/src/components/pipelines/deal-form.tsx @@ -79,7 +79,9 @@ export function DealForm({ setTitle(deal.title); setValue(String(deal.value ?? "")); setCurrency(deal.currency || "USD"); - setContactId(deal.contact_id); + // contact_id is nullable when the contact has been deleted + // (migration 004: ON DELETE SET NULL). "" means "no selection". + setContactId(deal.contact_id ?? ""); setStageId(deal.stage_id); setAssignedTo(deal.assigned_to ?? ""); setExpectedCloseDate(deal.expected_close_date ?? ""); diff --git a/src/types/index.ts b/src/types/index.ts index aed01e8..81be69e 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -141,7 +141,11 @@ export interface Deal { user_id: string; pipeline_id: string; stage_id: string; - contact_id: string; + /** + * Nullable after migration 004 — becomes NULL when the referenced + * contact is deleted (ON DELETE SET NULL). History preserved. + */ + contact_id: string | null; conversation_id?: string; assigned_to?: string; title: string; @@ -182,7 +186,12 @@ export interface Broadcast { export interface BroadcastRecipient { id: string; broadcast_id: string; - contact_id: string; + /** + * Nullable after migration 004 — becomes NULL when the referenced + * contact is deleted (ON DELETE SET NULL). History preserved; the + * UI renders "Unknown" for orphaned rows. + */ + contact_id: string | null; status: RecipientStatus; sent_at?: string; delivered_at?: string; diff --git a/supabase/migrations/004_contact_delete_set_null.sql b/supabase/migrations/004_contact_delete_set_null.sql new file mode 100644 index 0000000..f326d12 --- /dev/null +++ b/supabase/migrations/004_contact_delete_set_null.sql @@ -0,0 +1,65 @@ +-- ============================================================ +-- Allow contact deletion without wiping history. +-- +-- broadcast_recipients.contact_id and deals.contact_id were declared +-- NOT NULL REFERENCES contacts(id) with no ON DELETE action, so +-- Postgres defaults to NO ACTION. The first time a user tried to +-- delete a contact that had ever received a broadcast or been +-- attached to a deal, the delete failed with: +-- +-- ERROR 23503: update or delete on table "contacts" violates +-- foreign key constraint ... on table +-- +-- CASCADE is the wrong fix — it would silently wipe historical +-- broadcast recipient rows (breaking audit + retroactively moving +-- broadcasts.sent_count / delivered_count / read_count etc. via the +-- aggregate trigger) and deal rows. +-- +-- SET NULL is the right fix: history rows survive with a NULL +-- contact_id. The UI is already null-safe (contact?.name ?? 'Unknown', +-- contact?.phone, etc.). +-- +-- Idempotent — safe to run multiple times. +-- ============================================================ + +-- ── broadcast_recipients.contact_id ──────────────────────────── +ALTER TABLE broadcast_recipients + ALTER COLUMN contact_id DROP NOT NULL; + +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'broadcast_recipients_contact_id_fkey' + AND conrelid = 'broadcast_recipients'::regclass + ) THEN + ALTER TABLE broadcast_recipients + DROP CONSTRAINT broadcast_recipients_contact_id_fkey; + END IF; +END $$; + +ALTER TABLE broadcast_recipients + ADD CONSTRAINT broadcast_recipients_contact_id_fkey + FOREIGN KEY (contact_id) REFERENCES contacts(id) + ON DELETE SET NULL; + +-- ── deals.contact_id ─────────────────────────────────────────── +ALTER TABLE deals + ALTER COLUMN contact_id DROP NOT NULL; + +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conname = 'deals_contact_id_fkey' + AND conrelid = 'deals'::regclass + ) THEN + ALTER TABLE deals + DROP CONSTRAINT deals_contact_id_fkey; + END IF; +END $$; + +ALTER TABLE deals + ADD CONSTRAINT deals_contact_id_fkey + FOREIGN KEY (contact_id) REFERENCES contacts(id) + ON DELETE SET NULL;