fix rls trigger#41776
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
6 Skipped Deployments
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughA new PostgreSQL event trigger SQL constant ( Changes
🚥 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
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: 0
🧹 Nitpick comments (2)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts (1)
16-51: LGTM! Consider simplifying the schema condition.The SQL logic correctly implements auto-enabling RLS on new tables. Security best practices are followed with
SECURITY DEFINERandsearch_path = pg_catalog.However, line 32 contains redundant conditions. Since you're checking
cmd.schema_name IN ('public'), the subsequent checks for system schemas are unnecessary:♻️ Optional simplification
- IF cmd.schema_name IS NOT NULL AND cmd.schema_name IN ('public') AND cmd.schema_name NOT IN ('pg_catalog','information_schema') AND cmd.schema_name NOT LIKE 'pg_toast%' AND cmd.schema_name NOT LIKE 'pg_temp%' THEN + IF cmd.schema_name IS NOT NULL AND cmd.schema_name IN ('public') THENIf you plan to expand the enforced schema list beyond 'public', consider keeping the defensive checks but add a comment explaining the future-proofing intent.
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx (1)
37-37: Minor optimization: Consider using a constant empty array.Changing from
EMPTY_ARRto[]creates a new array instance on each render whendatais undefined. While the loading guard at line 72 mitigates most practical impact, this removes a minor optimization.♻️ Optional: Restore referential equality
If you want to maintain the optimization:
+const EMPTY_ARR: never[] = [] + export const BannerRlsEventTrigger = () => { // ... - const { data: eventTriggers = [], isLoading: isLoadingEventTriggers } = + const { data: eventTriggers = EMPTY_ARR, isLoading: isLoadingEventTriggers } =This prevents the
useMemoat line 43 from recalculating unnecessarily when the component renders before data loads.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.tsapps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/studio/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/studio-best-practices.mdc)
apps/studio/**/*.{ts,tsx}: Assign complex conditions to descriptive variables instead of using multiple conditions in a single expression
Use consistent naming conventions for boolean variables:isprefix for state/identity,hasprefix for possession,canprefix for capability/permission,shouldprefix for conditional behavior
Derive boolean state from existing state instead of storing it separately
Use early returns for guard clauses instead of deeply nested conditionals
Extract complex logic into custom hooks when logic becomes reusable or complex
Return objects from custom hooks instead of arrays for better extensibility and clearer API
Use discriminated unions for complex state management instead of multiple independent state fields
Avoid type casting (e.g.,as any,as Type); instead validate values at runtime using zod schemas
Files:
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsxapps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
apps/studio/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/studio-best-practices.mdc)
apps/studio/**/*.tsx: Components should ideally be under 200-300 lines; break down large components with multiple distinct UI sections, complex conditional rendering, or multiple unrelated useState hooks
Extract repeated JSX patterns into reusable components instead of copying similar JSX blocks
Use consistent loading/error/success pattern: handle loading state first with early returns, then error state, then empty state, then render success state
Keep state as local as possible and only lift up when needed
Group related state using objects or reducers instead of multiple useState calls, preferring react-hook-form for form state management
Name event handlers consistently: useonprefix for prop callbacks andhandleprefix for internal handlers
Avoid inline arrow functions for expensive operations; use useCallback to maintain stable function references
Use appropriate conditional rendering patterns: && for simple show/hide, ternary for binary choice, early returns or extracted components for multiple conditions
Avoid nested ternaries in JSX; use separate conditions or early returns instead
Use useMemo for expensive computations when the computation is genuinely expensive and the value is passed to memoized children
Define prop interfaces explicitly for React components with proper typing of props and callback handlers
Files:
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx
🧠 Learnings (2)
📚 Learning: 2025-12-12T05:20:17.409Z
Learnt from: joshenlim
Repo: supabase/supabase PR: 41258
File: apps/studio/pages/project/[ref]/storage/vectors/buckets/[bucketId].tsx:9-9
Timestamp: 2025-12-12T05:20:17.409Z
Learning: In apps/studio/**/*.{ts,tsx}, use named imports for DefaultLayout: import { DefaultLayout } from 'components/layouts/DefaultLayout' instead of default import. This is the new practice being adopted across the studio app.
Applied to files:
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsxapps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
📚 Learning: 2026-01-02T05:58:55.226Z
Learnt from: meGauravJain
Repo: supabase/supabase PR: 41658
File: apps/studio/components/interfaces/Connect/content/prisma/content.tsx:70-85
Timestamp: 2026-01-02T05:58:55.226Z
Learning: In Prisma configuration or related connection setup code, prefer the environment variable name DIRECT_URL for direct/non-pooled database connections instead of DATABASE_URL_UNPOOLED, per Prisma documentation. Update code that reads database connection strings to use DIRECT_URL where applicable, and document why this is preferred (explicit, documented behavior). Ensure your environment provides DIRECT_URL when using direct connections and fallback handling if needed.
Applied to files:
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsxapps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts
🧬 Code graph analysis (1)
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx (1)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts (1)
AUTO_ENABLE_RLS_EVENT_TRIGGER_SQL(16-51)
⏰ 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). (3)
- GitHub Check: typecheck
- GitHub Check: test (1)
- GitHub Check: test
🔇 Additional comments (3)
apps/studio/components/interfaces/Database/Triggers/EventTriggersList/EventTriggers.constants.ts (1)
57-57: Correct fix! The template now uses the appropriate SQL.This change correctly replaces the generic placeholder template with the specific RLS auto-enable SQL, aligning with the PR objective.
apps/studio/components/ui/BannerStack/Banners/BannerRlsEventTrigger.tsx (2)
7-7: Correct import for the RLS auto-enable SQL.The import change aligns with the new constant defined in EventTriggers.constants.ts.
68-68: Correctly uses the RLS auto-enable SQL template.This change fixes the core issue where the banner was creating a generic event trigger instead of the specific RLS auto-enable trigger.
🎭 Playwright Test ResultsDetails
Flaky testsFeatures › table-editor.spec.ts › table editor › add enums and show enums on table Skipped testsFeatures › sql-editor.spec.ts › SQL Editor › snippet favourite works as expected |
joshenlim
left a comment
There was a problem hiding this comment.
Verified changes on staging preview!
Braintrust eval report
|
Fixes trigger generated as part of policy banner to use correct template
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.