Conversation
keithwillcode
added
core
WalkthroughA new boolean property, Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/24/25)1 reviewer 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: 2
🧹 Nitpick comments (2)
docs/api-reference/v2/openapi.json (1)
15709-15712: Add explicitdefault: falsefor the new boolean flag.Down-stream code already uses
falseas the implicit default; documenting it here prevents drift and improves generated client typings."enableAutomaticRecordingForOrganizer": { "type": "boolean", - "description": "If true, enables the automatic recording for the event when organizer joins the call" + "description": "If true, enables the automatic recording for the event when organizer joins the call", + "default": false }apps/api/v2/swagger/documentation.json (1)
15709-15712: Tighten wording & add explicit default
- Wording is a bit clunky—“enables the automatic recording for the event when organizer joins the call”.
- The schema omits an explicit
"default": falsealthough the handler applies that default. Surfacing it in the spec prevents client confusion.- "enableAutomaticRecordingForOrganizer": { - "type": "boolean", - "description": "If true, enables the automatic recording for the event when organizer joins the call" - } + "enableAutomaticRecordingForOrganizer": { + "type": "boolean", + "description": "If true, automatically starts recording once the organizer joins the call.", + "default": false + }Ensure the OpenAPI file and the generated client types remain in sync after the default is added.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts(5 hunks)apps/api/v2/swagger/documentation.json(1 hunks)docs/api-reference/v2/openapi.json(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/create.handler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
packages/trpc/server/routers/viewer/eventTypes/create.handler.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/controllers/event-types.controller.e2e-spec.ts
docs/api-reference/v2/openapi.json
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
docs/api-reference/v2/openapi.json: When docs changes are made in /docs/api-reference/v2/openapi.json, ensure the following: 'summary' fields are short and concise.
When docs changes are made in /docs/api-reference/v2/openapi.json, ensure the following: 'summary' fields do not end with a period.
When docs changes are made in /docs/api-reference/v2/openapi.json, ensure the following: 'summary' fields are written in proper American english.
Files:
docs/api-reference/v2/openapi.json
🧠 Learnings (1)
apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts (3)
Learnt from: vijayraghav-io
PR: #21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like @@unique([outlookSubscriptionId, eventTypeId]) should not be added as they would break this subscription sharing functionality.
Learnt from: hariombalhara
PR: #22547
File: packages/embeds/embed-core/src/lib/eventHandlers/scrollByDistanceEventHandler.ts:11-14
Timestamp: 2025-07-16T11:46:28.759Z
Learning: In Cal.com's embed system, internal events like "__scrollByDistance" are fired by Cal.com's own code, so runtime validation of event data structure is unnecessary since TypeScript type system guarantees type safety for internal events.
Learnt from: vijayraghav-io
PR: #21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.264Z
Learning: The Outlook Calendar integration in Cal.com intentionally reuses subscription IDs across multiple event types for efficiency. The upsertSelectedCalendarsForEventTypeIds method creates separate SelectedCalendar records for each eventTypeId, all sharing the same outlookSubscriptionId. This subscription sharing pattern means that unique constraints like @@unique([outlookSubscriptionId, eventTypeId]) should not be applied as they would prevent this intended functionality.
⏰ 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). (1)
- GitHub Check: required
🔇 Additional comments (7)
packages/trpc/server/routers/viewer/eventTypes/create.handler.ts (1)
74-77: LGTM! Clean implementation of new calVideoSettings properties.The implementation correctly follows the established pattern with proper default values and conditional handling. All four new properties (
enableAutomaticTranscription,enableAutomaticRecordingForOrganizer,disableTranscriptionForGuests,disableTranscriptionForOrganizer) are consistently implemented using the nullish coalescing operator withfalsedefaults.apps/api/v2/swagger/documentation.json (1)
15709-15712: Confirm naming consistency across all specs & input DTOsDouble-check that every location (OpenAPI v2 JSON, TS input DTOs, Prisma models, e2e test payloads) uses the exact camel-case key
enableAutomaticRecordingForOrganizer. Any mismatch will silently drop the field on validation.apps/api/v2/src/ee/event-types/event-types_2024_06_14/controllers/event-types.controller.e2e-spec.ts (5)
970-970: LGTM: New property correctly included in update testThe
enableAutomaticRecordingForOrganizerproperty is properly included in thecalVideoSettingsobject for testing the update functionality.
1099-1101: LGTM: Proper assertion for the new propertyThe assertion correctly verifies that the
enableAutomaticRecordingForOrganizerproperty is properly persisted and returned in the API response after updating an event type.
1122-1122: LGTM: Proper test state maintenanceCorrectly updates the local
eventTypeobject with the updatedcalVideoSettingsto maintain test state consistency for subsequent operations.
1879-1879: LGTM: New property correctly included in creation testThe
enableAutomaticRecordingForOrganizerproperty is properly included in thecalVideoSettingsobject for testing the event type creation functionality.
1902-1902: LGTM: Proper assertion for creation functionalityThe assertion correctly verifies that the
enableAutomaticRecordingForOrganizerproperty is properly set and returned in the API response after creating an event type with cal video settings.
packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts
Show resolved
Hide resolved
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
ibex088
dismissed stale reviews from alishaz-polymath and joeauyeung
via
c314f2a
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
8 participants
What does this PR do?
Adds a new property to API v2
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?