feat(organization): persist website on orgs#1836
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a nullable org website end-to-end: frontend sends website when present, backend functions accept/normalize/validate website, DB schema and RPCs gain a Changes
Sequence Diagram(s)sequenceDiagram
participant FE as Frontend (Onboarding)
participant FN as Supabase Function (POST/PUT/GET)
participant UTIL as normalizeWebsiteUrl()
participant DB as Database (public.orgs)
FE->>FN: POST/PUT { name, website? }
FN->>UTIL: normalizeWebsiteUrl(input)
UTIL-->>FN: normalizedWebsite|null or throws
FN->>DB: INSERT/UPDATE orgs { name, website: normalizedWebsite|null }
DB-->>FN: inserted/updated row (includes website)
FN-->>FE: Respond with org object (includes website)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
📝 Coding Plan
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 346ee5c571
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
🧹 Nitpick comments (1)
tests/organization-api.test.ts (1)
410-491: Use concurrent test cases for the updated organization create/update coverage.The modified cases still use
it(...). Please switch these toit.concurrent(...); and for Line 469 onward, avoid sharedORG_IDmutation by creating a dedicated org fixture inside the test if running concurrently.♻️ Suggested test runner update
- it('create organization', async () => { + it.concurrent('create organization', async () => { // ... }) - it('update organization', async () => { + it.concurrent('update organization', async () => { // ... })As per coding guidelines:
tests/**/*.test.{ts,js}: ALL TEST FILES RUN IN PARALLEL; use it.concurrent() instead of it() ...and use dedicated seed data when tests modify shared resources.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/public/organization/post.ts`:
- Around line 14-24: normalizeWebsiteUrl currently uses a case-sensitive regex
(/^https?:\/\//) which mis-detects schemes like "HTTPS://" and also lets
non-http schemes (e.g., ftp://) through, causing malformed normalization; update
normalizeWebsiteUrl to test the scheme case-insensitively (use /^https?:\/\//i)
and explicitly reject any URL whose parsed protocol is not "http:" or "https:"
(throw the same simpleError('invalid_body', 'Invalid body', { error:
'website_must_be_a_valid_url' }) on invalid), and apply the identical change to
the matching function in organization/put.ts so POST and PUT behave
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4581c0c-052a-4df7-8a99-58a395eca10c
📒 Files selected for processing (7)
src/pages/onboarding/organization.vuesrc/types/supabase.types.tssupabase/functions/_backend/public/organization/post.tssupabase/functions/_backend/public/organization/put.tssupabase/functions/_backend/utils/supabase.types.tssupabase/migrations/20260320044548_add_org_website.sqltests/organization-api.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 768f85db06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@supabase/functions/_backend/public/organization/website.ts`:
- Around line 9-15: The scheme-detection regex is too permissive and treats
"example.com:3000" as having a scheme; change the hasScheme check to only detect
actual scheme prefixes with "//" (e.g. replace
/^[a-z][a-z\d+\-.]*:/i.test(trimmed) with
/^[a-z][a-z\d+\-.]*:\/\//i.test(trimmed)) so host:port values are not
misclassified, then keep the subsequent protocol validation and URL
normalization (references: hasScheme variable, the protocol check that throws
'invalid website protocol', and the normalized = new URL(...) line).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 83ebeb50-f8be-40f9-8b50-5d73cfaa328f
📒 Files selected for processing (4)
src/pages/onboarding/organization.vuesupabase/functions/_backend/public/organization/post.tssupabase/functions/_backend/public/organization/put.tssupabase/functions/_backend/public/organization/website.ts
✅ Files skipped from review due to trivial changes (1)
- src/pages/onboarding/organization.vue
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)
tests/organization-api.test.ts (1)
807-825:⚠️ Potential issue | 🟡 MinorIncorrect reset value will cause test flakiness.
The
previousWebsitevariable is set to'https://www.capgo.app/docs'but the originalwebsitevalue frombeforeAll(line 47) is'https://test-organization.example/'. After this test resets at line 825, the sharedORG_IDwill have an incorrect website value.This will cause the test at line 88 (
expect(safe.data).toEqual({ id: ORG_ID, name, website })) to fail intermittently depending on test execution order.🐛 Proposed fix to use the correct reset value
it('get_orgs_v7 returns enforce_hashed_api_keys field', async () => { // Set a known value const rpcWebsite = website - const previousWebsite = 'https://www.capgo.app/docs' await getSupabaseClient().from('orgs').update({ enforce_hashed_api_keys: true, website: rpcWebsite }).eq('id', ORG_ID) // Call get_orgs_v7 via RPC @@ -822,6 +821,6 @@ describe('[PUT] /organization - enforce_hashed_api_keys setting', () => { expect(testOrg!.website).toBe(rpcWebsite) // Reset - await getSupabaseClient().from('orgs').update({ enforce_hashed_api_keys: false, website: previousWebsite }).eq('id', ORG_ID) + await getSupabaseClient().from('orgs').update({ enforce_hashed_api_keys: false, website }).eq('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 807 - 825, The test resets the org website to a hardcoded value causing flakiness; change the reset to use the original value captured in the test's `website` variable by setting `previousWebsite` (or directly using `website`) instead of `'https://www.capgo.app/docs'`, so the final update call that resets `{ enforce_hashed_api_keys: false, website: previousWebsite }` (after calling the `get_orgs_v7` RPC and asserting `rpcWebsite`) restores the exact original `website` value.
🧹 Nitpick comments (2)
tests/organization-api.test.ts (2)
474-486: Useit.concurrent()for consistency and parallelism.Per coding guidelines, all tests should use
it.concurrent()to run in parallel. This test only reads (expects a 400 failure), so it has no shared resource conflicts.♻️ Proposed fix
- it('create organization rejects invalid website scheme', async () => { + it.concurrent('create organization rejects invalid website scheme', async () => {As per coding guidelines: "ALL TEST FILES RUN IN PARALLEL; use
it.concurrent()instead ofit()to run tests in parallel within the same file".🤖 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 474 - 486, The test case using it('create organization rejects invalid website scheme', ...) should be changed to use Jest's parallel runner by replacing the it invocation with it.concurrent so the test runs in parallel with other tests; locate the it call with the description "create organization rejects invalid website scheme" in tests/organization-api.test.ts and switch it to it.concurrent(...) without altering the test body or assertions.
590-603: Useit.concurrent()for consistency and parallelism.Per coding guidelines, all tests should use
it.concurrent()to run in parallel. This test only reads (expects a 400 failure), so it has no shared resource conflicts.♻️ Proposed fix
- it('update organization rejects invalid website scheme', async () => { + it.concurrent('update organization rejects invalid website scheme', async () => {As per coding guidelines: "ALL TEST FILES RUN IN PARALLEL; use
it.concurrent()instead ofit()to run tests in parallel within the same file".🤖 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 590 - 603, Replace the non-concurrent test declaration for the "update organization rejects invalid website scheme" case with a concurrent one: change the call to it() to it.concurrent() for the test named 'update organization rejects invalid website scheme' in tests/organization-api.test.ts so it runs in parallel with other tests; keep the test body, arguments, and assertions (fetch to `${BASE_URL}/organization`, headers, method 'PUT', payload, and expectations for 400 and error 'invalid_body') unchanged.
🤖 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 `@tests/organization-api.test.ts`:
- Around line 807-825: The test resets the org website to a hardcoded value
causing flakiness; change the reset to use the original value captured in the
test's `website` variable by setting `previousWebsite` (or directly using
`website`) instead of `'https://www.capgo.app/docs'`, so the final update call
that resets `{ enforce_hashed_api_keys: false, website: previousWebsite }`
(after calling the `get_orgs_v7` RPC and asserting `rpcWebsite`) restores the
exact original `website` value.
---
Nitpick comments:
In `@tests/organization-api.test.ts`:
- Around line 474-486: The test case using it('create organization rejects
invalid website scheme', ...) should be changed to use Jest's parallel runner by
replacing the it invocation with it.concurrent so the test runs in parallel with
other tests; locate the it call with the description "create organization
rejects invalid website scheme" in tests/organization-api.test.ts and switch it
to it.concurrent(...) without altering the test body or assertions.
- Around line 590-603: Replace the non-concurrent test declaration for the
"update organization rejects invalid website scheme" case with a concurrent one:
change the call to it() to it.concurrent() for the test named 'update
organization rejects invalid website scheme' in tests/organization-api.test.ts
so it runs in parallel with other tests; keep the test body, arguments, and
assertions (fetch to `${BASE_URL}/organization`, headers, method 'PUT', payload,
and expectations for 400 and error 'invalid_body') unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f1208914-ca90-4f82-b65b-c6cf016b02b7
📒 Files selected for processing (1)
tests/organization-api.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-website.unit.test.ts`:
- Around line 6-18: Replace the non-concurrent test declarations with concurrent
ones: for each test block that currently uses it('...') in this file (the tests
invoking normalizeWebsiteUrl), change it(...) to it.concurrent(...) so the three
test cases (the ones asserting explicit http/https behavior, adding https to
host/host:port, and rejecting non-web schemes) run concurrently using Jest's
concurrent test API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad851070-c1a7-4c38-aa7a-fa8fac5f6e50
📒 Files selected for processing (2)
supabase/functions/_backend/public/organization/website.tstests/organization-website.unit.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48d850e29d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
1 participant
Summary (AI generated)
websitecolumn to organizationsMotivation (AI generated)
Organization onboarding already asks for a website when importing name and logo, but that data was discarded after onboarding. Saving it on the org makes the imported source explicit and reusable later.
Business Impact (AI generated)
This preserves company metadata gathered during onboarding, reduces repeated manual entry, and gives Capgo a stable org-level website reference for future product surfaces that rely on company identity.
Test Plan (AI generated)
bun lint src/pages/onboarding/organization.vue tests/organization-api.test.tsbun lint:backend supabase/functions/_backend/public/organization/post.ts supabase/functions/_backend/public/organization/put.tsbun typecheckbun run supabase:startbun run supabase:with-env -- bunx vitest run tests/organization-api.test.tsGenerated with AI
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores