add backup deletion on user delete #576
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe convex/auth.ts file was expanded to integrate authentication triggers. Changes include importing AuthFunctions from better-auth, wiring authFunctions into the authComponent, and adding a user deletion trigger that removes associated backup documents. The triggers API is exported for external use. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Deploying edutools-testing with
|
| Latest commit: |
42a5d74
|
| Status: | 🚫 Build failed. |
![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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
convex/auth.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
**/*.{ts,tsx,js,jsx}: If using TypeScript, use an enum to store flag names. If using JavaScript, store flag names as strings to an object declared as a constant, to simulate an enum. Use a consistent naming convention for this storage. enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
Gate flag-dependent code on a check that verifies the flag's values are valid and expected.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object as defined for feature flags, with enum/const object members written UPPERCASE_WITH_UNDERSCORE.
Files:
convex/auth.ts
**/*.*
⚙️ CodeRabbit configuration file
**/*.*: Do not correct spelling errors or grammar mistakes.
Files:
convex/auth.ts
⏰ 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: Cloudflare Pages
🔇 Additional comments (3)
convex/auth.ts (3)
1-7: LGTM!The import additions are correct and necessary for the new trigger functionality.
17-18: LGTM!The authFunctions binding and integration into the authComponent configuration is correct.
Also applies to: 27-27
139-139: LGTM!The triggers API export is correct and provides the necessary public interface.
| onDelete: async (ctx, doc) => { | ||
| if (!doc?.userId) { | ||
| return; | ||
| } | ||
|
|
||
| const backups = await ctx.db | ||
| .query("backups") | ||
| .withIndex("by_user", (q) => q.eq("userId", doc.userId as string)) | ||
| .collect(); | ||
| for (const backup of backups) { | ||
| await ctx.db.delete(backup._id); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Add error handling and consider logging for the deletion loop.
The deletion loop lacks error handling, which could result in partial cleanup if any deletion fails. Additionally, there's no observability into how many backups were deleted or if any failures occurred.
Apply this diff to add error handling and logging:
onDelete: async (ctx, doc) => {
if (!doc?.userId) {
return;
}
const backups = await ctx.db
.query("backups")
.withIndex("by_user", (q) => q.eq("userId", doc.userId as string))
.collect();
+
+ let deletedCount = 0;
+ let failedCount = 0;
+
for (const backup of backups) {
- await ctx.db.delete(backup._id);
+ try {
+ await ctx.db.delete(backup._id);
+ deletedCount++;
+ } catch (error) {
+ console.error(`Failed to delete backup ${backup._id}:`, error);
+ failedCount++;
+ }
}
+
+ console.log(`User ${doc.userId} deleted: ${deletedCount} backups removed, ${failedCount} failed`);
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onDelete: async (ctx, doc) => { | |
| if (!doc?.userId) { | |
| return; | |
| } | |
| const backups = await ctx.db | |
| .query("backups") | |
| .withIndex("by_user", (q) => q.eq("userId", doc.userId as string)) | |
| .collect(); | |
| for (const backup of backups) { | |
| await ctx.db.delete(backup._id); | |
| } | |
| }, | |
| onDelete: async (ctx, doc) => { | |
| if (!doc?.userId) { | |
| return; | |
| } | |
| const backups = await ctx.db | |
| .query("backups") | |
| .withIndex("by_user", (q) => q.eq("userId", doc.userId as string)) | |
| .collect(); | |
| let deletedCount = 0; | |
| let failedCount = 0; | |
| for (const backup of backups) { | |
| try { | |
| await ctx.db.delete(backup._id); | |
| deletedCount++; | |
| } catch (error) { | |
| console.error(`Failed to delete backup ${backup._id}:`, error); | |
| failedCount++; | |
| } | |
| } | |
| console.log(`User ${doc.userId} deleted: ${deletedCount} backups removed, ${failedCount} failed`); | |
| }, |
🤖 Prompt for AI Agents
In convex/auth.ts around lines 30 to 42, the deletion loop for user backups has
no error handling or logging which can lead to partial cleanup and no
observability; wrap each ctx.db.delete call in a try/catch so one failure
doesn’t stop subsequent deletes, count successful and failed deletions, and log
a summary (and individual errors) after the loop; use the repository’s existing
logger (or console.error if none) and avoid swallowing errors silently —
optionally return or rethrow an aggregated error if the caller must know about
failures.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.