feat(Coordly): Add member management and invitation workflows - #185
Conversation
📝 WalkthroughWalkthroughAdds member CRUD and invitation lifecycle support across Prisma, API contracts, backend services, web hooks, authenticated management UI, and a public invitation-acceptance page. ChangesMember invitations and management
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant InvitePage
participant useAuth
participant MembersController
participant MembersService
participant SessionService
InvitePage->>useAuth: acceptMemberInvitation({ token })
useAuth->>MembersController: POST /members/invitations/accept
MembersController->>MembersService: acceptInvitation(token)
MembersService->>SessionService: createSession(userId, transaction)
SessionService-->>MembersService: sessionId
MembersService-->>MembersController: accepted user
MembersController-->>useAuth: user response and session cookie
useAuth-->>InvitePage: success response
InvitePage->>InvitePage: redirect to /dashboard
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/web/hooks/use-members.ts (1)
55-62: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSame invalidation anti-pattern in both hooks:
undefineddata arg wipes the cache andswrMutate()double-fetches. Both invalidation helpers reset matched/members*cache entries toundefined(causing an empty-state flash during revalidation) and redundantly callswrMutate()on a key already covered by the matcher. Revalidate without clearing.
apps/web/hooks/use-members.ts#L55-L62: replaceswrMutate()+mutate(matcher, undefined, { revalidate: true })withmutate(matcher)ininvalidateMembers.apps/web/hooks/use-members.ts#L132-L139: apply the identical change ininvalidateInvitations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/hooks/use-members.ts` around lines 55 - 62, The invalidation helpers clear cached data and redundantly fetch the same key. In apps/web/hooks/use-members.ts lines 55-62, update invalidateMembers to replace swrMutate() and the matcher call with mutate(matcher); apply the identical change to invalidateInvitations at lines 132-139, preserving cached data while triggering revalidation.apps/web/app/invite/accept/page.tsx (1)
19-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the acceptance call against double invocation.
Invitation acceptance is a non-idempotent, single-use-token operation (it marks the invitation accepted and creates a session). Under React 19 Strict Mode this effect mounts twice in development, firing
accept()twice; the second call runs against an already-consumed token and can flip a successful acceptance into the error state. AuseRefone-shot guard keeps the mutation firing exactly once.🛡️ Suggested guard
+ const hasAccepted = useRef(false); + useEffect(() => { const token = searchParams.get('token'); if (!token) { setStatus('error'); setErrorMessage('Invalid or missing invitation token'); return; } + if (hasAccepted.current) return; + hasAccepted.current = true; + const accept = async () => {Remember to import
useRef.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/invite/accept/page.tsx` around lines 19 - 49, Guard the invitation acceptance effect with a useRef one-shot flag so acceptMemberInvitation is invoked only once, including under React Strict Mode’s duplicate effect execution. Import useRef, check the guard before starting accept(), and mark it before the mutation; preserve the existing success, redirect, and error handling.apps/api/src/members/members.service.ts (1)
387-414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate token-generation/mail-enqueue logic between
invite()andresendInvitation().Both methods independently generate a token, hash it, compute
expiresAt, buildinvitationLinkfromprocess.env.APP_URL, and enqueueMAIL_JOBS.SEND_INVITATIONwith the same payload shape. Extracting a private helper (e.g.issueInvitationToken(invitation)) would remove this duplication and keep the mail-job payload in one place going forward.Also applies to: 439-461
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/members/members.service.ts` around lines 387 - 414, The invitation token and email queuing logic is duplicated between invite() and resendInvitation(). Extract the shared token generation, hashing, expiry, invitation-link construction, and MAIL_JOBS.SEND_INVITATION enqueue payload into a private helper such as issueInvitationToken, then have both methods call it while preserving their existing invitation data and response behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/members/members.controller.ts`:
- Around line 91-107: Apply Nest’s ParseUUIDPipe to every `@Param`('id') in the
members controller, including findOne, update, remove, resendInvitation, and
revokeInvitation. Keep the existing service calls and response behavior
unchanged so malformed IDs are rejected as 400 responses before reaching Prisma.
In `@apps/api/src/members/members.service.ts`:
- Around line 491-604: The acceptInvitation flow consumes the invitation before
session creation can succeed, risking a committed user/member with no retryable
invitation when createSession fails. Adjust acceptInvitation and its transaction
so session creation is included in the same atomic unit when supported, or move
the acceptedAt update until after sessionService.createSession succeeds while
preserving rollback/retry behavior on session-store failure.
- Around line 205-233: Update create() to validate that the resolved
organizationId references an existing Organization before calling
prisma.member.create, reusing the same lookup and NotFoundException behavior as
invite(). Keep the existing username validation and member creation flow
unchanged when the organization exists.
In `@apps/web/app/`(authenticated)/members/page.tsx:
- Around line 63-66: Update the ROLE_COLORS mapping to use the design-token
classes defined in globals.css instead of the arbitrary blue and purple Tailwind
palette classes, while preserving distinct styling for ADMIN and PRESENTER
badges.
In `@packages/database/prisma/schema.prisma`:
- Around line 207-210: Add a Prisma index for the invitedById foreign key in the
affected model, alongside the existing organizationId, email, and expiresAt
indexes, while preserving the private schema declaration.
---
Nitpick comments:
In `@apps/api/src/members/members.service.ts`:
- Around line 387-414: The invitation token and email queuing logic is
duplicated between invite() and resendInvitation(). Extract the shared token
generation, hashing, expiry, invitation-link construction, and
MAIL_JOBS.SEND_INVITATION enqueue payload into a private helper such as
issueInvitationToken, then have both methods call it while preserving their
existing invitation data and response behavior.
In `@apps/web/app/invite/accept/page.tsx`:
- Around line 19-49: Guard the invitation acceptance effect with a useRef
one-shot flag so acceptMemberInvitation is invoked only once, including under
React Strict Mode’s duplicate effect execution. Import useRef, check the guard
before starting accept(), and mark it before the mutation; preserve the existing
success, redirect, and error handling.
In `@apps/web/hooks/use-members.ts`:
- Around line 55-62: The invalidation helpers clear cached data and redundantly
fetch the same key. In apps/web/hooks/use-members.ts lines 55-62, update
invalidateMembers to replace swrMutate() and the matcher call with
mutate(matcher); apply the identical change to invalidateInvitations at lines
132-139, preserving cached data while triggering revalidation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c547eeaf-c16b-4294-9d25-d53d8f8100d9
📒 Files selected for processing (21)
apps/api/src/members/members.controller.tsapps/api/src/members/members.module.tsapps/api/src/members/members.service.tsapps/web/app/(authenticated)/members/page.tsxapps/web/app/invite/accept/page.tsxapps/web/hooks/use-auth.tsapps/web/hooks/use-members.tsapps/web/proxy.tspackages/contracts/src/members/index.tspackages/contracts/src/members/member-action.response.tspackages/contracts/src/members/member-create.request.tspackages/contracts/src/members/member-invitation-accept.request.tspackages/contracts/src/members/member-invitation-accept.response.tspackages/contracts/src/members/member-invitation-action.response.tspackages/contracts/src/members/member-invitation-list.response.tspackages/contracts/src/members/member-invitation.response.tspackages/contracts/src/members/member-invite.request.tspackages/contracts/src/members/member-update.request.tspackages/contracts/src/members/member.response.tspackages/database/prisma/migrations/20260717183454_add_member_invitations/migration.sqlpackages/database/prisma/schema.prisma
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/database/prisma/migrations/20260721183637_add_member_invitation_invited_by_index/migration.sql (1)
2-2: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid blocking writes during production index creation.
Use
CREATE INDEX CONCURRENTLYfor this live-table index, and verify that the migration runner does not wrap it in a transaction because PostgreSQL disallows concurrent index creation inside one.Proposed change
-CREATE INDEX "MemberInvitation_invitedById_idx" ON "private"."MemberInvitation"("invitedById"); +CREATE INDEX CONCURRENTLY "MemberInvitation_invitedById_idx" ON "private"."MemberInvitation"("invitedById");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/database/prisma/migrations/20260721183637_add_member_invitation_invited_by_index/migration.sql` at line 2, Update the MemberInvitation_invitedById_idx migration to use concurrent index creation, and configure this migration so the runner executes it outside a transaction, preserving PostgreSQL’s requirement for concurrent indexes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/auth/session.service.ts`:
- Around line 27-36: Update createSession so the Redis setex write occurs only
after the database transaction has successfully resolved, ensuring rollback
leaves no Redis-only session and Redis failures do not affect the transaction.
Keep the existing prisma.session.create persistence and return behavior intact,
and use the post-transaction path or established outbox/retry mechanism for the
Redis write.
---
Nitpick comments:
In
`@packages/database/prisma/migrations/20260721183637_add_member_invitation_invited_by_index/migration.sql`:
- Line 2: Update the MemberInvitation_invitedById_idx migration to use
concurrent index creation, and configure this migration so the runner executes
it outside a transaction, preserving PostgreSQL’s requirement for concurrent
indexes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f1f0a3c-788b-41c8-b8c8-a9a7190afbf0
📒 Files selected for processing (9)
apps/api/src/auth/session.service.tsapps/api/src/members/members.controller.tsapps/api/src/members/members.service.tsapps/web/app/(authenticated)/members/page.tsxapps/web/app/invite/accept/page.tsxapps/web/hooks/use-auth.tsapps/web/hooks/use-members.tspackages/database/prisma/migrations/20260721183637_add_member_invitation_invited_by_index/migration.sqlpackages/database/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/hooks/use-auth.ts
- packages/database/prisma/schema.prisma
- apps/web/app/invite/accept/page.tsx
- apps/api/src/members/members.controller.ts
- apps/web/hooks/use-members.ts
- apps/web/app/(authenticated)/members/page.tsx
- apps/api/src/members/members.service.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 (3)
apps/api/src/members/members.service.ts (3)
221-237: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake token rotation and email enqueue retry-safe.
The database token update completes before
mailQueue.add(). If queueing fails,invite()orresendInvitation()can leave a valid but unsent token, while resend also invalidates the previous working link. Use a transactional outbox/job record or a retry-safe compensating transition.Also applies to: 473-493, 525-528
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/members/members.service.ts` around lines 221 - 237, Make token rotation and invitation email enqueue atomic or retry-safe across the invitation flows around the Prisma member update, invite(), and resendInvitation(). Persist an outbox/job record transactionally with the token change, or add a compensating transition that restores the prior token state when mailQueue.add() fails, ensuring retries do not leave a valid unsent token or invalidate a previously working link.
221-227: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake invitation state transitions conditional.
Accept, revoke, and resend first check pending state, then update only by
id. Concurrent operations can therefore set bothacceptedAtandrevokedAt, or rotate a token after acceptance. IncludeorganizationId,acceptedAt: null, andrevokedAt: nullin the write predicate and verify the affected-row count; acceptance should re-read or lock the invitation inside the transaction.Also applies to: 507-513, 539-555, 659-662
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/members/members.service.ts` around lines 221 - 227, Make the invitation writes in the accept, revoke, and resend flows conditional on the invitation remaining pending: include organizationId, acceptedAt: null, and revokedAt: null alongside id in each Prisma update/updateMany predicate. Check the affected-row count and reject or stop when no pending row was updated; for acceptance, re-read or lock the invitation within the transaction before applying the transition.
456-488: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake active-invitation deduplication atomic. The schema only enforces
tokenHashuniqueness, so thefindFirst+createpath can still race and produce duplicate active invitations for the same organization/email or username. Add a database-level uniqueness or locking guard and map conflicts toConflictException.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/members/members.service.ts` around lines 456 - 488, The findFirst/create flow in the member invitation creation method is race-prone and must enforce active-invitation deduplication atomically. Add an appropriate database-level unique constraint or locking mechanism for organization plus active email/username invitations, then update the create path to catch and map resulting uniqueness conflicts to ConflictException while preserving the existing duplicate-invitation message.
🧹 Nitpick comments (1)
apps/api/src/auth/session.service.ts (1)
58-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the mandated
DatabaseServicefor the new session lookup.
cacheSessionintroduces database access throughPrismaService; inject and useDatabaseServiceinstead so this service follows the repository’s database boundary.As per coding guidelines, “Access the database only through
DatabaseService(extendsPrismaClient).”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/auth/session.service.ts` around lines 58 - 68, Update cacheSession to perform the session lookup through the mandated DatabaseService instead of PrismaService. Inject DatabaseService into the containing service and use it for the findUnique query, preserving the existing expiration check and cacheSessionData behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/members/members.service.ts`:
- Around line 672-678: Update the session caching warning in the
accepted-invitation flow around cacheSession to remove result.sessionId from the
log, since it is a bearer credential. Log only the invitation ID and a sanitized
error class or code, avoiding raw error messages and backend details.
---
Outside diff comments:
In `@apps/api/src/members/members.service.ts`:
- Around line 221-237: Make token rotation and invitation email enqueue atomic
or retry-safe across the invitation flows around the Prisma member update,
invite(), and resendInvitation(). Persist an outbox/job record transactionally
with the token change, or add a compensating transition that restores the prior
token state when mailQueue.add() fails, ensuring retries do not leave a valid
unsent token or invalidate a previously working link.
- Around line 221-227: Make the invitation writes in the accept, revoke, and
resend flows conditional on the invitation remaining pending: include
organizationId, acceptedAt: null, and revokedAt: null alongside id in each
Prisma update/updateMany predicate. Check the affected-row count and reject or
stop when no pending row was updated; for acceptance, re-read or lock the
invitation within the transaction before applying the transition.
- Around line 456-488: The findFirst/create flow in the member invitation
creation method is race-prone and must enforce active-invitation deduplication
atomically. Add an appropriate database-level unique constraint or locking
mechanism for organization plus active email/username invitations, then update
the create path to catch and map resulting uniqueness conflicts to
ConflictException while preserving the existing duplicate-invitation message.
---
Nitpick comments:
In `@apps/api/src/auth/session.service.ts`:
- Around line 58-68: Update cacheSession to perform the session lookup through
the mandated DatabaseService instead of PrismaService. Inject DatabaseService
into the containing service and use it for the findUnique query, preserving the
existing expiration check and cacheSessionData behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 77f81757-a6bc-44b5-a045-e4de6def2dbc
📒 Files selected for processing (3)
apps/api/src/auth/session.service.tsapps/api/src/members/members.service.tspackages/database/prisma/migrations/20260721183637_add_member_invitation_invited_by_index/migration.sql
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/database/prisma/migrations/20260721183637_add_member_invitation_invited_by_index/migration.sql
| await this.sessionService.cacheSession(result.sessionId).catch((error) => { | ||
| this.logger.warn( | ||
| `Failed to cache accepted invitation session ${result.sessionId}: ${ | ||
| error instanceof Error ? error.message : String(error) | ||
| }`, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not log the full session identifier.
result.sessionId is returned for cookie creation, so logging it can expose a bearer credential. The raw error message may also contain sensitive backend details; log only the invitation ID and a sanitized error class/code.
Proposed fix
- `Failed to cache accepted invitation session ${result.sessionId}: ${
- error instanceof Error ? error.message : String(error)
- }`,
+ `Failed to cache accepted invitation session for invitation ${invitation.id}`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await this.sessionService.cacheSession(result.sessionId).catch((error) => { | |
| this.logger.warn( | |
| `Failed to cache accepted invitation session ${result.sessionId}: ${ | |
| error instanceof Error ? error.message : String(error) | |
| }`, | |
| ); | |
| }); | |
| await this.sessionService.cacheSession(result.sessionId).catch((error) => { | |
| this.logger.warn( | |
| `Failed to cache accepted invitation session for invitation ${invitation.id}`, | |
| ); | |
| }); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 672-676: Avoid logging sensitive data
Context: this.logger.warn(
Failed to cache accepted invitation session ${result.sessionId}: ${ error instanceof Error ? error.message : String(error) },
)
Note: [CWE-532] Insertion of Sensitive Information into Log File.
(log-sensitive-data-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/members/members.service.ts` around lines 672 - 678, Update the
session caching warning in the accepted-invitation flow around cacheSession to
remove result.sessionId from the log, since it is a bearer credential. Log only
the invitation ID and a sanitized error class or code, avoiding raw error
messages and backend details.
Source: Linters/SAST tools
2 participants
Description
MemberInvitationPrisma model and migration for storing invitation metadata, token hashes, expiration, and acceptance state./invite/acceptto be accessed without an authenticated session so invited users can accept invitations.Link to issue or ticket
N/A
Steps to QA
npx turbo run db:generate.npx turbo run db:deploy.npx turbo run check-types./members./invite/accept?token=<valid-token>while signed out and verify the invitation is accepted, a session is created, and the user is redirected to/dashboard.Screenshots
Summary by CodeRabbit