Skip to content

Commit cc1efe1

Browse files
fix: Conditional fetch cache (calcom#24816)
* Fix cache to fetch only when it's available * Non hierarchical feature check * Fix tests: Add missing mock method and comprehensive CalendarCacheWrapper tests - Add checkIfUserHasFeatureNonHierarchical to features.repository mock to fix failing GoogleCalendar tests - Add comprehensive unit tests for CalendarCacheWrapper covering: - Calendars with sync only (cache-only path) - Calendars without sync only (original calendar path) - Mixed calendars (both cache and original) - Timezone handling with UTC defaults - Edge cases (empty arrays, undefined methods, null ids) - Use proper types instead of 'as any' to satisfy lint rules Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * Fix: Sanitize logging to avoid exposing PII - Replace logging full selectedCalendars objects with only calendar IDs and count - Prevents exposure of email fields and other sensitive information in logs - Addresses AI code reviewer feedback Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * Apply suggestion from @volnei --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 7635d33 commit cc1efe1

5 files changed

Lines changed: 625 additions & 32 deletions

File tree

packages/app-store/_utils/getCalendar.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export const getCalendar = async (
5050
featuresRepository.checkIfFeatureIsEnabledGlobally(
5151
CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE
5252
),
53-
featuresRepository.checkIfUserHasFeature(
53+
featuresRepository.checkIfUserHasFeatureNonHierarchical(
5454
credential.userId as number,
5555
CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE
5656
),

packages/app-store/googlecalendar/lib/__mocks__/features.repository.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const featuresRepositoryModuleMock = {
44
FeaturesRepository: vi.fn().mockImplementation(() => ({
55
checkIfFeatureIsEnabledGlobally: vi.fn().mockResolvedValue(true),
66
checkIfUserHasFeature: vi.fn().mockResolvedValue(true),
7+
checkIfUserHasFeatureNonHierarchical: vi.fn().mockResolvedValue(true),
78
})),
89
};
910

packages/features/calendar-subscription/lib/cache/CalendarCacheWrapper.ts

Lines changed: 92 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -50,54 +50,115 @@ export class CalendarCacheWrapper implements Calendar {
5050
}
5151

5252
/**
53-
* Override this method to use cache
53+
* Retrieves availability combining cache and live sources.
5454
*
55-
* @param dateFrom
56-
* @param dateTo
57-
* @param selectedCalendars
58-
* @param shouldServeCache
55+
* - Calendars **with** both `syncToken` and `syncSubscribedAt` → fetched from cache.
56+
* - Calendars **without** one of them → fetched directly from the original calendar.
57+
* - Results are merged into a single array.
58+
*
59+
* @param dateFrom - Start date (ISO string)
60+
* @param dateTo - End date (ISO string)
61+
* @param selectedCalendars - List of calendars to retrieve availability from
62+
* @returns Combined array of busy date ranges
5963
*/
6064
async getAvailability(
6165
dateFrom: string,
6266
dateTo: string,
6367
selectedCalendars: IntegrationCalendar[]
64-
// _shouldServeCache?: boolean
65-
// _fallbackToPrimary?: boolean
6668
): Promise<EventBusyDate[]> {
67-
log.debug("getAvailability from cache", { dateFrom, dateTo, selectedCalendars });
68-
const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id));
69-
if (!selectedCalendarIds.length) {
70-
return Promise.resolve([]);
69+
log.debug("getAvailability (mixed cache + original)", {
70+
dateFrom,
71+
dateTo,
72+
calendarIds: selectedCalendars.map((c) => c.id),
73+
calendarCount: selectedCalendars.length,
74+
});
75+
76+
if (!selectedCalendars?.length) return [];
77+
78+
const withSync = selectedCalendars.filter((c) => c.syncToken && c.syncSubscribedAt);
79+
const withoutSync = selectedCalendars.filter((c) => !c.syncToken || !c.syncSubscribedAt);
80+
81+
const results: EventBusyDate[] = [];
82+
83+
// Fetch from cache for synced calendars
84+
if (withSync.length) {
85+
const ids = withSync.map((c) => c.id).filter((id): id is string => Boolean(id));
86+
const cached = await this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
87+
ids,
88+
new Date(dateFrom),
89+
new Date(dateTo)
90+
);
91+
results.push(...cached);
7192
}
72-
return this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
73-
selectedCalendarIds,
74-
new Date(dateFrom),
75-
new Date(dateTo)
76-
);
93+
94+
// Fetch from original calendar for unsynced ones
95+
if (withoutSync.length) {
96+
const original = await this.deps.originalCalendar.getAvailability(dateFrom, dateTo, withoutSync);
97+
results.push(...original);
98+
}
99+
100+
return results;
77101
}
78102

79103
/**
80-
* Override this method to use cache
104+
* Retrieves availability with time zones, combining cache and live data.
105+
*
106+
* - Calendars **with** both `syncToken` and `syncSubscribedAt` → fetched from cache.
107+
* - Calendars **without** one of them → fetched directly from the original calendar.
108+
* - Results are merged into a single array with `{ start, end, timeZone }` format.
81109
*
82-
* @param dateFrom
83-
* @param dateTo
84-
* @param selectedCalendars
85-
* @returns
110+
* @param dateFrom - Start date (ISO string)
111+
* @param dateTo - End date (ISO string)
112+
* @param selectedCalendars - List of calendars to retrieve availability from
113+
* @returns Combined array of time-zone-aware availability ranges
86114
*/
87-
async getAvailabilityWithTimeZones?(
115+
async getAvailabilityWithTimeZones(
88116
dateFrom: string,
89117
dateTo: string,
90118
selectedCalendars: IntegrationCalendar[]
91-
// _fallbackToPrimary?: boolean
92119
): Promise<{ start: Date | string; end: Date | string; timeZone: string }[]> {
93-
log.debug("getAvailabilityWithTimeZones from cache", { dateFrom, dateTo, selectedCalendars });
94-
const selectedCalendarIds = selectedCalendars.map((e) => e.id).filter((id): id is string => Boolean(id));
95-
const result = await this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
96-
selectedCalendarIds,
97-
new Date(dateFrom),
98-
new Date(dateTo)
99-
);
100-
return result.map(({ start, end, timeZone }) => ({ start, end, timeZone: timeZone || "UTC" }));
120+
log.debug("getAvailabilityWithTimeZones (mixed cache + original)", {
121+
dateFrom,
122+
dateTo,
123+
calendarIds: selectedCalendars.map((c) => c.id),
124+
calendarCount: selectedCalendars.length,
125+
});
126+
127+
if (!selectedCalendars?.length) return [];
128+
129+
const withSync = selectedCalendars.filter((c) => c.syncToken && c.syncSubscribedAt);
130+
const withoutSync = selectedCalendars.filter((c) => !c.syncToken || !c.syncSubscribedAt);
131+
132+
const results: { start: Date | string; end: Date | string; timeZone: string }[] = [];
133+
134+
// Fetch from cache for synced calendars
135+
if (withSync.length) {
136+
const ids = withSync.map((c) => c.id).filter((id): id is string => Boolean(id));
137+
const cached = await this.deps.calendarCacheEventRepository.findAllBySelectedCalendarIdsBetween(
138+
ids,
139+
new Date(dateFrom),
140+
new Date(dateTo)
141+
);
142+
results.push(
143+
...cached.map(({ start, end, timeZone }) => ({
144+
start,
145+
end,
146+
timeZone: timeZone || "UTC",
147+
}))
148+
);
149+
}
150+
151+
// Fetch from original calendar for unsynced ones
152+
if (withoutSync.length) {
153+
const original = await this.deps.originalCalendar.getAvailabilityWithTimeZones?.(
154+
dateFrom,
155+
dateTo,
156+
withoutSync
157+
);
158+
if (original?.length) results.push(...original);
159+
}
160+
161+
return results;
101162
}
102163

103164
fetchAvailabilityAndSetCache?(selectedCalendars: IntegrationCalendar[]): Promise<unknown> {

0 commit comments

Comments
 (0)