wacrm/supabase/migrations/005_broadcast_counts_incremental.sql
Arnas Donauskas 66dd4ef97e
fix(broadcasts): senior-review hardening pass
Fixes 8 real issues found during a thorough review of the broadcast
feature (PRs #21-#29).

Security
  * Webhook POSTs had zero spoof protection. Anyone could forge a
    status update and drift broadcast counts arbitrarily. Added
    HMAC-SHA256 signature verification via the X-Hub-Signature-256
    header (Meta's standard). New helper src/lib/whatsapp/webhook-
    signature.ts with timing-safe comparison. Rollout fails open
    with a warning when META_APP_SECRET is unset so existing
    deploys don't break at rollout; set the env var to enforce.

Correctness
  * CSV audience was completely broken — synthesized "csv-N" strings
    as contact_id, which failed the UUID cast on insert. Every CSV
    broadcast silently created zero recipients. Replaced with a
    real find-or-create against the contacts table (batched lookup
    by phone, single INSERT for missing rows, de-duplicates CSV
    inputs). CSV contacts now end up as real contacts.rows with
    real UUIDs flowing through the rest of the pipeline.

  * Recipient insert errors were logged with console.error and the
    loop marched on — broadcasts then ran with incomplete recipient
    sets, webhook updates couldn't find rows, and aggregate counts
    drifted. Now: on batch insert failure, flip the broadcast to
    failed + failed_count=total + throw so handleSend's toast fires.

  * Webhook status guard ordered "failed" above "replied", which
    meant a webhook replay delivering "failed" for an already-
    replied recipient would upgrade replied → failed. Split the
    ladder: forward-only ordering across pending → sent →
    delivered → read → replied; failed is a terminal side branch
    that can only be reached from pending or sent (never from any
    success state). Net effect: a message that was read can no
    longer regress to failed on a replay.

  * Template sync only pulled page 1 (ignored paging.next). Users
    with >100 templates silently missed some, and broadcasts with
    those names failed with Meta's "template not found". Now
    follows paging.next with a 20-page safety cap, reports
    truncated=true in the response if hit.

  * Template sync reported success:true even when individual
    upserts failed. Now accumulates per-template errors and
    surfaces them via a separate toast.error in the UI — green
    toast no longer hides silent drift.

Performance
  * Migration 005: replace the broadcast_recipients aggregate
    trigger installed in migration 003. Old version ran
    COUNT(*) FILTER on every row change — O(n²) across a send
    loop (10k recipients × 10k index scans). New version
    increments / decrements by ±1 per status transition: O(1)
    per change. Kept the old recompute_broadcast_counts() as a
    callable safety-net in case of drift.

  * List page poll kept running while the tab was hidden. Added
    visibilitychange handler — polling pauses when hidden,
    resumes (and does an immediate fetch) on refocus.

UX
  * Step 3 (Personalize) let users skip past unmapped placeholders
    → broadcasts then sent to Meta with empty-string params.
    Next is now disabled until every {{N}} has a value; an amber
    warning lists the missing placeholders.

Documented accepted risks in the PR body.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-17 17:38:52 +03:00

130 lines
5.0 KiB
PL/PgSQL

-- ============================================================
-- Incremental broadcast aggregate trigger.
--
-- Migration 003 installed a trigger that recomputed every counter
-- (sent/delivered/read/replied/failed) via COUNT(*) FILTER on every
-- row change. For a 10k-recipient broadcast, the send loop produces
-- 10k INSERTs + 10k UPDATEs = 20k full aggregate scans, each walking
-- the (broadcast_id, status) index. Workable at small scale, but
-- O(n²) overall.
--
-- This migration replaces that with an incremental trigger that
-- adjusts the parent broadcast's counts by ±1 based on the OLD →
-- NEW.status delta. O(1) per recipient change; no scans at all.
--
-- Semantic model (same as the lib/broadcast-status.ts "forward-only
-- ladder" in the webhook):
-- sent_count = recipients whose status is at or past 'sent'
-- delivered_count = ... at or past 'delivered'
-- read_count = ... at or past 'read'
-- replied_count = status = 'replied'
-- failed_count = status = 'failed'
--
-- A webhook that advances a recipient pending → sent → delivered →
-- read → replied bumps every rung it crosses by 1. Going to 'failed'
-- only bumps failed_count (and can only happen from pending / sent,
-- enforced in the webhook).
--
-- Keeps the safety net: a public recompute_broadcast_counts() SQL
-- function is retained so ops can run it manually if counts ever
-- drift (e.g. after bulk DB surgery).
--
-- Idempotent — safe to run multiple times.
-- ============================================================
-- Delta a single column by +1 / -1.
CREATE OR REPLACE FUNCTION public._bcast_bump(bid UUID, col TEXT, delta INT)
RETURNS VOID AS $$
BEGIN
EXECUTE format(
'UPDATE broadcasts SET %I = GREATEST(0, %I + $1), updated_at = NOW() WHERE id = $2',
col, col
) USING delta, bid;
END;
$$ LANGUAGE plpgsql SECURITY DEFINER SET search_path = public;
-- Columns this recipient's status contributes to.
CREATE OR REPLACE FUNCTION public._bcast_cols_for_status(s TEXT)
RETURNS TEXT[] AS $$
BEGIN
-- 'pending' contributes to nothing.
IF s = 'pending' THEN RETURN ARRAY[]::TEXT[]; END IF;
IF s = 'sent' THEN RETURN ARRAY['sent_count']; END IF;
IF s = 'delivered' THEN RETURN ARRAY['sent_count','delivered_count']; END IF;
IF s = 'read' THEN RETURN ARRAY['sent_count','delivered_count','read_count']; END IF;
IF s = 'replied' THEN RETURN ARRAY['sent_count','delivered_count','read_count','replied_count']; END IF;
IF s = 'failed' THEN RETURN ARRAY['failed_count']; END IF;
RETURN ARRAY[]::TEXT[];
END;
$$ LANGUAGE plpgsql IMMUTABLE;
-- Replace the trigger body with the incremental version.
CREATE OR REPLACE FUNCTION public.broadcast_recipient_aggregate_trigger()
RETURNS TRIGGER AS $$
DECLARE
old_cols TEXT[];
new_cols TEXT[];
c TEXT;
BEGIN
IF TG_OP = 'INSERT' THEN
new_cols := _bcast_cols_for_status(NEW.status);
FOREACH c IN ARRAY new_cols LOOP
PERFORM _bcast_bump(NEW.broadcast_id, c, 1);
END LOOP;
RETURN NEW;
END IF;
IF TG_OP = 'DELETE' THEN
old_cols := _bcast_cols_for_status(OLD.status);
FOREACH c IN ARRAY old_cols LOOP
PERFORM _bcast_bump(OLD.broadcast_id, c, -1);
END LOOP;
RETURN OLD;
END IF;
-- UPDATE: only care if status changed.
IF OLD.status IS DISTINCT FROM NEW.status THEN
old_cols := _bcast_cols_for_status(OLD.status);
new_cols := _bcast_cols_for_status(NEW.status);
-- Subtract the old contributions, add the new.
FOREACH c IN ARRAY old_cols LOOP
PERFORM _bcast_bump(NEW.broadcast_id, c, -1);
END LOOP;
FOREACH c IN ARRAY new_cols LOOP
PERFORM _bcast_bump(NEW.broadcast_id, c, 1);
END LOOP;
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql SECURITY DEFINER SET search_path = public;
-- Trigger itself remains the same (INSERT/UPDATE/DELETE) — just its
-- body has been replaced.
-- Safety net — rebuild counts from scratch. Retained as-is so ops can
-- run it on demand if something ever drifts. Matches the incremental
-- trigger's semantic model exactly.
CREATE OR REPLACE FUNCTION public.recompute_broadcast_counts(bid UUID)
RETURNS VOID AS $$
BEGIN
UPDATE broadcasts b SET
sent_count = agg.sent_count,
delivered_count = agg.delivered_count,
read_count = agg.read_count,
replied_count = agg.replied_count,
failed_count = agg.failed_count,
updated_at = NOW()
FROM (
SELECT
COUNT(*) FILTER (WHERE status IN ('sent','delivered','read','replied')) AS sent_count,
COUNT(*) FILTER (WHERE status IN ('delivered','read','replied')) AS delivered_count,
COUNT(*) FILTER (WHERE status IN ('read','replied')) AS read_count,
COUNT(*) FILTER (WHERE status = 'replied') AS replied_count,
COUNT(*) FILTER (WHERE status = 'failed') AS failed_count
FROM broadcast_recipients
WHERE broadcast_id = bid
) agg
WHERE b.id = bid;
END;
$$ LANGUAGE plpgsql SECURITY DEFINER SET search_path = public;