diff --git a/packages/trpc/server/routers/viewer/bookings/get.handler.test.ts b/packages/trpc/server/routers/viewer/bookings/get.handler.test.ts index 2e8c6174cd..28bd067fee 100644 --- a/packages/trpc/server/routers/viewer/bookings/get.handler.test.ts +++ b/packages/trpc/server/routers/viewer/bookings/get.handler.test.ts @@ -335,11 +335,10 @@ describe("getBookings - PBAC Permission Checks", () => { }); }); - describe("Event type filtering", () => { - it("should get event types from teams where user has booking.read permission", async () => { + describe("Event type filtering with subqueries", () => { + it("should use subqueries for event types from teams where user has booking.read permission", async () => { mockGetTeamIdsWithPermission.mockResolvedValue([1]); mockPrisma.user.findMany = vi.fn().mockResolvedValue([]); - mockPrisma.$queryRaw = vi.fn().mockResolvedValue([{ id: 10 }, { id: 11 }]); mockPrisma.booking.groupBy = vi.fn().mockResolvedValue([]); await getBookings({ @@ -352,13 +351,15 @@ describe("getBookings - PBAC Permission Checks", () => { skip: 0, }); - // Verify event types are fetched from team 1 using raw SQL query - expect(mockPrisma.$queryRaw).toHaveBeenCalled(); + // PERFORMANCE: With the optimization, we no longer call $queryRaw to fetch event type IDs. + // Instead, we use subqueries directly in the SQL to avoid materializing large arrays. + // The query is built using Kysely and executed via executeQuery. + expect(mockKysely.executeQuery).toHaveBeenCalled(); }); }); - describe("User IDs and emails retrieval", () => { - it("should get user IDs and emails from teams with booking permission", async () => { + describe("User IDs retrieval for permission validation", () => { + it("should only fetch user IDs when userIds filter is provided", async () => { mockGetTeamIdsWithPermission.mockResolvedValue([1, 2]); mockPrisma.user.findMany = vi.fn().mockResolvedValue([ { id: 2, email: "user2@example.com" }, @@ -372,13 +373,38 @@ describe("getBookings - PBAC Permission Checks", () => { prisma: mockPrisma, kysely: mockKysely, bookingListingByStatus: ["upcoming"], - filters: {}, + filters: { + userIds: [2], // When userIds filter is provided, we need to validate permissions + }, take: 10, skip: 0, }); - // Verify users are fetched from teams 1 and 2 + // Verify users are fetched from teams 1 and 2 for permission validation expect(mockPrisma.user.findMany).toHaveBeenCalled(); }); + + it("should NOT fetch user IDs when no userIds filter is provided (uses subqueries instead)", async () => { + mockGetTeamIdsWithPermission.mockResolvedValue([1, 2]); + // Reset the mock to track calls + mockPrisma.user.findMany = vi.fn().mockResolvedValue([]); + mockPrisma.eventType.findMany = vi.fn().mockResolvedValue([]); + mockPrisma.booking.groupBy = vi.fn().mockResolvedValue([]); + + await getBookings({ + user: mockUser, + prisma: mockPrisma, + kysely: mockKysely, + bookingListingByStatus: ["upcoming"], + filters: {}, // No userIds filter + take: 10, + skip: 0, + }); + + // PERFORMANCE: Without userIds filter, we don't need to fetch user IDs for validation. + // The query uses subqueries instead of materializing all user IDs/emails. + // user.findMany should NOT be called for getUserIdsFromTeamIds + expect(mockPrisma.user.findMany).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/trpc/server/routers/viewer/bookings/get.handler.ts b/packages/trpc/server/routers/viewer/bookings/get.handler.ts index c565bafebc..77c30f2c23 100644 --- a/packages/trpc/server/routers/viewer/bookings/get.handler.ts +++ b/packages/trpc/server/routers/viewer/bookings/get.handler.ts @@ -118,25 +118,25 @@ export async function getBookings({ orgId: user.orgId ?? undefined, }); + // Only fetch user IDs from teams if we need to validate userIds filter + // PERFORMANCE: We no longer need to fetch all emails/IDs for the main query since we use subqueries + const needsUserIdsValidation = !!filters?.userIds && filters.userIds.length > 0; + const [ eventTypeIdsFromTeamIdsFilter, attendeeEmailsFromUserIdsFilter, eventTypeIdsFromEventTypeIdsFilter, - eventTypeIdsWhereUserHasBookingPermission, - userIdsAndEmailsWhereUserHasBookingPermission, + allAccessibleUserIds, ] = await Promise.all([ getEventTypeIdsFromTeamIdsFilter(prisma, filters?.teamIds), getAttendeeEmailsFromUserIdsFilter(prisma, user.email, filters?.userIds), getEventTypeIdsFromEventTypeIdsFilter(prisma, filters?.eventTypeIds), - getEventTypeIdsFromTeamIdsFilter(prisma, teamIdsWithBookingPermission), - getUserIdsAndEmailsFromTeamIds(prisma, teamIdsWithBookingPermission), + // Only fetch accessible user IDs when we need to validate the userIds filter + needsUserIdsValidation ? getUserIdsFromTeamIds(prisma, teamIdsWithBookingPermission) : Promise.resolve([]), ]); const bookingQueries: { query: BookingsUnionQuery; tables: (keyof DB)[] }[] = []; - // Get user IDs and emails from teams where user has booking permission - const [allAccessibleUserIds, allAccessibleUserEmails] = userIdsAndEmailsWhereUserHasBookingPermission; - // If userIds filter is provided if (!!filters?.userIds && filters.userIds.length > 0) { const areUserIdsWithinUserOrgOrTeam = filters.userIds.every((userId) => @@ -242,7 +242,8 @@ export async function getBookings({ }); // 4. Scope depends on `user.orgId`: // - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get bookings where organization/team members are attendees - if (allAccessibleUserEmails?.length) { + // PERFORMANCE: Use subquery with team membership instead of materializing all emails (can be 400+ for large orgs) + if (teamIdsWithBookingPermission?.length) { bookingQueries.push({ query: kysely .selectFrom("Booking") @@ -252,13 +253,20 @@ export async function getBookings({ .select("Booking.createdAt") .select("Booking.updatedAt") .innerJoin("Attendee", "Attendee.bookingId", "Booking.id") - .where("Attendee.email", "in", allAccessibleUserEmails), + .where("Attendee.email", "in", (eb) => + eb + .selectFrom("users") + .select("users.email") + .innerJoin("Membership", "Membership.userId", "users.id") + .where("Membership.teamId", "in", teamIdsWithBookingPermission) + ), tables: ["Booking", "Attendee"], }); } // 5. Scope depends on `user.orgId`: // - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get bookings where organization/team members are attendees via seatsReference - if (allAccessibleUserEmails?.length) { + // PERFORMANCE: Use subquery with team membership instead of materializing all emails + if (teamIdsWithBookingPermission?.length) { bookingQueries.push({ query: kysely .selectFrom("Booking") @@ -269,14 +277,21 @@ export async function getBookings({ .select("Booking.updatedAt") .innerJoin("Attendee", "Attendee.bookingId", "Booking.id") .innerJoin("BookingSeat", "Attendee.id", "BookingSeat.attendeeId") - .where("Attendee.email", "in", allAccessibleUserEmails), + .where("Attendee.email", "in", (eb) => + eb + .selectFrom("users") + .select("users.email") + .innerJoin("Membership", "Membership.userId", "users.id") + .where("Membership.teamId", "in", teamIdsWithBookingPermission) + ), tables: ["Booking", "Attendee", "BookingSeat"], }); } // 6. Scope depends on `user.orgId`: // - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get booking created for an event type within the organization/team - if (eventTypeIdsWhereUserHasBookingPermission?.length) { + // PERFORMANCE: Use subquery to get event type IDs instead of materializing them + if (teamIdsWithBookingPermission?.length) { bookingQueries.push({ query: kysely .selectFrom("Booking") @@ -285,15 +300,20 @@ export async function getBookings({ .select("Booking.endTime") .select("Booking.createdAt") .select("Booking.updatedAt") - .innerJoin("EventType", "EventType.id", "Booking.eventTypeId") - .where("Booking.eventTypeId", "in", eventTypeIdsWhereUserHasBookingPermission), - tables: ["Booking", "EventType"], + .where("Booking.eventTypeId", "in", (eb) => + eb + .selectFrom("EventType") + .select("EventType.id") + .where("EventType.teamId", "in", teamIdsWithBookingPermission) + ), + tables: ["Booking"], }); } // 7. Scope depends on `user.orgId`: // - If Current user is ORG_OWNER/ADMIN or has booking.read permission, get bookings created by users within the same organization/team - if (allAccessibleUserIds?.length) { + // PERFORMANCE: Use subquery with team membership instead of materializing all user IDs + if (teamIdsWithBookingPermission?.length) { bookingQueries.push({ query: kysely .selectFrom("Booking") @@ -302,7 +322,12 @@ export async function getBookings({ .select("Booking.endTime") .select("Booking.createdAt") .select("Booking.updatedAt") - .where("Booking.userId", "in", allAccessibleUserIds), + .where("Booking.userId", "in", (eb) => + eb + .selectFrom("Membership") + .select("Membership.userId") + .where("Membership.teamId", "in", teamIdsWithBookingPermission) + ), tables: ["Booking"], }); } @@ -944,17 +969,15 @@ async function getEventTypeIdsFromEventTypeIdsFilter(prisma: PrismaClient, event } /** - * Gets [IDs, Emails] of members from specified team IDs. + * Gets user IDs of members from specified team IDs. + * PERFORMANCE: This is a lighter version that only fetches IDs (not emails) for permission validation. * @param prisma The Prisma client. * @param teamIds Array of team IDs to get members from - * @returns {Promise<[number[], string[]]>} [UserIDs, UserEmails] for members in the specified teams. + * @returns {Promise} UserIDs for members in the specified teams. */ -async function getUserIdsAndEmailsFromTeamIds( - prisma: PrismaClient, - teamIds: number[] -): Promise<[number[], string[]]> { +async function getUserIdsFromTeamIds(prisma: PrismaClient, teamIds: number[]): Promise { if (teamIds.length === 0) { - return [[], []]; + return []; } const users = await prisma.user.findMany({ @@ -969,13 +992,9 @@ async function getUserIdsAndEmailsFromTeamIds( }, select: { id: true, - email: true, }, }); - const userIds = Array.from(new Set(users.map((user) => user.id))); - const userEmails = Array.from(new Set(users.map((user) => user.email))); - - return [userIds, userEmails]; + return Array.from(new Set(users.map((user) => user.id))); } function addStatusesQueryFilters(query: BookingsUnionQuery, statuses: InputByStatus[]) {