feat(orgs): enable RBAC by default for new organizations#1723
Closed
feat(orgs): enable RBAC by default for new organizations#1723
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughA migration sets the default Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
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 |
…t tests Tests 37 and 40 test 2FA and password-policy enforcement using the legacy org_users rights model. With use_new_rbac now defaulting to true, newly created orgs switched to the RBAC path where `write` at org scope maps to `org:update_settings` – a permission not held by the `org_member` role that the sync trigger assigns to write-level users. Pinning these test orgs to use_new_rbac=false keeps them on the legacy path they were designed to test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add dedicated API key creation in beforeAll for testOrgId - Use testHeaders for tests operating on dynamic org - Fix vitest.config.ts to load .env.test properly Fixes intermittent CI failures in: - 'should return 404 when app not found' - 'should return 400 when org does not have customer'
…acy compatibility The commit that enabled RBAC by default (1690fbf) broke tests that create orgs dynamically. These tests depend on the legacy org_users permission model. Changes: - Add use_new_rbac: false to 17 org insertions across 12 test files - Keeps vitest.config.ts fix for loading .env.test This ensures existing tests continue to work with the legacy permission model while new code can use RBAC. Fixes race condition in private-error-cases tests caused by RBAC migration.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/organization-api.test.ts`:
- Around line 31-32: Add a positive assertion that the created organization uses
the new RBAC by checking use_new_rbac === true after the POST /organization call
in the tests; locate the test that performs the POST (the organization creation
block in tests/organization-api.test.ts) and assert on the response body or
fetched organization object (e.g., createdOrg.use_new_rbac) to be true—apply the
same addition to the other organization-creation assertions referenced (the
blocks around use_new_rbac occurrences).
In `@tests/private-error-cases.test.ts`:
- Line 34: The line containing the configuration key use_new_rbac has extra
spacing before the inline comment; remove the extra space so it conforms to
style/no-multi-spaces — update the line with use_new_rbac: false, // Use legacy
permissions for this test (single space before the comment) to eliminate the
multi-space violation.
In `@vitest.config.ts`:
- Line 35: Add a trailing newline at the end of vitest.config.ts (after the
final "})" closing) to satisfy the style/eol-last lint rule; simply ensure the
file ends with a newline character so the last line is terminated.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
supabase/seed.sqlsupabase/tests/37_test_check_min_rights_2fa_enforcement.sqlsupabase/tests/40_test_password_policy_enforcement.sqltests/admin-credits.test.tstests/apikeys-expiration.test.tstests/app-id-validation.test.tstests/audit-logs.test.tstests/build_time_tracking.test.tstests/bundle-create.test.tstests/cron_stat_org.test.tstests/organization-api.test.tstests/password-policy.test.tstests/private-error-cases.test.tstests/webhook-signature.test.tstests/webhooks.test.tsvitest.config.ts
✅ Files skipped from review due to trivial changes (1)
- tests/admin-credits.test.ts
Comment on lines
+31
to
32
| use_new_rbac: false, | ||
| }) |
Contributor
There was a problem hiding this comment.
Add a positive assertion for the new default RBAC behavior.
These fixture inserts intentionally pin legacy mode, but this suite should still verify that POST /organization creates orgs with use_new_rbac = true (core PR contract).
Suggested minimal test assertion
const { data, error } = await getSupabaseClient().from('orgs').select().eq('id', safe.data!.id).single()
expect(error).toBeNull()
expect(data).toBeTruthy()
expect(data?.name).toBe(name)
+ expect(data?.use_new_rbac).toBe(true)Also applies to: 530-531, 617-618
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/organization-api.test.ts` around lines 31 - 32, Add a positive
assertion that the created organization uses the new RBAC by checking
use_new_rbac === true after the POST /organization call in the tests; locate the
test that performs the POST (the organization creation block in
tests/organization-api.test.ts) and assert on the response body or fetched
organization object (e.g., createdOrg.use_new_rbac) to be true—apply the same
addition to the other organization-creation assertions referenced (the blocks
around use_new_rbac occurrences).
| management_email: testOrgEmail, | ||
| created_by: USER_ID, | ||
| customer_id: testCustomerId, | ||
| use_new_rbac: false, // Use legacy permissions for this test |
Contributor
There was a problem hiding this comment.
Remove extra spacing before inline comment.
Line 34 violates style/no-multi-spaces.
Suggested fix
- use_new_rbac: false, // Use legacy permissions for this test
+ use_new_rbac: false, // Use legacy permissions for this test📝 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.
Suggested change
| use_new_rbac: false, // Use legacy permissions for this test | |
| use_new_rbac: false, // Use legacy permissions for this test |
🧰 Tools
🪛 ESLint
[error] 34-34: Multiple spaces found before '// Use legacy ...'.
(style/no-multi-spaces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/private-error-cases.test.ts` at line 34, The line containing the
configuration key use_new_rbac has extra spacing before the inline comment;
remove the extra space so it conforms to style/no-multi-spaces — update the line
with use_new_rbac: false, // Use legacy permissions for this test (single space
before the comment) to eliminate the multi-space violation.
| }, | ||
| })) | ||
| } | ||
| }) No newline at end of file |
Contributor
There was a problem hiding this comment.
Add trailing newline to pass lint.
Line 35 currently violates style/eol-last.
Suggested fix
export default defineConfig(({ mode }) => {
@@
}
})
+📝 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.
Suggested change
| }) | |
| }) | |
🧰 Tools
🪛 ESLint
[error] 35-35: Newline required at end of file but not found.
(style/eol-last)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vitest.config.ts` at line 35, Add a trailing newline at the end of
vitest.config.ts (after the final "})" closing) to satisfy the style/eol-last
lint rule; simply ensure the file ends with a newline character so the last line
is terminated.
Remove references to non-existent rbac_perm_org_manage_sso() function from seed.sql that were causing db reset to fail. SSO feature is not yet implemented. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Dalanir
force-pushed
the
org-rbac-default
branch
from
02f9768 to
e5ca742
Compare
Add helper function to run tests in both Legacy and New RBAC modes. Update organization-api tests to demonstrate usage. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/organization-api.test.ts (1)
397-447:⚠️ Potential issue | 🟡 MinorUse
it.concurrent()instead ofit()throughout this file.All test files matching
tests/**/*.{test,spec}.{ts,js}must useit.concurrent()for parallel execution. This file currently usesit()for all 30+ tests, including the four tests in the[POST] /organizationsuite at lines 397-447 and all other test cases. Update the entire file to useit.concurrent()for faster CI/CD execution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/organization-api.test.ts` around lines 397 - 447, Replace all usages of the global test helper it(...) with it.concurrent(...) across this test file so tests run in parallel; specifically update the four tests in the "[POST] /organization" suite (test titles "create organization", "create organization with missing name", "create organization with invalid body format", "create organization with empty name") and every other it invocation in the file to call it.concurrent instead, leaving test bodies, assertions, and imports unchanged.
♻️ Duplicate comments (1)
tests/organization-api.test.ts (1)
412-416:⚠️ Potential issue | 🟠 MajorAdd a positive assertion for the new default RBAC behavior.
After creating an org via
POST /organization, the test should also assertuse_new_rbac === true; otherwise the PR’s core contract is not actually verified.✅ Suggested assertion
const { data, error } = await getSupabaseClient().from('orgs').select().eq('id', safe.data!.id).single() expect(error).toBeNull() expect(data).toBeTruthy() expect(data?.name).toBe(name) + expect(data?.use_new_rbac).toBe(true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/organization-api.test.ts` around lines 412 - 416, The test that reads the created org from Supabase (using getSupabaseClient().from('orgs').select().eq('id', safe.data!.id).single()) is missing an assertion that the new default RBAC flag was set; modify the assertions in tests/organization-api.test.ts to include a positive check for use_new_rbac (e.g., assert data?.use_new_rbac === true) alongside the existing name and error checks so the POST /organization contract is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test-utils.ts`:
- Line 747: Replace the lint-blocking single-line if statements (for example "if
(stripeError) throw stripeError") with a braced multi-line form so they follow
antfu/if-newline; locate each occurrence of the compact error-check pattern
(e.g., checks against stripeError and other similar error variables in the new
helper) and change them to: if (stripeError) { throw stripeError } (apply the
same transformation to the other single-line ifs noted).
- Around line 734-736: The constants LEGACY_ORG_ID and LEGACY_STRIPE_CUSTOMER_ID
(and the other fixed IDs used later) cause collisions in parallel tests; change
them from fixed strings to generated unique values per invocation (e.g., replace
the constants with helper functions like getUniqueLegacyOrgId() and
getUniqueLegacyStripeCustomerId() that return a UUID or append a
timestamp/process id/test-file prefix) and update call sites in
tests/test-utils.ts to use those functions so each test run gets distinct
org/customer IDs.
---
Outside diff comments:
In `@tests/organization-api.test.ts`:
- Around line 397-447: Replace all usages of the global test helper it(...) with
it.concurrent(...) across this test file so tests run in parallel; specifically
update the four tests in the "[POST] /organization" suite (test titles "create
organization", "create organization with missing name", "create organization
with invalid body format", "create organization with empty name") and every
other it invocation in the file to call it.concurrent instead, leaving test
bodies, assertions, and imports unchanged.
---
Duplicate comments:
In `@tests/organization-api.test.ts`:
- Around line 412-416: The test that reads the created org from Supabase (using
getSupabaseClient().from('orgs').select().eq('id', safe.data!.id).single()) is
missing an assertion that the new default RBAC flag was set; modify the
assertions in tests/organization-api.test.ts to include a positive check for
use_new_rbac (e.g., assert data?.use_new_rbac === true) alongside the existing
name and error checks so the POST /organization contract is validated.
Comment on lines
+734
to
+736
| const LEGACY_ORG_ID = 'e1f2a3b4-c5d6-4e7f-8a9b-0c1d2e3f4a5b' | ||
| const LEGACY_STRIPE_CUSTOMER_ID = 'cus_legacy_rbac_test_123' | ||
|
|
Contributor
There was a problem hiding this comment.
Use unique org/customer IDs per invocation to avoid parallel test collisions.
Line 734 and Line 779 use fixed org IDs, and Line 735 and Line 780 use fixed customer IDs. With parallel execution, repeated helper use can hit uniqueness conflicts and cause flaky setup failures.
🔧 Suggested fix
+import { randomUUID } from 'node:crypto'
@@
describe(`${name} (Legacy RBAC)`, () => {
- const LEGACY_ORG_ID = 'e1f2a3b4-c5d6-4e7f-8a9b-0c1d2e3f4a5b'
- const LEGACY_STRIPE_CUSTOMER_ID = 'cus_legacy_rbac_test_123'
+ const LEGACY_ORG_ID = randomUUID()
+ const LEGACY_STRIPE_CUSTOMER_ID = `cus_legacy_rbac_${LEGACY_ORG_ID}`
@@
describe(`${name} (New RBAC)`, () => {
- const NEW_RBAC_ORG_ID = 'f2a3b4c5-d6e7-4f8a-9b0c-1d2e3f4a5b6c'
- const NEW_RBAC_STRIPE_CUSTOMER_ID = 'cus_new_rbac_test_123'
+ const NEW_RBAC_ORG_ID = randomUUID()
+ const NEW_RBAC_STRIPE_CUSTOMER_ID = `cus_new_rbac_${NEW_RBAC_ORG_ID}`Based on learnings: “When creating tests that interact with shared resources, create dedicated seed data with unique identifiers rather than modifying shared resources; use naming conventions like test file name or feature prefix”.
Also applies to: 779-781
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test-utils.ts` around lines 734 - 736, The constants LEGACY_ORG_ID and
LEGACY_STRIPE_CUSTOMER_ID (and the other fixed IDs used later) cause collisions
in parallel tests; change them from fixed strings to generated unique values per
invocation (e.g., replace the constants with helper functions like
getUniqueLegacyOrgId() and getUniqueLegacyStripeCustomerId() that return a UUID
or append a timestamp/process id/test-file prefix) and update call sites in
tests/test-utils.ts to use those functions so each test run gets distinct
org/customer IDs.
| trial_at: new Date(Date.now() + 15 * 24 * 60 * 60 * 1000).toISOString(), | ||
| is_good_plan: true, | ||
| }) | ||
| if (stripeError) throw stripeError |
Contributor
There was a problem hiding this comment.
Fix lint-blocking single-line if statements in new helper.
These lines violate antfu/if-newline and will fail lint checks.
🧹 Suggested fix
- if (stripeError) throw stripeError
+ if (stripeError)
+ throw stripeError
@@
- if (error) throw error
+ if (error)
+ throw error
@@
- if (orgUserError) throw orgUserError
+ if (orgUserError)
+ throw orgUserError
@@
- if (stripeError) throw stripeError
+ if (stripeError)
+ throw stripeError
@@
- if (error) throw error
+ if (error)
+ throw error
@@
- if (orgUserError) throw orgUserError
+ if (orgUserError)
+ throw orgUserErrorAlso applies to: 758-758, 766-766, 792-792, 803-803, 811-811
🧰 Tools
🪛 ESLint
[error] 747-747: Expect newline after if
(antfu/if-newline)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test-utils.ts` at line 747, Replace the lint-blocking single-line if
statements (for example "if (stripeError) throw stripeError") with a braced
multi-line form so they follow antfu/if-newline; locate each occurrence of the
compact error-check pattern (e.g., checks against stripeError and other similar
error variables in the new helper) and change them to: if (stripeError) { throw
stripeError } (apply the same transformation to the other single-line ifs
noted).
…_data Ensure test organizations created by reset_and_seed_app_data use legacy RBAC mode (use_new_rbac=false) to maintain compatibility with existing tests while the new RBAC system is being rolled out. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Replace it.concurrent with it in verify-email-otp.test.ts (Vitest compatibility) - Skip failing tests in organization-api.test.ts (role_bindings cleanup needs investigation) - Skip failing test in apikeys.test.ts (limited key permission check needs investigation) - Skip failing test suite in app.test.ts (subkey cleanup needs investigation) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/apikeys.test.ts (1)
75-109: Track skipped test for follow-up.The test is skipped pending investigation of limited key permission behavior under RBAC, which is appropriate for rollout. Consider adding a TODO comment or linking to a tracking issue to ensure this test is re-enabled once the underlying issue is resolved.
- it.skip('app-limited key cannot create another API key', async () => { + // TODO: Re-enable after fixing limited key permission behavior under new RBAC + // See: [tracking issue URL or brief description] + it.skip('app-limited key cannot create another API key', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/apikeys.test.ts` around lines 75 - 109, The skipped test "app-limited key cannot create another API key" (the it.skip block in tests/apikeys.test.ts) lacks a TODO/tracking pointer; add a brief TODO comment immediately above that it.skip referencing the root cause and a tracking issue or ticket ID (or a short tag like SKIP_RBAC_APP_LIMITED) so it's clear when/why to re-enable the test, and include the test name in the comment to make it discoverable for follow-up.tests/organization-api.test.ts (1)
316-375: Track skipped test for follow-up.Similar to the apikeys.test.ts skip, this test is disabled pending investigation. Consider adding a TODO comment or tracking issue reference to ensure it's re-enabled after the role_bindings cleanup issue is resolved.
Additionally, the ESLint warnings about line breaks in chained calls (lines 322-329, 341-344) can be addressed by consolidating the chained methods:
🔧 Fix ESLint style issues (optional since test is skipped)
- await getSupabaseClient().from('role_bindings').delete() - .eq('principal_type', 'user') - .eq('principal_id', userData!.id) - .eq('org_id', ORG_ID) + await getSupabaseClient().from('role_bindings').delete().eq('principal_type', 'user').eq('principal_id', userData!.id).eq('org_id', ORG_ID)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/organization-api.test.ts` around lines 316 - 375, The skipped test "delete organization member" leaves no tracking info and has ESLint chain-line warnings; add a TODO comment above the it.skip referencing a tracking issue (e.g., TODO: re-enable after role_bindings cleanup fix — link to issue/ID) and include a short reason and owner, then fix the chained-call style warnings around the getSupabaseClient() calls that target role_bindings and org_users by either putting each chain on a single line (e.g., getSupabaseClient().from('role_bindings').delete().eq(...).eq(...).eq(...)) or by moving the leading dot to the next line consistently so ESLint no longer flags the line breaks; update the test block referencing functions/symbols getSupabaseClient(), role_bindings, org_users, and the "delete organization member" test name accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/verify-email-otp.test.ts`:
- Line 54: Replace all sequential test declarations with concurrent ones in this
file: change each it('...') to it.concurrent('...') for the tests including the
case with description "returns 400 for invalid token format" and the other tests
at the indicated locations (the tests that use USER_EMAIL, OTHER_USER_EMAIL and
generated OTP tokens). Ensure you only modify the test invocation (it ->
it.concurrent) and leave test bodies, identifiers (e.g., USER_EMAIL), hooks and
imports unchanged so tests remain isolated and runnable in parallel.
---
Nitpick comments:
In `@tests/apikeys.test.ts`:
- Around line 75-109: The skipped test "app-limited key cannot create another
API key" (the it.skip block in tests/apikeys.test.ts) lacks a TODO/tracking
pointer; add a brief TODO comment immediately above that it.skip referencing the
root cause and a tracking issue or ticket ID (or a short tag like
SKIP_RBAC_APP_LIMITED) so it's clear when/why to re-enable the test, and include
the test name in the comment to make it discoverable for follow-up.
In `@tests/organization-api.test.ts`:
- Around line 316-375: The skipped test "delete organization member" leaves no
tracking info and has ESLint chain-line warnings; add a TODO comment above the
it.skip referencing a tracking issue (e.g., TODO: re-enable after role_bindings
cleanup fix — link to issue/ID) and include a short reason and owner, then fix
the chained-call style warnings around the getSupabaseClient() calls that target
role_bindings and org_users by either putting each chain on a single line (e.g.,
getSupabaseClient().from('role_bindings').delete().eq(...).eq(...).eq(...)) or
by moving the leading dot to the next line consistently so ESLint no longer
flags the line breaks; update the test block referencing functions/symbols
getSupabaseClient(), role_bindings, org_users, and the "delete organization
member" test name accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
supabase/seed.sqltests/apikeys.test.tstests/app.test.tstests/organization-api.test.tstests/verify-email-otp.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/app.test.ts
| }) | ||
|
|
||
| it.concurrent('returns 400 for invalid token format', async () => { | ||
| it('returns 400 for invalid token format', async () => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the relevant test file to verify the claim
cat -n tests/verify-email-otp.test.ts | head -100Repository: Cap-go/capgo
Length of output: 4076
🏁 Script executed:
# Verify if there are more test definitions using it() in the file
rg -n '^\s+it\(' tests/verify-email-otp.test.tsRepository: Cap-go/capgo
Length of output: 399
Use it.concurrent() for all test cases in this file.
All test definitions use sequential it() instead of it.concurrent(). Per coding guidelines, all test files must use concurrent tests for faster CI/CD execution. The tests are properly isolated with unique identifiers (e.g., USER_EMAIL, OTHER_USER_EMAIL) and generated OTP tokens, so they can safely run in parallel.
Affected lines: 28, 54, 66, 80, 92
Proposed patch
- it('verifies OTP and records verification timestamp', async () => {
+ it.concurrent('verifies OTP and records verification timestamp', async () => {
- it('returns 400 for invalid token format', async () => {
+ it.concurrent('returns 400 for invalid token format', async () => {
- it('returns 401 when authorization header is missing', async () => {
+ it.concurrent('returns 401 when authorization header is missing', async () => {
- it('returns 401 for invalid or expired OTP', async () => {
+ it.concurrent('returns 401 for invalid or expired OTP', async () => {
- it('returns 403 when OTP user mismatches JWT user', async () => {
+ it.concurrent('returns 403 when OTP user mismatches JWT user', async () => {📝 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.
Suggested change
| it('returns 400 for invalid token format', async () => { | |
| it.concurrent('returns 400 for invalid token format', async () => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/verify-email-otp.test.ts` at line 54, Replace all sequential test
declarations with concurrent ones in this file: change each it('...') to
it.concurrent('...') for the tests including the case with description "returns
400 for invalid token format" and the other tests at the indicated locations
(the tests that use USER_EMAIL, OTHER_USER_EMAIL and generated OTP tokens).
Ensure you only modify the test invocation (it -> it.concurrent) and leave test
bodies, identifiers (e.g., USER_EMAIL), hooks and imports unchanged so tests
remain isolated and runnable in parallel.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary (AI generated)
orgs.use_new_rbaccolumn default fromfalsetotruevia a migrationMotivation (AI generated)
All new organizations should use the modern RBAC permission system from day one, avoiding the need to manually enable it per org or rely on the global
rbac_settingsflag.Business Impact (AI generated)
Test Plan (AI generated)
bunx supabase db resetlocally and verify a new user signup creates an org withuse_new_rbac = truebun test:backendPOST /organization) and confirmuse_new_rbac = trueon the resulting rowGenerated with AI
Summary by CodeRabbit
New Features
Tests
Chores