Skip to content

Commit 16cb9a3

Browse files
refactor: join teams with feature enabled directly in findNextSubscriptionBatch query (calcom#27364)
* refactor: join teams with feature enabled directly in findNextSubscriptionBatch query - Changed findNextSubscriptionBatch to accept featureId instead of teamIds - Join TeamFeatures table directly in the query instead of separate queries - Updated CalendarSubscriptionService.checkForNewSubscriptions to pass featureId - Removed separate getTeamsWithFeatureEnabled call - Updated tests to reflect the new query structure Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> * refactor: change featureId to featureIds array for flexibility - Changed findNextSubscriptionBatch to accept featureIds array instead of single featureId - Updated Prisma query to use 'in' operator for multiple feature IDs - Updated CalendarSubscriptionService to pass array with single feature - Updated all tests to use featureIds array Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 844380e commit 16cb9a3

5 files changed

Lines changed: 47 additions & 116 deletions

File tree

packages/features/calendar-subscription/lib/CalendarSubscriptionService.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -440,14 +440,10 @@ export class CalendarSubscriptionService {
440440
*/
441441
// biome-ignore lint/nursery/useExplicitType: return type is void
442442
async checkForNewSubscriptions() {
443-
const teamIds = await this.deps.featuresRepository.getTeamsWithFeatureEnabled(
444-
CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE
445-
);
446-
447443
const rows = await this.deps.selectedCalendarRepository.findNextSubscriptionBatch({
448444
take: 100,
449445
integrations: this.deps.adapterFactory.getProviders(),
450-
teamIds,
446+
featureIds: [CalendarSubscriptionService.CALENDAR_SUBSCRIPTION_CACHE_FEATURE],
451447
genericCalendarSuffixes: this.deps.adapterFactory.getGenericCalendarSuffixes(),
452448
});
453449
log.debug("checkForNewSubscriptions", { count: rows.length });

packages/features/calendar-subscription/lib/__tests__/CalendarSubscriptionService.test.ts

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -422,13 +422,10 @@ describe("CalendarSubscriptionService", () => {
422422

423423
await service.checkForNewSubscriptions();
424424

425-
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
426-
"calendar-subscription-cache"
427-
);
428425
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
429426
take: 100,
430427
integrations: ["google_calendar", "office365_calendar"],
431-
teamIds: [1, 2, 3],
428+
featureIds: ["calendar-subscription-cache"],
432429
genericCalendarSuffixes: [
433430
"@group.v.calendar.google.com",
434431
"@group.calendar.google.com",
@@ -439,12 +436,10 @@ describe("CalendarSubscriptionService", () => {
439436
expect(subscribeSpy).toHaveBeenCalledWith(mockSelectedCalendar.id);
440437
});
441438

442-
test("should handle mixed cache scenario where some teams have cache enabled and some do not", async () => {
439+
test("should handle multiple calendars returned from batch", async () => {
443440
const calendarWithCache = { ...mockSelectedCalendar, id: "calendar-with-cache", userId: 1 };
444441
const calendarWithCache2 = { ...mockSelectedCalendar, id: "calendar-with-cache-2", userId: 2 };
445442

446-
mockFeaturesRepository.getTeamsWithFeatureEnabled.mockResolvedValue([10, 20]);
447-
448443
mockSelectedCalendarRepository.findNextSubscriptionBatch.mockResolvedValue([
449444
calendarWithCache,
450445
calendarWithCache2,
@@ -454,13 +449,10 @@ describe("CalendarSubscriptionService", () => {
454449

455450
await service.checkForNewSubscriptions();
456451

457-
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
458-
"calendar-subscription-cache"
459-
);
460452
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
461453
take: 100,
462454
integrations: ["google_calendar", "office365_calendar"],
463-
teamIds: [10, 20],
455+
featureIds: ["calendar-subscription-cache"],
464456
genericCalendarSuffixes: [
465457
"@group.v.calendar.google.com",
466458
"@group.calendar.google.com",
@@ -473,58 +465,17 @@ describe("CalendarSubscriptionService", () => {
473465
expect(subscribeSpy).toHaveBeenCalledWith("calendar-with-cache-2");
474466
});
475467

476-
test("should only fetch calendars for teams with feature enabled, not entire organization hierarchy", async () => {
477-
const teamId = 100;
478-
const parentOrgId = 1;
479-
480-
mockFeaturesRepository.getTeamsWithFeatureEnabled.mockResolvedValue([teamId]);
481-
482-
const calendarForTeamMember = { ...mockSelectedCalendar, id: "team-member-calendar", userId: 5 };
483-
mockSelectedCalendarRepository.findNextSubscriptionBatch.mockResolvedValue([calendarForTeamMember]);
484-
485-
const subscribeSpy = vi.spyOn(service, "subscribe").mockResolvedValue(undefined);
486-
487-
await service.checkForNewSubscriptions();
488-
489-
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
490-
"calendar-subscription-cache"
491-
);
492-
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
493-
take: 100,
494-
integrations: ["google_calendar", "office365_calendar"],
495-
teamIds: [teamId],
496-
genericCalendarSuffixes: [
497-
"@group.v.calendar.google.com",
498-
"@group.calendar.google.com",
499-
"@import.calendar.google.com",
500-
"@resource.calendar.google.com",
501-
],
502-
});
503-
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).not.toHaveBeenCalledWith(
504-
expect.objectContaining({
505-
teamIds: expect.arrayContaining([parentOrgId]),
506-
})
507-
);
508-
expect(subscribeSpy).toHaveBeenCalledTimes(1);
509-
expect(subscribeSpy).toHaveBeenCalledWith("team-member-calendar");
510-
});
511-
512-
test("should not process any calendars when no teams have the feature enabled", async () => {
513-
mockFeaturesRepository.getTeamsWithFeatureEnabled.mockResolvedValue([]);
514-
468+
test("should not process any calendars when no calendars are returned", async () => {
515469
mockSelectedCalendarRepository.findNextSubscriptionBatch.mockResolvedValue([]);
516470

517471
const subscribeSpy = vi.spyOn(service, "subscribe").mockResolvedValue(undefined);
518472

519473
await service.checkForNewSubscriptions();
520474

521-
expect(mockFeaturesRepository.getTeamsWithFeatureEnabled).toHaveBeenCalledWith(
522-
"calendar-subscription-cache"
523-
);
524475
expect(mockSelectedCalendarRepository.findNextSubscriptionBatch).toHaveBeenCalledWith({
525476
take: 100,
526477
integrations: ["google_calendar", "office365_calendar"],
527-
teamIds: [],
478+
featureIds: ["calendar-subscription-cache"],
528479
genericCalendarSuffixes: [
529480
"@group.v.calendar.google.com",
530481
"@group.calendar.google.com",

packages/features/selectedCalendar/repositories/SelectedCalendarRepository.interface.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,21 @@ export interface ISelectedCalendarRepository {
2020
* Will check if syncSubscribedAt is null or channelExpiration is greater than current date
2121
* Calendars with recent subscription errors (last 24h) are skipped
2222
* Calendars with 3+ subscription errors are skipped entirely
23+
* Joins with TeamFeatures to filter users belonging to teams with any of the specified features enabled
2324
*
2425
* @param take the number of calendars to take
26+
* @param featureIds the feature IDs to filter teams by (users in teams with any of these features enabled)
2527
* @param integrations the list of integrations
2628
* @param genericCalendarSuffixes the list of generic calendar suffixes to exclude
2729
*/
2830
findNextSubscriptionBatch({
2931
take,
30-
teamIds,
32+
featureIds,
3133
integrations,
3234
genericCalendarSuffixes,
3335
}: {
3436
take: number;
35-
teamIds: number[];
37+
featureIds: string[];
3638
integrations: string[];
3739
genericCalendarSuffixes?: string[];
3840
}): Promise<SelectedCalendar[]>;

packages/features/selectedCalendar/repositories/SelectedCalendarRepository.test.ts

Lines changed: 27 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ describe("SelectedCalendarRepository", () => {
117117

118118
const result = await repository.findNextSubscriptionBatch({
119119
take: 10,
120-
teamIds: [1, 2, 3],
120+
featureIds: ["calendar-subscription-cache"],
121121
integrations: ["google_calendar", "office365_calendar"],
122122
});
123123

@@ -127,8 +127,15 @@ describe("SelectedCalendarRepository", () => {
127127
user: {
128128
teams: {
129129
some: {
130-
teamId: { in: [1, 2, 3] },
131130
accepted: true,
131+
team: {
132+
features: {
133+
some: {
134+
featureId: { in: ["calendar-subscription-cache"] },
135+
enabled: true,
136+
},
137+
},
138+
},
132139
},
133140
},
134141
},
@@ -163,7 +170,7 @@ describe("SelectedCalendarRepository", () => {
163170

164171
const result = await repository.findNextSubscriptionBatch({
165172
take: 5,
166-
teamIds: [10, 20],
173+
featureIds: ["calendar-subscription-cache"],
167174
integrations: [],
168175
});
169176

@@ -173,8 +180,15 @@ describe("SelectedCalendarRepository", () => {
173180
user: {
174181
teams: {
175182
some: {
176-
teamId: { in: [10, 20] },
177183
accepted: true,
184+
team: {
185+
features: {
186+
some: {
187+
featureId: { in: ["calendar-subscription-cache"] },
188+
enabled: true,
189+
},
190+
},
191+
},
178192
},
179193
},
180194
},
@@ -203,52 +217,6 @@ describe("SelectedCalendarRepository", () => {
203217
expect(result).toEqual(mockCalendars);
204218
});
205219

206-
test("should handle empty teamIds array", async () => {
207-
const mockCalendars: SelectedCalendar[] = [];
208-
vi.mocked(mockPrismaClient.selectedCalendar.findMany).mockResolvedValue(mockCalendars);
209-
210-
const result = await repository.findNextSubscriptionBatch({
211-
take: 10,
212-
teamIds: [],
213-
integrations: ["google_calendar"],
214-
});
215-
216-
expect(mockPrismaClient.selectedCalendar.findMany).toHaveBeenCalledWith({
217-
where: {
218-
integration: { in: ["google_calendar"] },
219-
user: {
220-
teams: {
221-
some: {
222-
teamId: { in: [] },
223-
accepted: true,
224-
},
225-
},
226-
},
227-
AND: [
228-
{
229-
OR: [
230-
{ syncSubscribedAt: null },
231-
{ channelExpiration: null },
232-
{ channelExpiration: { lte: expect.any(Date) } },
233-
],
234-
},
235-
{
236-
OR: [
237-
{ syncSubscribedErrorAt: null },
238-
{ syncSubscribedErrorAt: { lt: expect.any(Date) } },
239-
],
240-
},
241-
{
242-
syncSubscribedErrorCount: { lt: 3 },
243-
},
244-
],
245-
},
246-
take: 10,
247-
});
248-
249-
expect(result).toEqual(mockCalendars);
250-
});
251-
252220
test("should filter out generic calendars when genericCalendarSuffixes is provided", async () => {
253221
const mockCalendars = [mockSelectedCalendar];
254222
vi.mocked(mockPrismaClient.selectedCalendar.findMany).mockResolvedValue(mockCalendars);
@@ -257,7 +225,7 @@ describe("SelectedCalendarRepository", () => {
257225

258226
const result = await repository.findNextSubscriptionBatch({
259227
take: 10,
260-
teamIds: [1, 2],
228+
featureIds: ["calendar-subscription-cache"],
261229
integrations: ["google_calendar"],
262230
genericCalendarSuffixes: genericSuffixes,
263231
});
@@ -268,8 +236,15 @@ describe("SelectedCalendarRepository", () => {
268236
user: {
269237
teams: {
270238
some: {
271-
teamId: { in: [1, 2] },
272239
accepted: true,
240+
team: {
241+
features: {
242+
some: {
243+
featureId: { in: ["calendar-subscription-cache"] },
244+
enabled: true,
245+
},
246+
},
247+
},
273248
},
274249
},
275250
},

packages/features/selectedCalendar/repositories/SelectedCalendarRepository.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ export class SelectedCalendarRepository implements ISelectedCalendarRepository {
1919

2020
async findNextSubscriptionBatch({
2121
take,
22-
teamIds,
22+
featureIds,
2323
integrations,
2424
genericCalendarSuffixes,
2525
}: {
2626
take: number;
27-
teamIds: number[];
27+
featureIds: string[];
2828
integrations: string[];
2929
genericCalendarSuffixes?: string[];
3030
}) {
@@ -65,8 +65,15 @@ export class SelectedCalendarRepository implements ISelectedCalendarRepository {
6565
user: {
6666
teams: {
6767
some: {
68-
teamId: { in: teamIds },
6968
accepted: true,
69+
team: {
70+
features: {
71+
some: {
72+
featureId: { in: featureIds },
73+
enabled: true,
74+
},
75+
},
76+
},
7077
},
7178
},
7279
},

0 commit comments

Comments
 (0)