-
Notifications
You must be signed in to change notification settings - Fork 10.2k
chore: add auto recording #22723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add auto recording #22723
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/api-reference/v2/openapi.json (1)
15709-15712
: Add explicitdefault: false
for the new boolean flag.Down-stream code already uses
false
as 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": false
although 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.ts
packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts
apps/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 withfalse
defaults.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
enableAutomaticRecordingForOrganizer
property is properly included in thecalVideoSettings
object for testing the update functionality.
1099-1101
: LGTM: Proper assertion for the new propertyThe assertion correctly verifies that the
enableAutomaticRecordingForOrganizer
property is properly persisted and returned in the API response after updating an event type.
1122-1122
: LGTM: Proper test state maintenanceCorrectly updates the local
eventType
object with the updatedcalVideoSettings
to maintain test state consistency for subsequent operations.
1879-1879
: LGTM: New property correctly included in creation testThe
enableAutomaticRecordingForOrganizer
property is properly included in thecalVideoSettings
object for testing the event type creation functionality.
1902-1902
: LGTM: Proper assertion for creation functionalityThe assertion correctly verifies that the
enableAutomaticRecordingForOrganizer
property 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 ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
c314f2a
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
What does this PR do?
Adds a new property to API v2
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?