ENG-1238 Group creation function #670
Conversation
|
Updates to Preview Branch (eng-1238-group-creation-function) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds a Deno config with import path aliases and a new Supabase Edge runtime HTTP POST handler that authenticates the caller, creates a deterministic group user via the Supabase admin client, and inserts a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant EdgeFn as "Edge Function (create-group)"
participant Auth as "Supabase Auth (token verification)"
participant Admin as "Supabase Admin Client"
participant DB as "Database (auth + public tables)"
Client->>EdgeFn: POST /create-group (Authorization: Bearer <token>)
Note over EdgeFn: CORS preflight/origin check\nENV validation
EdgeFn->>Auth: Verify Bearer token / getClaims
alt invalid or anonymous
Auth-->>EdgeFn: error
EdgeFn-->>Client: 401 Unauthorized
else valid
Auth-->>EdgeFn: claims
EdgeFn->>Admin: create user (deterministic email, gen password)
Admin->>DB: insert into auth.users
DB-->>Admin: user record (group_id)
Admin-->>EdgeFn: created user
EdgeFn->>DB: insert group_membership (group_id, requester_id, role=admin)
DB-->>EdgeFn: membership record
EdgeFn-->>Client: 200 { group_id } + CORS headers
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @packages/database/supabase/functions/create-group/index.ts:
- Around line 49-62: The code uses a non-null assertion on
req.headers.get('Authorization') and immediately calls .replace(), which throws
if the header is missing; modify the logic around authHeader/token in the
create-group handler so you first check that authHeader is a non-empty string
and starts with "Bearer " (or otherwise parse it safely), return a 401
Response.json when missing or malformed, then extract token and call
supabase.auth.getClaims(token); remove the `!` on authHeader and ensure
userEmail and is_anonymous checks remain after a successful getClaims call.
- Around line 71-74: The current direct throws after the Supabase user creation
(checking userResponse.error and !userResponse.data.user) are unhandled and will
crash the function; wrap the user creation call and the subsequent checks
(references: userResponse, userResponse.error, userResponse.data.user) in a
try/catch block, catch any errors or non-successful responses, and return or
throw a structured error response (e.g., return a JSON error object or an HTTP
response with status and message) instead of letting the exception bubble
unhandled so the function returns a predictable error to callers.
- Around line 78-79: membershipResponse from
supabaseAdmin.from("group_membership").insert(...) is only logged and not
checked, risking orphaned group records; after calling that insert, check
membershipResponse.error and if present log the error and rollback the
previously created group (use the group creation response variable, e.g.,
groupResponse or the inserted group id) by deleting it via
supabaseAdmin.from("groups").delete().eq("id", group_id) (or throw/return an
error to abort), ensuring you don't leave an ownerless group and that the
function returns an appropriate failure result.
🧹 Nitpick comments (2)
packages/database/supabase/functions/create-group/index.ts (2)
81-92: Inconsistent response format - consider returning JSON.Error responses use
Response.json()but the success response returns plain text. For API consistency and easier client parsing, consider returning JSON for the success case as well.🔎 Proposed fix
- const res = new Response(group_id); + const res = Response.json({ group_id });
20-22: Consider validating HTTP method for non-OPTIONS requests.After handling OPTIONS, the function proceeds with group creation for any HTTP method (GET, POST, PUT, etc.). Typically, resource creation should only accept POST requests.
🔎 Proposed fix
if (req.method === "OPTIONS") { // ... existing OPTIONS handling } + + if (req.method !== "POST") { + return Response.json( + { msg: 'Method not allowed' }, + { status: 405 } + ); + }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/database/supabase/functions/create-group/deno.jsonpackages/database/supabase/functions/create-group/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
packages/database/supabase/functions/create-group/index.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-21T23:47:06.754Z
Learning: Supabase edge functions in the discourse-graph codebase cannot reuse code from elsewhere in the codebase due to runtime environment isolation constraints. Code duplication in edge functions at packages/database/supabase/functions/ is necessary and expected.
📚 Learning: 2025-06-25T18:03:52.669Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-07-21T14:22:20.752Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
Applied to files:
packages/database/supabase/functions/create-group/deno.jsonpackages/database/supabase/functions/create-group/index.ts
📚 Learning: 2025-08-21T23:47:06.754Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-21T23:47:06.754Z
Learning: Supabase edge functions in the discourse-graph codebase cannot reuse code from elsewhere in the codebase due to runtime environment isolation constraints. Code duplication in edge functions at packages/database/supabase/functions/ is necessary and expected.
Applied to files:
packages/database/supabase/functions/create-group/deno.jsonpackages/database/supabase/functions/create-group/index.ts
📚 Learning: 2025-08-27T13:59:31.143Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 377
File: packages/database/cucumber.json:3-4
Timestamp: 2025-08-27T13:59:31.143Z
Learning: In the discourse-graph monorepo, repo/* imports work correctly through standard npm package resolution where packages/*/package.json files have "name": "repo/..." and consuming apps reference these packages by name in their dependencies. The imports resolve properly in both local development and deployment environments.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-09-01T19:09:07.052Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-09-01T19:09:07.052Z
Learning: User prefers to avoid making types explicit in each require statement due to anti-DRY concerns when working with module imports in TypeScript.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-09-11T23:54:41.861Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 431
File: packages/database/package.json:18-22
Timestamp: 2025-09-11T23:54:41.861Z
Learning: In the discourse-graph repository's packages/database/package.json, the typesVersions mapping for "./dbDotEnv" must retain the leading "./" prefix. Removing it (as suggested by standard TypeScript documentation) fails in practice in their specific build environment/tooling setup.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
🧬 Code graph analysis (1)
packages/database/supabase/functions/create-group/index.ts (2)
packages/database/src/dbDotEnv.mjs (1)
data(82-82)packages/database/src/lib/client.ts (1)
DGSupabaseClient(9-9)
⏰ 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: Supabase Preview
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (3)
packages/database/supabase/functions/create-group/index.ts (2)
9-17: LGTM - CORS utilities are correctly duplicated.As per learnings, code duplication in Supabase edge functions is expected due to runtime isolation. The comment on line 9 properly documents the source of the duplication.
23-34: LGTM - CORS preflight handling is correct.The OPTIONS handler properly returns 204 with conditional origin allowance and appropriate CORS headers.
packages/database/supabase/functions/create-group/deno.json (1)
6-9: Fix incorrect execContext import path in deno.json.The import alias for
@repo/utils/lib/execContextpoints to a non-existent path. The file exists atpackages/utils/src/execContext.ts(not in alib/subdirectory), so the alias should map to../../../../utils/src/execContext.tsinstead of../../../../utils/src/lib/execContext.ts. This error exists in bothcreate-group/deno.jsonandcreate-space/deno.json.⛔ Skipped due to learnings
Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 431 File: packages/database/package.json:18-22 Timestamp: 2025-09-11T23:54:41.861Z Learning: In the discourse-graph repository's packages/database/package.json, the typesVersions mapping for "./dbDotEnv" must retain the leading "./" prefix. Removing it (as suggested by standard TypeScript documentation) fails in practice in their specific build environment/tooling setup.Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 0 File: :0-0 Timestamp: 2025-08-21T23:47:06.754Z Learning: Supabase edge functions in the discourse-graph codebase cannot reuse code from elsewhere in the codebase due to runtime environment isolation constraints. Code duplication in edge functions at packages/database/supabase/functions/ is necessary and expected.Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 291 File: packages/database/supabase/functions/create-space/index.ts:0-0 Timestamp: 2025-07-21T14:22:20.752Z Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 400 File: packages/database/src/dbDotEnv.js:24-31 Timestamp: 2025-08-31T17:37:17.173Z Learning: The "all" variant for SUPABASE_USE_DB should only be used in createEnv.mts and not in the dbDotEnv module. It was incorrectly added to dbDotEnv.d.ts but should be removed to maintain proper separation of concerns.Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 241 File: packages/database/tsconfig.json:3-7 Timestamp: 2025-06-25T18:03:52.669Z Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 0 File: :0-0 Timestamp: 2025-09-01T19:09:07.052Z Learning: User prefers to avoid making types explicit in each require statement due to anti-DRY concerns when working with module imports in TypeScript.Learnt from: maparent Repo: DiscourseGraphs/discourse-graph PR: 0 File: :0-0 Timestamp: 2025-07-13T16:47:14.352Z Learning: In the discourse-graph codebase, types.gen.ts contains automatically generated database function type definitions that may have reordered signatures between versions. This reordering is expected behavior from the code generation process and should not be flagged as an issue requiring fixes.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
@CodeRabbit waiting for the review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4670 @maparent I've completed the review! Here are my findings: Critical IssueMissing Authorization Header Check const authHeader = req.headers.get('Authorization')!
const token = authHeader.replace('Bearer ', '')The non-null assertion operator ( const authHeader = req.headers.get('Authorization');
if (!authHeader) {
return Response.json(
{ msg: 'Missing Authorization header' },
{ status: 401 }
);
}
const token = authHeader.replace('Bearer ', '');Security & Design Questions
Code Quality
Minor Suggestions
The overall approach looks solid! The critical issue is the missing null check on the Authorization header. 🧠 Learnings used |
maparent
force-pushed
the
eng-1238-group-creation-function
branch
from
885bb0a to
8a580e2
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @packages/database/supabase/functions/create-group/deno.json:
- Around line 6-9: Update the module mapping for "@repo/utils/lib/execContext"
in the deno.json overrides: replace the incorrect path
"../../../../utils/src/lib/execContext.ts" with the correct
"../../../../utils/src/execContext.ts" (remove the extra "lib/" segment) so the
import points to packages/utils/src/execContext.ts.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/roam/src/index.tspackages/database/supabase/functions/create-group/deno.jsonpackages/database/supabase/functions/create-group/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/database/supabase/functions/create-group/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/index.ts
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/index.ts
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/index.ts
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/index.ts
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/index.ts
🧠 Learnings (10)
📚 Learning: 2025-06-25T18:03:52.669Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 241
File: packages/database/tsconfig.json:3-7
Timestamp: 2025-06-25T18:03:52.669Z
Learning: The packages/database directory in the discourse-graph repository has a unique structure as a database schema/migration package. It contains doc/, scripts/, supabase/ directories and TypeScript files at the root level, but no typical src/, test/, dist/, or node_modules directories. The current tsconfig.json with "include": ["."] and "exclude": ["supabase"] is appropriate for this structure.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-07-21T14:22:20.752Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 291
File: packages/database/supabase/functions/create-space/index.ts:0-0
Timestamp: 2025-07-21T14:22:20.752Z
Learning: In the discourse-graph codebase, types.gen.ts is not accessible from Supabase edge functions, requiring duplication of types and utilities when needed in the edge function environment at packages/database/supabase/functions/.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-08-21T23:47:06.754Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-08-21T23:47:06.754Z
Learning: Supabase edge functions in the discourse-graph codebase cannot reuse code from elsewhere in the codebase due to runtime environment isolation constraints. Code duplication in edge functions at packages/database/supabase/functions/ is necessary and expected.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-08-27T13:59:31.143Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 377
File: packages/database/cucumber.json:3-4
Timestamp: 2025-08-27T13:59:31.143Z
Learning: In the discourse-graph monorepo, repo/* imports work correctly through standard npm package resolution where packages/*/package.json files have "name": "repo/..." and consuming apps reference these packages by name in their dependencies. The imports resolve properly in both local development and deployment environments.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-09-01T19:09:07.052Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-09-01T19:09:07.052Z
Learning: User prefers to avoid making types explicit in each require statement due to anti-DRY concerns when working with module imports in TypeScript.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-09-11T23:54:41.861Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 431
File: packages/database/package.json:18-22
Timestamp: 2025-09-11T23:54:41.861Z
Learning: In the discourse-graph repository's packages/database/package.json, the typesVersions mapping for "./dbDotEnv" must retain the leading "./" prefix. Removing it (as suggested by standard TypeScript documentation) fails in practice in their specific build environment/tooling setup.
Applied to files:
packages/database/supabase/functions/create-group/deno.json
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{js,ts,tsx,jsx,json} : Prefer existing dependencies from package.json when working on the Roam Research extension
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/** : Implement the Discourse Graph protocol in the Roam Research extension
Applied to files:
apps/roam/src/index.ts
⏰ 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). (2)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
🔇 Additional comments (2)
packages/database/supabase/functions/create-group/deno.json (2)
1-11: Configuration structure looks appropriate for a Deno edge function.The import mappings provide access to Supabase JSR packages and local repository modules, which aligns with the edge function's requirements.
3-3: No issue — this import mapping is correct. The mapping tojsr:@supabase/functions-js/edge-runtime.d.tsfollows Supabase's recommended pattern. The.d.tsfile provides TypeScript type definitions for the globalEdgeRuntimeambient type; it is not a module with runtime exports. This side-effect import style is the intended way to enable intellisense for edge-runtime types in Supabase Deno functions.Likely an incorrect or invalid review comment.
maparent
force-pushed
the
eng-1238-group-creation-function
branch
from
c5bf6ba to
595b0a3
Compare
maparent
force-pushed
the
eng-1238-group-creation-function
branch
from
4877765 to
985452a
Compare
https://linear.app/discourse-graphs/issue/ENG-1238/group-creation-function
An edge function that will create a group, owned by the currently logged in user.
The user should then be able to add members normally.
Loom video: https://www.loom.com/share/2725c1614adf4b4eb4443b818908ad72
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.