feat: add toggle to disable recording download email for guests #22259
Conversation
- Add disableRecordingDownloadEmailForGuests field to CalVideoSettings schema - Update repository layer to handle new field with default false value - Add UI toggle in event type setup page alongside other Cal Video settings - Modify sendDailyVideoRecordingEmails to conditionally skip attendee emails - Update webhook handler to pass calVideoSettings to email function - Add i18n translation for new toggle option - Update all TypeScript types and TRPC schemas - Create database migration for new field Follows pattern from PR #21755 for transcription settings. Only attendee emails are skipped when enabled - organizer emails always sent. Co-Authored-By: udit@cal.com <udit222001@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
|
✅ No security or compliance issues detected. Reviewed everything up to d30c996. Security Overview
Detected Code Changes
Reply to this PR with |
|
This PR is being marked as stale due to inactivity. |
WalkthroughThis change introduces a new boolean setting, Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/app/api/recorded-daily-video/route.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. apps/web/lib/video/[uid]/getServerSideProps.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. packages/emails/daily-video-emails.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team.
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 (
|
- Merge disableRecordingDownloadEmailForGuests with enableAutomaticRecordingForOrganizer - Use consistent .nullish() pattern in TypeScript schemas - Maintain compatibility between both recording features Co-Authored-By: udit@cal.com <udit222001@gmail.com>
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/lib/server/repository/calVideoSettings.ts (1)
15-23: Missing field in createCalVideoSettings method.The
createCalVideoSettingsmethod doesn't include the newdisableRecordingDownloadEmailForGuestsfield in its type definition or implementation, whilecreateOrUpdateCalVideoSettingsdoes. This inconsistency could lead to issues if the create method is used elsewhere.Consider adding the field to maintain consistency:
calVideoSettings: { disableRecordingForGuests?: boolean | null; disableRecordingForOrganizer?: boolean | null; + disableRecordingDownloadEmailForGuests?: boolean | null; enableAutomaticTranscription?: boolean | null; enableAutomaticRecordingForOrganizer?: boolean | null; disableTranscriptionForGuests?: boolean | null; disableTranscriptionForOrganizer?: boolean | null; redirectUrlOnExit?: string | null; };And in the create data object:
data: { disableRecordingForGuests: calVideoSettings.disableRecordingForGuests ?? false, disableRecordingForOrganizer: calVideoSettings.disableRecordingForOrganizer ?? false, + disableRecordingDownloadEmailForGuests: calVideoSettings.disableRecordingDownloadEmailForGuests ?? false, enableAutomaticTranscription: calVideoSettings.enableAutomaticTranscription ?? false, enableAutomaticRecordingForOrganizer: calVideoSettings.enableAutomaticRecordingForOrganizer ?? false, disableTranscriptionForGuests: calVideoSettings.disableTranscriptionForGuests ?? false, disableTranscriptionForOrganizer: calVideoSettings.disableTranscriptionForOrganizer ?? false, redirectUrlOnExit: calVideoSettings.redirectUrlOnExit ?? null, eventTypeId, },
🧹 Nitpick comments (3)
packages/prisma/migrations/20250704084455_add_disable_recording_download_email_for_guests/migration.sql (1)
2-2: Consider using nullable Boolean field to align with team preferences.While this migration is technically correct and uses a default value to avoid updating existing rows, the retrieved learning suggests Cal.com prefers nullable Boolean fields (
Boolean?) with defaults rather than non-nullable fields (Boolean NOT NULL) to avoid expensive database migrations.Consider this alternative approach:
-ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingDownloadEmailForGuests" BOOLEAN NOT NULL DEFAULT false; +ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingDownloadEmailForGuests" BOOLEAN DEFAULT false;This would align with the team's established pattern while maintaining the same functionality.
packages/prisma/schema.prisma (1)
79-79: Consider using nullable Boolean field to follow Cal.com's schema design patterns.Based on the retrieved learnings, Cal.com typically prefers nullable Boolean fields (
Boolean?) with defaults rather than non-nullable (Boolean) to avoid expensive database migrations that would update existing rows. Consider changing this to:- disableRecordingDownloadEmailForGuests Boolean @default(false) + disableRecordingDownloadEmailForGuests Boolean? @default(false)This approach aligns with the team's established pattern for Boolean schema fields.
packages/features/eventtypes/components/Locations.tsx (1)
399-412: Consider adding UpgradeTeamsBadge for consistency.The new toggle follows the correct pattern but is missing the
UpgradeTeamsBadgecomponent that other recording-related toggles have (lines 359, 375, 392). This inconsistency might confuse users about feature availability.Apply this diff to maintain consistency:
<SettingsToggle title={t("disable_recording_download_email_for_guests")} labelClassName="text-sm" checked={value} onCheckedChange={onChange} + Badge={<UpgradeTeamsBadge checkForActiveStatus />} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/app/api/recorded-daily-video/route.ts(1 hunks)apps/web/lib/daily-webhook/getBooking.ts(1 hunks)apps/web/lib/video/[uid]/getServerSideProps.ts(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/emails/daily-video-emails.ts(1 hunks)packages/features/eventtypes/components/Locations.tsx(1 hunks)packages/features/eventtypes/lib/types.ts(1 hunks)packages/features/schedules/components/Schedule.tsx(0 hunks)packages/lib/server/repository/booking.ts(1 hunks)packages/lib/server/repository/calVideoSettings.ts(3 hunks)packages/lib/server/repository/eventTypeRepository.ts(1 hunks)packages/prisma/migrations/20250704084455_add_disable_recording_download_email_for_guests/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)packages/prisma/zod/custom/eventtype.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/create.handler.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/types.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/features/schedules/components/Schedule.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:
apps/web/lib/video/[uid]/getServerSideProps.tspackages/features/eventtypes/lib/types.tsapps/web/app/api/recorded-daily-video/route.tspackages/lib/server/repository/eventTypeRepository.tspackages/prisma/zod/custom/eventtype.tspackages/lib/server/repository/booking.tsapps/web/lib/daily-webhook/getBooking.tspackages/lib/server/repository/calVideoSettings.tspackages/trpc/server/routers/viewer/eventTypes/create.handler.tspackages/features/eventtypes/components/Locations.tsxpackages/trpc/server/routers/viewer/eventTypes/types.tspackages/emails/daily-video-emails.ts
🧠 Learnings (5)
packages/prisma/migrations/20250704084455_add_disable_recording_download_email_for_guests/migration.sql (1)
Learnt from: vijayraghav-io
PR: #16579
File: packages/prisma/schema.prisma:149-153
Timestamp: 2025-07-16T07:14:49.248Z
Learning: In Cal.com's schema design, the team prefers to keep Boolean fields nullable (Boolean?) with defaults rather than making them non-nullable (Boolean) to avoid expensive database migrations that would update existing rows.
apps/web/lib/daily-webhook/getBooking.ts (3)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-21T13:54:11.770Z
Learning: Applies to backend/**/*.{ts,tsx} : For Prisma queries: Select selects only the fields you specify explicitly.
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
packages/features/eventtypes/components/Locations.tsx (2)
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the currentLink.maxUsageCount ?? 1 fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Learnt from: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.343Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
packages/prisma/schema.prisma (3)
Learnt from: vijayraghav-io
PR: #16579
File: packages/prisma/schema.prisma:149-153
Timestamp: 2025-07-16T07:14:49.248Z
Learning: In Cal.com's schema design, the team prefers to keep Boolean fields nullable (Boolean?) with defaults rather than making them non-nullable (Boolean) to avoid expensive database migrations that would update existing rows.
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: 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.
apps/web/public/static/locales/en/common.json (1)
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.233Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
🧬 Code Graph Analysis (2)
apps/web/app/api/recorded-daily-video/route.ts (1)
packages/emails/daily-video-emails.ts (1)
sendDailyVideoRecordingEmails(35-55)
packages/features/eventtypes/components/Locations.tsx (1)
packages/ui/components/form/switch/SettingsToggle.tsx (1)
SettingsToggle(29-125)
🔇 Additional comments (15)
apps/web/public/static/locales/en/common.json (1)
50-50: Ensure new key is replicated in all locale filesThe English entry looks good and the key name is consistent with the TypeScript flag
disableRecordingDownloadEmailForGuests.
To avoid runtime fall-back to English, remember to add the same key (even if just as an empty or copied value) to every othercommon.jsonlocale underpublic/static/locales/<lang>/.
Failing to do so will surface untranslated strings in non-EN environments.apps/web/lib/video/[uid]/getServerSideProps.ts (1)
25-25: LGTM! Type definition follows established patterns.The addition of
disableRecordingDownloadEmailForGuests: booleanis consistent with the existing CalVideoSettings interface structure and naming conventions.apps/web/lib/daily-webhook/getBooking.ts (1)
26-26: LGTM! Necessary change to support the new email setting.Adding
calVideoSettings: trueto the select clause enables fetching the complete CalVideoSettings object, including the newdisableRecordingDownloadEmailForGuestsfield that will be used in the email sending logic.apps/web/app/api/recorded-daily-video/route.ts (1)
140-140: LGTM! Correct integration of calVideoSettings into email workflow.The addition of
booking.eventType?.calVideoSettingsas the third parameter correctly passes the video settings (including the newdisableRecordingDownloadEmailForGuestsflag) to the email function. The optional chaining handles potential null values appropriately.packages/lib/server/repository/eventTypeRepository.ts (1)
711-711: LGTM! Correctly extends calVideoSettings selection.The addition of
disableRecordingDownloadEmailForGuests: trueto the calVideoSettings select clause ensures the new field is included when fetching complete event type data. The placement and formatting are consistent with existing fields.packages/features/eventtypes/lib/types.ts (1)
161-161: LGTM! Type definition correctly added.The new
disableRecordingDownloadEmailForGuestsproperty is properly typed as an optional boolean and correctly positioned within thecalVideoSettingsobject alongside related recording settings.packages/trpc/server/routers/viewer/eventTypes/types.ts (1)
34-34: LGTM! Schema validation properly added.The new
disableRecordingDownloadEmailForGuestsfield is correctly defined withz.boolean().nullish(), maintaining consistency with other optional boolean fields in thecalVideoSettingsSchema.packages/lib/server/repository/booking.ts (1)
400-400: LGTM! Database field selection properly added.The new
disableRecordingDownloadEmailForGuestsfield is correctly added to the select statement within thecalVideoSettingsselection, enabling the booking repository to fetch this setting for meeting page functionality.packages/prisma/zod/custom/eventtype.ts (1)
12-15: LGTM! Schema extensions properly implemented.The new fields are correctly defined with
z.boolean().optional().nullable(), maintaining consistency with the existing schema pattern. Note that this includes transcription-related fields (enableAutomaticTranscription,disableTranscriptionForGuests,disableTranscriptionForOrganizer) in addition to the main PR focus ofdisableRecordingDownloadEmailForGuests.packages/prisma/schema.prisma (1)
75-84: Good field placement and consistent formatting.The new field is logically positioned among other recording-related settings, and the indentation adjustments maintain consistency across the model definition.
packages/lib/server/repository/calVideoSettings.ts (2)
47-47: Type definition looks good.The new optional field follows the established pattern with proper nullable boolean type definition.
60-61: Consistent implementation in upsert operation.The new field is properly handled in both
updateandcreatebranches with appropriate default fallback using nullish coalescing.Also applies to: 72-73
packages/emails/daily-video-emails.ts (2)
35-39: Well-implemented function signature update.The optional parameter with proper typing maintains backward compatibility while enabling the new functionality.
47-53: Correct conditional email logic implementation.The logic properly preserves organizer emails while conditionally skipping attendee emails based on the flag. The optional chaining ensures safe property access and maintains backward compatibility.
packages/trpc/server/routers/viewer/eventTypes/create.handler.ts (1)
74-78: Consistent implementation of new CalVideo settings.The new properties follow the established pattern with proper null coalescing defaults and maintain type safety. The addition of transcription-related properties alongside the recording email setting aligns with the PR's broader video settings enhancements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/prisma/migrations/20250704084455_add_disable_recording_download_email_for_guests/migration.sql (1)
2-2: Consider using nullable Boolean for consistency with team preferences.Based on previous team discussions, Cal.com prefers using nullable Boolean fields (
Boolean?) with defaults rather than non-nullable (Boolean) to avoid expensive database migrations. While this specific migration adds a new column with a default (which shouldn't require updating existing rows), consider usingBOOLEAN DEFAULT falsewithoutNOT NULLfor consistency with established patterns.-ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingDownloadEmailForGuests" BOOLEAN NOT NULL DEFAULT false; +ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingDownloadEmailForGuests" BOOLEAN DEFAULT false;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/web/app/api/recorded-daily-video/route.ts(1 hunks)apps/web/lib/daily-webhook/getBooking.ts(1 hunks)apps/web/lib/video/[uid]/getServerSideProps.ts(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/emails/daily-video-emails.ts(1 hunks)packages/features/eventtypes/components/Locations.tsx(1 hunks)packages/features/eventtypes/lib/types.ts(1 hunks)packages/features/schedules/components/Schedule.tsx(0 hunks)packages/lib/server/repository/booking.ts(1 hunks)packages/lib/server/repository/calVideoSettings.ts(3 hunks)packages/lib/server/repository/eventTypeRepository.ts(1 hunks)packages/prisma/migrations/20250704084455_add_disable_recording_download_email_for_guests/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)packages/prisma/zod/custom/eventtype.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/create.handler.ts(1 hunks)packages/trpc/server/routers/viewer/eventTypes/types.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/features/schedules/components/Schedule.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:
apps/web/lib/daily-webhook/getBooking.tspackages/lib/server/repository/eventTypeRepository.tspackages/prisma/zod/custom/eventtype.tspackages/trpc/server/routers/viewer/eventTypes/types.tspackages/lib/server/repository/calVideoSettings.tsapps/web/app/api/recorded-daily-video/route.tspackages/features/eventtypes/components/Locations.tsxpackages/lib/server/repository/booking.tspackages/trpc/server/routers/viewer/eventTypes/create.handler.tsapps/web/lib/video/[uid]/getServerSideProps.tspackages/emails/daily-video-emails.tspackages/features/eventtypes/lib/types.ts
🧠 Learnings (5)
packages/prisma/migrations/20250704084455_add_disable_recording_download_email_for_guests/migration.sql (1)
Learnt from: vijayraghav-io
PR: #16579
File: packages/prisma/schema.prisma:149-153
Timestamp: 2025-07-16T07:14:49.248Z
Learning: In Cal.com's schema design, the team prefers to keep Boolean fields nullable (Boolean?) with defaults rather than making them non-nullable (Boolean) to avoid expensive database migrations that would update existing rows.
apps/web/public/static/locales/en/common.json (1)
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.233Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
apps/web/lib/daily-webhook/getBooking.ts (3)
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-21T13:54:11.770Z
Learning: Applies to backend/**/*.{ts,tsx} : For Prisma queries: Select selects only the fields you specify explicitly.
Learnt from: vijayraghav-io
PR: #21072
File: packages/app-store/office365calendar/api/webhook.ts:120-123
Timestamp: 2025-07-18T17:57:16.395Z
Learning: The office365calendar webhook handler in packages/app-store/office365calendar/api/webhook.ts is specifically designed for Office365 calendar integration, not as a generic webhook handler. Therefore, it's safe to assume that fetchAvailabilityAndSetCache method will be implemented in the Office365CalendarService, making explicit validation checks unnecessary.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/prisma/schema.prisma (3)
Learnt from: vijayraghav-io
PR: #16579
File: packages/prisma/schema.prisma:149-153
Timestamp: 2025-07-16T07:14:49.248Z
Learning: In Cal.com's schema design, the team prefers to keep Boolean fields nullable (Boolean?) with defaults rather than making them non-nullable (Boolean) to avoid expensive database migrations that would update existing rows.
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: 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.
packages/features/eventtypes/components/Locations.tsx (2)
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the currentLink.maxUsageCount ?? 1 fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Learnt from: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.343Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
🧬 Code Graph Analysis (1)
apps/web/app/api/recorded-daily-video/route.ts (1)
packages/emails/daily-video-emails.ts (1)
sendDailyVideoRecordingEmails(35-55)
⏰ 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: Install dependencies / Yarn install & cache
🔇 Additional comments (15)
apps/web/public/static/locales/en/common.json (1)
50-50: Key addition looks good and consistentThe new
disable_recording_download_email_for_guestskey follows existing naming conventions (snake_case) and sits logically next to related Cal Video toggles. No collisions or typos detected.apps/web/lib/video/[uid]/getServerSideProps.ts (1)
25-25: LGTM! Type definition correctly extended.The addition of
disableRecordingDownloadEmailForGueststo theCalVideoSettingstype is consistent with the other boolean properties and aligns with the broader feature implementation.apps/web/lib/daily-webhook/getBooking.ts (1)
26-26: LGTM! Necessary field addition for downstream functionality.Adding
calVideoSettings: trueto the selection ensures the newdisableRecordingDownloadEmailForGuestsflag is available for the email sending logic downstream. The change follows the established query pattern.packages/lib/server/repository/booking.ts (1)
400-400: LGTM! Consistent field addition to calVideoSettings selection.The addition of
disableRecordingDownloadEmailForGuests: truefollows the established pattern of other boolean fields in the calVideoSettings selection block and ensures the new field is available when booking data is retrieved for meeting pages.packages/prisma/schema.prisma (1)
79-79: LGTM! New field follows existing patterns.The new
disableRecordingDownloadEmailForGuestsfield is well-positioned alongside other recording-related settings and follows the existing naming convention and type pattern within theCalVideoSettingsmodel. The default value offalseensures backward compatibility.packages/features/eventtypes/lib/types.ts (1)
161-161: LGTM! Type definition is consistent and well-structured.The new optional property
disableRecordingDownloadEmailForGuestsis appropriately positioned within thecalVideoSettingsobject, follows the existing naming convention, and maintains consistency with the corresponding Prisma schema field.packages/lib/server/repository/eventTypeRepository.ts (1)
711-711: LGTM! Field addition follows established pattern.The new
disableRecordingDownloadEmailForGuestsfield is correctly added to the calVideoSettings selection, maintaining consistency with other recording-related settings.packages/trpc/server/routers/viewer/eventTypes/types.ts (1)
34-34: LGTM! Schema extension is properly implemented.The new field uses the appropriate
.nullish()modifier, maintaining consistency with other boolean settings in the calVideoSettingsSchema.packages/trpc/server/routers/viewer/eventTypes/create.handler.ts (1)
74-78: LGTM! Default values are properly handled.The implementation correctly uses nullish coalescing (
?? false) to provide sensible defaults. The conditional creation within the CalVideo location check is appropriate.Note: Ensure the transcription-related fields (
enableAutomaticTranscription,disableTranscriptionForGuests,disableTranscriptionForOrganizer) are fully utilized in the email logic, as the PR description suggests onlydisableRecordingDownloadEmailForGuestshas complete end-to-end integration.apps/web/app/api/recorded-daily-video/route.ts (1)
140-140: LGTM! Correctly integrates the new calVideoSettings parameter.The addition of
booking.eventType?.calVideoSettingsas the third parameter enables the email function to respect thedisableRecordingDownloadEmailForGuestssetting. The optional chaining handles potential null/undefined values appropriately.packages/lib/server/repository/calVideoSettings.ts (2)
47-47: Property addition follows established pattern correctly.The new
disableRecordingDownloadEmailForGuestsproperty is properly typed as optional boolean | null, consistent with other CalVideo settings in the interface.
60-61: Implementation correctly handles both update and create paths.The property is properly added to both the
updateandcreateobjects in the upsert operation, with appropriate default fallback tofalse. The implementation is consistent with existing boolean properties.Also applies to: 72-73
packages/features/eventtypes/components/Locations.tsx (1)
399-412: UI toggle implementation follows established patterns correctly.The new toggle for disabling recording download emails for guests is properly integrated using the Controller pattern, with consistent styling and form binding. The implementation matches other CalVideo settings toggles in structure and behavior.
packages/emails/daily-video-emails.ts (2)
35-39: Function signature correctly adds optional settings parameter.The addition of the optional
calVideoSettingsparameter maintains backward compatibility while providing the necessary configuration for conditional email sending behavior.
47-53: Conditional email logic correctly implements the feature requirement.The implementation properly checks the flag using optional chaining and only skips attendee emails when explicitly disabled, while maintaining backward compatibility and always sending organizer emails.
| disableRecordingDownloadEmailForGuests: z.boolean().optional().nullable(), | ||
| enableAutomaticTranscription: z.boolean().optional().nullable(), | ||
| disableTranscriptionForGuests: z.boolean().optional().nullable(), | ||
| disableTranscriptionForOrganizer: z.boolean().optional().nullable(), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider consistency in nullish field definitions and verify transcription field integration.
Two observations:
-
Pattern inconsistency: These fields use
.optional().nullable()while the same fields intypes.tsuse.nullish(). Consider using.nullish()consistently as it's more concise and the preferred Zod pattern. -
Incomplete integration concern: The AI summary suggests only
disableRecordingDownloadEmailForGuestsis fully integrated end-to-end. If the transcription fields (enableAutomaticTranscription,disableTranscriptionForGuests,disableTranscriptionForOrganizer) are not fully implemented, consider either completing their integration or documenting them as future work to avoid confusion.
- disableRecordingDownloadEmailForGuests: z.boolean().optional().nullable(),
- enableAutomaticTranscription: z.boolean().optional().nullable(),
- disableTranscriptionForGuests: z.boolean().optional().nullable(),
- disableTranscriptionForOrganizer: z.boolean().optional().nullable(),
+ disableRecordingDownloadEmailForGuests: z.boolean().nullish(),
+ enableAutomaticTranscription: z.boolean().nullish(),
+ disableTranscriptionForGuests: z.boolean().nullish(),
+ disableTranscriptionForOrganizer: z.boolean().nullish(),🏁 Script executed:
#!/bin/bash
echo "=== Checking .nullish() usage in packages/prisma schema files ==="
rg "z\.boolean\(\)\.nullish" -n packages/prisma
echo ""
echo "=== Scanning occurrences of the new fields across the repository ==="
for field in disableRecordingDownloadEmailForGuests enableAutomaticTranscription disableTranscriptionForGuests disableTranscriptionForOrganizer; do
echo ""
echo ">>> Occurrences of '$field':"
rg "$field" -n
doneLength of output: 15245
Ensure consistent use of .nullish() in Zod schema
All four new boolean fields in packages/prisma/zod/custom/eventtype.ts currently use .optional().nullable(). Replace these with the more concise and preferred .nullish() shorthand to align with other Zod type definitions in the codebase.
• File: packages/prisma/zod/custom/eventtype.ts
Lines 12–15:
- disableRecordingDownloadEmailForGuests: z.boolean().optional().nullable(),
- enableAutomaticTranscription: z.boolean().optional().nullable(),
- disableTranscriptionForGuests: z.boolean().optional().nullable(),
- disableTranscriptionForOrganizer: z.boolean().optional().nullable(),
+ disableRecordingDownloadEmailForGuests: z.boolean().nullish(),
+ enableAutomaticTranscription: z.boolean().nullish(),
+ disableTranscriptionForGuests: z.boolean().nullish(),
+ disableTranscriptionForOrganizer: z.boolean().nullish(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| disableRecordingDownloadEmailForGuests: z.boolean().optional().nullable(), | |
| enableAutomaticTranscription: z.boolean().optional().nullable(), | |
| disableTranscriptionForGuests: z.boolean().optional().nullable(), | |
| disableTranscriptionForOrganizer: z.boolean().optional().nullable(), | |
| disableRecordingDownloadEmailForGuests: z.boolean().nullish(), | |
| enableAutomaticTranscription: z.boolean().nullish(), | |
| disableTranscriptionForGuests: z.boolean().nullish(), | |
| disableTranscriptionForOrganizer: z.boolean().nullish(), |
🤖 Prompt for AI Agents
In packages/prisma/zod/custom/eventtype.ts at lines 12 to 15, replace the use of
.optional().nullable() on the four boolean fields with the more concise
.nullish() method to ensure consistency with other Zod schema definitions in the
codebase.
feat: add toggle to disable recording download email for guests
Fixes: https://linear.app/calcom/issue/CAL-4181/add-option-to-allow-disabling-recording-download-link-to-attendees
Summary
This PR adds a new toggle option "Disable recording download email for guests" to Cal Video settings in event types. When enabled, the system will skip sending recording download emails to attendees while still sending them to organizers.
Key Changes:
disableRecordingDownloadEmailForGuestsfield toCalVideoSettingsdatabase modelsendDailyVideoRecordingEmailsfunction to conditionally skip attendee emailsPattern: Follows the established pattern from PR #21755 for transcription settings.
Review & Testing Checklist for Human
Recommended Test Plan:
Diagram
%%{ init : { "theme" : "default" }}%% graph TD subgraph "Database Layer" Schema["packages/prisma/schema.prisma"]:::major-edit Migration["packages/prisma/migrations/..."]:::major-edit end subgraph "Repository Layer" CalVideoRepo["packages/lib/server/repository/calVideoSettings.ts"]:::major-edit EventTypeRepo["packages/lib/server/repository/eventType.ts"]:::minor-edit BookingRepo["packages/lib/server/repository/booking.ts"]:::minor-edit end subgraph "UI Layer" LocationsUI["packages/features/eventtypes/components/Locations.tsx"]:::major-edit i18n["apps/web/public/static/locales/en/common.json"]:::minor-edit end subgraph "Email System" EmailFunc["packages/emails/daily-video-emails.ts"]:::major-edit WebhookHandler["apps/web/app/api/recorded-daily-video/route.ts"]:::minor-edit end subgraph "Type System" TRPCTypes["packages/trpc/server/routers/viewer/eventTypes/types.ts"]:::minor-edit ZodSchemas["packages/prisma/zod/custom/eventtype.ts"]:::minor-edit EventTypeTypes["packages/features/eventtypes/lib/types.ts"]:::minor-edit end Schema --> CalVideoRepo CalVideoRepo --> LocationsUI LocationsUI --> EventTypeRepo EventTypeRepo --> WebhookHandler WebhookHandler --> EmailFunc subgraph Legend L1["Major Edit"]:::major-edit L2["Minor Edit"]:::minor-edit L3["Context/No Edit"]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
falsevalue to existing CalVideoSettings recordsThe implementation maintains backward compatibility and follows existing code patterns throughout the Cal.com codebase.