feat: add custom Google Calendar reminders for booked events#23638
feat: add custom Google Calendar reminders for booked events#23638ShashwatPS wants to merge 22 commits intocalcom:mainfrom
Conversation
|
@ShashwatPS is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds per-destination-calendar reminder support end-to-end: DB schema (customCalendarReminder Int default 10) and migration; UI component DestinationReminderSelector and integration into DestinationCalendarSettings with onReminderChange; web wrapper wiring to a new TRPC mutation setDestinationReminder; server-side schema and handler to persist reminders; CalendarService reads the stored reminder and sets explicit Google Calendar reminder overrides (popup and email) with a 10-minute fallback; tests updated accordingly. Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
📜 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)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings✨ 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. Comment |
github-actions
bot
added
$20
calendar-apps
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/app-store/googlecalendar/lib/CalendarService.ts (1)
378-395: Bug: updateEvent resets reminders to Google defaults, dropping the custom reminder.This overwrites the custom overrides set on create and can remove notifications after edits/reschedules.
Apply same reminder logic as createEvent:
async updateEvent(uid: string, event: CalendarServiceEvent, externalCalendarId: string): Promise<any> { - const payload: calendar_v3.Schema$Event = { + const defaultReminder = await this.getReminderDuration(); + const payload: calendar_v3.Schema$Event = { summary: event.title, description: event.calendarDescription, start: { dateTime: event.startTime, timeZone: event.organizer.timeZone, }, end: { dateTime: event.endTime, timeZone: event.organizer.timeZone, }, attendees: this.getAttendees({ event, hostExternalCalendarId: externalCalendarId }), - reminders: { - useDefault: true, - }, + reminders: defaultReminder + ? { + useDefault: false, + overrides: [ + { method: "popup", minutes: defaultReminder }, + { method: "email", minutes: defaultReminder }, + ], + } + : { useDefault: true }, guestsCanSeeOtherGuests: !!event.seatsPerTimeSlot ? event.seatsShowAttendees : true, };
🧹 Nitpick comments (13)
packages/prisma/schema.prisma (3)
303-304: Add bounds (and optional enum) for reminder minutes to prevent bad data.As-is, any Int is allowed. Either clamp in code or enforce at DB level (0–40320) and/or limit to the UI-supported set (10, 30, 60).
If you opt for DB-side bounds, add a follow-up migration:
-- 20250905172511_add_custom_calendar_reminder/migration.sql +ALTER TABLE "DestinationCalendar" + ADD CONSTRAINT "customCalendarReminder_range_chk" + CHECK ("customCalendarReminder" BETWEEN 0 AND 40320);
303-304: Consider storage type and indexing for the update pattern.
- Use SmallInt since values are tiny.
- If mutations filter by (credentialId, integration), a composite index will help.
Apply:
- customCalendarReminder Int @default(10) + customCalendarReminder Int @default(10) @db.SmallIntAnd in a new migration:
+CREATE INDEX "DestinationCalendar_credential_integration_idx" +ON "DestinationCalendar" ("credentialId","integration");
303-304: Revisit default semantics (do we want “always override” by default?).Defaulting to 10 guarantees overrides are sent (useDefault=false). If you want “respect Google defaults unless configured,” make this nullable and treat null/0 as “useDefault:true”.
Happy to draft the code changes if you choose this behavior.
packages/prisma/migrations/20250905172511_add_custom_calendar_reminder/migration.sql (1)
1-2: Migration is fine; consider adding a CHECK and the composite index mentioned.Add the range CHECK and (credentialId, integration) index if your handler updates by both keys.
Proposed additions:
ALTER TABLE "DestinationCalendar" ADD COLUMN "customCalendarReminder" INTEGER NOT NULL DEFAULT 10; +ALTER TABLE "DestinationCalendar" + ADD CONSTRAINT "customCalendarReminder_range_chk" + CHECK ("customCalendarReminder" BETWEEN 0 AND 40320); +CREATE INDEX "DestinationCalendar_credential_integration_idx" +ON "DestinationCalendar" ("credentialId","integration");packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts (1)
9-11: Export the schema constant for router reuse.Ensure the TRPC router uses this runtime schema (not just the inferred type).
Keep the type:
export type TSetDestinationCalendarReminderSchema = z.infer< - typeof ZUpdateDestinationCalendarReminderInputSchema + typeof ZUpdateDestinationCalendarReminderInputSchema >;Then in the router, set
.input(ZUpdateDestinationCalendarReminderInputSchema).packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts (1)
1613-1624: Configure reminders and add default-path test
- Allow configuring reminder methods instead of hardcoding both “popup” and “email” (dual notifications can be noisy).
- Add a test covering the
useDefault: truepath (no overrides provided) to verify default-reminder behavior.packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsPlatformWrapper.tsx (1)
57-58: Don’t pass null for optional callback props.Prefer omitting the prop; passing null forces consumers to type it as null | fn.
Apply:
- onReminderChange={null}packages/app-store/googlecalendar/lib/CalendarService.ts (1)
204-234: Unreachable fallback and duplication risk in reminders; consider centralizing.getReminderDuration always returns a positive number (10 on error), so the useDefault:true branch won’t run. Extract a small helper to build the reminders object and reuse it across create/update to avoid drift.
Example:
+ private buildReminders(minutes: number): calendar_v3.Schema$EventReminders { + return { + useDefault: false, + overrides: [ + { method: "popup", minutes }, + { method: "email", minutes }, + ], + }; + } … - reminders: defaultReminder - ? { - useDefault: false, - overrides: [ - { method: "popup", minutes: defaultReminder }, - { method: "email", minutes: defaultReminder }, - ], - } - : { useDefault: true }, + reminders: this.buildReminders(defaultReminder),packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsx (1)
18-25: Localize error toast and invalidate cache on success.Avoid raw strings; keep UX consistent and refresh server state after mutation.
const mutationReminder = trpc.viewer.calendars.setDestinationReminder.useMutation({ - onSuccess: () => { - showToast(t("save_changes"), "success"); - }, - onError(error) { - showToast(`Error updating reminder: ${error.message}`, "error"); - }, + onSuccess: () => { + utils.viewer.calendars.connectedCalendars.invalidate(); + showToast(t("save_changes"), "success"); + }, + onError() { + showToast(t("something_went_wrong"), "error"); + }, });packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (4)
58-68: Null-safe onChange; don’t silently coerce to 10 on clearIf the select can emit null, returning 10 is surprising. Guard and call the callback only when a value exists; also use optional chaining per the prop change.
- onChange={(event) => { - const reminderValue = event?.value || 10; - - onReminderChange({ - credentialId: destinationCalendar.credentialId, - integration: destinationCalendar.integration, - defaultReminder: reminderValue, - }); - - setSelectedOption(memoOptions.find((option) => option.value === reminderValue) || null); - }} + onChange={(event) => { + const reminderValue = event?.value; + if (reminderValue == null) return; // If clearing is allowed, ignore; else enforce selection. + onReminderChange?.({ + credentialId: destinationCalendar.credentialId, + integration: destinationCalendar.integration, + defaultReminder: reminderValue, + }); + setSelectedOption(memoOptions.find((option) => option.value === reminderValue) || null); + }}If the intended UX is “never clearable,” explicitly set isClearable={false} on Select (if supported) to prevent null emissions. Do you want this behavior?
37-46: Use nullish check for numeric reminder valueTruthiness fails for 0; prefer a nullish check to future-proof.
- useEffect(() => { - if (destinationCalendar && destinationCalendar.customCalendarReminder) { - const defaultOption = memoOptions.find( - (option) => option.value === destinationCalendar.customCalendarReminder - ); - setSelectedOption(defaultOption || null); - } else { - setSelectedOption(null); - } - }, [destinationCalendar, memoOptions]); + useEffect(() => { + const value = destinationCalendar?.customCalendarReminder; + if (value != null) { + setSelectedOption(memoOptions.find((option) => option.value === value) ?? null); + } else { + setSelectedOption(null); + } + }, [destinationCalendar, memoOptions]);
5-13: Reduce tight coupling to ConnectedDestinationCalendars shapeUsing ConnectedDestinationCalendars["destinationCalendar"] ties this atom to a backend return type. Consider a minimal local interface to stabilize the component API.
Example:
type DestinationCalendarLike = { credentialId: number; integration: string; customCalendarReminder?: number | null; };Then: destinationCalendar: DestinationCalendarLike;
48-55: Minor i18n consistencyYou already localize placeholder and title with t("reminder"). Ensure the corresponding translation key exists in all supported locales.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
packages/app-store/googlecalendar/lib/CalendarService.ts(3 hunks)packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts(1 hunks)packages/platform/atoms/destination-calendar/DestinationCalendar.tsx(3 hunks)packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx(1 hunks)packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsPlatformWrapper.tsx(1 hunks)packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsx(2 hunks)packages/prisma/migrations/20250905172511_add_custom_calendar_reminder/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)packages/trpc/server/routers/viewer/calendars/_router.tsx(1 hunks)packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts(1 hunks)packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.tspackages/app-store/googlecalendar/lib/__tests__/CalendarService.test.tspackages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.tspackages/app-store/googlecalendar/lib/CalendarService.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/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.tspackages/app-store/googlecalendar/lib/__tests__/CalendarService.test.tspackages/trpc/server/routers/viewer/calendars/_router.tsxpackages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.tspackages/app-store/googlecalendar/lib/CalendarService.tspackages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsxpackages/platform/atoms/destination-calendar/DestinationReminderSelector.tsxpackages/platform/atoms/destination-calendar/DestinationCalendar.tsxpackages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsPlatformWrapper.tsx
**/*.{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/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.tspackages/app-store/googlecalendar/lib/__tests__/CalendarService.test.tspackages/trpc/server/routers/viewer/calendars/_router.tsxpackages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.tspackages/app-store/googlecalendar/lib/CalendarService.tspackages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsxpackages/platform/atoms/destination-calendar/DestinationReminderSelector.tsxpackages/platform/atoms/destination-calendar/DestinationCalendar.tsxpackages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsPlatformWrapper.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/trpc/server/routers/viewer/calendars/_router.tsxpackages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsxpackages/platform/atoms/destination-calendar/DestinationReminderSelector.tsxpackages/platform/atoms/destination-calendar/DestinationCalendar.tsxpackages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsPlatformWrapper.tsx
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/googlecalendar/lib/CalendarService.ts
🧠 Learnings (2)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/app-store/googlecalendar/lib/CalendarService.ts
🧬 Code graph analysis (6)
packages/trpc/server/routers/viewer/calendars/_router.tsx (7)
packages/trpc/server/trpc.ts (1)
router(13-13)packages/trpc/server/routers/viewer/calendars/connectedCalendars.schema.ts (1)
ZConnectedCalendarsInputSchema(3-10)packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts (1)
connectedCalendarsHandler(15-42)packages/trpc/server/routers/viewer/calendars/setDestinationCalendar.schema.ts (1)
ZSetDestinationCalendarInputSchema(3-8)packages/trpc/server/routers/viewer/calendars/setDestinationCalendar.handler.ts (1)
setDestinationCalendarHandler(54-124)packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts (1)
deleteCacheHandler(13-33)packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts (1)
setDestinationReminderHandler(12-33)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts (2)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts (1)
TSetDestinationCalendarReminderSchema(9-11)packages/platform/libraries/index.ts (1)
TRPCError(66-66)
packages/app-store/googlecalendar/lib/CalendarService.ts (2)
scripts/prepare-local-for-delegation-credentials-testing.js (1)
prisma(7-7)apps/api/v2/src/lib/logger.bridge.ts (1)
error(77-79)
packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsx (1)
apps/web/app/_trpc/trpc.ts (1)
trpc(7-7)
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (1)
packages/lib/getConnectedDestinationCalendars.ts (1)
ConnectedDestinationCalendars(32-34)
packages/platform/atoms/destination-calendar/DestinationCalendar.tsx (2)
packages/platform/atoms/destination-calendar/DestinationCalendarSelector.tsx (1)
DestinationCalendarProps(15-24)packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (1)
DestinationReminderSelector(15-74)
⏰ 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: Linters / lint
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (6)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts (1)
3-7: Credential.key isn’t exposed—this mutation only returns a boolean success flag.packages/trpc/server/routers/viewer/calendars/_router.tsx (1)
19-23: LGTM: lazy-loading handler cache pattern is correct and consistent.The guarded dynamic imports avoid repeated module loads and keep cold start work minimal.
Also applies to: 29-33, 39-44, 55-60
packages/app-store/googlecalendar/lib/CalendarService.ts (1)
78-97: Good: minimal Prisma select + safe fallback.Only selecting the needed field aligns with our Prisma guidance; logging + default of 10 ensures safe behavior.
packages/platform/atoms/destination-calendar/wrappers/DestinationCalendarSettingsWebWrapper.tsx (1)
40-41: LGTM: wires onReminderChange to tRPC mutation.Hooking the selector to the new mutation is straightforward and idiomatic.
packages/platform/atoms/destination-calendar/DestinationCalendar.tsx (1)
47-52: LGTM: conditional render protects UI when reminder handler not provided.The guard avoids rendering the selector when unsupported.
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (1)
15-19: Named export looks goodComplies with the “prefer named exports” guideline.
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx
Show resolved
Hide resolved
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts
Outdated
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts
Outdated
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts
Show resolved
Hide resolved
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts
Outdated
Show resolved
Hide resolved
|
@ShashwatPS Thanks for the PR. Could you please fix the failing type check and address the coderabbit suggestions as well? Please also add tests for this feature. |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/lib/server/service/BookingWebhookFactory.ts (1)
64-66: Fix destinationCalendar shape mismatch (array vs single item)BaseWebhookPayload declares destinationCalendar as a single DestinationCalendar | null, but createBasePayload emits an array. Align the contract.
Apply:
-interface BaseWebhookPayload { +interface BaseWebhookPayload { @@ - destinationCalendar: DestinationCalendar | null; + destinationCalendar: DestinationCalendar[]; // always an array @@ - private getDestinationCalendar(params: BaseWebhookPayload) { - return params.destinationCalendar ? [params.destinationCalendar] : []; + private getDestinationCalendar(params: BaseWebhookPayload) { + return params.destinationCalendar; } @@ - destinationCalendar: this.getDestinationCalendar(params), + destinationCalendar: this.getDestinationCalendar(params),If external consumers rely on the array form already, instead update the type only:
- destinationCalendar: DestinationCalendar | null; + destinationCalendar: DestinationCalendar[];Also applies to: 95-112
♻️ Duplicate comments (1)
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (1)
22-29: Localize option labels and react to locale changesUse t() per guidelines and include it in deps.
- const memoOptions = useMemo( - () => [ - { label: "Remind 10 minutes before", value: 10 }, - { label: "Remind 30 minutes before", value: 30 }, - { label: "Remind 1 hour before", value: 60 }, - ], - [] - ); + const memoOptions = useMemo( + () => [ + { label: t("remind_minutes_before", { count: 10 }), value: 10 }, + { label: t("remind_minutes_before", { count: 30 }), value: 30 }, + { label: t("remind_hours_before", { count: 1 }), value: 60 }, + ], + [t] + );
🧹 Nitpick comments (3)
packages/lib/buildCalEventFromBooking.ts (1)
20-21: Add runtime guarantees or narrow the type for customCalendarReminderNow that this field is required on non-null DestinationCalendar, please ensure upstream selectors always fetch it. Optionally narrow to a literal union (10 | 30 | 60) to prevent accidental values.
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (2)
38-46: Avoid truthy check; allow 0 if ever introducedCheck for nullish instead of truthiness to prevent misfires if 0 becomes valid.
- if (destinationCalendar && destinationCalendar.customCalendarReminder) { + if (destinationCalendar && destinationCalendar.customCalendarReminder != null) {
61-62: Prefer nullish checks over falsy for IDs0 would short-circuit; use null/undefined checks.
- if (!destinationCalendar?.credentialId || !destinationCalendar?.integration || !onReminderChange) + if (destinationCalendar?.credentialId == null || !destinationCalendar?.integration || !onReminderChange) return;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
packages/app-store/googlecalendar/lib/__tests__/CalendarService.test.ts(3 hunks)packages/lib/buildCalEventFromBooking.ts(1 hunks)packages/lib/server/service/BookingWebhookFactory.ts(1 hunks)packages/platform/atoms/destination-calendar/DestinationCalendar.tsx(3 hunks)packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx(1 hunks)packages/trpc/server/routers/viewer/calendars/_router.tsx(1 hunks)packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/trpc/server/routers/viewer/calendars/setDestinationReminder.schema.ts
- packages/platform/atoms/destination-calendar/DestinationCalendar.tsx
- packages/trpc/server/routers/viewer/calendars/_router.tsx
- packages/app-store/googlecalendar/lib/tests/CalendarService.test.ts
🧰 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:
packages/lib/server/service/BookingWebhookFactory.tspackages/lib/buildCalEventFromBooking.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/server/service/BookingWebhookFactory.tspackages/platform/atoms/destination-calendar/DestinationReminderSelector.tsxpackages/lib/buildCalEventFromBooking.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/lib/server/service/BookingWebhookFactory.tspackages/platform/atoms/destination-calendar/DestinationReminderSelector.tsxpackages/lib/buildCalEventFromBooking.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx
🧠 Learnings (1)
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/lib/buildCalEventFromBooking.ts
🧬 Code graph analysis (1)
packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx (1)
packages/lib/getConnectedDestinationCalendars.ts (1)
ConnectedDestinationCalendars(32-34)
⏰ 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). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
packages/lib/server/service/BookingWebhookFactory.ts (1)
20-21: Type addition looks fine; confirm producers populate itSince customCalendarReminder is required here, verify all construction paths for DestinationCalendar include it after the Prisma migration.
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:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/public/static/locales/en/common.json(1 hunks)packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/platform/atoms/destination-calendar/DestinationReminderSelector.tsx
⏰ 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: Codacy Static Code Analysis
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts (3)
25-49: Trim heavy scenario setup to speed up unit tests.This handler test doesn’t need org/team or end-to-end booking scaffolding. Directly inserting the minimal
destinationCalendar+ credential rows would make the suite faster and less brittle.Also applies to: 54-83
114-120: Useselectin Prisma reads (even in tests).Limit fields to what you assert to keep with repo guidelines and avoid accidental sensitive loads.
Apply:
- const destinationCalendar = await prisma.destinationCalendar.findFirst({ + const destinationCalendar = await prisma.destinationCalendar.findFirst({ where: { credentialId: delegationCredential.id, integration: "google_calendar", }, + select: { customCalendarReminder: true }, });And similarly for the second query later in the file.
Also applies to: 186-191
200-224: Stabilize error assertion.Comparing against a new
TRPCErrorinstance is brittle. Match on shape or message.Apply:
- ).rejects.toThrow(new TRPCError({ code: "NOT_FOUND", message: "Selected calendar not found" })); + ).rejects.toMatchObject({ code: "NOT_FOUND", message: "Selected calendar not found" });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.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/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.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/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.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/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts (2)
apps/web/test/utils/bookingScenario/bookingScenario.ts (4)
getOrganizer(1520-1576)TestData(1239-1511)createBookingScenario(978-1009)getScenarioData(1578-1664)packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.ts (1)
setDestinationReminderHandler(15-36)
⏰ 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: Codacy Static Code Analysis
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (4)
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts (4)
1-1: Ensure the handler uses the same Prisma mock as the test.You import the Prisma mock directly. Confirm that
setDestinationReminderHandleralso resolves to this mock (likely viasetupAndTeardown()), otherwise the handler might use a different client.If needed, explicitly mock the module:
import { vi } from "vitest"; import prisma from "../../../../../../tests/libs/__mocks__/prisma"; vi.mock("@calcom/prisma", () => ({ default: prisma, prisma }));
23-24: Good: test harness and lifecycle setup.
setupAndTeardown()inclusion is correct and keeps DB state isolated.
85-93: Verify delegation credential field usage.You persist
delegationCredential.idintodestinationCalendar.credentialId. Confirm the schema expects delegation IDs incredentialId(vs. a dedicateddelegationCredentialId). If a distinct field exists, update both the test seed and handler filter accordingly.
102-109: Clarify scope: per-credential vs per-calendar update.The handler filters by
{ credentialId, integration }, which will update all destination calendars for that account. If the feature is truly per-destination-calendar, includeexternalIdin input and the handlerwhereclause, and add a test that proves only the targeted calendar is updated.Example test to lock semantics:
it("updates only the specified destination calendar (if per-calendar is intended)", async () => { // seed two destination calendars under same credential, different externalIds await prisma.destinationCalendar.createMany({ data: [ { integration: "google_calendar", externalId: "A@group.calendar.google.com", userId: organizer.id, credentialId: delegationCredential.id, customCalendarReminder: 10 }, { integration: "google_calendar", externalId: "B@group.calendar.google.com", userId: organizer.id, credentialId: delegationCredential.id, customCalendarReminder: 10 }, ], }); await setDestinationReminderHandler({ ctx, input: { credentialId: delegationCredential.id, integration: "google_calendar", externalId: "A@group.calendar.google.com", // add to schema if needed defaultReminder: 60, }, }); const [a, b] = await Promise.all([ prisma.destinationCalendar.findFirst({ where: { credentialId: delegationCredential.id, integration: "google_calendar", externalId: "A@group.calendar.google.com" }, select: { customCalendarReminder: true } }), prisma.destinationCalendar.findFirst({ where: { credentialId: delegationCredential.id, integration: "google_calendar", externalId: "B@group.calendar.google.com" }, select: { customCalendarReminder: true } }), ]); expect(a?.customCalendarReminder).toBe(60); expect(b?.customCalendarReminder).toBe(10); });
packages/trpc/server/routers/viewer/calendars/setDestinationReminder.handler.test.ts
Show resolved
Hide resolved
|
@kart1ka Fixed the type checks, went through the CodeRabbit suggestions and addressed them. Also added tests for the custom reminders in Google Calendar. Please have a look whenever you are free and suggest if anything needs to be changed. |
kart1ka
left a comment
kart1ka
left a comment
There was a problem hiding this comment.
Looks Good. Thanks for the PR.
|
This PR is being marked as stale due to inactivity. |
|
I would like to pick this up since the existing PR is stale. |
|
Issue CAL-4716 is fully fixed All issues are resolved feature is added. CAL-4716: Custom Google Calendar Notifications This PR lets users set custom reminder times for events booked through Cal.com and synced to Google Calendar. Instead of relying on Google Calendar’s default notifications, users can now choose 10, 30, or 60 minutes before the event to receive a notification. Key Changes Backend: Added customCalendarReminder to the DestinationCalendar model (default: 10 minutes) Created a new tRPC endpoint setDestinationReminder to save users’ reminder preferences Updated Google Calendar integration to apply custom reminders for both creating and updating events Both popup and email notifications now respect the selected reminder time Frontend: Added DestinationReminderSelector component with a dropdown for 10/30/60 minutes Integrated the selector into the calendar settings UI Shows a success toast when preferences are saved Testing: Unit tests added for the reminder handler Verified Google Calendar events now include correct custom reminders Tested error handling for invalid calendars How It Works User connects their Google Calendar Dropdown appears next to each calendar in settings User selects a preferred reminder time (10, 30, 60 minutes) Preference is saved in the database for that calendar All future bookings automatically include this custom reminder Rescheduled bookings preserve the chosen reminder |
Co-Authored-By: unknown <>
Co-Authored-By: unknown <>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 15 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/prisma/migrations/20250905172511_add_custom_calendar_reminder/migration.sql">
<violation number="1" location="packages/prisma/migrations/20250905172511_add_custom_calendar_reminder/migration.sql:2">
P2: Add a DB-level CHECK constraint for allowed reminder values (10/30/60) to prevent invalid data from being stored.</violation>
</file>
<file name="packages/app-store/googlecalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/googlecalendar/lib/CalendarService.ts:72">
P2: Reminder defaults always overridden: `|| 10` makes useDefault unreachable and coerces 0 to 10</violation>
<violation number="2" location="packages/app-store/googlecalendar/lib/CalendarService.ts:157">
P2: Custom reminders only applied on create; update/reschedule still force default reminders, causing inconsistent behavior</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,2 @@ | |||
| -- AlterTable | |||
| ALTER TABLE "DestinationCalendar" ADD COLUMN "customCalendarReminder" INTEGER NOT NULL DEFAULT 10; | |||
There was a problem hiding this comment.
P2: Add a DB-level CHECK constraint for allowed reminder values (10/30/60) to prevent invalid data from being stored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/prisma/migrations/20250905172511_add_custom_calendar_reminder/migration.sql, line 2:
<comment>Add a DB-level CHECK constraint for allowed reminder values (10/30/60) to prevent invalid data from being stored.</comment>
<file context>
@@ -0,0 +1,2 @@
+-- AlterTable
+ALTER TABLE "DestinationCalendar" ADD COLUMN "customCalendarReminder" INTEGER NOT NULL DEFAULT 10;
</file context>
| reminders: { | ||
| useDefault: true, | ||
| }, | ||
| reminders: defaultReminder |
There was a problem hiding this comment.
P2: Custom reminders only applied on create; update/reschedule still force default reminders, causing inconsistent behavior
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/googlecalendar/lib/CalendarService.ts, line 157:
<comment>Custom reminders only applied on create; update/reschedule still force default reminders, causing inconsistent behavior</comment>
<file context>
@@ -131,9 +154,23 @@ export default class GoogleCalendarService implements Calendar {
- reminders: {
- useDefault: true,
- },
+ reminders: defaultReminder
+ ? {
+ useDefault: false,
</file context>
| customCalendarReminder: true, | ||
| }, | ||
| }); | ||
| return calendarData?.customCalendarReminder || 10; |
There was a problem hiding this comment.
P2: Reminder defaults always overridden: || 10 makes useDefault unreachable and coerces 0 to 10
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/googlecalendar/lib/CalendarService.ts, line 72:
<comment>Reminder defaults always overridden: `|| 10` makes useDefault unreachable and coerces 0 to 10</comment>
<file context>
@@ -57,6 +58,27 @@ export default class GoogleCalendarService implements Calendar {
+ customCalendarReminder: true,
+ },
+ });
+ return calendarData?.customCalendarReminder || 10;
+ } catch (error) {
+ this.log.error(
</file context>
|
Thanks for your contribution! We ended up merging #26078 for this. |
6 participants
What does this PR do?
Video Demo (if applicable):
https://www.loom.com/share/35ad2beac0714a5d97a56901912c190d?sid=c1606560-2276-4513-8094-ddf5a9cfe7f8
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist