feat: authentication secured event types#23217
Conversation
WalkthroughThe PR adds a bookingRequiresAuthentication boolean across the stack (Prisma schema + migration, defaults, types, inputs/outputs, transformers, and OpenAPI docs). BookingsController now accepts optional auth and forwards an optional user to BookingsService. BookingsService injects memberships deps and adds checkBookingRequiresAuthenticationSetting to enforce multi-path authorization before booking creation. EventTypesRepository gained host/assigned-user checks. MembershipsRepository/Service gained methods to evaluate org/team admin/owner relationships. E2E tests were added for managed-user booking behavior (one suite is duplicated). Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 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 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/20/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/22/25)1 label was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts (1)
2195-2195: Fix stray unary + causing runtime error in testThe leading + before expect coerces the matcher to NaN, making .toBe undefined at runtime.
Apply this diff:
- +expect(updatedEventType.hidden).toBe(false); + expect(updatedEventType.hidden).toBe(false);
🧹 Nitpick comments (7)
apps/api/v2/src/modules/slots/slots-2024-09-04/controllers/slots.controller.ts (1)
261-264: Remove duplicate ApiHeader decoratorOPTIONAL_X_CAL_CLIENT_ID_HEADER is declared twice. Keep a single declaration.
Apply this diff:
@ApiHeader(OPTIONAL_X_CAL_CLIENT_ID_HEADER) - @ApiHeader(OPTIONAL_X_CAL_CLIENT_ID_HEADER) @ApiHeader(OPTIONAL_API_KEY_OR_ACCESS_TOKEN_HEADER)packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts (1)
448-454: Mark bookingRequiresAuthentication as optional in OpenAPIThe field is optional in the type, but using ApiProperty (DocsProperty) without required: false will mark it as required in the Swagger schema. Use ApiPropertyOptional for consistency with other optional fields.
Apply this diff:
@IsOptional() @IsBoolean() - @DocsProperty({ + @ApiPropertyOptional({ description: "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type.", }) bookingRequiresAuthentication?: boolean;apps/api/v2/src/modules/memberships/services/memberships.service.ts (1)
25-47: Clear, sequential authorization check; consider clarifying intent via naming/docsThe flow is correct and efficient (early return when no admin/owner orgs; then org membership check, then teams within those orgs). Two minor polish items:
- The parameter name anotherUserId is a bit vague; targetUserId or subjectUserId would be clearer.
- A short JSDoc would help future readers understand that “of another user” means “admin/owner in any org where that user is a member or a member via a team”.
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (1)
781-804: Add an explicit unauthenticated booking test for the “requires authentication” pathYou validate owner and org-admin tokens, and a non-owner managed user. Missing is the “no Authorization header” case, which is the core of this flag. Add a test to assert 401 when booking without any credentials.
Apply this diff to add the missing test right after the first case:
@@ beforeAll(async () => { @@ }); it("can't be booked with managed user credentials who is not admin and not event type owner", async () => { @@ }); + it("can't be booked without authentication", async () => { + await request(app.getHttpServer()) + .post(`/v2/bookings`) + .send(body) + .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) + .expect(401); + }); +apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts (1)
102-138: Document optional auth and gated access in Swagger.Consider adding explicit 401/403 responses and a note that when eventType.bookingRequiresAuthentication is true, valid credentials of an authorized user are required.
Apply this diff to annotate responses:
@Post("/") @UseGuards(OptionalApiAuthGuard) @ApiOperation({ summary: "Create a booking", @@ }) + @ApiUnauthorizedResponse({ description: "Missing or invalid credentials" }) + @ApiForbiddenResponse({ description: "Authenticated but not permitted to book this event type" })And add imports at the top of this file:
import { ApiForbiddenResponse, ApiUnauthorizedResponse } from "@nestjs/swagger";docs/api-reference/v2/openapi.json (2)
5326-5349: Tighten parameter typings and fix header description capitalization
- teamId, take, and skip should be integers (paging/IDs are not fractional).
- Capitalize “API key” and start the sentence with “Value” for consistency.
Apply this diff:
- "description": "For non-platform customers - value must be `Bearer <token>` where `<token>` is api key prefixed with cal_", + "description": "For non-platform customers - Value must be `Bearer <token>`, where `<token>` is an API key prefixed with `cal_`", ... - "schema": { - "type": "number" - } + "schema": { + "type": "integer" + } ... - "schema": { - "minimum": 1, - "maximum": 250, - "default": 250, - "type": "number" - } + "schema": { + "minimum": 1, + "maximum": 250, + "default": 250, + "type": "integer" + } ... - "schema": { - "minimum": 0, - "default": 0, - "type": "number" - } + "schema": { + "minimum": 0, + "default": 0, + "type": "integer" + }Also applies to: 5351-5357, 5359-5369, 5371-5382
12721-12726: Tighten parameter typings and fix header description capitalization
- teamId, take, and skip should be integers.
- Capitalize and clarify Authorization header text.
Apply this diff:
- "description": "value must be `Bearer <token>` where `<token>` is api key prefixed with cal_ or managed user access token", + "description": "Value must be `Bearer <token>`, where `<token>` is an API key prefixed with `cal_` or a managed-user access token", ... - "schema": { - "type": "number" - } + "schema": { + "type": "integer" + } ... - "schema": { - "minimum": 1, - "maximum": 250, - "default": 250, - "type": "number" - } + "schema": { + "minimum": 1, + "maximum": 250, + "default": 250, + "type": "integer" + } ... - "schema": { - "minimum": 0, - "default": 0, - "type": "number" - } + "schema": { + "minimum": 0, + "default": 0, + "type": "integer" + }Also applies to: 12728-12734, 12736-12746, 12749-12758
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
apps/api/v2/src/ee/bookings/2024-08-13/bookings.module.ts(2 hunks)apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts(3 hunks)apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts(3 hunks)apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts(5 hunks)apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts(4 hunks)apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts(1 hunks)apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts(3 hunks)apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts(1 hunks)apps/api/v2/src/modules/memberships/memberships.repository.ts(3 hunks)apps/api/v2/src/modules/memberships/services/memberships.service.ts(1 hunks)apps/api/v2/src/modules/organizations/event-types/services/output.service.ts(1 hunks)apps/api/v2/src/modules/slots/slots-2024-09-04/controllers/slots.controller.ts(2 hunks)apps/api/v2/swagger/documentation.json(18 hunks)docs/api-reference/v2/openapi.json(18 hunks)packages/lib/defaultEvents.ts(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts(1 hunks)packages/prisma/migrations/20250820094118_add_event_type_booking_requires_auth_column/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.tspackages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.tspackages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.tsapps/api/v2/src/modules/organizations/event-types/services/output.service.tsapps/api/v2/src/ee/bookings/2024-08-13/bookings.module.tspackages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.tsapps/api/v2/src/modules/memberships/services/memberships.service.tspackages/lib/defaultEvents.tsapps/api/v2/src/modules/memberships/memberships.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.tsapps/api/v2/src/modules/slots/slots-2024-09-04/controllers/slots.controller.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.tspackages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.tspackages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.tsapps/api/v2/src/modules/organizations/event-types/services/output.service.tsapps/api/v2/src/ee/bookings/2024-08-13/bookings.module.tspackages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.tsapps/api/v2/src/modules/memberships/services/memberships.service.tspackages/lib/defaultEvents.tsapps/api/v2/src/modules/memberships/memberships.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.tsapps/api/v2/src/modules/slots/slots-2024-09-04/controllers/slots.controller.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts
docs/api-reference/v2/openapi.json
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
When docs changes are made in
/docs/api-reference/v2/openapi.json, ensure 'summary' fields are short, concise, do not end with a period, and are written in proper American English
Files:
docs/api-reference/v2/openapi.json
**/*.{service,repository}.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/modules/organizations/event-types/services/output.service.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.tsapps/api/v2/src/modules/memberships/services/memberships.service.tsapps/api/v2/src/modules/memberships/memberships.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.tsapps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts
🧬 Code Graph Analysis (5)
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts (1)
apps/api/v2/src/modules/auth/strategies/api-auth/api-auth.strategy.ts (1)
ApiAuthGuardUser(25-25)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (2)
apps/api/v2/test/utils/randomString.ts (1)
randomString(3-6)packages/platform/constants/api.ts (2)
CAL_API_VERSION_HEADER(72-72)VERSION_2024_08_13(59-59)
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (1)
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts (1)
AuthOptionalUser(5-5)
apps/api/v2/src/modules/slots/slots-2024-09-04/controllers/slots.controller.ts (1)
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts (2)
GetOptionalUser(7-32)AuthOptionalUser(5-5)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts (2)
apps/api/v2/src/modules/auth/guards/optional-api-auth/optional-api-auth.guard.ts (1)
OptionalApiAuthGuard(4-14)apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts (2)
GetOptionalUser(7-32)AuthOptionalUser(5-5)
⏰ 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: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Atoms E2E Tests
🔇 Additional comments (38)
packages/prisma/schema.prisma (1)
231-232: Field addition looks correct and non-breakingAdding bookingRequiresAuthentication with a false default is safe for existing rows and aligns with the migration.
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts (1)
5-6: Good: clear, reusable alias for optional authAuthOptionalUser = ApiAuthGuardUser | null provides a concise shared type for optional auth flows.
apps/api/v2/src/modules/slots/slots-2024-09-04/controllers/slots.controller.ts (2)
3-6: LGTM: imports updated to optional-auth patternImporting AuthOptionalUser alongside GetOptionalUser keeps signatures precise without leaking Prisma types into controllers.
266-266: Signature change is correctAccepting AuthOptionalUser and passing user?.id down maintains behavior while allowing unauthenticated usage.
apps/api/v2/src/modules/organizations/event-types/services/output.service.ts (1)
78-79: Type surface alignedIncluding bookingRequiresAuthentication in Input keeps org event-type outputs in sync with DB and shared types.
packages/prisma/migrations/20250820094118_add_event_type_booking_requires_auth_column/migration.sql (1)
2-2: Migration adds bookingRequiresAuthentication column with sane defaults — LGTMNon-null with default false is appropriate and backward-compatible for existing rows.
apps/api/v2/src/ee/bookings/2024-08-13/bookings.module.ts (1)
25-25: Wiring MembershipsModule into BookingsModule is correctThis enables DI for membership checks during booking authorization. No issues spotted.
Also applies to: 57-58
packages/lib/defaultEvents.ts (1)
149-150: Default event shape updated with bookingRequiresAuthentication — LGTMAdding the flag to commons ensures downstream defaults are consistent with the new schema.
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts (2)
501-502: E2E coverage for bookingRequiresAuthentication on create — LGTMPayload includes bookingRequiresAuthentication: true and asserts it in the response. This validates output mapping as expected.
Also applies to: 574-575
1031-1032: E2E coverage for bookingRequiresAuthentication on update — LGTMThe update payload sets bookingRequiresAuthentication: false and the assertion verifies it. Good coverage for both create and update flows.
Also applies to: 1127-1128
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (4)
6-6: Wiring MembershipsModule into the test app looks goodEnsures the membership-based authorization paths are available for these e2e tests.
84-84: Including MembershipsModule in Test.createTestingModule imports is appropriateThis aligns the test container with production wiring for membership checks.
805-812: Confirm semantics: 401 vs 403 for authenticated-but-insufficient roleHere you expect 401 for a user who is authenticated but lacks permission (not owner/host/assigned/admin). Typical REST semantics would be 403 Forbidden for authenticated users without sufficient authorization, reserving 401 for unauthenticated requests. If the service intentionally returns 401 for both, consider documenting this; otherwise, adjust expectation to 403.
Do you want me to open a follow-up to align status codes (401 unauthenticated, 403 forbidden) and update tests accordingly?
814-836: Positive coverage for owner and org-admin pathsThese assertions exercise two allowed roles and clean up created bookings. LGTM.
apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts (1)
93-94: End-to-end propagation of bookingRequiresAuthentication in outputs looks correct
- Included in Input pick, destructured in getResponseEventType, and returned in the output shape. This keeps API responses in sync with the new schema.
Also applies to: 133-134, 211-212
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts (4)
16-19: Imports for optional auth user decorator look correct.The new GetOptionalUser/AuthOptionalUser imports are aligned with the optional-auth flow introduced in the PR.
23-23: OptionalApiAuthGuard import aligns with the new endpoint behavior.This correctly enables optional authentication on POST /v2/bookings.
146-149: Parameter wiring matches service signature.createBooking(request, body, user) aligns with the service’s new signature.
103-103: PermissionsGuard No-Op Behavior ConfirmedPermissionsGuard’s
canActivatereturnstruewhenever no@Permissionsmetadata is found, so controller-level@UseGuards(PermissionsGuard)will not block methods (such as the@Post("/")handler at line 103) that only use@UseGuards(OptionalApiAuthGuard)without any@Permissions. No further changes are needed.apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (3)
120-123: Potential null-dereference if errors handler doesn’t throw.If this.errorsBookingsService.handleEventTypeToBeBookedNotFound(body) does not throw, eventType may be null, and checkBookingRequiresAuthenticationSetting(eventType, ...) will crash accessing eventType.bookingRequiresAuthentication.
Please confirm handleEventTypeToBeBookedNotFound always throws. If not, add an explicit throw/return after the call.
if (!eventType) { this.errorsBookingsService.handleEventTypeToBeBookedNotFound(body); + return; }
107-110: BookingsModule_2024_08_13 correctly imports MembershipsModule
TheBookingsModule_2024_08_13decorator includesMembershipsModule, which exports bothMembershipsRepositoryandMembershipsService. No further wiring is needed.
112-112: Signature change ripple-check completeThe only call site for
bookingsService.createBookingis in
• apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.ts
– Invoked asthis.bookingsService.createBooking(request, body, user)No other references to the service method were found; tests and library functions use a different
createBooking.docs/api-reference/v2/openapi.json (9)
5321-5321: Summary format looks goodSummary is concise, American English, and has no trailing period.
12716-12716: Summary format looks goodSummary is concise, American English, and has no trailing period.
21053-21059: Per-DTO discriminators look fineUsing a single-value enum for the type discriminator in each DTO is acceptable and keeps the schema explicit.
21074-21080: Per-DTO discriminators look fineConsistent with the other trigger DTOs.
21087-21093: Per-DTO discriminators look fine
21099-21105: Per-DTO discriminators look fine
21111-21117: Per-DTO discriminators look fine
21131-21137: Per-DTO discriminators look fine
21152-21158: Per-DTO discriminators look fineapps/api/v2/swagger/documentation.json (7)
22812-22814: LGTM: specific trigger enum is correctly constrainedThis schema intentionally constrains the trigger to beforeEvent only. Type and enum align.
22838-22840: LGTM: specific trigger enum is correctly constrainedafterEvent trigger enum looks correct.
22856-22858: LGTM: specific trigger enum is correctly constrainedeventCancelled trigger enum looks correct.
22872-22874: LGTM: specific trigger enum is correctly constrainednewEvent trigger enum looks correct.
22888-22890: LGTM: specific trigger enum is correctly constrainedrescheduleEvent trigger enum looks correct.
22912-22914: LGTM: specific trigger enum is correctly constrainedafterGuestsCalVideoNoShow trigger enum looks correct.
22938-22940: LGTM: specific trigger enum is correctly constrainedafterHostsCalVideoNoShow trigger enum looks correct.
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts
Show resolved
Hide resolved
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts
Show resolved
Hide resolved
apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts
Show resolved
Hide resolved
packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts
Show resolved
Hide resolved
packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lib/test/builder.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/test/builder.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/lib/test/builder.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts (1)
136-145: Prefer O(1) existence check via Host composite key; also add explicit return typeUsing eventType.findFirst with hosts.some performs an extra join. If the Host model has a composite unique key (userId_eventTypeId), query Host directly for an O(1) existence check and return a boolean. Also add an explicit return type for stronger typing.
- async isUserHostOfEventType(userId: number, eventTypeId: number) { - const eventType = await this.dbRead.prisma.eventType.findFirst({ - where: { - id: eventTypeId, - hosts: { some: { userId: userId } }, - }, - select: { id: true }, - }); - return !!eventType; - } + async isUserHostOfEventType(userId: number, eventTypeId: number): Promise<boolean> { + const host = await this.dbRead.prisma.host.findUnique({ + where: { userId_eventTypeId: { userId, eventTypeId } }, + select: { userId: true }, + }); + return !!host; + }If the composite unique is not available, fall back to host.findFirst({ where: { userId, eventTypeId }, select: { id: true } }).
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (1)
24-31: LGTM: Use ForbiddenException for authenticated-but-not-authorizedThe switch to ForbiddenException aligns with correct HTTP semantics for authenticated callers lacking permission. Good catch.
🧹 Nitpick comments (3)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts (1)
147-156: Add explicit return type; minor nit on object shorthandThe implementation looks good and adheres to “select-only.” Add an explicit return type, and consider property shorthand for readability.
- async isUserAssignedToEventType(userId: number, eventTypeId: number) { + async isUserAssignedToEventType(userId: number, eventTypeId: number): Promise<boolean> { const eventType = await this.dbRead.prisma.eventType.findFirst({ where: { - id: eventTypeId, - users: { some: { id: userId } }, + id: eventTypeId, + users: { some: { id: userId } }, }, select: { id: true }, }); return !!eventType; }apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (2)
120-124: Return early when event type is not found to aid control-flow and typesIf handleEventTypeToBeBookedNotFound throws (as expected), return explicitly so TS narrows eventType and future lines don't rely on a potentially null value.
- if (!eventType) { - this.errorsBookingsService.handleEventTypeToBeBookedNotFound(body); - } + if (!eventType) { + return this.errorsBookingsService.handleEventTypeToBeBookedNotFound(body); + }If handleEventTypeToBeBookedNotFound indeed always throws, consider annotating its return type as never for even stronger type narrowing.
158-206: Normalize checkBookingRequiresAuthenticationSetting to return void and remove boolean returnsThis method is used for gating and throws on failure. Returning a boolean is unused and slightly muddles intent. Prefer a Promise signature with early returns only for the allowed paths. Also trim internal function names from error messages to avoid leaking implementation details.
- async checkBookingRequiresAuthenticationSetting( - eventType: EventTypeWithOwnerAndTeam, - authUser: AuthOptionalUser - ) { - if (!eventType.bookingRequiresAuthentication) return true; + async checkBookingRequiresAuthenticationSetting( + eventType: EventTypeWithOwnerAndTeam, + authUser: AuthOptionalUser + ): Promise<void> { + if (!eventType.bookingRequiresAuthentication) return; @@ - throw new UnauthorizedException( - "checkBookingRequiresAuthentication - request must be authenticated by passing credentials belonging to event type owner, host or team or org admin or owner." - ); + throw new UnauthorizedException( + "Authentication required: provide credentials for the event type owner/host, or a team/org admin/owner." + ); @@ - const eventTypeId = eventType.id; + const eventTypeId = eventType.id; @@ - if (eventTypeId) { - const [isUserHost, isUserAssigned] = await Promise.all([ - this.eventTypesRepository.isUserHostOfEventType(authUserId, eventTypeId), - this.eventTypesRepository.isUserAssignedToEventType(authUserId, eventTypeId), - ]); - - if (isUserHost || isUserAssigned) return; - } + const [isUserHost, isUserAssigned] = await Promise.all([ + this.eventTypesRepository.isUserHostOfEventType(authUserId, eventTypeId), + this.eventTypesRepository.isUserAssignedToEventType(authUserId, eventTypeId), + ]); + if (isUserHost || isUserAssigned) return; @@ - throw new ForbiddenException( - "checkBookingRequiresAuthentication - user is not authorized to access this event type. User has to be either event type owner, host, team admin or owner or org admin or owner." - ); + throw new ForbiddenException( + "Not authorized to book this event type. Allowed roles: event type owner/host, team admin/owner, or org admin/owner." + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts(5 hunks)apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.tsapps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts
🧬 Code graph analysis (1)
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (1)
apps/api/v2/src/modules/auth/decorators/get-optional-user/get-optional-user.decorator.ts (1)
AuthOptionalUser(5-5)
⏰ 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: Install dependencies / Yarn install & cache
- GitHub Check: Atoms E2E Tests
🔇 Additional comments (3)
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts (3)
9-9: LGTM: Optional auth type import is correct and matches controller changesImporting AuthOptionalUser cleanly communicates the optional-credentials contract for public booking endpoints.
13-14: LGTM: Memberships dependencies are properly injectedMembershipsRepository and MembershipsService DI looks consistent with the new authorization path.
Also applies to: 108-110
113-113: All createBooking call sites updated to include authUserThe
rgsearch shows only two occurrences ofcreateBooking(in the API code:
- The service definition itself in
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts(accepts three parameters).- The controller invocation in
apps/api/v2/src/ee/bookings/2024-08-13/controllers/bookings.controller.tsat line 149, which now correctly passes(request, body, user).No invocations with fewer than three arguments were found.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (13)
docs/api-reference/v2/openapi.json (4)
17224-17230: Clarify bookingRequiresAuthentication wording; capitalize APISame clarification as above for Update input. Keep schema neutral and point to service-level enforcement.
"bookingRequiresAuthentication": { "type": "boolean", - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "Indicates booking this event type via the API requires authorized access; enforcement is handled by the booking service" },
17257-17264: Don’t mark hidden and bookingRequiresAuthentication as requiredThese are optional fields with server-side defaults. Listing them in required breaks clients.
"successRedirectUrl", "isInstantEvent", "scheduleId", - "hidden", - "bookingRequiresAuthentication", "ownerId", "users"
21061-21066: Invalid OpenAPI typing: string enum declared as objectThis property is a scalar enum; type must be string.
"unit": { - "type": "object", + "type": "string", "enum": ["hour", "minute", "day"], "description": "Unit for the offset time", "example": "hour" }
21652-21668: Fix nested enum array and incorrect type; use flat string enum“type” is currently declared as object with enum nested array, which will fail validation and codegen.
"properties": { "type": { - "type": "object", - "enum": [ - [ - "beforeEvent", - "eventCancelled", - "newEvent", - "afterEvent", - "rescheduleEvent", - "afterHostsCalVideoNoShow", - "afterGuestsCalVideoNoShow" - ] - ], + "type": "string", + "enum": [ + "beforeEvent", + "eventCancelled", + "newEvent", + "afterEvent", + "rescheduleEvent", + "afterHostsCalVideoNoShow", + "afterGuestsCalVideoNoShow" + ], "description": "Trigger type for the workflow" } },apps/api/v2/swagger/documentation.json (9)
20985-20991: Team event-type output: fix wording to authorization-based gatingRepeat of the same semantics issue; please harmonize.
"bookingRequiresAuthentication": { "type": "boolean", - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "When true, API bookings are restricted to callers with sufficient permissions (e.g., event owner/host, team admin/owner, or org admin/owner)." },
5814-5829: Use integer for orgId/teamId path paramsSame issue as above in this route: path params should be integer (ids), not number.
{ "name": "orgId", "required": true, "in": "path", "schema": { - "type": "number" + "type": "integer", + "format": "int64" } }, { "name": "teamId", "required": true, "in": "path", "schema": { - "type": "number" + "type": "integer", + "format": "int64" } },
13325-13389: Team schedules: change teamId/take/skip to integer (repeat of earlier feedback)These parameters are still typed as number. Please update to integer (int64 for IDs). This prevents float inputs and matches API semantics.
{ "name": "teamId", "required": true, "in": "path", "schema": { - "type": "number" + "type": "integer", + "format": "int64" } }, { "name": "take", "required": false, "in": "query", "description": "Maximum number of items to return", "example": 25, "schema": { "minimum": 1, "maximum": 250, "default": 250, - "type": "number" + "type": "integer" } }, { "name": "skip", "required": false, "in": "query", "description": "Number of items to skip", "example": 0, "schema": { "minimum": 0, "default": 0, - "type": "number" + "type": "integer" } }
18349-18355: Update input: align bookingRequiresAuthentication wording and add explicit default=falseMirror the semantics used by the service (privileged authorization) and declare the default for consistency with other inputs.
"bookingRequiresAuthentication": { "type": "boolean", - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "default": false, + "description": "If true, bookings via API require a caller with sufficient permissions (e.g., event owner/host, team admin/owner, or org admin/owner). Anonymous or unauthorized callers are rejected." },
22811-22816: Invalid enum schema for unit: type must be string, not objectDeclaring type: object with a string enum will break client generation.
"unit": { - "type": "object", - "enum": [ + "type": "string", + "enum": [ "hour", "minute", "day" ], "description": "Unit for the offset time", "example": "hour" }
23560-23576: Invalid nested enum array and object type (workflow trigger): flatten and use stringEnum is wrapped as a nested array and the schema type is object, which is invalid OpenAPI.
"properties": { "type": { - "type": "object", - "enum": [ - [ - "beforeEvent", - "eventCancelled", - "newEvent", - "afterEvent", - "rescheduleEvent", - "afterHostsCalVideoNoShow", - "afterGuestsCalVideoNoShow" - ] - ], + "type": "string", + "enum": [ + "beforeEvent", + "eventCancelled", + "newEvent", + "afterEvent", + "rescheduleEvent", + "afterHostsCalVideoNoShow", + "afterGuestsCalVideoNoShow" + ], "description": "Trigger type for the workflow" } },
18382-18389: Do not make hidden/bookingRequiresAuthentication required (breaking change)These are feature flags with sensible defaults. Making them required will break existing clients and is unnecessary.
"scheduleId", - "hidden", - "bookingRequiresAuthentication", "ownerId", "users"
5550-5640: IDs and pagination params must be integer-typed (not number) for client correctnessPath/query schemas here use "type": "number". Use integer types to avoid floats slipping into IDs/pagination and to align with prior endpoints.
Apply:
{ "name": "orgId", "required": true, "in": "path", "schema": { - "type": "number" + "type": "integer", + "format": "int64" } }, { "name": "teamId", "required": true, "in": "path", "schema": { - "type": "number" + "type": "integer", + "format": "int64" } }, { "name": "take", "required": false, "in": "query", "description": "Maximum number of items to return", "example": 25, "schema": { "minimum": 1, "maximum": 250, "default": 250, - "type": "number" + "type": "integer" } }, { "name": "skip", "required": false, "in": "query", "description": "Number of items to skip", "example": 0, "schema": { "minimum": 0, "default": 0, - "type": "number" + "type": "integer" } }
16679-16686: Clarify authorization semantics of bookingRequiresAuthentication (create input)Description implies “any authenticated user,” but the implementation enforces privileged roles (owner/host/team admin/org admin). Keep inputs explicit and consistent.
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "If true, bookings via API require a caller with sufficient permissions (e.g., event owner/host, team admin/owner, or org admin/owner). Anonymous or unauthorized callers are rejected." },
🧹 Nitpick comments (9)
docs/api-reference/v2/openapi.json (5)
15737-15744: Clarify bookingRequiresAuthentication to reflect service-enforced authorization (not “any authenticated user”)Per the implementation, this is an indicator; enforcement/role checks happen in service logic. Avoid implying that any authenticated user can book.
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "Indicates booking this event type via the API requires authorized access; enforcement is handled by the booking service" },
17560-17567: Align description with service-enforced authorization and capitalize APIRepeat of clarification for org create input.
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "Indicates booking this event type via the API requires authorized access; enforcement is handled by the booking service" },
19131-19138: Clarify authorization semantics; avoid implying “any authenticated user”Same fix for team event type output.
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "Indicates booking this event type via the API requires authorized access; enforcement is handled by the booking service" },
19542-19548: Clarify authorization semantics; capitalize APISame as above for team update input.
"bookingRequiresAuthentication": { "type": "boolean", - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "Indicates booking this event type via the API requires authorized access; enforcement is handled by the booking service" },
19958-19965: Clarify authorization semantics; capitalize APISame as above for user create input (hosts variant).
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "Indicates booking this event type via the API requires authorized access; enforcement is handled by the booking service" },apps/api/v2/swagger/documentation.json (4)
18707-18714: Team create input: clarify bookingRequiresAuthentication semantics (authZ vs. authN)Same wording fix as above to avoid implying “any authenticated user.”
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "If true, API bookings are restricted to callers with sufficient permissions (e.g., event owner/host, team admin/owner, or org admin/owner)." },
20543-20550: Event-type output: reflect authorization semantics in descriptionOutputs should describe what the flag means operationally; keep wording consistent across all variants.
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "When true, API bookings are restricted to callers with sufficient permissions (e.g., event owner/host, team admin/owner, or org admin/owner)." },
21438-21445: Org-scoped event-type input: unify description and defaultKeep this variant consistent with others: default=false and authorization semantics.
"bookingRequiresAuthentication": { "type": "boolean", "default": false, - "description": "Boolean to require authentication for booking this event type via api. If true, only authenticated users can book this event type." + "description": "If true, API bookings are restricted to callers with sufficient permissions (e.g., event owner/host, team admin/owner, or org admin/owner)." },
22840-22971: Workflow trigger single-value enums: consider $ref to a shared enum to reduce duplicationThese schemas constrain "type" to a single literal via enum of one value. That’s valid but verbose and diverges from the shared trigger enum used elsewhere. Prefer referencing a single components schema (e.g., components/schemas/WorkflowTriggerType) or at least centralize allowed values to avoid drift.
Example:
- "type": { - "type": "string", - "default": "beforeEvent", - "enum": ["beforeEvent"], - "description": "Trigger type for the workflow", - "example": "beforeEvent" - } + "type": { + "$ref": "#/components/schemas/WorkflowTriggerType" + }If literals are intentional per-DTO, keep, but ensure a single source of truth exists for the enum.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/api/v2/src/modules/organizations/teams/schedules/organizations-teams-schedules.controller.ts(2 hunks)apps/api/v2/swagger/documentation.json(19 hunks)docs/api-reference/v2/openapi.json(19 hunks)packages/lib/defaultEvents.ts(1 hunks)packages/lib/test/builder.ts(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts(1 hunks)
🔥 Files not summarized due to errors (1)
- docs/api-reference/v2/openapi.json: Error: Server error: no LLM provider could handle the message
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/lib/defaultEvents.ts
- packages/lib/test/builder.ts
- packages/platform/types/event-types/event-types_2024_06_14/inputs/update-event-type.input.ts
- packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts
🧰 Additional context used
📓 Path-based instructions (4)
docs/api-reference/v2/openapi.json
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
When docs changes are made in
/docs/api-reference/v2/openapi.json, ensure 'summary' fields are short, concise, do not end with a period, and are written in proper American English
Files:
docs/api-reference/v2/openapi.json
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/modules/organizations/teams/schedules/organizations-teams-schedules.controller.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/modules/organizations/teams/schedules/organizations-teams-schedules.controller.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/modules/organizations/teams/schedules/organizations-teams-schedules.controller.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23217
File: docs/api-reference/v2/openapi.json:17226-17229
Timestamp: 2025-08-21T13:59:04.198Z
Learning: In the cal.com codebase, permission logic is handled by business logic in specific endpoints/services rather than being enforced at the API schema level. The schema fields like `bookingRequiresAuthentication` serve as indicators, but the actual permission decisions are made by the individual services.
📚 Learning: 2025-08-21T13:59:04.198Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: docs/api-reference/v2/openapi.json:17226-17229
Timestamp: 2025-08-21T13:59:04.198Z
Learning: In the cal.com codebase, permission logic is handled by business logic in specific endpoints/services rather than being enforced at the API schema level. The schema fields like `bookingRequiresAuthentication` serve as indicators, but the actual permission decisions are made by the individual services.
Applied to files:
apps/api/v2/swagger/documentation.jsondocs/api-reference/v2/openapi.json
📚 Learning: 2025-08-21T12:28:42.000Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.000Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
apps/api/v2/swagger/documentation.jsondocs/api-reference/v2/openapi.json
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Atoms E2E Tests
🔇 Additional comments (11)
docs/api-reference/v2/openapi.json (7)
21081-21087: LGTM on string enum for trigger “beforeEvent”This matches correct scalar enum typing and is consistent with examples.
21102-21108: LGTM on string enum for trigger “afterEvent”Consistent with the above.
21115-21121: LGTM on string enum for trigger “eventCancelled”Consistent scalar enum typing.
21127-21133: LGTM on string enum for trigger “newEvent”Looks good.
21139-21145: LGTM on string enum for trigger “rescheduleEvent”Looks good.
21159-21165: LGTM on string enum for trigger “afterGuestsCalVideoNoShow”Looks good.
21180-21186: LGTM on string enum for trigger “afterHostsCalVideoNoShow”Looks good.
apps/api/v2/src/modules/organizations/teams/schedules/organizations-teams-schedules.controller.ts (2)
70-73: Fix typo and clarify Swagger path params commentConfirmed that the Swagger documentation for
/v2/organizations/{orgId}/teams/{teamId}/users/{userId}/schedulesincludesorgId,teamId, anduserIdas path parameters, and theIsUserInOrgTeamguard reads those exact param names. Applying the diff below corrects the typo and makes the intent explicit:- // note(Lauirs): putting orgId and teamId so swagger is generacted correctly + // note(Lauris): include orgId and teamId params so Swagger generates path params correctly (intentionally unused)
46-48: Fix typos in inline note and clarify Swagger intentThe inline comment on lines 46–48 has two typos (“Lauirs” and “generacted”) and could more clearly state that the parameter is only included to generate the path param in Swagger (and isn’t used at runtime).
• File: apps/api/v2/src/modules/organizations/teams/schedules/organizations-teams-schedules.controller.ts
• Lines: 46–48Proposed diff:
- // note(Lauirs): putting orgId so swagger is generacted correctly + // note(Lauris): include orgId param so Swagger generates path params correctly (intentionally unused)Optional: If you’d rather be explicit in your OpenAPI spec instead of relying on inference, you can add
@ApiParamdecorators for bothorgIdandteamId.apps/api/v2/swagger/documentation.json (2)
16679-16686: No changes needed for bookingRequiresAuthentication consistencyConfirmed that across all schema variants:
- Input definitions (lines 16682–16686, 18710–18714, 20546–20550, 21441–21445) include
"default": falseand share the identical, concise description.- Output definitions (lines 18352–18355, 20988–20991) omit the default as per convention and use the same description text.
All six occurrences are already aligned with the intended authorization‐centric wording and default placement.
5550-5640: Review parameter types for common IDs and pagination fieldsI’ve scanned all path/query parameters named
orgId,teamId,userId,take, andskipthat are currently defined with"type": "number". Please manually verify each occurrence and confirm whether the parameter should indeed be a number or be changed to a string for consistency with the API’s design.• File:
apps/api/v2/swagger/documentation.json
– Sections around lines 5550–5640 (e.g./v2/organizations/{orgId}/teams/{teamId}/schedules)
– Sections around lines 13325–13389 (other endpoints)• To regenerate the full list of matches, run:
rg -z -n -P '"in":\s*"(path|query)".*?"name":\s*"(orgId|teamId|userId|take|skip)".*?"schema":\s*\{[^}]*"type":\s*"number"' apps/api/v2/swagger/documentation.json
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/prisma/zod-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/prisma/zod-utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/prisma/zod-utils.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/prisma/zod-utils.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23217
File: docs/api-reference/v2/openapi.json:17226-17229
Timestamp: 2025-08-21T13:59:04.198Z
Learning: In the cal.com codebase, permission logic is handled by business logic in specific endpoints/services rather than being enforced at the API schema level. The schema fields like `bookingRequiresAuthentication` serve as indicators, but the actual permission decisions are made by the individual services.
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/prisma/zod-utils.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Atoms E2E Tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (4)
782-793: Isolate the variable under test: drop requiresConfirmation for the auth-gated event typeThis suite validates bookingRequiresAuthentication. Setting requiresConfirmation here mixes concerns and can obscure failures (e.g., booking flows that return 201 but PENDING status). Recommend disabling requiresConfirmation in this describe to keep assertions targeted.
length: 60, - requiresConfirmation: true, + requiresConfirmation: false, bookingRequiresAuthentication: true,
822-833: Assert response shape and semantics on successful booking (owner path)Minor: Assert SUCCESS_STATUS and sanity-check the eventTypeId to catch regressions in output mapping. Also re-use the parsed typed body for cleanup.
const response = await request(app.getHttpServer()) .post(`/v2/bookings`) .send(body) .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) .set("Authorization", `Bearer ${secondManagedUser.accessToken}`) .expect(201); - const bookingId = response.body.data.id; + const responseBody: ApiSuccessResponse<BookingOutput_2024_08_13> = response.body; + expect(responseBody.status).toEqual(SUCCESS_STATUS); + expect(responseBody.data.eventTypeId).toEqual(eventTypeRequiringAuthenticationId); + const bookingId = responseBody.data.id; await bookingsRepositoryFixture.deleteById(bookingId);
834-845: Mirror the success assertions for the admin path as wellSame reasoning as above for the admin-allowed path.
const response = await request(app.getHttpServer()) .post(`/v2/bookings`) .send(body) .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) .set("Authorization", `Bearer ${orgAdminManagedUser.accessToken}`) .expect(201); - const bookingId = response.body.data.id; + const responseBody: ApiSuccessResponse<BookingOutput_2024_08_13> = response.body; + expect(responseBody.status).toEqual(SUCCESS_STATUS); + expect(responseBody.data.eventTypeId).toEqual(eventTypeRequiringAuthenticationId); + const bookingId = responseBody.data.id; await bookingsRepositoryFixture.deleteById(bookingId);
847-856: Optional cleanup: delete the auth-gated event type created in this suiteYou delete the created bookings, but not the event type introduced in this describe. It will likely be cascaded by later cleanup, but an explicit delete makes the suite self-contained.
afterAll(async () => { + // If not cascaded automatically: + // await eventTypesRepositoryFixture.delete(eventTypeRequiringAuthenticationId); await userRepositoryFixture.delete(firstManagedUser.user.id); await userRepositoryFixture.delete(secondManagedUser.user.id); await userRepositoryFixture.delete(thirdManagedUser.user.id); await userRepositoryFixture.delete(orgAdminManagedUser.user.id); await userRepositoryFixture.delete(platformAdmin.id); await oauthClientRepositoryFixture.delete(oAuthClient.id); await teamRepositoryFixture.delete(organization.id); await app.close(); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23217
File: docs/api-reference/v2/openapi.json:17226-17229
Timestamp: 2025-08-21T13:59:04.198Z
Learning: In the cal.com codebase, permission logic is handled by business logic in specific endpoints/services rather than being enforced at the API schema level. The schema fields like `bookingRequiresAuthentication` serve as indicators, but the actual permission decisions are made by the individual services.
📚 Learning: 2025-08-21T13:44:06.784Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.784Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧬 Code graph analysis (1)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (2)
apps/api/v2/test/utils/randomString.ts (1)
randomString(3-6)packages/platform/constants/api.ts (2)
CAL_API_VERSION_HEADER(72-72)VERSION_2024_08_13(59-59)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Atoms E2E Tests
🔇 Additional comments (3)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (3)
805-812: Good distinction between 401 (no credentials) and 403 (insufficient privileges)The expected status codes align with the contract introduced in this PR. Nice coverage of the unauthenticated vs. unauthorized paths.
Also applies to: 813-821
6-6: MembershipsModule import is required in this spec; AppModule does not include it
AppModule’s decorator includes SentryModule, ConfigModule, RedisModule, BullModule, ThrottlerModule, PrismaModule, EndpointsModule, AuthModule, and JwtModule—but not MembershipsModule—so the explicit import in your E2E spec is necessary to wire up that module’s providers. Remove the suggestion to drop it.Likely an incorrect or invalid review comment.
776-845: Expand coverage: add tests for host, team admin/owner (team-owned event types), and system adminShort: the new auth gate is only exercised for event-type owner and org admin — please add tests for (a) a host of the event type, (b) a team admin/owner when the event type is team-owned, and (c) a system admin.
What I checked
- Target test file: apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (the describe block "event type booking requires authentication", ~lines 776–845).
- I attempted to find fixture helpers to attach hosts or create team-owned event types (EventTypesRepositoryFixture implementation and helpers like addHost/addHosts or an ownerTeamId/teamId creation field) but could not locate the EventTypesRepositoryFixture implementation in the repository. The test imports test/fixtures/repository/event-types.repository.fixture but the class/host helpers weren't found by search.
Actionable next steps (pick one)
- If the fixture exposes helpers to assign hosts and create team-owned event types: add the three tests inside the existing describe block (examples below).
- If the fixture does not yet expose those helpers: add helpers to the fixture (e.g., addHosts(eventTypeId, userIds) and/or createTeamOwned({ ... }, teamId) or accept ownerTeamId in create) or point me to the fixture implementation so I can wire exact calls.
Suggested test snippets to add (inside describe "event type booking requires authentication"):
+ it("can be booked by a host of the event type", async () => { + const hostEventType = await eventTypesRepositoryFixture.create( + { title: `auth-host-${randomString()}`, slug: `auth-host-${randomString()}`, length: 30, bookingRequiresAuthentication: true }, + secondManagedUser.user.id + ); + // if fixture supports assignment: + // await eventTypesRepositoryFixture.addHosts(hostEventType.id, [firstManagedUser.user.id]); + + const hostBody = { start: new Date(Date.UTC(2030,0,10,10,0,0)).toISOString(), eventTypeId: hostEventType.id, attendee: { email: "external@example.com", name: "External", timeZone: "Europe/Rome" } }; + const res = await request(app.getHttpServer()) + .post(`/v2/bookings`) + .send(hostBody) + .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) + .set("Authorization", `Bearer ${firstManagedUser.accessToken}`) + .expect(201); + await bookingsRepositoryFixture.deleteById(res.body.data.id); + }); + + it("can be booked by a team admin/owner when event type is team-owned", async () => { + // Arrange: create team, make orgAdminManagedUser an ADMIN/OWNER via membership fixture, create event type with teamId/ownerTeamId + // book with orgAdminManagedUser and expect 201 + }); + + it("can be booked by a system admin", async () => { + // Clarify which account/flag represents system admin in tests (platform admin vs membership). If platform admin exists, assert booking succeeds with its credentials. + });I can implement the exact tests and wire fixture calls once you either (A) confirm where EventTypesRepositoryFixture is implemented, or (B) expose helpers to assign hosts/create team-owned event types, or (C) tell me the intended pattern for creating a "system admin" test user in the fixtures.
6 participants
Linear CAL-6281