Skip to content

Commit 5f80917

Browse files
malek10xdevmalek10xdev
authored
fix(migration): delete orphan extension rows on phone-only migration (#2842)
* feat: create oauth client utilities for mining-sources edge function * fix: address code review issues - pin deps, add guards, fix types * feat: create mining-sources edge function with OAuth routes * fix: address code review issues - add onError handler, fix credentials JSON, add getRequiredEnv * test: add tests for mining-sources utility functions and route schemas * test: extract schemas to shared module, strengthen test assertions, add edge cases * feat: update frontend to call mining-sources edge function * chore: remove deprecated Express OAuth routes and methods * chore: remove deprecated add-mining-source edge function (replaced by mining-sources) * test: remove deprecated OAuth helper tests (functionality moved to edge function) * fix: resolve CI lint and DeepSource issues * feat: make oauth_callback_base_url a required env var with no fallbacks * docs: mark oauth_callback_base_url as required in env template * feat: add mining-sources config and document oauth_callback_base_url env var * fix: hmac-sign oauth state to prevent userId forgery * feat: add callbackQuerySchema for oauth callback route validation * test: add callbackQuerySchema tests * feat: use zod validation for oauth callback route params * chore: remove orphaned validator schemas and stale add-mining-source directory * fix: remove dead OAuth code from mining controller (post-merge cleanup) * feat: add create_smtp_sender_for_oauth rpc for smtp twin creation * feat: add smtp twin creation to mining-sources edge function callback * test: add smtp twin creation tests for mining-sources edge function * chore: remove plan docs from remote * chore: remove workmux config from remote * fix: use const instead of let for accessToken/refreshToken (deepsource) * feat(db): drop person_email, key extension tables by persons.id * chore(types): regenerate supabase types after person_id migration * feat: support phone-only contacts across backend stack Update all backend code to use person UUIDs (persons.id) instead of email as the primary identifier, matching the DB migration that drops the NOT NULL constraint on persons.email. - PgContacts: SQL constants and method bodies now use person_id; addManyAndReturn returns id+email+tags; new getPersonIdByEmail helper - Contacts interface: renamed email params to ids, new helper method, updateManyPersonsStatus uses {id, status} tuples - types.ts: Person.email is optional with Person.id added; Contact and ContactFrontend gain email? and identifier? fields - contacts.controller: request body field 'emails' renamed to 'ids', registerExportedContacts receives UUIDs - contacts-verification.controller: looks up person_id via getPersonIdByEmail before calling updateManyPersonsStatus - enrichments: filters contacts by id+email, registers engagements with person_id - engagements.register: payload uses person_id instead of email - Extractors (EmailMessage, FileImport, GoogleContactsExtractor, PostgreSQLImport) and Google export updated to handle optional email; phone-only contacts are skipped or guarded where appropriate - Tests updated: enrichments fixtures include id; pg-contacts mock rows include {id, email} * feat(frontend): support phone-only contacts across the UI Update the Vue stack to identify contacts by person UUID instead of email, matching the DB migration that drops the NOT NULL constraint on persons.email. Frontend Contact type: - email is now optional; new getContactIdentifier() helper returns email or first phone for display - ContactEdit.email also optional MiningTable.vue: - data-key switched from 'email' to 'id' - contactsToTreat now maps to id (used by ExportContacts and RemoveContactButton) - 'Send by Email' button disables when no selected contact has email - Refresh-status pill guards against contacts without an email - Identifier (email or phone) shown in the contacts column - selectedEmails store key renamed to selectedIds ExportContacts.vue: - Sends 'ids' instead of 'emails' in the export body - contactsToTreat v-model is now ids ContactInformationSidebar.vue: - contacts-to-treat prop, real-time subscription, and delete operations all key off person.id - copy button hides for phone-only contacts - refresh status no-ops without an email Pinia contacts store: - selectedEmails ref renamed to selectedIds - Cache, updates, removes and realtime handler all key off contact.id (was keyed off email) - hasPersons probe selects 'id' instead of 'email' utils/contacts.ts: - updateContact / updateContactTags now key off person_id - removeContactsFromDatabase passes 'ids' to delete_contacts RPC - delete_contacts RPC signature changed from emails text[] to ids uuid[] so phone-only contacts can be partially deleted CampaignComposerDialog.vue: - selectedEmails computed filters out phone-only contacts (email campaign can't reach them) Regenerated database.types.ts after the migration was applied (strips CLI noise, 1509 lines). * test(backend): cover phone-only contact round-trip in PgContacts * test(backend): ensure updateManyPersonsStatus works for phone-only contacts * feat(frontend): hide email-only widgets in sidebar for phone-only contacts * fix(phone-only): address critical and major review findings - C1: email-campaigns edge function rewrites getContactsByEmails to resolve emails -> person_ids -> get_contacts_table_by_ids (replaces call to dropped get_contacts_table_by_emails RPC) - C2: frontend removeContactsFromDatabase now passes ids[] to private.delete_contacts RPC (was passing emails[]) - M1: backend Google export mapToPerson skips null/empty email instead of writing { value: undefined } into emailAddresses - M2: delete_contacts message DELETE now scopes on m.user_id to prevent cross-user message leak when message_ids are shared - M3: delete_contacts parameter renamed user_id->p_user_id, ids->p_ids, deleteallcontacts->p_delete_all for consistency with other private.* functions and to avoid column shadowing Verified: - Local Supabase: 2-step email->id->contact lookup returns email contacts and correctly excludes phone-only contacts - delete_contacts partial delete and bulk delete both work - Backend 588/588 tests pass, build clean - Frontend 72/100 tests pass (28 pre-existing failures, same as baseline of 34 on main) - Frontend typecheck 14 errors (down from 29 baseline; all remaining are pre-existing) - Prettier clean * fix(phone-only): address minor review cleanups - m1: INSERT_TAGS_SQL now uses explicit ON CONFLICT (person_id, name, user_id) target instead of relying on Postgres inference (was 'ON CONFLICT DO NOTHING') - g: remove-contact confirmation dialog falls back to getContactIdentifier (phone) when name and email are empty - h: MiningTable global search now includes 'identifier' and 'telephone' so phone-only contacts are findable - i: EnrichButton filters out contacts with null email instead of casting to string (phone-only contacts cannot be enriched by email) - j: frontend callers updated to p_user_id parameter name; stale @ts-expect-error directives removed where the regenerated database.types.ts now types RPCs correctly (contacts store: get_contacts_table, refine_persons; sources: get_mining_source_overview; campaigns: get_campaigns_overview, get_sms_campaigns_overview; contacts utils: organizations CRUD, persons update) - regen: database.types.ts regenerated against updated migration; CLI noise stripped (1986 lines) * fix(frontend): replace non-null assertion in getContactIdentifier DeepSource JS-0339: forbidden non-null assertion. Use ?? '' fallback to match the function's existing return semantics. * fix(frontend): rename unused destructure members to silence DeepSource DeepSource JS-0356: 'tags' and 'sources' are assigned but never used in the destructure block. Follow the existing _id/_userId convention. * fix(extractor): add early-skip guard for person.email in EmailMessage DeepSource JS-0339: 4x forbidden non-null assertions on person.email. The extractContacts method is email-specific; in practice person.email is always set. Add an early return guard to make the type system happy and prevent any future crash if a phone-only contact somehow flows in. * fix(extractor): skip phone-only contacts in FileImport DeepSource JS-0339: forbidden non-null assertion on email in FileImport.ts getContacts(). Phone-only contacts from CSV/XLSX imports have no email to verify. The existing assert(Boolean(email)) in extractPerson was a runtime non-null assertion in disguise; remove it so phone-only contacts flow through extractPerson, and add an early return guard in getContacts before the email.split('@') call. Add a unit test that constructs a phone-only contact and verifies: - getContacts() returns an empty persons array - domainStatusVerification is not called - the internal Promise.allSettled does not silently reject (catches the underlying null-split crash that was previously swallowed by Promise.allSettled) * fix(extractor): skip phone-only contacts in PostgreSQLImport DeepSource JS-0339: forbidden non-null assertion on email in PostgreSQLImport.ts getContacts(). Phone-only contacts from a PostgreSQL mining source have no email to verify. Add an early return guard before the email.split('@') call. Add a unit test that constructs a phone-only contact and verifies: - getContacts() returns an empty persons array - domainStatusVerification is not called - the internal Promise.allSettled does not silently reject (catches the underlying null-split crash that was previously swallowed by Promise.allSettled) * style(backend): apply prettier to new extractor tests CI prettier check failed on fileImport.test.ts and postgresqlImport.test.ts. Auto-format applied. * fix(backend): address 3 DeepSource issues in new test files * chore(google-contacts-export): add skipcq comment on mapToPerson DeepSource JS-R1005: mapToPerson has cyclomatic complexity 33. Pre-existing on main; refactor tracked in #2831. Silenced with skipcq to clean up the PR's DeepSource report. The function maps 7+ schema field types (names, emails, phones, orgs, urls, addresses, memberships) in one pass, and the per-field 'is duplicate' checks against existing fields inherently inflate complexity. Splitting into helpers would be a behavioral change and out of scope for PR #2830. * fix(extractor): process phone-only contacts in FileImport and PostgreSQLImport The earlier DeepSource cleanup added 'if (!email) return;' guards to skip phone-only contacts in both extractors. This was wrong: these extractors can receive phone-only contacts from CSV/PostgreSQL mining sources, and the whole point of the phone-only support refactor is to surface them in the contact list. Behavior now matches GoogleContactsExtractor (which always adds the person to persons and defaults to a 'personal' tag with REACHABILITY.NONE when no email is available). Pre-existing on main in GoogleContactsExtractor; this aligns FileImport and PostgreSQLImport with that pattern. Changes: - FileImport: phone-only contacts added with personal/REACHABILITY.DIRECT_PERSON/ refined#phone_only tag, bypassing domain verification and email-based tagging - PostgreSQLImport: same, plus mapRowToContact no longer throws on missing email (was filtering out phone-only rows at the mapping layer) - TagSource union extended with 'refined#phone_only' - Tests updated: fileImport now asserts phone-only IS processed; added a second test verifying invalid-domain email contacts are still dropped - postgresqlImport now asserts phone-only IS processed EmailMessage engine is unchanged: it is email-specific by design, and contacts from email mailboxes always have emails. Tests: 592/592 pass (was 591; +1 new test case for invalid-domain drop). * fix(migration): delete orphan extension rows before person_id backfill
1 parent 7903e9b commit 5f80917

1 file changed

Lines changed: 109 additions & 0 deletions

File tree

‎supabase/migrations/20260603120000_drop_person_email_use_persons_id.sql‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,38 @@ ALTER TABLE private.persons
6868
-- partitioned parent propagates to all child partitions automatically.
6969
ALTER TABLE private.tags ADD COLUMN IF NOT EXISTS person_id UUID;
7070

71+
-- Delete orphan rows: for any (user_id, person_email) in tags that has
72+
-- no matching person, delete the tag. The original schema had no FK
73+
-- from tags.person_email to persons.email, so deleted persons left
74+
-- orphan tags. The backfill UPDATE below can only match existing
75+
-- persons, so orphan tags would leave NULL person_id and trip the
76+
-- RAISE EXCEPTION guard. We delete them here, respecting user intent
77+
-- (they deleted the contact, so the tag goes too). The count is
78+
-- logged via RAISE NOTICE for ops visibility.
79+
DO $$
80+
DECLARE
81+
orphan_count int;
82+
BEGIN
83+
SELECT COUNT(*) INTO orphan_count
84+
FROM private.tags t
85+
WHERE t.person_email IS NOT NULL
86+
AND NOT EXISTS (
87+
SELECT 1 FROM private.persons p
88+
WHERE p.email = t.person_email
89+
AND p.user_id = t.user_id
90+
);
91+
IF orphan_count > 0 THEN
92+
RAISE NOTICE 'Migration: deleting % orphan tag row(s) (person_email not in persons)', orphan_count;
93+
DELETE FROM private.tags t
94+
WHERE t.person_email IS NOT NULL
95+
AND NOT EXISTS (
96+
SELECT 1 FROM private.persons p
97+
WHERE p.email = t.person_email
98+
AND p.user_id = t.user_id
99+
);
100+
END IF;
101+
END $$;
102+
71103
-- Backfill person_id from persons on the parent
72104
UPDATE private.tags t
73105
SET person_id = p.id
@@ -114,6 +146,31 @@ ALTER TABLE private.tags DROP COLUMN person_email;
114146
-- partitioned parent propagates to all child partitions automatically.
115147
ALTER TABLE private.pointsofcontact ADD COLUMN IF NOT EXISTS person_id UUID;
116148

149+
-- Delete orphan pointsofcontact rows (see tags section for rationale).
150+
DO $$
151+
DECLARE
152+
orphan_count int;
153+
BEGIN
154+
SELECT COUNT(*) INTO orphan_count
155+
FROM private.pointsofcontact poc
156+
WHERE poc.person_email IS NOT NULL
157+
AND NOT EXISTS (
158+
SELECT 1 FROM private.persons p
159+
WHERE p.email = poc.person_email
160+
AND p.user_id = poc.user_id
161+
);
162+
IF orphan_count > 0 THEN
163+
RAISE NOTICE 'Migration: deleting % orphan pointsofcontact row(s) (person_email not in persons)', orphan_count;
164+
DELETE FROM private.pointsofcontact poc
165+
WHERE poc.person_email IS NOT NULL
166+
AND NOT EXISTS (
167+
SELECT 1 FROM private.persons p
168+
WHERE p.email = poc.person_email
169+
AND p.user_id = poc.user_id
170+
);
171+
END IF;
172+
END $$;
173+
117174
UPDATE private.pointsofcontact poc
118175
SET person_id = p.id
119176
FROM private.persons p
@@ -152,6 +209,32 @@ ALTER TABLE private.refinedpersons
152209

153210
ALTER TABLE private.refinedpersons ADD COLUMN IF NOT EXISTS person_id UUID;
154211

212+
-- Delete orphan refinedpersons rows. Note: column is 'email', not
213+
-- 'person_email'. Same rationale as the tags section.
214+
DO $$
215+
DECLARE
216+
orphan_count int;
217+
BEGIN
218+
SELECT COUNT(*) INTO orphan_count
219+
FROM private.refinedpersons r
220+
WHERE r.email IS NOT NULL
221+
AND NOT EXISTS (
222+
SELECT 1 FROM private.persons p
223+
WHERE p.email = r.email
224+
AND p.user_id = r.user_id
225+
);
226+
IF orphan_count > 0 THEN
227+
RAISE NOTICE 'Migration: deleting % orphan refinedpersons row(s) (email not in persons)', orphan_count;
228+
DELETE FROM private.refinedpersons r
229+
WHERE r.email IS NOT NULL
230+
AND NOT EXISTS (
231+
SELECT 1 FROM private.persons p
232+
WHERE p.email = r.email
233+
AND p.user_id = r.user_id
234+
);
235+
END IF;
236+
END $$;
237+
155238
UPDATE private.refinedpersons r
156239
SET person_id = p.id
157240
FROM private.persons p
@@ -185,6 +268,32 @@ ALTER TABLE private.refinedpersons DROP COLUMN email;
185268

186269
ALTER TABLE private.engagement ADD COLUMN IF NOT EXISTS person_id UUID;
187270

271+
-- Delete orphan engagement rows. Note: column is 'email', not
272+
-- 'person_email'. Same rationale as the tags section.
273+
DO $$
274+
DECLARE
275+
orphan_count int;
276+
BEGIN
277+
SELECT COUNT(*) INTO orphan_count
278+
FROM private.engagement e
279+
WHERE e.email IS NOT NULL
280+
AND NOT EXISTS (
281+
SELECT 1 FROM private.persons p
282+
WHERE p.email = e.email
283+
AND p.user_id = e.user_id
284+
);
285+
IF orphan_count > 0 THEN
286+
RAISE NOTICE 'Migration: deleting % orphan engagement row(s) (email not in persons)', orphan_count;
287+
DELETE FROM private.engagement e
288+
WHERE e.email IS NOT NULL
289+
AND NOT EXISTS (
290+
SELECT 1 FROM private.persons p
291+
WHERE p.email = e.email
292+
AND p.user_id = e.user_id
293+
);
294+
END IF;
295+
END $$;
296+
188297
UPDATE private.engagement e
189298
SET person_id = p.id
190299
FROM private.persons p

0 commit comments

Comments
 (0)