fix: delete_user properly call on_user_delete#1262
Conversation
WalkthroughA new SQL migration replaces Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant DB_Function as public.delete_user()
participant Auth as auth.uid()
participant Queue as queue.on_user_delete
participant ToDelete as to_delete_accounts (table)
participant APIKeys as apikeys (table)
Client->>DB_Function: CALL public.delete_user()
DB_Function->>Auth: auth.uid() (resolve current user id)
Auth-->>DB_Function: user_id
DB_Function->>DB_Function: SELECT user row -> row_to_json() as old_record_json
DB_Function->>Queue: publish on_user_delete(payload: {old_record, table, type})
Queue-->>DB_Function: publish ACK
DB_Function->>ToDelete: INSERT removed_data, removed_at (now + 30 days)
DB_Function->>APIKeys: DELETE WHERE owner = user_id
APIKeys-->>DB_Function: deletion result
DB_Function-->>Client: function completes (commit/delete done)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 SQLFluff (3.5.0)supabase/migrations/20251208175306_fix_user_delete_old_record.sqlUser Error: No dialect was specified. You must configure a dialect or specify one on the command line using --dialect after the command. Available dialects: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
supabase/migrations/20251208175306_fix_user_delete_old_record.sql (1)
18-20: Qualify table reference for clarity in SECURITY DEFINER context.While unqualified
userswill resolve correctly, explicitly usingpublic.usersimproves clarity and consistency, especially in SECURITY DEFINER functions where search_path is restrictive.SELECT row_to_json(u)::jsonb INTO old_record_json FROM ( SELECT * - FROM users + FROM public.users WHERE id = user_id_fn ) AS u;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/20251208175306_fix_user_delete_old_record.sql(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{supabase/functions/_backend/utils/postgress_schema.ts,supabase/migrations/**/*.sql}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database schema must be defined in
utils/postgress_schema.tsusing Drizzle ORM and mirrored in SQL migrations
Files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/migrations/**/*.sql: Never edit previously committed migration files; create new migration files withsupabase migration new <feature_slug>for schema changes
Runbun typesafter schema changes in migrations to regenerate TypeScript types for Supabase
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file withsupabase migration new <feature_slug>and keep editing that file until the feature ships; never edit previously committed migrations
A migration that introduces a new table may include seed inserts for that table, treating that seeding as part of the current feature and not modifying previously committed migrations
Do not create new cron jobs; instead update theprocess_all_cron_tasksfunction in a new migration file to add your job if needed
Files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
🧠 Learnings (7)
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` to back new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:12.533Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T17:34:12.533Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` with new or changed test fixtures when adding database schema changes
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:12.533Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T17:34:12.533Z
Learning: Applies to supabase/migrations/**/*.sql : Never edit previously committed migration files; create new migration files with `supabase migration new <feature_slug>` for schema changes
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : Do not create new cron jobs; instead update the `process_all_cron_tasks` function in a new migration file to add your job if needed
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : When a feature requires schema changes, create a single migration file with `supabase migration new <feature_slug>` and keep editing that file until the feature ships; never edit previously committed migrations
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : A migration that introduces a new table may include seed inserts for that table, treating that seeding as part of the current feature and not modifying previously committed migrations
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:12.533Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T17:34:12.533Z
Learning: Active migration from Supabase Postgres (V1) to Cloudflare D1 (V2); V1 functions use `pg.ts` with `getPgClient()` and `getDrizzleClient()`, V2 functions use `pg_d1.ts` with `getPgClientD1()` and `getDrizzleClientD1()`
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
supabase/migrations/20251208175306_fix_user_delete_old_record.sql (1)
45-45: Consider explicit handling for missing API keys in removed_data.Line 45 uses
jsonb_agg()to aggregate API keys, which returnsnullif no API keys exist for the user. This results in{"apikeys": null}inremoved_data. If downstream processing expects an empty array instead, consider usingCOALESCE:- "jsonb_build_object"('email', user_email, 'apikeys', (SELECT "jsonb_agg"("to_jsonb"(a.*)) FROM "public"."apikeys" a WHERE a."user_id" = user_id_fn)) + "jsonb_build_object"('email', user_email, 'apikeys', COALESCE((SELECT "jsonb_agg"("to_jsonb"(a.*)) FROM "public"."apikeys" a WHERE a."user_id" = user_id_fn), '[]'::jsonb))Verify whether downstream consumers of
to_delete_accounts.removed_datahandlenullfor theapikeysfield or expect an empty array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
supabase/migrations/20251208175306_fix_user_delete_old_record.sql(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{supabase/functions/_backend/utils/postgress_schema.ts,supabase/migrations/**/*.sql}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Database schema must be defined in
utils/postgress_schema.tsusing Drizzle ORM and mirrored in SQL migrations
Files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
supabase/migrations/**/*.sql
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
supabase/migrations/**/*.sql: Never edit previously committed migration files; create new migration files withsupabase migration new <feature_slug>for schema changes
Runbun typesafter schema changes in migrations to regenerate TypeScript types for Supabase
supabase/migrations/**/*.sql: When a feature requires schema changes, create a single migration file withsupabase migration new <feature_slug>and keep editing that file until the feature ships; never edit previously committed migrations
A migration that introduces a new table may include seed inserts for that table, treating that seeding as part of the current feature and not modifying previously committed migrations
Do not create new cron jobs; instead update theprocess_all_cron_tasksfunction in a new migration file to add your job if needed
Files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
🧠 Learnings (6)
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` to back new or evolved tests; keep fixtures focused on current behavior while leaving committed migrations unchanged
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:12.533Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T17:34:12.533Z
Learning: Applies to supabase/seed.sql : Update `supabase/seed.sql` with new or changed test fixtures when adding database schema changes
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:12.533Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T17:34:12.533Z
Learning: Applies to supabase/migrations/**/*.sql : Never edit previously committed migration files; create new migration files with `supabase migration new <feature_slug>` for schema changes
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:12.533Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T17:34:12.533Z
Learning: Active migration from Supabase Postgres (V1) to Cloudflare D1 (V2); V1 functions use `pg.ts` with `getPgClient()` and `getDrizzleClient()`, V2 functions use `pg_d1.ts` with `getPgClientD1()` and `getDrizzleClientD1()`
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : When a feature requires schema changes, create a single migration file with `supabase migration new <feature_slug>` and keep editing that file until the feature ships; never edit previously committed migrations
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
📚 Learning: 2025-12-05T17:34:36.569Z
Learnt from: CR
Repo: Cap-go/capgo PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-05T17:34:36.569Z
Learning: Applies to supabase/migrations/**/*.sql : Do not create new cron jobs; instead update the `process_all_cron_tasks` function in a new migration file to add your job if needed
Applied to files:
supabase/migrations/20251208175306_fix_user_delete_old_record.sql
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests
🔇 Additional comments (1)
supabase/migrations/20251208175306_fix_user_delete_old_record.sql (1)
24-34: Syntax correction from previous review is now fixed.The
jsonb_build_object()call now correctly uses alternating key-value pairs ('payload', "jsonb_build_object"(...), 'function_name', 'on_user_delete') instead of invalid object-literal syntax. The structure properly includes'table': 'users'and'type': 'DELETE'as required by the queue event handler.
|
2 participants
The business issue
Currently, deleting the account doesn't cancel the Capgo subscription. This has caused one user to get charged when he shouldn't have been. Furthermore, we had to issue a refund, which is a bad experience for the user and Martin had to waste his time on issuing the refund.
Motivation
I strongly believe that if I have implemented a feature where deleting the account calls
on_user_deleteimmediately, which consequently cancels the subscription, it should work. Therefore, I am making this PRThe fix
The
on_user_deletecontainstriggerValidator('users', 'DELETE'). I have missed that in the original PR #1191. As such, I have corrected theon_user_deletequeue callPERFORM "pgmq"."send"( 'on_user_delete'::text, "jsonb_build_object"( + 'old_record', old_record_json, - 'user_id', user_id_fn, - 'email', user_email + "table": "users", + "type": "DELETE", ) );PR's checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.