feat: add calendar cache status and actions (#22532)#1
feat: add calendar cache status and actions (#22532)#1akshayutture-augment wants to merge 1 commit into
Conversation
* feat: add calendar cache status dropdown - Add updatedAt field to CalendarCache schema with migration - Create tRPC cacheStatus endpoint for fetching cache timestamps - Add action dropdown to CalendarSwitch for Google Calendar entries - Display formatted last updated timestamp in dropdown - Add placeholder for cache deletion functionality - Include translation strings for dropdown content The dropdown only appears for Google Calendar integrations that have active cache entries and provides cache management options for future extensibility. Co-Authored-By: zomars@cal.com <zomars@me.com> * fix: resolve Prisma type incompatibilities in repository files - Remove problematic satisfies clause in selectedCalendar.ts - Add missing cacheStatus parameter to ConnectedCalendarList component - Fixes type errors that were preventing CI from passing Co-Authored-By: zomars@cal.com <zomars@me.com> * refactor: integrate cache status into connectedCalendars handler - Remove separate cacheStatus tRPC endpoint as requested - Return cache status as separate field in connectedCalendars response - Update UI components to use cache data from connectedCalendars - Fix Prisma type incompatibilities in repository files Co-Authored-By: zomars@cal.com <zomars@me.com> * fix: resolve Prisma type incompatibilities and fix data flow for cache status - Fix Prisma.SortOrder usage in membership.ts orderBy clauses - Remove problematic satisfies clause in selectedCalendar.ts - Fix TeamSelect type reference in team.ts - Update SelectedCalendarsSettingsWebWrapper to properly pass cacheStatus data flow Co-Authored-By: zomars@cal.com <zomars@me.com> * Discard changes to packages/lib/server/repository/membership.ts * Discard changes to packages/lib/server/repository/team.ts * fix: improve calendar cache dropdown with proper formatting and subscription logic - Fix timestamp HTML entity encoding with interpolation escapeValue: false - Only show dropdown for subscribed Google calendars (googleChannelId exists) - Hide delete option when no cache data exists - Include updatedAt and googleChannelId fields upstream in user repository - Update data flow to pass subscription status through components Co-Authored-By: zomars@cal.com <zomars@me.com> * feat: update SelectedCalendar.updatedAt when Google webhooks trigger cache refresh - Add updateManyByCredentialId method to SelectedCalendarRepository - Update fetchAvailabilityAndSetCache to refresh SelectedCalendar timestamps - Ensure webhook flow updates both CalendarCache and SelectedCalendar records - Maintain proper timestamp tracking for calendar cache operations Co-Authored-By: zomars@cal.com <zomars@me.com> * Add script to automate Tunnelmole webhook setup Introduces test-gcal-webhooks.sh to start Tunnelmole, extract the public URL, and update GOOGLE_WEBHOOK_URL in the .env file. Handles process management, rate limits, and ensures environment configuration for Google Calendar webhooks. * Update dev:cron script to use npx tsx Replaces 'ts-node' with 'npx tsx' in the dev:cron script for running cron-tester.ts, likely to improve compatibility or leverage tsx features. * Update cache status string and improve CalendarSwitch UI Renamed 'last_updated' to 'cache_last_updated' in locale file for clarity and updated CalendarSwitch to use the new string. Also added dark mode text color support for cache status display. * refactor: move cache management to credential-level dropdown with Remove App - Create CredentialActionsDropdown component consolidating cache and app removal actions - Add deleteCache tRPC mutation for credential-level cache deletion - Update connectedCalendars handler to include cacheUpdatedAt at credential level - Move dropdown from individual CalendarSwitch to credential level in SelectedCalendarsSettingsWebWrapper - Remove cache-related props from CalendarSwitch component - Add translation strings for cache management actions - Consolidate all credential-level actions (cache management + Remove App) in one dropdown Co-Authored-By: zomars@cal.com <zomars@me.com> * fix: remove duplicate translation keys in common.json - Remove duplicate cache-related keys at lines 51-56 - Keep properly positioned keys later in file - Addresses GitHub comment from zomars about duplicate keys Co-Authored-By: zomars@cal.com <zomars@me.com> * fix: rename translation key to cache_last_updated - Address GitHub comment from zomars - Rename 'last_updated' to 'cache_last_updated' for specificity - Update usage in CredentialActionsDropdown component Co-Authored-By: zomars@cal.com <zomars@me.com> * fix: remove duplicate last_updated translation key Co-Authored-By: zomars@cal.com <zomars@me.com> * fix: add confirmation dialog for cache deletion and use repository pattern - Add confirmation dialog for destructive cache deletion action - Replace direct Prisma calls with CalendarCacheRepository pattern - Add getCacheStatusByCredentialIds method to repository interface - Fix import paths for UI components - Address GitHub review comments from zomars Co-Authored-By: zomars@cal.com <zomars@me.com> * Update CredentialActionsDropdown.tsx * Update common.json * Update common.json * fix: remove nested div wrapper to resolve HTML structure error - Remove wrapping div around DisconnectIntegration component - Fixes nested <p> tag validation error preventing Remove App functionality - Maintains existing confirmation dialog patterns Co-Authored-By: zomars@cal.com <zomars@me.com> * Fix API handler response termination logic Removed unnecessary return values after setting status in the integrations API handler. This clarifies response handling and prevents returning the response object when not needed. Resolves "API handler should not return a value, received object". * fix: 400 is correct error code for computing slot for past booking (#22574) * fix * add test * chore: release v5.5.1 * Refactor credential disconnect to use confirmation dialog Replaces the DisconnectIntegration component with an inline confirmation dialog for removing app credentials. Adds disconnect mutation logic and updates UI to improve user experience and consistency. * Set default value for CalendarCache.updatedAt Added a default value of NOW() for the updatedAt column in the CalendarCache table to ensure existing and future rows have a valid timestamp. Updated the Prisma schema to reflect this change and provide compatibility for legacy data and raw inserts. --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Benny Joo <sldisek783@gmail.com> Co-authored-by: emrysal <me@alexvanandel.com>
|
@coderabbitai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis PR introduces calendar cache management features by adding cache status tracking to the CalendarCache model, implementing cache deletion via a new TRPC endpoint, creating a CredentialActionsDropdown React component for managing app credentials, and enriching calendar data with cache metadata across repository, handler, and frontend layers. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as CredentialActionsDropdown
participant API as TRPC deleteCache Mutation
participant Handler as deleteCacheHandler
participant DB as CalendarCacheRepository
rect rgb(200, 220, 255)
note over UI,DB: Cache Deletion Flow
UI->>UI: User confirms delete in dialog
UI->>API: mutation.mutate({ credentialId })
API->>Handler: deleteCacheHandler({ ctx, input })
Handler->>Handler: Verify credential ownership
Handler->>DB: deleteMany({ credentialId })
DB->>DB: Remove calendarCache records
DB-->>Handler: success
Handler-->>API: { success: true }
API->>UI: onSuccess callback
UI->>UI: Show success toast
UI->>UI: Invalidate cache queries
end
rect rgb(220, 255, 220)
note over UI,DB: Cache Status Enrichment
UI->>API: getConnectedCalendars()
API->>Handler: connectedCalendarsHandler()
Handler->>DB: getCacheStatusByCredentialIds([...])
DB->>DB: GROUP BY credentialId, MAX(updatedAt)
DB-->>Handler: [{ credentialId, updatedAt }]
Handler->>Handler: Map & enrich calendars with cacheUpdatedAt
Handler-->>API: connectedCalendars[]
API-->>UI: Display with cache timestamps
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
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)
990-1025:updateManyByCredentialIdis called with an empty data object – will fail at runtime
fetchAvailabilityAndSetCacheends with:// Update SelectedCalendar.updatedAt for all calendars under this credential await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {});With Prisma,
updateManyrequires at least one field indata;{}is invalid and will cause a runtime error when this path is hit. The repository method passesdatadirectly to Prisma without modification, so the empty object reaches Prisma and fails.Change the call to explicitly update
updatedAt:- await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, {}); + await SelectedCalendarRepository.updateManyByCredentialId(this.credential.id, { + updatedAt: new Date(), + });Location:
packages/app-store/googlecalendar/lib/CalendarService.ts:1024
🧹 Nitpick comments (9)
scripts/test-gcal-webhooks.sh (2)
41-41: Use convention for unused loop variable.The loop counter
iis not used in the loop body. Use the bash convention_for intentionally unused variables to silence the shellcheck warning (SC2034).Apply this diff:
- for i in {1..20}; do + for _ in {1..20}; do
12-12: InitializeOWNED_PIDat the start for clarity.The
OWNED_PIDvariable is checked in the cleanup function before being set elsewhere. While bash safely treats undefined variables as empty strings in conditional tests, explicitly initializing variables at the top improves readability and prevents subtle bugs.Apply this diff:
# Configuration LOG_FILE="/tmp/tmole.log" ENV_FILE="../.env" TM_PORT=3000 TM_KEYWORD="https://.*\.tunnelmole\.net" TMOLE_RUNNING=$(pgrep -f "tmole $TM_PORT") +OWNED_PID=falsepackages/trpc/server/routers/viewer/calendars/_router.tsx (1)
1-2: NewdeleteCacheTRPC route is wired correctly; consider updating handler-cache typeThe new route:
deleteCache: authedProcedure .input(z.object({ credentialId: z.number() })) .mutation(async ({ ctx, input }) => { const { deleteCacheHandler } = await import("./deleteCache.handler"); return deleteCacheHandler({ ctx, input }); }),follows the same pattern as the existing routes (authed procedure, inline zod schema, dynamic handler import) and looks correct.
Minor optional polish: if
CalendarsRouterHandlerCacheis actually used to cache handlers, you may want to add adeleteCache?entry there for consistency.Also applies to: 28-33
packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts (1)
27-37: Defensively filter credential IDs and skip DB call when noneThe enrichment logic looks sound, but two defensive tweaks would make it safer and a bit cheaper:
- If
connectedCalendarscan ever contain items without a validcredentialId(e.g.null/undefinedfor some integrations), consider filtering before passing to Prisma to avoid anIN [null]query:const credentialIds = connectedCalendars .map((cal) => cal.credentialId) .filter((id): id is number => id != null);
- If
credentialIds.length === 0, you can skip instantiatingCalendarCacheRepositoryand the DB call and just return the originalconnectedCalendarswithcacheUpdatedAt: null:if (credentialIds.length === 0) { return { connectedCalendars: connectedCalendars.map((c) => ({ ...c, cacheUpdatedAt: null })), destinationCalendar }; }Also applies to: 39-39
packages/features/calendar-cache/calendar-cache.repository.ts (1)
173-188: Handle empty credential lists and optionally de‑duplicate IDsImplementation is correct and matches the interface, but two small robustness/perf tweaks are worth considering:
- Short‑circuit when no IDs to avoid an unnecessary
groupBy:if (credentialIds.length === 0) { return []; }
- If
credentialIdscan contain duplicates, de‑duplicating before the query can reduce the size of theINlist with no behavioral change:const uniqueCredentialIds = [...new Set(credentialIds)]; const cacheStatuses = await prisma.calendarCache.groupBy({ by: ["credentialId"], where: { credentialId: { in: uniqueCredentialIds } }, _max: { updatedAt: true }, });packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts (1)
4-33: Use TRPC-native errors instead of genericErrorfor better API semanticsThe auth check and deletion logic look correct, but throwing a bare
Errorwill typically surface as a 500 instead of a typed TRPC error:if (!credential) { throw new Error("Credential not found or access denied"); }Consider using
TRPCError(or the project’s standard error helper) so callers can reliably differentiate “not found” vs “forbidden”:import { TRPCError } from "@trpc/server"; if (!credential) { throw new TRPCError({ code: "NOT_FOUND", // or "FORBIDDEN" depending on semantics message: "Credential not found or access denied", }); }This keeps error handling consistent across routers and avoids misclassifying expected failures as server errors.
packages/platform/atoms/selected-calendars/wrappers/SelectedCalendarsSettingsWebWrapper.tsx (1)
5-5: Verify that disconnect behavior should ignoreisDisabledscope flagWiring
CredentialActionsDropdownin both the normal and error states looks consistent, and passingcacheUpdatedAt+delegationCredentialIdgives the dropdown what it needs. NormalizingdelegationCredentialIdtonullforCalendarSwitchis also a nice touch.One subtle behavioral difference to double-check:
SelectedCalendarsSettingsWebWrappercomputes:const isDisabled = disabledScope ? disabledScope === scope : false; const shouldDisableConnectionModification = isDisabled || disableConnectionModification;…but
ConnectedCalendarListreceives onlydisableConnectionModification, and this is what gets passed toCredentialActionsDropdown.
- Inside the dropdown,
canDisconnect = !delegationCredentialId && !disableConnectionModification.So when
isDisabled === truebut the top-leveldisableConnectionModificationprop isfalse, calendar toggles are disabled viaisDisabled, yet the dropdown will still allow disconnection.If the intention is “no connection modifications when the scope is disabled”, you may want to pass
shouldDisableConnectionModificationdown instead:<ConnectedCalendarList // ... disableConnectionModification={shouldDisableConnectionModification} />Also applies to: 70-79, 98-98, 123-132
packages/features/apps/components/CredentialActionsDropdown.tsx (2)
42-65: Consider invalidating TRPC queries after cache deletion as wellThe mutation wiring looks good overall, and disconnect invalidates the relevant queries:
async onSettled() { await utils.viewer.calendars.connectedCalendars.invalidate(); await utils.viewer.apps.integrations.invalidate(); }For cache deletion, the component currently relies only on the optional
onSuccesscallback to refresh callers:const deleteCacheMutation = trpc.viewer.calendars.deleteCache.useMutation({ onSuccess: () => { showToast(t("cache_deleted_successfully"), "success"); onSuccess?.(); }, });To make the dropdown self-contained and ensure the cache status (
cacheUpdatedAt) always reflects the latest state, you could also invalidate the connected-calendars query here:const utils = trpc.useUtils(); const deleteCacheMutation = trpc.viewer.calendars.deleteCache.useMutation({ onSuccess: async () => { showToast(t("cache_deleted_successfully"), "success"); await utils.viewer.calendars.connectedCalendars.invalidate(); onSuccess?.(); }, onError: () => { showToast(t("error_deleting_cache"), "error"); }, });Optionally, you might also disable the confirm buttons while the respective mutation is in flight to prevent duplicate submissions:
<ConfirmationDialogContent // ... confirmBtnLoading={deleteCacheMutation.isPending} onConfirm={() => { deleteCacheMutation.mutate({ credentialId }); // dialog can be closed in onSuccess/onError instead of immediately }} />Also applies to: 130-151
67-73: Localize cache timestamp formatting instead of hard‑codingen-USThe visibility logic around
hasCache/canDisconnectlooks correct, and only Google Calendar connections expose cache actions, which matches the feature.For the cache status display:
{t("cache_last_updated", { timestamp: new Intl.DateTimeFormat("en-US", { dateStyle: "short", timeStyle: "short", }).format(new Date(cacheUpdatedAt)), interpolation: { escapeValue: false }, })}Two improvements:
- Instead of hard‑coding
"en-US", pull the active locale fromuseLocale()(many codepaths expose something likeconst { i18n } = useLocale();), so dates respect the user’s language/region settings.- You can precompute
lastUpdateddefensively:const lastUpdated = cacheUpdatedAt ? new Date(cacheUpdatedAt) : null;and only render the status block when
lastUpdatedis non‑null, avoiding any edge cases wherecacheUpdatedAtis unexpectedlynull/undefined.Example:
const { t, i18n } = useLocale(); const lastUpdated = cacheUpdatedAt ? new Date(cacheUpdatedAt) : null; // ... {lastUpdated && ( <DropdownMenuItem className="focus:ring-muted"> <div className="px-2 py-1"> <div className="text-sm font-medium text-gray-900 dark:text-white">{t("cache_status")}</div> <div className="text-xs text-gray-500 dark:text-white"> {t("cache_last_updated", { timestamp: new Intl.DateTimeFormat(i18n.language, { dateStyle: "short", timeStyle: "short", }).format(lastUpdated), interpolation: { escapeValue: false }, })} </div> </div> </DropdownMenuItem> )}Also applies to: 82-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
apps/web/package.json(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/app-store/googlecalendar/lib/CalendarService.ts(1 hunks)packages/features/apps/components/CredentialActionsDropdown.tsx(1 hunks)packages/features/calendar-cache/calendar-cache.repository.interface.ts(1 hunks)packages/features/calendar-cache/calendar-cache.repository.mock.ts(1 hunks)packages/features/calendar-cache/calendar-cache.repository.ts(1 hunks)packages/lib/getConnectedDestinationCalendars.ts(1 hunks)packages/lib/server/repository/selectedCalendar.ts(2 hunks)packages/lib/server/repository/user.ts(1 hunks)packages/platform/atoms/selected-calendars/wrappers/SelectedCalendarsSettingsWebWrapper.tsx(5 hunks)packages/prisma/migrations/20250715160635_add_calendar_cache_updated_at/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)packages/trpc/server/routers/viewer/calendars/_router.tsx(2 hunks)packages/trpc/server/routers/viewer/calendars/connectedCalendars.handler.ts(2 hunks)packages/trpc/server/routers/viewer/calendars/deleteCache.handler.ts(1 hunks)scripts/test-gcal-webhooks.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/test-gcal-webhooks.sh
[warning] 41-41: i appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (7)
scripts/test-gcal-webhooks.sh (1)
28-31: Add robustness check when reusing Tunnelmole log.When tmole is already running, the script extracts the URL from the existing log file (line 30). If the log file is missing or has been cleaned up,
TUNNEL_URLwill be empty and the validation at line 56 will trigger cleanup. Consider either ensuring the log file persists or providing a more explicit error message.Verify the expected behavior: Should the script fail gracefully if the log file is not found when reusing an existing tmole process? Consider adding a check:
if [ -n "$TMOLE_RUNNING" ]; then echo "Tunnelmole already running (PID: $TMOLE_RUNNING), reusing..." + if [ ! -f "$LOG_FILE" ]; then + echo "⚠️ Log file not found. Starting a fresh Tunnelmole session..." + TMOLE_RUNNING="" + fiThis way, if the log is missing, you fall through to the normal startup path instead of silently failing the URL extraction.
packages/lib/server/repository/user.ts (1)
894-901: Extending selectedCalendars shape for sessions looks consistentAdding
updatedAtandgoogleChannelIdto theselectedCalendarsselect aligns with theSelectedCalendarschema and broader cache-status feature usage; no behavioral impact beyond exposing more metadata towithSelectedCalendars.packages/prisma/migrations/20250715160635_add_calendar_cache_updated_at/migration.sql (1)
1-9: Migration forCalendarCache.updatedAtis sane and matches schemaAdding a non-null
updatedAt TIMESTAMP(3) NOT NULL DEFAULT NOW()safely backfills existing rows and aligns with the Prisma model’s@default(now()) @updatedAt. No behavioral issues spotted.packages/prisma/schema.prisma (1)
1718-1720: CalendarCacheupdatedAtfield is defined correctlyAdding
// Provide an initial value for legacy rows and future raw inserts updatedAt DateTime @default(now()) @updatedAtgives Prisma the right semantics for both new rows and subsequent updates, and matches the migration. This should work well for cache freshness tracking.
packages/lib/getConnectedDestinationCalendars.ts (1)
20-28: ExtendingUserWithCalendarswith cache-related fields is appropriateIncluding
updatedAtandgoogleChannelIdin bothallSelectedCalendarsanduserLevelSelectedCalendars:Pick<SelectedCalendar, "externalId" | "integration" | "eventTypeId" | "updatedAt" | "googleChannelId">[]matches the
SelectedCalendarschema and aligns this helper’s data shape with the cache/watch metadata needed elsewhere. As long as the loader supplyingUserWithCalendarsselects these fields (which other changes in the PR address), this looks good.packages/features/calendar-cache/calendar-cache.repository.interface.ts (1)
30-32: Interface addition matches repository behaviorThe
getCacheStatusByCredentialIdssignature aligns with the PrismagroupByimplementation and the connected-calendars usage (credentialId+ nullableupdatedAt). Looks good.apps/web/public/static/locales/en/common.json (1)
3382-3388: New cache-related translation keys are consistent and correctly parameterizedThe added strings (
cache_status,cache_last_updated,delete_cached_data,cache_deleted_successfully,error_deleting_cache,confirm_delete_cache,yes_delete_cache) align with the usages inCredentialActionsDropdown.tsx, and the{{timestamp}}placeholder incache_last_updatedmatches the interpolation at the call site. Looks good.
| "clean": "rm -rf .turbo && rm -rf node_modules && rm -rf .next", | ||
| "dev": "yarn copy-static && next dev --turbopack", | ||
| "dev:cron": "ts-node cron-tester.ts", | ||
| "dev:cron": "npx tsx cron-tester.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
Add tsx as a devDependency or clarify workspace availability.
The script switches from ts-node to npx tsx, but tsx is not listed in this package's devDependencies. Using npx tsx will attempt to fetch the package from npm if it's not available locally, which may fail in offline environments or CI systems with restricted network access.
Verify the following:
- Is
tsxavailable as a workspace dependency (e.g., in a rootpackage.jsonor in packages matching the monorepo pattern)? - Should
tsxbe added todevDependenciesalongside or instead ofts-node? - Is
ts-node(line 196) still used elsewhere in this package or should it be removed?
Run this script to verify:
🏁 Script executed:
#!/bin/bash
# Check if tsx is listed in any workspace package.json files
fd --type f "package.json" --exec grep -l '"tsx"' {} \;
# Check all usages of ts-node and tsx in package.json files
echo "=== ts-node usages ===" && fd --type f "package.json" --exec grep -n "ts-node" {} + | grep -v node_modules
echo "=== tsx usages ===" && fd --type f "package.json" --exec grep -n "tsx" {} + | grep -v node_modules
# Verify cron-tester.ts exists
fd --type f "cron-tester.ts"Length of output: 14685
Add tsx to devDependencies or replace npx tsx with ts-node.
The script change switches from ts-node to npx tsx, but tsx is not listed as a devDependency in apps/web/package.json or anywhere in the monorepo. Using npx tsx will attempt to fetch the package from npm on each execution, creating non-deterministic behavior and failing in offline or restricted network environments. Additionally, ts-node remains in devDependencies but is no longer used in this file (it is still used in lines 24–25 for other scripts).
Either:
- Add
tsxtodevDependenciesalongsidets-node, or - Use
ts-nodeconsistently for this script to maintain dependency clarity in the workspace
🤖 Prompt for AI Agents
In apps/web/package.json at line 11, the "dev:cron" script uses "npx tsx" but
the tsx package is not declared in devDependencies, causing nondeterministic
installs and failures offline; either add "tsx" to devDependencies (pick a
compatible version with the monorepo/tooling) so the script runs from the local
install instead of npx, or change the script back to use the already-declared
"ts-node" for consistency; update package.json accordingly and run the workspace
install to verify the change.
| async getCacheStatusByCredentialIds() { | ||
| log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Mock’s getCacheStatusByCredentialIds signature doesn’t match the interface
CalendarCacheRepositoryMock implements ICalendarCacheRepository, but defines:
async getCacheStatusByCredentialIds() {
log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`);
return [];
}The interface (per summary) expects a parameter and a specific result shape (e.g. (credentialIds: number[]) => Promise<{ credentialId: number; updatedAt: Date | null }[]>). This mismatch will cause a TypeScript implementation error.
You can fix it by aligning the signature and leveraging the interface’s return type:
- async getCacheStatusByCredentialIds() {
- log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`);
- return [];
- }
+ async getCacheStatusByCredentialIds(
+ _credentialIds: number[],
+ ): ReturnType<ICalendarCacheRepository["getCacheStatusByCredentialIds"]> {
+ log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`);
+ return [];
+ }🌐 Web query:
In TypeScript, when a class `implements` an interface that declares `getCacheStatusByCredentialIds(credentialIds: number[]): Promise<...>`, is it valid for the class method to omit the parameter, or must the signature include it?
💡 Result:
Short answer: the class method must be compatible with the interface signature — you should declare the parameter (with a compatible type). Implementing an interface does not automatically copy parameter types into your class, and omitting the parameter or leaving it untyped can produce errors or implicit any. See the TypeScript handbook and discussion. [1][2]
Sources:
[1] TypeScript Handbook — Implementing an interface.
[2] Stack Overflow — "Implementing a Typescript class interface does not preserve method types."
🏁 Script executed:
#!/bin/bash
# Find and examine the ICalendarCacheRepository interface definition
fd -e ts -e tsx | xargs rg -l "ICalendarCacheRepository" | head -5Length of output: 251
🏁 Script executed:
#!/bin/bash
# Search for the actual interface definition
rg -nA 10 "interface ICalendarCacheRepository" --type tsLength of output: 1217
🏁 Script executed:
#!/bin/bash
# Check the mock file to see if there are any TypeScript comments indicating errors
cat packages/features/calendar-cache/calendar-cache.repository.mock.tsLength of output: 1058
🏁 Script executed:
#!/bin/bash
# Get full interface definition including getCacheStatusByCredentialIds
rg -nA 30 "interface ICalendarCacheRepository" packages/features/calendar-cache/calendar-cache.repository.interface.tsLength of output: 896
🏁 Script executed:
#!/bin/bash
# Search specifically for getCacheStatusByCredentialIds in interface
rg -nB 2 -A 5 "getCacheStatusByCredentialIds" packages/features/calendar-cache/calendar-cache.repository.interface.tsLength of output: 207
Mock's getCacheStatusByCredentialIds signature doesn't match the interface
CalendarCacheRepositoryMock implements ICalendarCacheRepository, but the mock method at lines 27-30 omits the required parameter:
Interface requires:
getCacheStatusByCredentialIds(credentialIds: number[]): Promise<{ credentialId: number; updatedAt: Date | null }[]>Mock defines:
async getCacheStatusByCredentialIds() {
log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`);
return [];
}TypeScript will error: the class method must declare the parameter to satisfy the interface contract. Add the parameter:
- async getCacheStatusByCredentialIds() {
+ async getCacheStatusByCredentialIds(_credentialIds: number[]) {
log.info(`Skipping getCacheStatusByCredentialIds due to calendar-cache being disabled`);
return [];
}Note: This issue affects all mock methods in the class—each omits its interface parameters.
🤖 Prompt for AI Agents
In packages/features/calendar-cache/calendar-cache.repository.mock.ts around
lines 27 to 30, the mock method getCacheStatusByCredentialIds (and other mock
methods in this class) omit the parameters required by ICalendarCacheRepository;
update each mock method signature to accept the same parameters as the interface
(e.g., getCacheStatusByCredentialIds(credentialIds: number[])) and keep the
current behavior (log and return []/appropriate stub) — if parameters are
unused, name them with a leading underscore (e.g., _credentialIds) to satisfy
TypeScript and linter rules, and ensure return types match the interface
promises.
| static async findMany({ where, select, orderBy }: FindManyArgs) { | ||
| const args = { where, select, orderBy } satisfies Prisma.SelectedCalendarFindManyArgs; | ||
| return await prisma.selectedCalendar.findMany(args); | ||
| return await prisma.selectedCalendar.findMany({ where, select, orderBy }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
updateManyByCredentialId helper is fine, but callers must not pass {}
The refactor of findMany to directly call prisma.selectedCalendar.findMany({ where, select, orderBy }) is a nice simplification.
For updateManyByCredentialId:
static async updateManyByCredentialId(credentialId: number, data: Prisma.SelectedCalendarUpdateInput) {
return await prisma.selectedCalendar.updateMany({
where: { credentialId },
data,
});
}This is a reasonable wrapper, but it assumes callers always provide at least one field in data. The current usage in GoogleCalendarService.fetchAvailabilityAndSetCache passes {}, which will cause Prisma to throw.
Two small follow-ups:
- Ensure all call sites pass a non-empty
dataobject (e.g.,{ updatedAt: new Date() }). - Optionally tighten the type to match
updateManymore closely:
-static async updateManyByCredentialId(credentialId: number, data: Prisma.SelectedCalendarUpdateInput) {
+static async updateManyByCredentialId(
+ credentialId: number,
+ data: Prisma.SelectedCalendarUpdateManyMutationInput,
+) {Also applies to: 400-405
🌐 Web query:
What is the correct `data` type and requirements for `prisma.<model>.updateMany` in Prisma Client (e.g. does it accept `data: {}` and which TypeScript type should be used)?
💡 Result:
Short answer
- updateMany requires a non‑empty data argument typed as XOR<ModelUpdateManyMutationInput, ModelUncheckedUpdateManyInput> (exposed via Prisma's generated types). [1][2]
Details & example
- Signature (docs): data: XOR<ModelUpdateManyMutationInput, ModelUncheckedUpdateManyInput> — and data is required. [1][2]
- Practically: you must supply at least one updatable field (even though individual fields on the input type are optional). Passing an empty {} is not meaningful and will fail (you must specify which columns to change). [1][2]
TypeScript example
- Use the generated types or the args type:
- prisma.user.updateMany({ where: {...}, data: { role: 'ADMIN' } })
- The data param is of the generated union type (UserUpdateManyMutationInput | UserUncheckedUpdateManyInput via XOR). [1]
Sources
- Prisma Client API — updateMany (prisma docs). [1]
- Prisma CRUD guide — updateMany examples. [2]
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the GoogleCalendarService call to updateManyByCredentialId
cd /repo && cat packages/lib/server/services/googleCalendarService.ts | sed -n '400,410p'
Length of output: 55
🏁 Script executed:
#!/bin/bash
# Also search for all calls to updateManyByCredentialId to see usage patterns
cd /repo && rg -n 'updateManyByCredentialId' --type=ts -A2 -B1Length of output: 55
🏁 Script executed:
#!/bin/bash
# Check the Prisma schema or generated types for SelectedCalendar to confirm correct type names
cd /repo && rg -n 'SelectedCalendarUpdateManyMutationInput|SelectedCalendarUpdateInput' --type=ts | head -20Length of output: 55
🏁 Script executed:
#!/bin/bash
# Look at the helper definition again to see the exact type used
cd /repo && sed -n '255,270p' packages/lib/server/repository/selectedCalendar.tsLength of output: 55
---
🏁 Script executed:
```shell
#!/bin/bash
# Inspect the GoogleCalendarService call to updateManyByCredentialId
sed -n '400,410p' packages/lib/server/services/googleCalendarService.ts
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Search for all calls to updateManyByCredentialId to see usage patterns
rg -n 'updateManyByCredentialId' --type=ts -A2 -B1Length of output: 835
🏁 Script executed:
#!/bin/bash
# Look at the helper definition to see the exact type used
sed -n '255,270p' packages/lib/server/repository/selectedCalendar.tsLength of output: 474
🏁 Script executed:
#!/bin/bash
# Check for Prisma types in generated or referenced files
rg -n 'SelectedCalendarUpdateManyMutationInput|SelectedCalendarUpdateInput' --type=ts | head -20Length of output: 297
Fix updateManyByCredentialId call and type signature—passes empty data object
Verification confirms both concerns:
-
Runtime error: The call at
packages/app-store/googlecalendar/lib/CalendarService.ts:1024passes{}toupdateManyByCredentialId, but Prisma'supdateManyrequires at least one field indataand will throw. -
Incorrect type: The helper uses
Prisma.SelectedCalendarUpdateInput(for single-record updates), butupdateManyrequires the union typePrisma.SelectedCalendarUpdateManyMutationInput | Prisma.SelectedCalendarUpdateManyInput.
Required fixes:
- Line 1024 in
packages/app-store/googlecalendar/lib/CalendarService.ts: Change{}to at least one field, e.g.,{ updatedAt: new Date() }. - Line 400 in
packages/lib/server/repository/selectedCalendar.ts: Update the type parameter:data: Prisma.SelectedCalendarUpdateManyMutationInput
🤖 Prompt for AI Agents
In packages/lib/server/repository/selectedCalendar.ts around line 400 and
packages/app-store/googlecalendar/lib/CalendarService.ts around line 1024, the
updateMany helper uses the wrong Prisma type and callers pass an empty data
object which causes Prisma to throw; change the helper's data parameter type to
Prisma.SelectedCalendarUpdateManyMutationInput (the correct type for updateMany)
and update the call at CalendarService.ts:1024 to supply at least one field (for
example set updatedAt to new Date()) instead of passing an empty object so
Prisma.updateMany receives a non-empty data payload.
| fi | ||
|
|
||
| if grep -q '^GOOGLE_WEBHOOK_URL=' "$ENV_FILE"; then | ||
| sed -i '' -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" |
There was a problem hiding this comment.
Use portable sed syntax for cross-platform compatibility.
Line 68 uses sed -i '' which is macOS/BSD-specific. On Linux systems with GNU sed, this will fail with an error. Use a portable pattern instead.
Apply this diff for cross-platform compatibility:
- sed -i '' -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE"
+ sed -i.bak -E "s|^GOOGLE_WEBHOOK_URL=.*|GOOGLE_WEBHOOK_URL=$TUNNEL_URL|" "$ENV_FILE" && rm -f "$ENV_FILE.bak"Alternatively, if you want to avoid creating a backup file:
# Using printf + temp file (fully portable)
{
while IFS= read -r line; do
if [[ $line =~ ^GOOGLE_WEBHOOK_URL= ]]; then
echo "GOOGLE_WEBHOOK_URL=$TUNNEL_URL"
else
echo "$line"
fi
done < "$ENV_FILE"
} > "$ENV_FILE.tmp" && mv "$ENV_FILE.tmp" "$ENV_FILE"🤖 Prompt for AI Agents
scripts/test-gcal-webhooks.sh around line 68: the current sed invocation uses
macOS/BSD-specific in-place syntax ("sed -i ''") which fails on GNU sed; replace
it with a portable approach that writes changes to a temporary file and then
atomically moves it over the original: read the env file line-by-line, if a line
starts with "GOOGLE_WEBHOOK_URL=" output "GOOGLE_WEBHOOK_URL=$TUNNEL_URL"
otherwise output the original line, direct the output to "$ENV_FILE.tmp" and
then mv "$ENV_FILE.tmp" "$ENV_FILE".
Test 1
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Replicated from ai-code-review-evaluation/cal.com-coderabbit#11