fix: recreate group when the ID changes but the link does not#31
fix: recreate group when the ID changes but the link does not#31lorenzocorallo merged 2 commits intomainfrom
Conversation
WalkthroughThis PR introduces a unique constraint on the Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routers/tg/groups.ts (1)
79-95:⚠️ Potential issue | 🔴 CriticalMake delete+upsert atomic to avoid partial data loss and race conditions.
Right now the cleanup delete and the insert/upsert run in separate statements without a transaction. If insert fails (including unique conflicts inside a batch), previously deleted rows are already gone.
🛠️ Suggested direction
- for (const group of input) { - await DB.delete(...); - } - const rows = await DB.insert(...).onConflictDoUpdate(...).returning() + // Wrap cleanup + insert/upsert in a single DB transaction + // so either all changes commit or none do.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routers/tg/groups.ts` around lines 79 - 95, Wrap the cleanup deletes and the subsequent insert/upsert in a single database transaction so they execute atomically: open a transaction via DB.transaction (or the project's transactional API), run the looped DB.delete(GROUPS).where(and(eq(GROUPS.link, group.link), ne(GROUPS.telegramId, group.telegramId))) calls against the transaction handle (e.g., trx.delete(...)), then run the DB.insert(GROUPS).values(input).onConflictDoUpdate({...}).returning() also against the same trx; return the resulting rows' telegramId and let the transaction commit/rollback automatically to avoid partial deletes on insert failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/routers/tg/groups.ts`:
- Line 80: Before deleting the old GROUPS row via
DB.delete(GROUPS).where(and(eq(GROUPS.link, group.link), ne(GROUPS.telegramId,
group.telegramId))), first update all dependent references (e.g.,
groupAdmins.groupId which maps to groups.telegramId and any other FKs joined in
src/routers/tg/permissions.ts) to the new group.telegramId; perform these
updates with DB.update on the dependent tables to set their groupId =
group.telegramId where groupId = oldTelegramId, then run the delete, and wrap
the update+delete in a single transaction to avoid race conditions and ensure
referential integrity.
---
Outside diff comments:
In `@src/routers/tg/groups.ts`:
- Around line 79-95: Wrap the cleanup deletes and the subsequent insert/upsert
in a single database transaction so they execute atomically: open a transaction
via DB.transaction (or the project's transactional API), run the looped
DB.delete(GROUPS).where(and(eq(GROUPS.link, group.link), ne(GROUPS.telegramId,
group.telegramId))) calls against the transaction handle (e.g.,
trx.delete(...)), then run the
DB.insert(GROUPS).values(input).onConflictDoUpdate({...}).returning() also
against the same trx; return the resulting rows' telegramId and let the
transaction commit/rollback automatically to avoid partial deletes on insert
failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b06795a2-7973-4cd6-9811-05a4d0fee285
📒 Files selected for processing (5)
drizzle/0009_bouncy_unicorn.sqldrizzle/meta/0009_snapshot.jsondrizzle/meta/_journal.jsonsrc/db/schema/tg/groups.tssrc/routers/tg/groups.ts
No description provided.