-
Notifications
You must be signed in to change notification settings - Fork 12k
feat: disable recording emails for guests #22457
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
Changes from all commits
10f06f4
88b0468
0e8181a
131d1d3
bfbd879
40bf312
04186b5
3693038
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -423,6 +423,21 @@ const Locations: React.FC<LocationsProps> = ({ | |
| /> | ||
| )} | ||
|
|
||
| <Controller | ||
| name="calVideoSettings.disableRecordingEmailsForGuests" | ||
| defaultValue={!!eventType.calVideoSettings?.disableRecordingEmailsForGuests} | ||
| render={({ field: { onChange, value } }) => { | ||
| return ( | ||
| <SettingsToggle | ||
| title={t("disable_recording_emails_for_guests")} | ||
| labelClassName="text-sm leading-6 whitespace-normal break-words" | ||
| checked={value} | ||
| onCheckedChange={onChange} | ||
| /> | ||
| ); | ||
| }} | ||
| /> | ||
|
Comment on lines
+426
to
+439
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good implementation pattern but fix TypeScript error. The new toggle follows the established pattern of other CalVideo settings toggles and correctly uses However, the pipeline failure indicates a TypeScript error where Ensure that the 🧰 Tools🪛 GitHub Actions: PR Update[error] 428-428: TypeScript error TS2551: Property 'disableRecordingEmailsForGuests' does not exist on type of 'calVideoSettings'. Did you mean 'disableRecordingForGuests'? 🤖 Prompt for AI Agents |
||
|
|
||
| <TextField | ||
| label={t("enter_redirect_url_on_exit_description")} | ||
| defaultValue={eventType.calVideoSettings?.redirectUrlOnExit || ""} | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1 @@ | ||||||
| ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingEmailsForGuests" BOOLEAN NOT NULL DEFAULT false; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider using nullable Boolean field following team preferences. Based on Cal.com's established schema design patterns, the team prefers nullable Boolean fields ( Consider changing the migration to: -ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingEmailsForGuests" BOOLEAN NOT NULL DEFAULT false;
+ALTER TABLE "CalVideoSettings" ADD COLUMN "disableRecordingEmailsForGuests" BOOLEAN DEFAULT false;This approach aligns with the team's preference and avoids the cost of updating all existing rows during migration. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -78,6 +78,7 @@ model CalVideoSettings { | |||||
| redirectUrlOnExit String? | ||||||
| disableTranscriptionForGuests Boolean @default(false) | ||||||
| disableTranscriptionForOrganizer Boolean @default(false) | ||||||
| disableRecordingEmailsForGuests Boolean @default(false) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Follow team preference for nullable Boolean fields. The new field uses a non-nullable Boolean, but Cal.com's established schema design pattern prefers nullable Boolean fields with defaults to avoid expensive database migrations. Consider changing to: - disableRecordingEmailsForGuests Boolean @default(false)
+ disableRecordingEmailsForGuests Boolean? @default(false)This approach aligns with the team's preference and avoids costly row updates during migration. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| createdAt DateTime @default(now()) | ||||||
| updatedAt DateTime @updatedAt | ||||||
|
|
||||||
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.
Fix TypeScript error: Handle null eventTypeId.
The pipeline failure indicates that
booking.eventTypeIdcan benull, but the Prisma query expects anumber. This will cause a runtime error.Apply this fix to handle the null case:
📝 Committable suggestion
🧰 Tools
🪛 GitHub Actions: PR Update
[error] 142-142: TypeScript error TS2322: Type 'number | null' is not assignable to type 'number | undefined'. Type 'null' is not assignable to type 'number | undefined' for property 'eventTypeId'.
🤖 Prompt for AI Agents