Merge pull request #178 from ArnasDon/feat/account-sharing-backend-perf

perf(db,webhook): backend follow-ups from multi-user review (M1, M2, S4, S5)
This commit is contained in:
Arnas Donauskas 2026-05-31 18:25:44 +02:00 committed by GitHub
commit b4901c2aae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 187 additions and 8 deletions

View File

@ -40,6 +40,28 @@ as the sole owner of their own account and sees identical data.
### Changed ### Changed
- **Flow-media storage is now account-scoped.** Migration 016
pathed uploaded files under `auth.uid()/...`, which orphaned
flow media when a teammate left a shared account. New uploads
go under `account-<account_id>/...` and any account member
with the right role can edit them. Legacy paths remain
writable by the original uploader for backward compatibility.
- **Webhook contact lookup now pre-filters in SQL.** Previously
pulled every contact in an account just to JS-filter to one
row by phone — fine when account = one user, painful when
account = team. Pre-filter by phone suffix on the database
side; re-apply `phonesMatch` on the (typically 0-2 row)
candidate set.
### Migration required
- `supabase/migrations/020_account_sharing_followups.sql` —
composite partial indexes on `automations(account_id,
trigger_type) WHERE is_active` and `flows(account_id) WHERE
status='active'` for the engine dispatch hot path; updated
`flow-media` storage RLS to allow account-member writes under
the new path convention. Idempotent.
- **Role-aware UI gating across the app.** The inbox composer's - **Role-aware UI gating across the app.** The inbox composer's
send button + textarea, the "New broadcast / automation / flow" send button + textarea, the "New broadcast / automation / flow"
buttons, the "Add pipeline / deal" buttons, and the "Add / buttons, the "Add pipeline / deal" buttons, and the "Add /

View File

@ -875,21 +875,42 @@ async function findOrCreateContact(
phone: string, phone: string,
name: string name: string
): Promise<ContactOutcome | null> { ): Promise<ContactOutcome | null> {
// Look up existing contacts for this account. Switched from // Look up existing contacts for this account. We pre-filter in SQL
// user_id to account_id in PR 8 of the multi-user series so an // by the phone's last-8-digit suffix so we don't ship every contact
// inbound message lands on the team's existing contact row even // in the account over the wire just to JS-filter to one row. This
// if a different teammate created it. // matters at scale: a shared account with 5 teammates × 100 contacts
// each is 500 rows — the prior implementation pulled all of them on
// every inbound message.
//
// The `phonesMatch` helper considers two phones equal if they share
// the last 8 digits (trunk-prefix tolerance). We mirror that here as
// a `like` pattern, then re-run the strict comparison in JS on the
// narrowed candidate set. The candidate set is typically 0-2 rows,
// so the JS pass is effectively free.
//
// The trailing-suffix `like` cannot use a B-tree index, but the
// `account_id` filter on top of `idx_contacts_account` (017) means
// we sequential-scan a small, account-scoped subset.
const normalizedSender = phone.replace(/\D/g, '')
const phoneSuffix =
normalizedSender.length >= 8
? normalizedSender.slice(-8)
: normalizedSender
const { data: contacts, error: contactsError } = await supabaseAdmin() const { data: contacts, error: contactsError } = await supabaseAdmin()
.from('contacts') .from('contacts')
.select('*') .select('*')
.eq('account_id', accountId) .eq('account_id', accountId)
.like('phone', `%${phoneSuffix}`)
if (contactsError) { if (contactsError) {
console.error('Error fetching contacts:', contactsError) console.error('Error fetching contacts:', contactsError)
return null return null
} }
// Use phonesMatch for flexible matching // Re-apply phonesMatch on the candidate set for correctness — the
// SQL `like` is a coarse pre-filter; phonesMatch handles edge cases
// like leading-`+` and explicit trunk-zero handling.
const existingContact = contacts?.find((c: ContactRow) => phonesMatch(c.phone, phone)) const existingContact = contacts?.find((c: ContactRow) => phonesMatch(c.phone, phone))
if (existingContact) { if (existingContact) {

View File

@ -913,8 +913,22 @@ function SendMediaForm({
if (userErr || !user) { if (userErr || !user) {
throw new Error("Not signed in."); throw new Error("Not signed in.");
} }
// Path convention matches migration 016's RLS policy: first // Resolve the caller's account_id so the path is account-
// segment must equal auth.uid(). Timestamp + random suffix // scoped rather than user-scoped. The 020 migration's RLS
// policy allows any account member to write under
// `account-<account_id>/...`, so a flow's media survives a
// teammate leaving the account (which was a data-integrity
// hole in the original 016 storage policies).
const { data: profile, error: profileErr } = await supabase
.from("profiles")
.select("account_id")
.eq("user_id", user.id)
.maybeSingle();
if (profileErr || !profile?.account_id) {
throw new Error("Could not resolve your account.");
}
// Path convention matches migration 020's RLS policy: first
// segment is `account-<uuid>`. Timestamp + random suffix
// prevents collisions between two builders open at once. // prevents collisions between two builders open at once.
const ext = file.name.split(".").pop()?.toLowerCase() ?? "bin"; const ext = file.name.split(".").pop()?.toLowerCase() ?? "bin";
const safeBase = const safeBase =
@ -922,7 +936,7 @@ function SendMediaForm({
.replace(/\.[^.]+$/, "") .replace(/\.[^.]+$/, "")
.replace(/[^a-zA-Z0-9_-]+/g, "_") .replace(/[^a-zA-Z0-9_-]+/g, "_")
.slice(0, 40) || "file"; .slice(0, 40) || "file";
const path = `${user.id}/${Date.now()}-${safeBase}.${ext}`; const path = `account-${profile.account_id}/${Date.now()}-${safeBase}.${ext}`;
const { error: upErr } = await supabase.storage const { error: upErr } = await supabase.storage
.from(FLOW_MEDIA_BUCKET) .from(FLOW_MEDIA_BUCKET)
.upload(path, file, { .upload(path, file, {

View File

@ -0,0 +1,122 @@
-- ============================================================
-- 020_account_sharing_followups.sql — review-board fixes for
-- the multi-user accounts series (#167-#177).
--
-- Two concerns this migration addresses:
--
-- 1. Engine dispatch indexes — the per-inbound automations and
-- flows lookups now scope by `account_id + trigger_type/status
-- + is_active/status='active'`. The pre-017 partial indexes
-- (`idx_automations_active_trigger`, no flows equivalent) were
-- account-blind. For shared accounts with 100+ teammates each
-- authoring rules, the planner ends up post-filtering by
-- account_id. Composite partial indexes drop the post-filter
-- cost to zero on the hot path.
--
-- 2. Flow-media storage scoping — migration 016 created the
-- `flow-media` bucket with per-user RLS policies keyed on
-- `auth.uid() = path[0]`. After the multi-user move, flows
-- are account-scoped but the storage paths remained user-
-- scoped: an agent who left the account would orphan every
-- flow node referencing media they had uploaded. This
-- migration switches the write policies to account-scoped
-- paths (`account-<account_id>/...`) while leaving the
-- legacy `<auth.uid()>/...` paths writable by their original
-- uploader for backward compatibility. The bucket is public,
-- so reads are unchanged.
--
-- Idempotent — safe to run multiple times.
-- ============================================================
-- ============================================================
-- COMPOSITE INDEXES — engine dispatch hot path
-- ============================================================
-- `runAutomationsForTrigger` queries
-- automations WHERE account_id = X AND trigger_type = Y AND is_active = TRUE
-- Migration 006 added a partial index on (trigger_type) WHERE is_active.
-- Composite + partial index lets the planner answer all three predicates
-- from one index lookup. The existing partial index can stay as belt-and-
-- braces for any code path that filters only by trigger_type.
CREATE INDEX IF NOT EXISTS idx_automations_account_active_trigger
ON automations(account_id, trigger_type)
WHERE is_active = TRUE;
-- `findEntryFlow` queries
-- flows WHERE account_id = X AND status = 'active'
-- Migration 017 only added `idx_flows_account`; this partial composite
-- is tuned for the engine's lookup and skips archived/draft rows.
CREATE INDEX IF NOT EXISTS idx_flows_account_active
ON flows(account_id)
WHERE status = 'active';
-- ============================================================
-- FLOW-MEDIA STORAGE — account-scoped writes
--
-- New path convention: `account-<uuid>/<timestamp>-<base>.<ext>`
-- Legacy path convention: `<uuid>/<timestamp>-<base>.<ext>` (where
-- the uuid is auth.uid() — preserved for back-compat).
--
-- Reads stay public (the bucket is public so Meta can fetch media
-- URLs without credentials). Only the write policies change.
--
-- Drop existing per-user policies and replace with account-aware
-- ones that accept either path convention.
-- ============================================================
DROP POLICY IF EXISTS "Users can upload their own flow media" ON storage.objects;
DROP POLICY IF EXISTS "Users can update their own flow media" ON storage.objects;
DROP POLICY IF EXISTS "Users can delete their own flow media" ON storage.objects;
DROP POLICY IF EXISTS "Members can upload flow media" ON storage.objects;
CREATE POLICY "Members can upload flow media"
ON storage.objects FOR INSERT
WITH CHECK (
bucket_id = 'flow-media'
AND (
-- New: any account member uploading under their account's folder.
-- `'account-' || account_id` is how we namespace the folder, so
-- two accounts that happen to be in the same Supabase project
-- can never accidentally collide.
EXISTS (
SELECT 1 FROM public.profiles p
WHERE p.user_id = auth.uid()
AND ('account-' || p.account_id::text) = (storage.foldername(name))[1]
)
-- Legacy: the original uploader keeps write access to files they
-- already uploaded under the pre-020 path convention.
OR auth.uid()::text = (storage.foldername(name))[1]
)
);
DROP POLICY IF EXISTS "Members can update flow media" ON storage.objects;
CREATE POLICY "Members can update flow media"
ON storage.objects FOR UPDATE
USING (
bucket_id = 'flow-media'
AND (
EXISTS (
SELECT 1 FROM public.profiles p
WHERE p.user_id = auth.uid()
AND ('account-' || p.account_id::text) = (storage.foldername(name))[1]
)
OR auth.uid()::text = (storage.foldername(name))[1]
)
);
DROP POLICY IF EXISTS "Members can delete flow media" ON storage.objects;
CREATE POLICY "Members can delete flow media"
ON storage.objects FOR DELETE
USING (
bucket_id = 'flow-media'
AND (
EXISTS (
SELECT 1 FROM public.profiles p
WHERE p.user_id = auth.uid()
AND ('account-' || p.account_id::text) = (storage.foldername(name))[1]
)
OR auth.uid()::text = (storage.foldername(name))[1]
)
);
-- Public read policy from 016 stays as-is; reads cross both path
-- conventions without modification.