Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
supabase/functions/_backend/utils/supabase.ts (1)
138-149:⚠️ Potential issue | 🟠 MajorHandle lookup errors before building
upsertPayload.At Line 138–143, lookup errors are ignored. If that query fails, Line 147 falls back to
update.created_by, and the upsert can overwrite ownership despite this PR’s intent.🔧 Proposed fix
- const { data: existingChannel } = await supabaseAdmin(c) + const { data: existingChannel, error: existingChannelError } = await supabaseAdmin(c) .from('channels') .select('*') .eq('app_id', update.app_id) .eq('name', update.name) - .single() + .maybeSingle() + + if (existingChannelError) { + cloudlogErr({ requestId: c.get('requestId'), message: 'channel lookup failed', error: existingChannelError }) + return Promise.reject(existingChannelError) + } const upsertPayload = { ...update, - created_by: existingChannel?.created_by || update.created_by, + created_by: existingChannel?.created_by ?? update.created_by, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/supabase.ts` around lines 138 - 149, The lookup result from supabaseAdmin(c).from('channels').select(...).single() may return an error which is currently ignored before constructing upsertPayload; update.created_by should not silently override existing ownership when the lookup failed. After the query, check the returned error (the variable paired with data) and if present, surface/propagate it (throw, return an error response, or log and abort) instead of proceeding to build upsertPayload; then only set created_by from existingChannel?.created_by when existingChannel is present, otherwise fail the operation so upsertPayload cannot overwrite ownership inadvertently (refer to supabaseAdmin, existingChannel, upsertPayload, and update.created_by).
🧹 Nitpick comments (1)
supabase/functions/_backend/utils/supabase.ts (1)
160-163: Migrate this DB write path to the PG/Drizzle client helpers.This changed upsert path still goes through
supabaseAdmin(c).As per coding guidelines "All database operations must use
getPgClient()orgetDrizzleClient()fromutils/pg.tsfor PostgreSQL access during active migration to Cloudflare D1".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/utils/supabase.ts` around lines 160 - 163, The current upsert to the channels table uses supabaseAdmin(c); replace it with the PG/Drizzle client helpers by obtaining a client via getDrizzleClient(c) (or getPgClient(c) if preferred) and perform the equivalent upsert of upsertPayload into the channels table using Drizzle's insert().onConflict().doUpdate() (or the corresponding upsert helper on the pg client), ensuring the conflict target is app_id and name and preserving the same values as the original upsert, and remove the call to throwOnError in favor of proper error handling from the Drizzle/PG client.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@supabase/functions/_backend/utils/supabase.ts`:
- Around line 138-149: The lookup result from
supabaseAdmin(c).from('channels').select(...).single() may return an error which
is currently ignored before constructing upsertPayload; update.created_by should
not silently override existing ownership when the lookup failed. After the
query, check the returned error (the variable paired with data) and if present,
surface/propagate it (throw, return an error response, or log and abort) instead
of proceeding to build upsertPayload; then only set created_by from
existingChannel?.created_by when existingChannel is present, otherwise fail the
operation so upsertPayload cannot overwrite ownership inadvertently (refer to
supabaseAdmin, existingChannel, upsertPayload, and update.created_by).
---
Nitpick comments:
In `@supabase/functions/_backend/utils/supabase.ts`:
- Around line 160-163: The current upsert to the channels table uses
supabaseAdmin(c); replace it with the PG/Drizzle client helpers by obtaining a
client via getDrizzleClient(c) (or getPgClient(c) if preferred) and perform the
equivalent upsert of upsertPayload into the channels table using Drizzle's
insert().onConflict().doUpdate() (or the corresponding upsert helper on the pg
client), ensuring the conflict target is app_id and name and preserving the same
values as the original upsert, and remove the call to throwOnError in favor of
proper error handling from the Drizzle/PG client.
* fix(auth): block account deletion for unverified users * fix(auth): refresh session fields for email verification gate * fix(auth): make delete_user insert idempotent * fix(auth): explain blocked delete/settings when email unverified * fix(auth): block delete action when email is unverified * fix(auth): localize resend email block and make delete_user idempotent
* fix(db): restrict invite_user_to_org public rpc * fix(db): use caller identity in invite 2FA check
* fix(db): secure record_build_time rpc writes * fix(db): preserve service-role record_build_time path
…apgo into riderx/channel-post-fix
|
1 participant
Summary (AI generated)
This change prevents
POST /channelfrom overwriting immutable ownership on existing channel updates by preserving the originalcreated_bywhen a channel with the sameapp_idandnamealready exists.Test plan (AI generated)
I ran
bunx eslint supabase/functions/_backend/utils/supabase.tsand verified no lint errors.Screenshots (AI generated)
No screenshots are needed because this is backend-only.
Checklist (AI generated)
bun run lint:backend && bun run lint.Summary by CodeRabbit