Shared group metadata for invites, adds join requests acceptance tracking#137
Shared group metadata for invites, adds join requests acceptance tracking#137lourou wants to merge 20 commits into
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughMoves invite-level metadata into a new GroupMetadata table, updates Prisma schema and migrations, refactors invite handlers/tests to read/upsert group metadata, adds expiresAt/autoApprove/timestamps, adds accept-request endpoint and auth allowance, and performs transactional cleanup of orphaned GroupMetadata on invite deletion. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant API as create-invite-code handler
participant DB as Prisma
Note over API,DB #DFF2E1: Transactional creation/upsert
C->>API: POST /invites (groupId, optional metadata...)
API->>DB: BEGIN
API->>DB: UPSERT GroupMetadata WHERE id=groupId (set metadata if provided)
DB-->>API: GroupMetadata
API->>DB: CREATE InviteCode referencing GroupMetadata.id
DB-->>API: InviteCode + groupMetadata
API->>DB: COMMIT
API-->>C: invite payload (metadata from groupMetadata)
sequenceDiagram
autonumber
participant C as Client
participant API as delete-invite handler
participant DB as Prisma
Note over API,DB #FFF2D9: Transactional deletion with orphan cleanup
C->>API: DELETE /invites/:id
API->>DB: BEGIN
API->>DB: DELETE InviteCode WHERE id=...
DB-->>API: deleted invite
API->>DB: SELECT COUNT(*) FROM InviteCode WHERE groupId=deleted.groupId
DB-->>API: count = N
alt N = 0
API->>DB: DELETE GroupMetadata WHERE id=groupId
DB-->>API: deleted GroupMetadata
end
API->>DB: COMMIT
API-->>C: deletion response
sequenceDiagram
autonumber
participant C as Client
participant API as accept-request-to-join handler
participant DB as Prisma
Note over API,DB #E8F0FF: Accepting a join request (idempotent, transactional)
C->>API: PUT /requests/:requestId/accept
API->>DB: SELECT Request JOIN InviteCode (include groupMetadata, notificationTargets, creator)
DB-->>API: request + invite
API->>DB: BEGIN
API->>DB: CHECK maxUses/expiry, INCREMENT InviteCode.usesCount
API->>DB: CREATE InviteCodeUse for requester
API->>DB: DELETE InviteCodeRequest
DB-->>API: committed changes
API-->>C: AcceptRequestToJoinResponse (inviteCodeUse details)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/api/v1/invites/handlers/get-invite-requests.ts (1)
46-51: Auth bug: xmtpId may be undefined → Prisma filter dropped, first identity returned.Prisma ignores undefined fields in where, so this can authorize the wrong user. Guard before querying.
- const identity = await prisma.deviceIdentity.findFirst({ + if (!xmtpId) { + res.status(401).json({ success: false, message: "Authentication required" }); + return; + } + const identity = await prisma.deviceIdentity.findFirst({ where: { xmtpId }, });src/api/v1/invites/handlers/create-invite-code.ts (1)
58-66: Auth bug: guard xmtpId before querying identity.Prevents Prisma from dropping the filter and authenticating as the first record.
- // Get the authenticated user's identity from the JWT - const { xmtpId } = res.locals; + // Get the authenticated user's identity from the JWT + const { xmtpId } = res.locals; + if (!xmtpId) { + res.status(401).json({ success: false, message: "Authentication required" }); + return; + }src/api/v1/invites/handlers/update-invite-code.ts (2)
68-73: Auth bug: guard xmtpId before querying identity.Same undefined-filter issue as elsewhere.
- // Get the authenticated user's identity from the JWT - const { xmtpId } = res.locals; + // Get the authenticated user's identity from the JWT + const { xmtpId } = res.locals; + if (!xmtpId) { + res.status(401).json({ success: false, message: "Authentication required" }); + return; + }
96-102: Disallow groupId changes here or add explicit authorization.Letting this endpoint move invites between groups can be abused to target other groups’ metadata. Either forbid changing groupId here or enforce ownership of the target group.
Option A (forbid change):
- if (existingInvite.createdById !== identity.id) { + if (existingInvite.createdById !== identity.id) { res.status(403).json({ success: false, message: "Not authorized to update this invite", }); return; } + if (body.groupId !== existingInvite.groupId) { + res.status(400).json({ + success: false, + message: "Cannot change groupId via this endpoint", + }); + return; + }src/api/v1/invites/handlers/request-to-join.ts (3)
42-43: Missing auth guard for xmtpId; potential unauthorized access path.If res.locals.xmtpId is absent/undefined, the subsequent deviceIdentity lookup may degrade to an unscoped query depending on Prisma’s handling, risking incorrect identity resolution.
- const { xmtpId } = res.locals; + const { xmtpId } = res.locals as { xmtpId?: string }; + if (!xmtpId || typeof xmtpId !== "string") { + res.status(401).json({ + success: false, + message: "Unauthorized", + }); + return; + }
52-58: Use 401 Unauthorized for missing identity, not 404.Returning 404 for a missing requester identity obscures an auth failure and conflicts with our API guidelines.
- if (!requesterIdentity) { - res.status(404).json({ - success: false, - message: "Identity not found", - }); - return; - } + if (!requesterIdentity) { + res.status(401).json({ + success: false, + message: "Unauthorized", + }); + return; + }
231-245: Handle unique‑constraint race on create to return 409 instead of 500.If two requests race, Prisma will throw P2002 on (inviteCodeId, requesterId). Map it to 409 for consistency with the preflight check.
+import { Prisma } from "@prisma/client";} catch (error) { if (error instanceof z.ZodError) { res.status(400).json({ success: false, message: "Invalid request body", errors: error.errors, }); return; } + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") { + res.status(409).json({ + success: false, + message: "You already have a request for this group", + }); + return; + } req.log.error({ error }, "Error creating join request"); res.status(500).json({ success: false, message: "Failed to create join request", }); }
🧹 Nitpick comments (11)
prisma/schema.prisma (2)
123-150: Add composite indexes to match query patterns (status+expiry, author+group).You frequently filter by status/expiry and by createdById with optional groupId. Composite indexes will reduce scans on larger tables.
Apply:
model InviteCode { id String @id @default(cuid()) maxUses Int? // null = unlimited uses usesCount Int @default(0) status InviteCodeStatus @default(ACTIVE) expiresAt DateTime? // null = no expiration autoApprove Boolean @default(false) // true = auto-join, false = requires approval // Group ID to identify which group this invite code is for groupId String // Shared metadata reference groupMetadata GroupMetadata @relation(fields: [groupId], references: [id], onDelete: Cascade) // Keep track of who created it for admin purposes createdById String createdBy DeviceIdentity @relation(fields: [createdById], references: [id], onDelete: Cascade) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt inviteCodeUses InviteCodeUse[] inviteRequests InviteCodeRequest[] notificationTargets InviteCodeNotificationTarget[] @@index([status]) + @@index([status, expiresAt]) @@index([createdById]) @@index([groupId]) + @@index([createdById, groupId]) }
178-190: Index InviteCodeRequest by (inviteCodeId, createdAt) to support ordered reads.get-invite-requests orders by createdAt; add a composite index to help.
model InviteCodeRequest { id String @id @default(cuid()) inviteCodeId String inviteCode InviteCode @relation(fields: [inviteCodeId], references: [id], onDelete: Cascade) requesterId String requester DeviceIdentity @relation(fields: [requesterId], references: [id], onDelete: Cascade) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@unique([inviteCodeId, requesterId]) @@index([inviteCodeId]) @@index([requesterId]) + @@index([inviteCodeId, createdAt]) }src/api/v1/invites/handlers/get-invite-requests.ts (1)
67-92: Consider pagination to prevent unbounded result sets.Add limit/offset or cursor to avoid large responses for heavy creators.
src/api/v1/invites/handlers/get-invite-details.ts (2)
52-56: Use enum instead of string literal for status filter.Improves type-safety and prevents typos.
- status: "ACTIVE", + status: "ACTIVE", // consider: InviteCodeStatus.ACTIVEIf you import the enum:
-import type { InviteCodeStatus } from "@prisma/client"; +import { InviteCodeStatus } from "@prisma/client"; ... - status: "ACTIVE", + status: InviteCodeStatus.ACTIVE,
78-84: Defensive null check on groupMetadata (optional).If legacy rows lack GroupMetadata, these property accesses will throw. Coalesce to nulls or assert non-null.
- name: invite.groupMetadata.name, - description: invite.groupMetadata.description, - imageUrl: invite.groupMetadata.imageUrl, + name: invite.groupMetadata?.name ?? null, + description: invite.groupMetadata?.description ?? null, + imageUrl: invite.groupMetadata?.imageUrl ?? null,Apply similarly in other handlers here.
Also applies to: 151-164, 247-255
prisma/migrations/20250919000000_shared_invite_metadata/migration.sql (1)
25-26: Add an index for the new FK/lookup path.Queries will frequently join/filter by InviteCode.groupId. Add an index.
ALTER TABLE "InviteCode" ADD CONSTRAINT "InviteCode_groupId_fkey" FOREIGN KEY ("groupId") REFERENCES "GroupMetadata"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- Helpful index for group-scoped lookups/joins +CREATE INDEX IF NOT EXISTS "InviteCode_groupId_idx" ON "InviteCode"("groupId");src/api/v1/invites/handlers/request-to-join.ts (5)
171-174: Null‑safety for groupMetadata in notification payload.Avoid potential runtime error if groupMetadata is null.
- name: requestToJoin.inviteCode.groupMetadata.name, - description: requestToJoin.inviteCode.groupMetadata.description, + name: requestToJoin.inviteCode.groupMetadata?.name ?? null, + description: requestToJoin.inviteCode.groupMetadata?.description ?? null,
210-226: Null‑safety for groupMetadata in API response.Same concern for the API response mapping.
- name: requestToJoin.inviteCode.groupMetadata.name, - description: requestToJoin.inviteCode.groupMetadata.description, - imageUrl: requestToJoin.inviteCode.groupMetadata.imageUrl, + name: requestToJoin.inviteCode.groupMetadata?.name ?? null, + description: requestToJoin.inviteCode.groupMetadata?.description ?? null, + imageUrl: requestToJoin.inviteCode.groupMetadata?.imageUrl ?? null,
207-228: Response assembly LGTM.ISO serialization and inviteLinkURL computation are correct. Ensure WEBSITE_URL is configured in prod to avoid relative links.
92-107: Preflight duplicate check is good; consider DB‑level handling only.You can rely solely on the unique index + P2002 mapping to 409 (see above) and drop the preflight read to reduce a round‑trip, if desired.
47-51: Prefer findUnique if xmtpId is unique.If deviceIdentity.xmtpId has a unique constraint, use findUnique for intent clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
prisma/migrations/20250919000000_shared_invite_metadata/migration.sql(1 hunks)prisma/schema.prisma(2 hunks)src/api/v1/invites/handlers/create-invite-code.ts(1 hunks)src/api/v1/invites/handlers/get-invite-details.ts(6 hunks)src/api/v1/invites/handlers/get-invite-requests.ts(2 hunks)src/api/v1/invites/handlers/request-to-join.ts(4 hunks)src/api/v1/invites/handlers/update-invite-code.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tsprisma/migrations/20250919000000_shared_invite_metadata/migration.sqlprisma/schema.prismasrc/api/v1/invites/handlers/update-invite-code.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/get-invite-requests.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/*.prisma
📄 CodeRabbit inference engine (.cursorrules)
**/*.prisma: Follow schema naming conventions (PascalCase for models)
Use appropriate field types and constraints
Add indexes for frequently queried fields
Use appropriate database indexes
Files:
prisma/schema.prisma
🧬 Code graph analysis (2)
src/api/v1/invites/handlers/create-invite-code.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
src/api/v1/invites/handlers/request-to-join.ts (1)
src/utils/invites.ts (1)
getInviteLink(1-4)
⏰ 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: Review for correctness
- GitHub Check: push_to_registry
🔇 Additional comments (9)
src/api/v1/invites/handlers/get-invite-requests.ts (1)
80-86: LGTM: switched to groupMetadata for name/description.Selection and response mapping look correct.
Also applies to: 112-116
src/api/v1/invites/handlers/get-invite-details.ts (2)
60-67: LGTM: public details sourced from groupMetadata.Select and mapping are consistent with the schema change.
Also applies to: 78-84
215-237: LGTM: authenticated details include creator and group metadata.Query shape and mapping look good.
Also applies to: 247-256
src/api/v1/invites/handlers/create-invite-code.ts (1)
141-156: LGTM: response maps metadata from relation.Looks consistent with the new model.
src/api/v1/invites/handlers/update-invite-code.ts (1)
182-195: LGTM: response returns metadata from relation.Aligned with schema changes.
src/api/v1/invites/handlers/request-to-join.ts (3)
15-31: Response shape changes look consistent.The nested invite object with ISO date strings matches the new GroupMetadata model and is coherent.
Please ensure client code has been updated to consume invite.name/description/imageUrl from the nested invite object, not from top-level properties.
121-137: Selection of groupMetadata is correct; ensure null-safety downstream.groupId can be nullable; groupMetadata may be null. Downstream property access should use optional chaining.
61-71: Invite validation LGTM.Active check, expiry check, and consistent 404 for inactive help prevent enumeration.
Also applies to: 73-80, 83-89
prisma/migrations/20250919000000_shared_invite_metadata/migration.sql (1)
24-25: Reconsider ON DELETE CASCADE from InviteCode.groupId → GroupMetadata.id.Location: prisma/migrations/20250919000000_shared_invite_metadata/migration.sql (lines 24–25).
Deleting a GroupMetadata row will cascade-delete all InviteCode rows — confirm that is intended. If not, change to RESTRICT or use SET NULL (make groupId nullable).Suggested change:
-ALTER TABLE "InviteCode" ADD CONSTRAINT "InviteCode_groupId_fkey" FOREIGN KEY ("groupId") REFERENCES "GroupMetadata"("id") ON DELETE CASCADE ON UPDATE CASCADE; +ALTER TABLE "InviteCode" ADD CONSTRAINT "InviteCode_groupId_fkey" FOREIGN KEY ("groupId") REFERENCES "GroupMetadata"("id") ON DELETE RESTRICT ON UPDATE CASCADE;Or: ON DELETE SET NULL (ensure groupId is nullable).
Move invite metadata to shared
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
prisma/schema.prisma (1)
123-150: Manual cleanup exists — make delete-invite.ts resilient to race/fk errors.delete-invite.ts deletes the invite inside a transaction then removes GroupMetadata if count === 0; create/update handlers correctly upsert metadata. Actions to harden against races and DB errors:
- Make the invite deletion idempotent: use deleteMany({ where: { id: params.inviteId, createdById: identity.id } }) and assert affected count, or catch Prisma P2025 and return a 404 instead of bubbling a 500. (src/api/v1/invites/handlers/delete-invite.ts — around tx.inviteCode.delete).
- Make the GroupMetadata removal resilient to concurrent inserts: perform a conditional/atomic delete (e.g., raw DELETE ... WHERE id=$1 AND NOT EXISTS(...) inside the transaction), or use deleteMany/try-catch and ignore foreign-key errors (Prisma P2003) so transient races don’t cause hard failures. (src/api/v1/invites/handlers/delete-invite.ts — around tx.groupMetadata.delete).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
prisma/migrations/20250919000002_remove_cascade_from_group_metadata/migration.sql(1 hunks)prisma/schema.prisma(2 hunks)src/api/v1/invites/handlers/delete-invite.ts(1 hunks)src/api/v1/invites/handlers/request-to-join.ts(4 hunks)tests/invite-requests.test.ts(1 hunks)tests/notifications-invites.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
tests/invite-requests.test.tstests/notifications-invites.test.tssrc/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
tests/invite-requests.test.tsprisma/migrations/20250919000002_remove_cascade_from_group_metadata/migration.sqltests/notifications-invites.test.tssrc/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.tsprisma/schema.prisma
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Write tests for API endpoints
Use helper functions for test setup
Mock external services
Test error cases
Follow AAA pattern (Arrange, Act, Assert)
Files:
tests/invite-requests.test.tstests/notifications-invites.test.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/delete-invite.tssrc/api/v1/invites/handlers/request-to-join.ts
**/*.prisma
📄 CodeRabbit inference engine (.cursorrules)
**/*.prisma: Follow schema naming conventions (PascalCase for models)
Use appropriate field types and constraints
Add indexes for frequently queried fields
Use appropriate database indexes
Files:
prisma/schema.prisma
🧬 Code graph analysis (4)
tests/invite-requests.test.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
tests/notifications-invites.test.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
src/api/v1/invites/handlers/delete-invite.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
src/api/v1/invites/handlers/request-to-join.ts (1)
src/utils/invites.ts (1)
getInviteLink(1-4)
⏰ 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, format check, lint
- GitHub Check: push_to_registry
- GitHub Check: Review for correctness
🔇 Additional comments (11)
prisma/schema.prisma (1)
111-121: LGTM! Well-designed GroupMetadata model with proper schema patterns.The GroupMetadata model follows Prisma best practices with appropriate field types, database-level defaults, and clear relational structure. The relation to InviteCode via groupId → GroupMetadata.id establishes proper referential integrity.
src/api/v1/invites/handlers/delete-invite.ts (1)
56-72: LGTM! Proper cleanup implementation with transaction safety.The transaction correctly handles orphaned GroupMetadata cleanup by:
- Deleting the InviteCode first
- Counting remaining invites for the same group
- Conditionally removing GroupMetadata when no invites remain
This approach ensures data consistency and prevents orphaned metadata records, which is necessary since the foreign key constraint was changed to not cascade (as shown in the migration).
tests/invite-requests.test.ts (2)
102-110: LGTM! Proper test data setup aligned with new schema structure.The upsert operation correctly establishes GroupMetadata before creating InviteCode, which is necessary given the foreign key relationship. The test properly reflects the new data model where group metadata is centralized.
112-119: LGTM! Clean InviteCode creation following new schema.The InviteCode creation correctly references the existing GroupMetadata via groupId and removes the previously embedded name/description fields. This aligns perfectly with the normalized schema design.
tests/notifications-invites.test.ts (2)
203-212: LGTM! Consistent test pattern with proper GroupMetadata setup.The test follows the same pattern as
invite-requests.test.tsby upserting GroupMetadata before creating InviteCode. The group ID ("test-group-notifications-123") is unique to this test context, preventing cross-test contamination.
215-221: LGTM! InviteCode creation properly aligned with schema changes.The InviteCode creation correctly references the GroupMetadata and removes direct embedding of group metadata fields, maintaining consistency with the new normalized design.
src/api/v1/invites/handlers/request-to-join.ts (4)
16-31: LGTM! Well-structured response type with comprehensive invite details.The nested invite structure properly exposes all necessary fields from both InviteCode and GroupMetadata while maintaining a clean API contract. The serialization of date fields to ISO strings ensures consistent client-side handling.
118-149: LGTM! Proper data fetching with normalized schema relations.The query correctly includes
groupMetadatawith selective field retrieval (name, description, imageUrl) while maintaining all necessary InviteCode fields. This approach optimizes data transfer while ensuring complete information availability.
170-171: LGTM! Notification payload correctly sources group metadata.The notification data properly maps group metadata from
groupMetadata.nameandgroupMetadata.descriptioninstead of directly from InviteCode, aligning with the normalized schema structure.
208-225: LGTM! Response construction properly handles normalized data structure.The response correctly maps:
- Group metadata fields from
requestToJoin.inviteCode.groupMetadata- InviteCode fields directly from
requestToJoin.inviteCode- Proper date serialization with
toISOString()- Invite link generation preserved via
getInviteLink()This maintains API compatibility while leveraging the new normalized schema.
prisma/migrations/20250919000002_remove_cascade_from_group_metadata/migration.sql (1)
1-7: Resolved — application handles InviteCode → GroupMetadata cleanup transactionallydelete-invite.ts deletes the InviteCode inside a transaction, then checks remainingInvites and calls tx.groupMetadata.delete when remainingInvites === 0 (src/api/v1/invites/handlers/delete-invite.ts, ~lines 57–70). No direct prisma.$executeRaw / $queryRaw found; tests use inviteCode.deleteMany for teardown. No further changes required.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/api/v1/invites/handlers/get-invite-requests.ts (1)
68-92: Consider pagination to avoid unbounded result setsLarge creators may return tens of thousands of requests. Add cursor/limit and a count if needed.
I can draft a cursor-based pagination patch (query schema + Prisma take/skip + nextCursor) if you want.
src/api/v1/invites/handlers/get-invite-details.ts (1)
129-131: Narrow the include to a select to avoid over-fetchingOnly name/description/imageUrl are needed.
Apply this diff:
- include: { - groupMetadata: true, - }, + select: { + id: true, + maxUses: true, + usesCount: true, + status: true, + expiresAt: true, + autoApprove: true, + groupId: true, + createdAt: true, + groupMetadata: { + select: { name: true, description: true, imageUrl: true }, + }, + },src/api/v1/invites/handlers/create-invite-code.ts (2)
103-140: Authorization gap: invite creation for arbitrary groupIdEndpoint allows creating invites for any groupId. If policy requires membership/ownership, add a check (e.g., existing membership/InviteCodeUse for that group) before proceeding.
Want me to wire a membership check consistent with update handler’s policy (InviteCodeUse existence)?
143-147: Null-safe mapping of groupMetadata in responseGuard mapping with optional chaining.
- name: inviteCode.groupMetadata.name ?? null, - description: inviteCode.groupMetadata.description ?? null, - imageUrl: inviteCode.groupMetadata.imageUrl ?? null, + name: inviteCode.groupMetadata?.name ?? null, + description: inviteCode.groupMetadata?.description ?? null, + imageUrl: inviteCode.groupMetadata?.imageUrl ?? null,src/api/v1/invites/handlers/update-invite-code.ts (2)
259-261: Null-safe mapping of groupMetadata in responseOptional-chain to avoid crashes.
- name: inviteCode.groupMetadata.name ?? null, - description: inviteCode.groupMetadata.description ?? null, - imageUrl: inviteCode.groupMetadata.imageUrl ?? null, + name: inviteCode.groupMetadata?.name ?? null, + description: inviteCode.groupMetadata?.description ?? null, + imageUrl: inviteCode.groupMetadata?.imageUrl ?? null,
185-190: Auth policy check: updating metadata based solely on InviteCodeUse may be too permissiveIf only owners/admins should edit group metadata, enforce role/ownership here instead of membership.
I can propose a role-based check once you confirm the intended policy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/api/v1/invites/handlers/create-invite-code.ts(1 hunks)src/api/v1/invites/handlers/get-invite-details.ts(6 hunks)src/api/v1/invites/handlers/get-invite-requests.ts(2 hunks)src/api/v1/invites/handlers/request-to-join.ts(4 hunks)src/api/v1/invites/handlers/update-invite-code.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/get-invite-details.tssrc/api/v1/invites/handlers/get-invite-requests.ts
🧬 Code graph analysis (2)
src/api/v1/invites/handlers/create-invite-code.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
src/api/v1/invites/handlers/request-to-join.ts (1)
src/utils/invites.ts (1)
getInviteLink(1-4)
⏰ 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: push_to_registry
- GitHub Check: Review for correctness
🔇 Additional comments (12)
src/api/v1/invites/handlers/get-invite-requests.ts (2)
80-86: Null-safe relation: groupMetadata may be nullAccessing fields later without optional chaining can throw. Keep as-is here, but fix usage below.
112-116: Fix possible runtime crash: optional-chain groupMetadata fieldsIf groupMetadata is missing, these property reads throw. Use optional chaining.
Apply this diff:
- name: request.inviteCode.groupMetadata.name ?? null, - description: request.inviteCode.groupMetadata.description ?? null, + name: request.inviteCode.groupMetadata?.name ?? null, + description: request.inviteCode.groupMetadata?.description ?? null,src/api/v1/invites/handlers/get-invite-details.ts (5)
60-66: Selection looks good; ensure downstream uses are null-safeSince groupMetadata can be absent, make sure mapping guards with ?. below.
224-230: Selection ok; ensure null-safe usage in mapping belowCallers should not assume groupMetadata always exists.
79-84: Null-safe mapping for groupMetadata on public detailsAdd optional chaining to prevent crashes.
Apply this diff:
- name: invite.groupMetadata.name ?? null, - description: invite.groupMetadata.description ?? null, - imageUrl: invite.groupMetadata.imageUrl ?? null, + name: invite.groupMetadata?.name ?? null, + description: invite.groupMetadata?.description ?? null, + imageUrl: invite.groupMetadata?.imageUrl ?? null,
153-156: Null-safe mapping for owner detailsSame crash risk; use optional chaining.
- name: invite.groupMetadata.name ?? null, - description: invite.groupMetadata.description ?? null, - imageUrl: invite.groupMetadata.imageUrl ?? null, + name: invite.groupMetadata?.name ?? null, + description: invite.groupMetadata?.description ?? null, + imageUrl: invite.groupMetadata?.imageUrl ?? null,
249-252: Null-safe mapping for authenticated detailsUse optional chaining.
- name: invite.groupMetadata.name ?? null, - description: invite.groupMetadata.description ?? null, - imageUrl: invite.groupMetadata.imageUrl ?? null, + name: invite.groupMetadata?.name ?? null, + description: invite.groupMetadata?.description ?? null, + imageUrl: invite.groupMetadata?.imageUrl ?? null,src/api/v1/invites/handlers/request-to-join.ts (4)
121-137: Selection good; ensure null-safe mapping downstreamYou select groupMetadata fields; treat relation as nullable in mappings below.
72-79: Status check returns 404; confirm intentionalReturning 404 for inactive invites hides existence (anti-enum). If you want explicit feedback, use 409 or 400; otherwise 404 is fine.
170-173: Null-safe mapping in notification payloadOptional-chain groupMetadata fields.
- name: requestToJoin.inviteCode.groupMetadata.name ?? null, - description: requestToJoin.inviteCode.groupMetadata.description ?? null, + name: requestToJoin.inviteCode.groupMetadata?.name ?? null, + description: requestToJoin.inviteCode.groupMetadata?.description ?? null,
209-225: Null-safe mapping in API responseOptional-chain name/description/imageUrl.
- name: requestToJoin.inviteCode.groupMetadata.name ?? null, - description: requestToJoin.inviteCode.groupMetadata.description ?? null, - imageUrl: requestToJoin.inviteCode.groupMetadata.imageUrl ?? null, + name: requestToJoin.inviteCode.groupMetadata?.name ?? null, + description: requestToJoin.inviteCode.groupMetadata?.description ?? null, + imageUrl: requestToJoin.inviteCode.groupMetadata?.imageUrl ?? null,src/api/v1/invites/handlers/update-invite-code.ts (1)
207-216: Remove upsert: this endpoint can still overwrite arbitrary group metadataEven with checkCanUpdateGroupMetadata, upsert lets any “member” edit canonical group fields. Move metadata editing to a dedicated, authorized endpoint; here only ensure existence.
Apply this diff:
- await tx.groupMetadata.upsert({ - where: { id: existingInvite.groupId }, - update: metadataUpdate, - create: { - id: existingInvite.groupId, - name: body.name, - description: body.description, - imageUrl: body.imageUrl, - }, - }); + const gm = await tx.groupMetadata.findUnique({ + where: { id: existingInvite.groupId }, + select: { id: true }, + }); + if (!gm) { + await tx.groupMetadata.create({ + data: { + id: existingInvite.groupId, + name: body.name, + description: body.description, + imageUrl: body.imageUrl, + }, + }); + } + // Do not update existing metadata here.
lourou
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/middleware/auth.ts (1)
77-83: Fix route matching: use RegExp.test directly (current code can throw and never match).
pathToRegexpreturns a RegExp. Accessing.regexp.test(...)is incorrect and may cause 403s for valid notification-extension requests.Apply this diff:
- return NOTIFICATION_EXTENSION_ALLOWED_ROUTES.some( - (route) => route.method === method && route.regexp.regexp.test(path), - ); + return NOTIFICATION_EXTENSION_ALLOWED_ROUTES.some( + (route) => route.method === method && route.regexp.test(path), + );
🧹 Nitpick comments (3)
src/api/v1/invites/handlers/accept-request-to-join.ts (3)
166-176: Map atomic-guard error to 400 response.Handle the "MAX_USES_REACHED" error explicitly to keep existing response semantics.
Apply this diff:
} catch (error) { + if (error instanceof Error && error.message === "MAX_USES_REACHED") { + res.status(400).json({ + success: false, + message: "Invite has reached maximum uses", + }); + return; + } if (error instanceof z.ZodError) { res.status(400).json({ success: false, message: "Invalid request data", errors: error.errors, }); return; }
165-165: Return after sending the success response.Aligns with codebase guideline to return immediately after res.status(...).json(...).
Apply this diff:
- res.status(200).json(response); + res.status(200).json(response); + return;
46-47: Trim unused include to reduce payload.
requesterisn’t used later; drop it to cut query cost.Apply this diff:
- requester: true,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/api/v1/invites/handlers/accept-request-to-join.ts(1 hunks)src/api/v1/invites/invites.router.ts(2 hunks)src/middleware/auth.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/middleware/auth.tssrc/api/v1/invites/invites.router.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/invites.router.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/invites.router.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/middleware/auth.tssrc/api/v1/invites/invites.router.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/middleware/auth.tssrc/api/v1/invites/invites.router.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/invites.router.ts
**/middleware/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Keep middleware in dedicated directory
Files:
src/middleware/auth.ts
🧬 Code graph analysis (2)
src/api/v1/invites/handlers/accept-request-to-join.ts (2)
src/utils/prisma.ts (1)
prisma(3-3)src/api/v1/invites/handlers/request-to-join.ts (1)
requestToJoin(35-245)
src/api/v1/invites/invites.router.ts (1)
src/api/v1/invites/handlers/accept-request-to-join.ts (1)
acceptRequestToJoin(24-182)
⏰ 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: push_to_registry
- GitHub Check: Typecheck, format check, lint
- GitHub Check: Review for correctness
🔇 Additional comments (3)
src/middleware/auth.ts (1)
62-62: Route allowlist update looks good.The new PUT /api/v1/invites/requests/:requestId/accept route is correctly added and matches the router path.
src/api/v1/invites/invites.router.ts (2)
2-2: Import addition looks correct.
19-19: Route wiring LGTM and matches middleware allowlist.Path, method, and ordering are appropriate.
This comment was marked as outdated.
This comment was marked as outdated.
lourou
force-pushed
the
lr/shared-group-metadata
branch
from
ab62b42 to
d60513c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/api/v1/invites/handlers/update-invite-code.ts (4)
8-42: Align auth policy and avoid redundant DB reads (helper is effectively redundant here).You already gate the entire handler to the invite creator (Line 131), so
checkCanUpdateGroupMetadataadds extra queries and a broader policy (“valid InviteCodeUse”) that can’t be reached. Simplify by relying on the earlier check or move all permission checks into one consistent place.Apply this minimal change to remove the extra queries here:
- // Check authorization for metadata updates - const canUpdateMetadata = await checkCanUpdateGroupMetadata({ - tx, - identityId: identity.id, - groupId: existingInvite.groupId, // Use groupId from existing invite - inviteId: params.inviteId, - }); + // Metadata updates are allowed because we already verified invite ownership above. + const canUpdateMetadata = true;Also applies to: 178-184
53-56: Allow clearing expiresAt consistently (schema vs. implementation mismatch).Code attempts to set null, but schema disallows it. Permit null in Zod and map accordingly.
- expiresAt: z.string().datetime().optional(), + expiresAt: z.union([z.string().datetime(), z.null()]).optional(),- ...(body.expiresAt !== undefined && { - expiresAt: body.expiresAt ? new Date(body.expiresAt) : null, - }), + ...(body.expiresAt !== undefined && { + expiresAt: + body.expiresAt === null ? null : new Date(body.expiresAt), + }),Also applies to: 218-221
251-256: Defensive read of groupMetadata in response.If the FK guarantees existence, fine; otherwise this can throw at runtime. Safer:
- name: inviteCode.groupMetadata.name ?? null, - description: inviteCode.groupMetadata.description ?? null, - imageUrl: inviteCode.groupMetadata.imageUrl ?? null, + name: inviteCode.groupMetadata?.name ?? null, + description: inviteCode.groupMetadata?.description ?? null, + imageUrl: inviteCode.groupMetadata?.imageUrl ?? null,
268-269: Add return after sending the response (repo guideline).- res.status(200).json(response); + res.status(200).json(response); + return;src/api/v1/invites/handlers/accept-request-to-join.ts (4)
24-27: Follow object parameter syntax guideline.Per the coding guidelines, functions should use object parameter syntax for better readability and extensibility.
Apply this diff to use object parameter syntax:
-export async function acceptRequestToJoin(req: Request, res: Response) { +export async function acceptRequestToJoin({ req, res }: { req: Request, res: Response }) {
68-84: Optimize duplicate usage check with early return.The function queries for existing usage after loading the request, but this could be done more efficiently by including it in the initial query or combining both checks.
Apply this diff to include the usage check in the initial query:
// Load the join request with related entities const requestToJoin = await prisma.inviteCodeRequest.findUnique({ where: { id: params.requestId }, include: { requester: true, inviteCode: { include: { createdBy: true, notificationTargets: { include: { deviceIdentity: true, }, }, + uses: { + where: { + usedById: { /* we'll need the requester ID here */ } + } + } }, }, }, }); if (!requestToJoin) { res.status(404).json({ success: false, message: "Request not found", }); return; } + // Check if user has already been accepted + const hasExistingUse = requestToJoin.inviteCode.uses.length > 0; + if (hasExistingUse) { + res.status(400).json({ + success: false, + message: "Request has already been accepted", + }); + return; + } - // Check if user has already been accepted (has InviteCodeUse record) - const existingUse = await prisma.inviteCodeUse.findUnique({ - where: { - inviteCodeId_usedById: { - inviteCodeId: requestToJoin.inviteCodeId, - usedById: requestToJoin.requesterId, - }, - }, - }); - - if (existingUse) { - res.status(400).json({ - success: false, - message: "Request has already been accepted", - }); - return; - }However, this requires knowing the requester ID beforehand, so the current approach is actually reasonable.
167-167: Add return statement on new line per guidelines.Per the coding guidelines, always place the 'return;' statement on a new line immediately after sending a response.
Apply this diff:
res.status(200).json(response); + return;
174-175: Add return statements on new lines per guidelines.Per the coding guidelines, always place the 'return;' statement on a new line immediately after sending a response.
Apply this diff:
errors: error.errors, }); + return; - return; } if (error instanceof Error && error.message === "INVITE_MAX_USES_REACHED") { res.status(400).json({ success: false, message: "Invite has reached maximum uses", }); + return; - return; } req.log.error({ error }, "Error accepting request to join"); res.status(500).json({ success: false, message: "Failed to accept request", }); + return;Also applies to: 182-183, 189-190
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/v1/invites/handlers/accept-request-to-join.ts(1 hunks)src/api/v1/invites/handlers/update-invite-code.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/accept-request-to-join.tssrc/api/v1/invites/handlers/update-invite-code.ts
🧬 Code graph analysis (1)
src/api/v1/invites/handlers/accept-request-to-join.ts (2)
src/utils/prisma.ts (1)
prisma(3-3)src/api/v1/invites/handlers/request-to-join.ts (1)
requestToJoin(35-245)
🔇 Additional comments (2)
src/api/v1/invites/handlers/update-invite-code.ts (1)
201-211: Avoid updating arbitrary group-level metadata in this invite-scoped endpoint.Even with the invite-creator check, updating GroupMetadata here is broad (affects all invites in the group) and reintroduces the earlier upsert concern. Consider only ensuring existence here and moving edits to a dedicated, authorized group-metadata endpoint.
Apply this safe existence-only change:
- await tx.groupMetadata.upsert({ - where: { id: existingInvite.groupId }, - update: metadataUpdate, - create: { - id: existingInvite.groupId, - name: body.name, - description: body.description, - imageUrl: body.imageUrl, - }, - }); + const gm = await tx.groupMetadata.findUnique({ + where: { id: existingInvite.groupId }, + select: { id: true }, + }); + if (!gm) { + await tx.groupMetadata.create({ + data: { + id: existingInvite.groupId, + name: body.name, + description: body.description, + imageUrl: body.imageUrl, + }, + }); + }src/api/v1/invites/handlers/accept-request-to-join.ts (1)
1-192: LGTM! Solid transaction-based implementation with good error handling.The handler correctly implements the acceptance flow with proper authorization checks, transaction safety, and comprehensive error handling. The atomic slot-claiming approach prevents race conditions when multiple requests try to use the same invite simultaneously.
…me API request and reponse
…lete the last invite
lourou
force-pushed
the
lr/shared-group-metadata
branch
from
4ab0a02 to
b4c2579
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
src/api/v1/invites/handlers/delete-invite.ts (1)
76-76: Return after sending the responseFollow our API guideline to return immediately after
res.status().json(...).- res.status(200).json(response); + res.status(200).json(response); + return;prisma/migrations/20250923131751_add_composite_indexes/migration.sql (1)
4-15: Indexes look good; confirm FK behavior and consider index redundancy follow-up
- Composite indexes align with query patterns. Nice.
- FK now RESTRICTs deletes; app logic (delete-invite) already guards with a count. Ensure there are no orphan invites before applying to avoid migration failure.
Consider dropping redundant single-column indexes in schema if covered by leftmost columns of composites (e.g., status vs (status, expiresAt); createdById vs (createdById, groupId)) in a subsequent migration after checking query plans.
src/api/v1/invites/handlers/create-invite-code.ts (3)
61-68: Remove redundant 401 guard for xmtpIdAuth middleware guarantees
res.locals.xmtpIdfor authenticated routes; this branch adds noise.- if (!xmtpId) { - res.status(401).json({ - success: false, - message: "Authentication required", - }); - return; - }
118-127: Race-safe create-if-missing for GroupMetadata (handle P2002)Two concurrent creates can hit a unique violation. Catch P2002 and ignore.
- if (!existingMetadata) { - await tx.groupMetadata.create({ - data: { - id: body.groupId, - name: body.name, - description: body.description, - imageUrl: body.imageUrl, - }, - }); - } + if (!existingMetadata) { + try { + await tx.groupMetadata.create({ + data: { + id: body.groupId, + name: body.name, + description: body.description, + imageUrl: body.imageUrl, + }, + }); + } catch (e) { + if ( + !(e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2002") + ) { + throw e; + } + } + }Also add:
import { Prisma } from "@prisma/client";
167-168: Return after sending the responseAdd
return;to comply with our handler pattern.- res.status(201).json(response); + res.status(201).json(response); + return;src/api/v1/invites/handlers/request-to-join.ts (2)
108-150: Handle unique race (P2002) on inviteCodeRequest create to return 409Concurrent requests can bypass the pre-check and hit the unique constraint. Catch P2002 and respond 409 instead of 500.
- const requestToJoin = await prisma.inviteCodeRequest.create({ - data: { - inviteCodeId: body.inviteId, - requesterId: requesterIdentity.id, - }, - include: { - requester: { - include: { profile: true }, - }, - inviteCode: { - select: { - id: true, - maxUses: true, - usesCount: true, - status: true, - expiresAt: true, - autoApprove: true, - groupId: true, - createdAt: true, - updatedAt: true, - createdById: true, - groupMetadata: { - select: { - name: true, - description: true, - imageUrl: true, - }, - }, - createdBy: { - select: { xmtpId: true }, - }, - notificationTargets: { include: { deviceIdentity: true } }, - }, - }, - }, - }); + let requestToJoin; + try { + requestToJoin = await prisma.inviteCodeRequest.create({ + data: { + inviteCodeId: body.inviteId, + requesterId: requesterIdentity.id, + }, + include: { + requester: { include: { profile: true } }, + inviteCode: { + select: { + id: true, + maxUses: true, + usesCount: true, + status: true, + expiresAt: true, + autoApprove: true, + groupId: true, + createdAt: true, + updatedAt: true, + createdById: true, + groupMetadata: { select: { name: true, description: true, imageUrl: true } }, + createdBy: { select: { xmtpId: true } }, + notificationTargets: { include: { deviceIdentity: true } }, + }, + }, + }, + }); + } catch (e) { + if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2002") { + res.status(409).json({ + success: false, + message: "You already have a request for this group", + }); + return; + } + throw e; + }Also add:
import { Prisma } from "@prisma/client";
229-229: Return after sending the responseAdd
return;.- res.status(201).json(response); + res.status(201).json(response); + return;prisma/schema.prisma (2)
134-136: Align referential actions in schema with migration (RESTRICT/CASCADE)Reflect the FK behavior in the Prisma schema to avoid drift.
- groupMetadata GroupMetadata @relation(fields: [groupId], references: [id]) + groupMetadata GroupMetadata @relation( + fields: [groupId], + references: [id], + onDelete: Restrict, + onUpdate: Cascade + )
147-152: Trim redundant indexes to reduce write overhead
@@index([status, expiresAt])coversstatus-only queries;@@index([createdById, groupId])coverscreatedById-only. Consider removing the single-column indexes.- @@index([status]) - @@index([createdById]) @@index([groupId]) @@index([status, expiresAt]) @@index([createdById, groupId])src/api/v1/invites/handlers/update-invite-code.ts (1)
106-112: Remove unnecessary authentication guard.Based on the retrieved learnings, authenticated endpoints in this codebase rely on auth middleware to validate and set
res.locals.xmtpId. The middleware returns 401 before reaching handlers if authentication fails, so this additional guard is redundant.- if (!xmtpId) { - res.status(401).json({ - success: false, - message: "Authentication required", - }); - return; - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
prisma/migrations/20250919000000_shared_invite_metadata/migration.sql(1 hunks)prisma/migrations/20250919000002_remove_cascade_from_group_metadata/migration.sql(1 hunks)prisma/migrations/20250923131751_add_composite_indexes/migration.sql(1 hunks)prisma/schema.prisma(4 hunks)src/api/v1/invites/handlers/accept-request-to-join.ts(1 hunks)src/api/v1/invites/handlers/create-invite-code.ts(2 hunks)src/api/v1/invites/handlers/delete-invite.ts(1 hunks)src/api/v1/invites/handlers/get-invite-details.ts(6 hunks)src/api/v1/invites/handlers/get-invite-requests.ts(2 hunks)src/api/v1/invites/handlers/request-to-join.ts(4 hunks)src/api/v1/invites/handlers/update-invite-code.ts(6 hunks)src/api/v1/invites/invites.router.ts(2 hunks)src/middleware/auth.ts(1 hunks)tests/invite-requests.test.ts(1 hunks)tests/notifications-invites.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/api/v1/invites/handlers/accept-request-to-join.ts
- prisma/migrations/20250919000002_remove_cascade_from_group_metadata/migration.sql
- prisma/migrations/20250919000000_shared_invite_metadata/migration.sql
- tests/notifications-invites.test.ts
- src/api/v1/invites/handlers/get-invite-details.ts
- tests/invite-requests.test.ts
- src/api/v1/invites/invites.router.ts
- src/middleware/auth.ts
- src/api/v1/invites/handlers/get-invite-requests.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
prisma/migrations/20250923131751_add_composite_indexes/migration.sqlsrc/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.tsprisma/schema.prisma
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/request-to-join.tssrc/api/v1/invites/handlers/create-invite-code.tssrc/api/v1/invites/handlers/delete-invite.ts
**/*.prisma
📄 CodeRabbit inference engine (.cursorrules)
**/*.prisma: Follow schema naming conventions (PascalCase for models)
Use appropriate field types and constraints
Add indexes for frequently queried fields
Use appropriate database indexes
Files:
prisma/schema.prisma
🧠 Learnings (4)
📚 Learning: 2025-08-05T14:39:39.983Z
Learnt from: CR
PR: ephemeraHQ/convos-backend#0
File: .cursorrules:0-0
Timestamp: 2025-08-05T14:39:39.983Z
Learning: Applies to **/*.prisma : Add indexes for frequently queried fields
Applied to files:
prisma/migrations/20250923131751_add_composite_indexes/migration.sql
📚 Learning: 2025-08-05T14:39:39.983Z
Learnt from: CR
PR: ephemeraHQ/convos-backend#0
File: .cursorrules:0-0
Timestamp: 2025-08-05T14:39:39.983Z
Learning: Applies to **/*.prisma : Use appropriate database indexes
Applied to files:
prisma/migrations/20250923131751_add_composite_indexes/migration.sql
📚 Learning: 2025-09-04T14:37:41.542Z
Learnt from: nmalzieu
PR: ephemeraHQ/convos-backend#133
File: src/api/v1/devices/handlers/get-device.handler.ts:15-16
Timestamp: 2025-09-04T14:37:41.542Z
Learning: For authenticated endpoints in the convos-backend Express.js application, res.locals.xmtpId is guaranteed to be present when handlers execute because the auth middleware validates JWT tokens and returns 401 before reaching handlers if authentication fails. Additional guards for res.locals.xmtpId are not needed in authenticated route handlers.
Applied to files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/create-invite-code.ts
📚 Learning: 2025-09-04T14:37:49.215Z
Learnt from: nmalzieu
PR: ephemeraHQ/convos-backend#133
File: src/api/v1/devices/handlers/list-devices.handler.ts:6-0
Timestamp: 2025-09-04T14:37:49.215Z
Learning: In the convos-backend codebase, authenticated endpoints rely on auth middleware to validate and set res.locals.xmtpId, so individual handlers don't need additional auth guards for undefined xmtpId values.
Applied to files:
src/api/v1/invites/handlers/update-invite-code.tssrc/api/v1/invites/handlers/create-invite-code.ts
🧬 Code graph analysis (4)
src/api/v1/invites/handlers/update-invite-code.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
src/api/v1/invites/handlers/request-to-join.ts (1)
src/utils/invites.ts (1)
getInviteLink(1-4)
src/api/v1/invites/handlers/create-invite-code.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
src/api/v1/invites/handlers/delete-invite.ts (1)
src/utils/prisma.ts (1)
prisma(3-3)
⏰ 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). (4)
- GitHub Check: push_to_registry
- GitHub Check: Typecheck, format check, lint
- GitHub Check: claude-review
- GitHub Check: Review for correctness
🔇 Additional comments (15)
src/api/v1/invites/handlers/delete-invite.ts (1)
65-67: Good: safe cleanup with deleteMany for potentially missing metadataSwitching to
deleteManyavoids failing the transaction when metadata is absent.src/api/v1/invites/handlers/request-to-join.ts (2)
119-137: LGTM: selecting groupMetadata as canonical sourceSelecting
groupMetadata { name, description, imageUrl }is correct and consistent with the new model.
206-227: LGTM: response maps invite fields from groupMetadata and serializes datesResponse shape and ISO serialization look consistent.
prisma/schema.prisma (2)
111-121: LGTM: central GroupMetadata modelCanonicalizing group fields here is the right move.
192-193: LGTM: composite index on InviteCodeRequestSupports paging by createdAt per invite.
src/api/v1/invites/handlers/update-invite-code.ts (10)
1-1: LGTM: Added Prisma type for transaction client.The Prisma import correctly includes the type needed for the transaction client parameter in the helper function.
8-42: LGTM: Well-designed authorization helper with proper scope.The
checkCanUpdateGroupMetadatafunction correctly implements the authorization logic, allowing both the invite creator and users with valid invite usage to update metadata. The function is well-documented and uses appropriate database queries with proper scoping.
54-55: LGTM: Optional fields align with flexible update semantics.Making
autoApproveandnotificationTargetsoptional correctly supports partial updates where users can choose which fields to modify.
139-152: LGTM: Authorization check correctly replaces ownership guard.The authorization flow now properly uses
checkCanUpdateGroupMetadatainstead of the strict ownership check, allowing both creators and valid invite users to update metadata as intended.
156-156: LGTM: Improved conditional handling for notification targets.The validation now correctly processes notification targets only when provided, eliminating unnecessary strict checks when the field is omitted.
186-191: LGTM: Conditional cleanup prevents unnecessary deletions.The logic correctly deletes existing notification targets only when new targets are provided (
!== undefined), preserving existing targets when the field is not specified in the request.
229-252: LGTM: Dynamic update object construction.The code correctly builds update objects from only the provided fields, using proper filtering to avoid updating fields with
undefinedvalues. The conditional handling of notification targets is also correctly implemented.
255-261: LGTM: Proper relation inclusion for response construction.The query correctly includes
groupMetadatarelation needed for the response, ensuring all required data is available for the response mapping.
268-270: LGTM: Response correctly sources metadata from GroupMetadata.The response construction properly maps name, description, and imageUrl from the
groupMetadatarelation instead of the invite code directly, aligning with the new data model.
216-226: Resolved — authorization is rechecked inside the transaction.rg output shows checkCanUpdateGroupMetadata is called with the transaction client (tx) immediately before the groupMetadata.upsert in src/api/v1/invites/handlers/update-invite-code.ts, so the authorization decision is enforced inside the transaction.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/api/v1/invites/handlers/accept-request-to-join.ts (5)
12-19: Unify response contract; addalreadyAcceptedto the type and set it consistently.The handler returns
alreadyAccepted: truein the idempotent path, but the exported type doesn’t include it, and the main success path omits it. Make it optional in the type and always set it (true/false) in responses.export type AcceptRequestToJoinResponse = { id: string; accepted: boolean; + // Present when the request was previously processed + alreadyAccepted?: boolean; inviteCodeUse: { id: string; usedAt: string; }; };const response: AcceptRequestToJoinResponse = { id: params.requestId, accepted: true, + alreadyAccepted: false, inviteCodeUse: { id: result.inviteCodeUse.id, usedAt: result.inviteCodeUse.usedAt.toISOString(), }, };Also applies to: 95-106, 182-189
35-41: Return 401 for unauthenticated caller.Identity missing should be 401 Unauthorized, not 404. Keep 404 for unauthorized-but-existing resources later as you already do.
- if (!authenticatedIdentity) { - res.status(404).json({ - success: false, - message: "Request not found", - }); - return; - } + if (!authenticatedIdentity) { + res.status(401).json({ + success: false, + message: "Unauthorized", + }); + return; + }
47-56: Trim over-fetching in the join request load.You don’t use
requester, and you only needxmtpIdfromcreatedByanddeviceIdentity. Narrow the selection.const requestToJoin = await prisma.inviteCodeRequest.findUnique({ where: { id: params.requestId }, - include: { - requester: true, - inviteCode: { - include: { - createdBy: true, - notificationTargets: { - include: { - deviceIdentity: true, - }, - }, - }, - }, - }, + include: { + inviteCode: { + select: { + status: true, + expiresAt: true, + createdBy: { select: { xmtpId: true } }, + notificationTargets: { + include: { deviceIdentity: { select: { xmtpId: true } } }, + }, + }, + }, + }, });
61-68: Capture keys for idempotent fallback.Store identifiers now so the catch block can deterministically look up the existing use without relying on the (soon-to-be-deleted) request record.
if (!requestToJoin) { res.status(404).json({ success: false, message: "Request not found", }); return; } + // Capture for idempotent handling in the catch block + context.inviteCodeId = requestToJoin.inviteCodeId; + context.usedById = requestToJoin.requesterId;Add this in the function body before the try block (outside try/catch):
// Captured for idempotent response paths in catch const context: { inviteCodeId?: string; usedById?: string } = {};
130-134: Remove unused selection.
usesCountis fetched but not used in this branch.- select: { maxUses: true, usesCount: true }, + select: { maxUses: true },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/api/v1/invites/handlers/accept-request-to-join.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: TypeScript with strict mode and ESNext features
Implement consistent error handling using AppError class
Use object parameter syntax for function arguments: '(args: { paramOne: string }) => {}' instead of '(paramOne: string) => {}' for better readability and extensibility
Keep functions focused and avoid optional parameters. If a function has many optional parameters, consider splitting it into multiple specialized functions for clearer intent and better maintainability
Prefer type inference from Prisma models
Keep types close to their usage (same directory)
Use type exports for shared types
Leverage path aliases (@/) for imports
Don't specify return type on functions. Prefer inferring the value.
Use transactions for multi-table operations
Include necessary relations in queries using include
Use custom AppError class for application errors
Log errors with context using logError utility
Log security-relevant events
Implement caching where needed
Optimize database queries
Use appropriate batch operations
Add JSDoc comments for public APIs
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/{api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes}/**/*.{ts,tsx}: For API route Response types, use simple typing (e.g., 'res: Response') instead of verbose generic types (e.g., 'res: Response<unknown, any>') to keep code clean and readable
Follow RESTful principles for endpoint naming
Always place the 'return;' statement on a new line immediately after sending a response with 'res.status().json()' or similar methods
Use Zod schemas for request validation and type inference
Return appropriate HTTP status codes
Handle file uploads efficiently
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/handlers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use dedicated handlers directory for route logic
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{api,routes,handlers, middleware}/**/*.{ts,tsx}: Implement consistent error responses
Handle validation errors separately from other errors
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Follow consistent file naming (kebab-case)
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/{middleware,api,routes}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/{middleware,api,routes}/**/*.{ts,tsx}: Implement rate limiting
Use security headers (Helmet)
Handle authentication consistently
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
**/{api,routes, middleware}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Validate and sanitize inputs
Files:
src/api/v1/invites/handlers/accept-request-to-join.ts
🧬 Code graph analysis (1)
src/api/v1/invites/handlers/accept-request-to-join.ts (2)
src/utils/prisma.ts (1)
prisma(3-3)src/api/v1/invites/handlers/request-to-join.ts (1)
requestToJoin(35-245)
⏰ 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). (4)
- GitHub Check: Typecheck, format check, lint
- GitHub Check: push_to_registry
- GitHub Check: claude-review
- GitHub Check: Review for correctness
🔇 Additional comments (4)
src/api/v1/invites/handlers/accept-request-to-join.ts (4)
69-83: Auth “not found” strategy — LGTM.Returning 404 for unauthorized (creator/notification-target) avoids leaking request existence.
95-107: Idempotent pre-check after auth — LGTM.Good placement to avoid information disclosure. Will align once the response type is updated per above.
139-159: Atomic slot claim — LGTM.The guarded
updateManyinside the transaction removes the TOCTOU onmaxUses.Also applies to: 161-165
218-269: Make P2002 idempotency handling robust; avoid 500s and “unknown” placeholders.
- Prisma’s
meta.targetcan be a string or string[]; current check misses the string case.- Don’t depend on the request row (it may be deleted). Use captured keys.
- Return a proper idempotent success with the real InviteCodeUse.
- // Handle race condition where another request already created the InviteCodeUse - if ( - error instanceof Prisma.PrismaClientKnownRequestError && - error.code === "P2002" && - Array.isArray(error.meta?.target) && - (error.meta.target as string[]).some((field) => - ["inviteCodeId_usedById", "inviteCodeId", "usedById"].includes(field), - ) - ) { - // Another request already created the InviteCodeUse - fetch it for idempotent response - // We need the original request data to look up the correct record - const requestId = req.params.requestId; - const requestData = await prisma.inviteCodeRequest.findUnique({ - where: { id: requestId }, - }); - - if (!requestData) { - // Request was already deleted, which means it was processed successfully - // Return success for idempotency - res.status(200).json({ - id: requestId, - accepted: true, - inviteCodeUse: { - id: "unknown", - usedAt: new Date().toISOString(), - }, - }); - return; - } - - const existingUse = await prisma.inviteCodeUse.findUnique({ - where: { - inviteCodeId_usedById: { - inviteCodeId: requestData.inviteCodeId, - usedById: requestData.requesterId, - }, - }, - }); - - if (existingUse) { - const response: AcceptRequestToJoinResponse = { - id: requestId, - accepted: true, - inviteCodeUse: { - id: existingUse.id, - usedAt: existingUse.usedAt.toISOString(), - }, - }; - res.status(200).json(response); - return; - } - } + // Handle race condition where another request already created the InviteCodeUse + if ( + error instanceof Prisma.PrismaClientKnownRequestError && + error.code === "P2002" && + String(error.meta?.target ?? "").includes("inviteCodeId_usedById") + ) { + const existingUse = await prisma.inviteCodeUse.findUnique({ + where: { + inviteCodeId_usedById: { + inviteCodeId: context.inviteCodeId!, + usedById: context.usedById!, + }, + }, + }); + if (existingUse) { + res.status(200).json({ + id: req.params.requestId, + accepted: true, + alreadyAccepted: true, + inviteCodeUse: { + id: existingUse.id, + usedAt: existingUse.usedAt.toISOString(), + }, + }); + return; + } + }Add (outside this hunk) the
contextvariable and its assignment as suggested above.
|
Claude finished @lourou's task —— View job PR Review: Shared Group Metadata for InvitesTodo List
Critical Issues 🚨1. Data Loss in Migration (Blocking Issue)The initial migration (20250919000000_shared_invite_metadata) will permanently lose all existing invite metadata. Lines 14-17 only copy Required Fix: -- Replace lines 14-17 with:
INSERT INTO "GroupMetadata" ("id", "name", "description", "imageUrl", "updatedAt")
SELECT DISTINCT ON (ic."groupId")
ic."groupId" AS id,
ic."name",
ic."description",
ic."imageUrl",
COALESCE(ic."updatedAt", CURRENT_TIMESTAMP) AS "updatedAt"
FROM "InviteCode" ic
WHERE ic."groupId" IS NOT NULL
ORDER BY ic."groupId", ic."updatedAt" DESC NULLS LAST;2. Security: Privilege Escalation in Group Metadata UpdatesBoth Issues:
Recommended Fix: Only create metadata if missing; move updates to a dedicated, properly-authorized group-metadata endpoint. 3. Race Conditions in Accept-Request Handler
Issues:
4. Inconsistent Error Handling
Security Concerns 🔒Authentication & Authorization
Data Validation
Information Disclosure
Performance Considerations ⚡Database Efficiency
Query Optimization
Code Quality & Best Practices 📝Positive Aspects
Areas for Improvement
Test Coverage Assessment 🧪Current Coverage
Missing Coverage
Database Design Review 📊Schema Design
Migration Strategy
Summary & RecommendationsThis PR introduces a solid architectural improvement by centralizing group metadata, but has several critical issues that must be addressed before merging: Must Fix Before Merge:
Recommended Improvements:
Overall Assessment:The core functionality is well-designed and the code quality is generally high. However, the migration data loss issue is a blocking concern that could cause permanent data loss in production. The security and race condition issues also need resolution before this can be safely deployed. Recommendation: Address critical issues before approval, then this will be a solid improvement to the invite system architecture. |
|
Claude finished @lourou's task —— View job I'll analyze this pull request and provide comprehensive feedback on code quality, security, performance, and test coverage. Todo List
PR Review: Shared Group Metadata for InvitesCritical Issues 🚨1. Data Loss in Migration (Blocking Issue)The initial migration (20250919000000_shared_invite_metadata) will permanently lose all existing invite metadata. Lines 14-17 only copy Required Fix: -- Replace lines 14-17 with:
INSERT INTO "GroupMetadata" ("id", "name", "description", "imageUrl", "updatedAt")
SELECT DISTINCT ON (ic."groupId")
ic."groupId" AS id,
ic."name",
ic."description",
ic."imageUrl",
COALESCE(ic."updatedAt", CURRENT_TIMESTAMP) AS "updatedAt"
FROM "InviteCode" ic
WHERE ic."groupId" IS NOT NULL
ORDER BY ic."groupId", ic."updatedAt" DESC NULLS LAST;2. Security: Privilege Escalation in Group Metadata UpdatesBoth Issues:
Recommended Fix: Only create metadata if missing; move updates to a dedicated, properly-authorized group-metadata endpoint. 3. Race Conditions in Accept-Request Handler
Issues:
4. Inconsistent Error Handling
Security Concerns 🔒Authentication & Authorization
Data Validation
Information Disclosure
Performance Considerations ⚡Database Efficiency
Query Optimization
Code Quality & Best Practices 📝Positive Aspects
Areas for Improvement
Test Coverage Assessment 🧪Current Coverage
Missing Coverage
Database Design Review 📊Schema Design
Migration Strategy
Summary & RecommendationsThis PR introduces a solid architectural improvement by centralizing group metadata, but has several critical issues that must be addressed before merging: Must Fix Before Merge:
Recommended Improvements:
Overall Assessment:The core functionality is well-designed and the code quality is generally high. However, the migration data loss issue is a blocking concern that could cause permanent data loss in production. The security and race condition issues also need resolution before this can be safely deployed. Recommendation: Address critical issues before approval, then this will be a solid improvement to the invite system architecture. |
1 participant
Store invite metadata per group by introducing
GroupMetadataand updating invite handlers to read and write metadata viaInviteCode.groupMetadatafor shared group metadata for invitesThis change moves invite
name/description/imageUrlfromInviteCodeto a newGroupMetadatatable keyed bygroupId, updates Prisma schema and migrations, and refactors invite handlers to read and write metadata via thegroupMetadatarelation. It also makes related responses build metadata fromgroupMetadataand performs upserts within transactions when creating or updating invites.GroupMetadatatable and foreign key fromInviteCode.groupIdwith cascade behavior in migration.sql; seed from existingInviteCoderecordsGroupMetadatamodel and relations; remove metadata fields fromInviteCodein schema.prismacreateInviteCodeto upsertGroupMetadataand createInviteCodein a transaction and returngroupMetadatain create-invite-code.tsgroupMetadatain get-invite-details.tsgroupMetadatain get-invite-requests.tsgroupMetadataand serialize timestamps in request-to-join.tsupdateInviteCodeto upsertGroupMetadataand returngroupMetadatain update-invite-code.ts📍Where to Start
Start with the migration and schema to understand the data model change: migration.sql and schema.prisma, then review the transaction logic in
createInviteCodein create-invite-code.ts.Macroscope summarized e366356.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests