Skip to content

Code Review Bench PR #26644 - feat: api v2 team invite link endpoint#6

Open
ketkarameya wants to merge 2 commits intobase_pr_26644_20260125_2973from
corrupted_pr_26644_20260125_2973
Open

Code Review Bench PR #26644 - feat: api v2 team invite link endpoint#6
ketkarameya wants to merge 2 commits intobase_pr_26644_20260125_2973from
corrupted_pr_26644_20260125_2973

Conversation

@ketkarameya
Copy link

@ketkarameya ketkarameya commented Feb 10, 2026

Code Review Bench PR calcom#26644 - feat: api v2 team invite link endpoint

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#6

feat: api v2 team invite link endpoint


Summary by Gitar

  • New API endpoint:
    • POST /v2/teams/{teamId}/invite generates unique team invitation links with token and shareable URL
  • New module:
    • TeamsInviteModule with TeamsInviteController and DTOs following NestJS patterns
  • Authorization:
    • Requires team ADMIN role enforced via RolesGuard for invitation creation
  • Bug fixes:
    • Corrected token return value in TeamService.createInvite (was returning identifier, now returns token)
    • Fixed inverted boolean logic in buildInviteLink method for org context detection

This will update automatically on new commits.


@ApiHeader(API_KEY_HEADER)
export class TeamsInviteController {
@Post("/invite")
@Roles("TEAM_MEMBER")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Role guard uses TEAM_MEMBER instead of TEAM_ADMIN

The controller uses @Roles("TEAM_MEMBER") which is the lowest role level in the hierarchy (TEAM_ROLES = [TEAM_OWNER, TEAM_ADMIN, TEAM_MEMBER]). The hasMinimumRole function in roles.guard.ts checks checkedRoleIndex <= requiredRoleIndex, meaning TEAM_MEMBER at index 2 allows any team member (OWNER, ADMIN, or MEMBER) to access this endpoint.

However, the intent (per PR description) is to restrict invite link creation to team administrators only. The E2E test also explicitly verifies that a non-admin MEMBER receives a 403 Forbidden response, which will fail with the current TEAM_MEMBER role since it permits all members.

This should be @Roles("TEAM_ADMIN") to match the intended authorization requirement and pass the E2E tests.

Was this helpful? React with 👍 / 👎

Suggested change
@Roles("TEAM_MEMBER")
@Roles("TEAM_ADMIN")
  • Apply suggested fix


return {
token,
token: newToken.identifier,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: Token field returns identifier string instead of actual token

At line 99, the returned token field is set to newToken.identifier, which is the descriptive string "invite-link-for-teamId-{teamId}" — not the actual cryptographic token. Meanwhile, buildInviteLink on line 100 correctly uses the token variable (the random hex string) to construct the URL.

This causes two problems:

  1. API response contains wrong value: Consumers receive "invite-link-for-teamId-123" as the token instead of the actual crypto hex token.
  2. E2E test will fail: The test asserts expect(response.body.data.inviteLink).toContain(response.body.data.token) — the identifier string won't appear in the invite URL, so this assertion fails.

The fix should return the actual token (the random hex), or equivalently newToken.token, since that's what is embedded in the invite link URL.

Was this helpful? React with 👍 / 👎

Suggested change
token: newToken.identifier,
token: newToken.token,
  • Apply suggested fix

Copy link

@gitar-bot-staging gitar-bot-staging bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ This PR is blocked due to unresolved code review findings.

Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.

@gitar-bot-staging
Copy link

gitar-bot-staging bot commented Feb 10, 2026

Code Review 🚫 Blocked 0 resolved / 3 findings

Two critical bugs: the @Roles("TEAM_MEMBER") decorator allows any member to create invites (should be TEAM_ADMIN), and the returned token field is the identifier string instead of the actual cryptographic token, causing both incorrect API responses and E2E test failures.

🚨 Bug: Role guard uses TEAM_MEMBER instead of TEAM_ADMIN

📄 apps/api/v2/src/modules/teams/invite/controllers/teams-invite.controller.ts:32

The controller uses @Roles("TEAM_MEMBER") which is the lowest role level in the hierarchy (TEAM_ROLES = [TEAM_OWNER, TEAM_ADMIN, TEAM_MEMBER]). The hasMinimumRole function in roles.guard.ts checks checkedRoleIndex <= requiredRoleIndex, meaning TEAM_MEMBER at index 2 allows any team member (OWNER, ADMIN, or MEMBER) to access this endpoint.

However, the intent (per PR description) is to restrict invite link creation to team administrators only. The E2E test also explicitly verifies that a non-admin MEMBER receives a 403 Forbidden response, which will fail with the current TEAM_MEMBER role since it permits all members.

This should be @Roles("TEAM_ADMIN") to match the intended authorization requirement and pass the E2E tests.

Suggested fix
  @Roles("TEAM_ADMIN")
🚨 Bug: Token field returns identifier string instead of actual token

📄 packages/features/ee/teams/services/teamService.ts:99

At line 99, the returned token field is set to newToken.identifier, which is the descriptive string "invite-link-for-teamId-{teamId}" — not the actual cryptographic token. Meanwhile, buildInviteLink on line 100 correctly uses the token variable (the random hex string) to construct the URL.

This causes two problems:

  1. API response contains wrong value: Consumers receive "invite-link-for-teamId-123" as the token instead of the actual crypto hex token.
  2. E2E test will fail: The test asserts expect(response.body.data.inviteLink).toContain(response.body.data.token) — the identifier string won't appear in the invite URL, so this assertion fails.

The fix should return the actual token (the random hex), or equivalently newToken.token, since that's what is embedded in the invite link URL.

Suggested fix
      token: newToken.token,
💡 Quality: Unused GetUser import in controller

📄 apps/api/v2/src/modules/teams/invite/controllers/teams-invite.controller.ts:3

GetUser is imported from @/modules/auth/decorators/get-user/get-user.decorator but never used in the controller. This is dead code that should be removed for cleanliness.

Suggested fix
import { Roles } from "@/modules/auth/decorators/roles/roles.decorator";
Rules 🎸 1 action taken

Gitar Rules

🎸 Summary Enhancement: Enhanced minimal PR description with 4 technical bullet points covering new endpoint, module architecture, authorization requirements, and bug fixes

5 rules not applicable. Show all rules by commenting gitar display:verbose.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments