From 594bb606b8e27280f2cab805ad93c749710df8e3 Mon Sep 17 00:00:00 2001 From: Keith Williams Date: Wed, 7 May 2025 14:44:56 -0400 Subject: [PATCH] perf: Refactor bookings queries for API (#16407) * perf: Refactor bookings queries for API * fix syntax * fixing vars * Inverted the filter by attendees if * fixed prisma filter issue * Defaulting list of emails to empty array * Only running 2 queries if we have data to filter by * Reverted changes that have been separated out to other PR * fix: ts build error * adds tests + extracts logic for units * fix integration tests * fix tests expectes * Fix test * switching back * attempt : fix tests * test: add test for duplicate bookings (#21165) * feat: Add tests for booking and duration limits (#21158) * Add tests for booking and duration limits in getSchedule.test.ts Co-Authored-By: keith@cal.com * fix: use SchedulingType enum instead of string for schedulingType Co-Authored-By: keith@cal.com --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com * test: add test to ensure no duplicate bookings when merging query results Co-Authored-By: keith@cal.com --------- Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com Co-authored-by: Keith Williams * fix: deduplicate bookings when merging results from multiple queries Co-Authored-By: keith@cal.com * fix: ensure pagination works correctly when merging results from multiple queries (#21167) Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com --------- Co-authored-by: Tushar Bhatt <95581504+TusharBhatt1@users.noreply.github.com> Co-authored-by: Tushar Co-authored-by: sean-brydon Co-authored-by: Peer Richelsen Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com --- .../utils/bookings/get/buildWhereClause.ts | 37 +++++ apps/api/v1/pages/api/bookings/_get.ts | 136 +++++++++------- .../lib/bookings/_get.integration-test.ts | 150 +++++++++++++++++- .../lib/bookings/get/buildWhereClause.test.ts | 75 +++++++++ 4 files changed, 340 insertions(+), 58 deletions(-) create mode 100644 apps/api/v1/lib/utils/bookings/get/buildWhereClause.ts create mode 100644 apps/api/v1/test/lib/bookings/get/buildWhereClause.test.ts diff --git a/apps/api/v1/lib/utils/bookings/get/buildWhereClause.ts b/apps/api/v1/lib/utils/bookings/get/buildWhereClause.ts new file mode 100644 index 0000000000..ae6b0348b0 --- /dev/null +++ b/apps/api/v1/lib/utils/bookings/get/buildWhereClause.ts @@ -0,0 +1,37 @@ +/** + * Constructs the WHERE clause for Prisma booking findMany operation. + * + * @param userId - The ID of the user making the request. This is used to filter bookings where the user is either the host or an attendee. + * @param attendeeEmails - An array of emails provided in the request for filtering bookings by attendee emails, used in case of Admin calls. + * @param userIds - An array of user IDs to be included in the filter. Defaults to an empty array, and an array of user IDs in case of Admin call containing it. + * @param userEmails - An array of user emails to be included in the filter if it is an Admin call and contains userId in query parameter. Defaults to an empty array. + * + * @returns An object that represents the WHERE clause for the findMany/findUnique operation. + */ +export function buildWhereClause(userId: number | null, attendeeEmails: string[], userIds: number[] = []) { + const filterByAttendeeEmails = attendeeEmails.length > 0; + const userFilter = userIds.length > 0 ? { userId: { in: userIds } } : !!userId ? { userId } : {}; + + let whereClause = {}; + + if (filterByAttendeeEmails) { + whereClause = { + AND: [ + userFilter, + { + attendees: { + some: { + email: { in: attendeeEmails }, + }, + }, + }, + ], + }; + } else { + whereClause = { + ...userFilter, + }; + } + + return whereClause; +} diff --git a/apps/api/v1/pages/api/bookings/_get.ts b/apps/api/v1/pages/api/bookings/_get.ts index 8115cb11d9..96983afab3 100644 --- a/apps/api/v1/pages/api/bookings/_get.ts +++ b/apps/api/v1/pages/api/bookings/_get.ts @@ -1,4 +1,4 @@ -import type { Prisma } from "@prisma/client"; +import type { Prisma, Booking } from "@prisma/client"; import type { NextApiRequest } from "next"; import { HttpError } from "@calcom/lib/http-error"; @@ -6,6 +6,7 @@ import { defaultResponder } from "@calcom/lib/server/defaultResponder"; import prisma from "@calcom/prisma"; import { withMiddleware } from "~/lib/helpers/withMiddleware"; +import { buildWhereClause } from "~/lib/utils/bookings/get/buildWhereClause"; import { getAccessibleUsers, retrieveOrgScopedAccessibleUsers, @@ -151,58 +152,6 @@ type GetAdminArgsType = { userId: number; }; -/** - * Constructs the WHERE clause for Prisma booking findMany operation. - * - * @param userId - The ID of the user making the request. This is used to filter bookings where the user is either the host or an attendee. - * @param attendeeEmails - An array of emails provided in the request for filtering bookings by attendee emails, used in case of Admin calls. - * @param userIds - An array of user IDs to be included in the filter. Defaults to an empty array, and an array of user IDs in case of Admin call containing it. - * @param userEmails - An array of user emails to be included in the filter if it is an Admin call and contains userId in query parameter. Defaults to an empty array. - * - * @returns An object that represents the WHERE clause for the findMany/findUnique operation. - */ -function buildWhereClause( - userId: number | null, - attendeeEmails: string[], - userIds: number[] = [], - userEmails: string[] = [] -) { - const filterByAttendeeEmails = attendeeEmails.length > 0; - const userFilter = userIds.length > 0 ? { userId: { in: userIds } } : !!userId ? { userId } : {}; - - let whereClause = {}; - - if (filterByAttendeeEmails) { - whereClause = { - AND: [ - userFilter, - { - attendees: { - some: { - email: { in: attendeeEmails }, - }, - }, - }, - ], - }; - } else { - whereClause = { - OR: [ - userFilter, - { - attendees: { - some: { - email: { in: userEmails }, - }, - }, - }, - ], - }; - } - - return whereClause; -} - export async function handler(req: NextApiRequest) { const { userId, @@ -238,6 +187,7 @@ export async function handler(req: NextApiRequest) { ? [queryFilterForAttendeeEmails.attendeeEmail] : []; const filterByAttendeeEmails = attendeeEmails.length > 0; + let userEmailsToFilterBy: string[] = []; /** Only admins can query other users */ if (isSystemWideAdmin) { @@ -251,7 +201,8 @@ export async function handler(req: NextApiRequest) { userId, }; const { userId: argUserId, userIds, userEmails } = await handleSystemWideAdminArgs(systemWideAdminArgs); - args.where = buildWhereClause(argUserId, attendeeEmails, userIds, userEmails); + userEmailsToFilterBy = userEmails; + args.where = buildWhereClause(argUserId, attendeeEmails, userIds); } } else if (isOrganizationOwnerOrAdmin) { let requestedUserIds = [userId]; @@ -265,7 +216,8 @@ export async function handler(req: NextApiRequest) { userId, }; const { userId: argUserId, userIds, userEmails } = await handleOrgWideAdminArgs(orgWideAdminArgs); - args.where = buildWhereClause(argUserId, attendeeEmails, userIds, userEmails); + userEmailsToFilterBy = userEmails; + args.where = buildWhereClause(argUserId, attendeeEmails, userIds); } else { const user = await prisma.user.findUnique({ where: { id: userId }, @@ -276,7 +228,7 @@ export async function handler(req: NextApiRequest) { if (!user) { throw new HttpError({ message: "User not found", statusCode: 404 }); } - args.where = buildWhereClause(userId, attendeeEmails, [], []); + args.where = buildWhereClause(userId, attendeeEmails, []); } if (dateFrom) { @@ -317,7 +269,77 @@ export async function handler(req: NextApiRequest) { } } - const data = await prisma.booking.findMany(args); + let data: Booking[] = []; + + if (!filterByAttendeeEmails && userEmailsToFilterBy.length > 0) { + const queryOne = prisma.booking.findMany(args); + + const whereClauseForQueryTwo: Prisma.BookingWhereInput = { + attendees: { + some: { + email: { in: userEmailsToFilterBy }, + }, + }, + }; + + if (dateFrom) { + whereClauseForQueryTwo.startTime = { gte: dateFrom }; + } + if (dateTo) { + whereClauseForQueryTwo.endTime = { lte: dateTo }; + } + if (status === "upcoming") { + whereClauseForQueryTwo.startTime = { gte: new Date().toISOString() }; + } + + const argsForQueryTwo: Prisma.BookingFindManyArgs = { + ...args, + where: whereClauseForQueryTwo, + }; + + const queryTwo = prisma.booking.findMany(argsForQueryTwo); + + const [resultOne, resultTwo] = await Promise.all([queryOne, queryTwo]); + + const bookingMap = new Map(); + [...resultOne, ...resultTwo].forEach((booking) => { + bookingMap.set(booking.id, booking); + }); + + const dedupedResults = Array.from(bookingMap.values()); + + if (args.orderBy) { + let sortField: keyof Booking; + let sortDirection: "asc" | "desc" = "asc"; + + if (typeof args.orderBy === "object" && !Array.isArray(args.orderBy)) { + const orderByKey = Object.keys(args.orderBy)[0] as keyof typeof args.orderBy; + sortField = orderByKey as keyof Booking; + sortDirection = args.orderBy[orderByKey] as "asc" | "desc"; + } else { + sortField = "id"; + } + + const sortOrder = sortDirection === "desc" ? -1 : 1; + + dedupedResults.sort((a, b) => { + const aValue = a[sortField]; + const bValue = b[sortField]; + + if (aValue < bValue) return -1 * sortOrder; + if (aValue > bValue) return 1 * sortOrder; + return 0; + }); + } + + if (args.take !== undefined && args.skip !== undefined) { + data = dedupedResults.slice(args.skip, args.skip + args.take); + } else { + data = dedupedResults; + } + } else { + data = await prisma.booking.findMany(args); + } return { bookings: data.map((booking) => schemaBookingReadPublic.parse(booking)) }; } diff --git a/apps/api/v1/test/lib/bookings/_get.integration-test.ts b/apps/api/v1/test/lib/bookings/_get.integration-test.ts index 4d915a634a..efca0fe4cc 100644 --- a/apps/api/v1/test/lib/bookings/_get.integration-test.ts +++ b/apps/api/v1/test/lib/bookings/_get.integration-test.ts @@ -2,6 +2,7 @@ import type { Request, Response } from "express"; import type { NextApiRequest, NextApiResponse } from "next"; import { createMocks } from "node-mocks-http"; import { describe, expect, it } from "vitest"; +import { ZodError } from "zod"; import prisma from "@calcom/prisma"; @@ -206,8 +207,155 @@ describe("GET /api/bookings", async () => { console.log("bookings=>", responseData.bookings); responseData.bookings.forEach((booking) => { if (booking.id === 31) expect(booking.eventType?.team?.slug).toBe("team1"); - if (booking.id === 19) expect(booking.eventType?.team).toBe(null); + if (booking.id === 19) { + // The team field can be either null or undefined due to nullish() in the schema + expect(booking.eventType?.team === null || booking.eventType?.team === undefined).toBe(true); + } }); }); }); + + describe("Date filtering", () => { + it("filters bookings by dateFrom", async () => { + const { req } = createMocks({ + method: "GET", + query: { + dateFrom: "2023-01-01T00:00:00Z", + }, + pagination: DefaultPagination, + }); + + req.userId = proUser.id; + + const responseData = await handler(req); + responseData.bookings.forEach((booking) => { + expect(new Date(booking.startTime).getTime()).toBeGreaterThanOrEqual( + new Date("2023-01-01T00:00:00Z").getTime() + ); + }); + }); + + it("filters bookings by dateTo", async () => { + const { req } = createMocks({ + method: "GET", + query: { + dateTo: "2024-12-31T23:59:59Z", + }, + pagination: DefaultPagination, + }); + + req.userId = proUser.id; + + const responseData = await handler(req); + responseData.bookings.forEach((booking) => { + expect(new Date(booking.endTime).getTime()).toBeLessThanOrEqual( + new Date("2024-12-31T23:59:59Z").getTime() + ); + }); + }); + }); + + describe("Sorting and ordering", () => { + it("sorts bookings by createdAt in descending order", async () => { + const { req } = createMocks({ + method: "GET", + query: { + sortBy: "createdAt", + order: "desc", + }, + pagination: DefaultPagination, + }); + + req.userId = proUser.id; + + const responseData = await handler(req); + const timestamps = responseData.bookings.map((b) => new Date(b.createdAt).getTime()); + expect(timestamps).toEqual([...timestamps].sort((a, b) => b - a)); + }); + }); + + describe("Multiple attendee email filtering", () => { + it("filters bookings by multiple attendee emails", async () => { + const attendeeEmails = ["test1@example.com", "test2@example.com"]; + const { req } = createMocks({ + method: "GET", + query: { + attendeeEmail: attendeeEmails, + }, + pagination: DefaultPagination, + }); + + req.userId = proUser.id; + + const responseData = await handler(req); + responseData.bookings.forEach((booking) => { + const bookingAttendeeEmails = booking.attendees?.map((a) => a.email); + expect(bookingAttendeeEmails?.some((email) => attendeeEmails.includes(email))).toBe(true); + }); + }); + }); + + describe("Error cases", () => { + it("throws error for invalid status parameter", async () => { + const { req } = createMocks({ + method: "GET", + query: { + status: "invalid_status", + }, + pagination: DefaultPagination, + }); + + req.userId = proUser.id; + + await expect(handler(req)).rejects.toThrow(ZodError); + }); + }); + + describe("Result merging", () => { + it("does not return duplicate bookings when merging results from multiple queries", async () => { + const adminUser = await prisma.user.findFirstOrThrow({ where: { email: "owner1-acme@example.com" } }); + + const testUser = await prisma.user.findFirstOrThrow({ where: { email: "pro@example.com" } }); + + const testUserBooking = await prisma.booking.findFirstOrThrow({ + where: { userId: testUser.id }, + include: { attendees: true }, + }); + + const { req } = createMocks({ + method: "GET", + query: { + userId: testUser.id, // This will make userEmailsToFilterBy contain the test user's email + }, + pagination: { + take: 100, + skip: 0, + }, + }); + + req.isSystemWideAdmin = true; + req.userId = adminUser.id; + + const responseData = await handler(req); + + const bookingIds = responseData.bookings.map((booking) => booking.id); + + const uniqueBookingIds = new Set(bookingIds); + + expect(uniqueBookingIds.size).toBe(bookingIds.length); + + if (uniqueBookingIds.size !== bookingIds.length) { + const counts = bookingIds.reduce((acc, id) => { + acc[id] = (acc[id] || 0) + 1; + return acc; + }, {} as Record); + + const duplicates = Object.entries(counts) + .filter(([_, count]) => count > 1) + .map(([id]) => id); + + console.log(`Found duplicate booking IDs: ${duplicates.join(", ")}`); + } + }); + }); }); diff --git a/apps/api/v1/test/lib/bookings/get/buildWhereClause.test.ts b/apps/api/v1/test/lib/bookings/get/buildWhereClause.test.ts new file mode 100644 index 0000000000..1060eaf928 --- /dev/null +++ b/apps/api/v1/test/lib/bookings/get/buildWhereClause.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from "vitest"; + +import { buildWhereClause } from "~/lib/utils/bookings/get/buildWhereClause"; + +describe("buildWhereClause", () => { + it("should return empty object when no filters are provided", () => { + const result = buildWhereClause(null, [], []); + expect(result).toEqual({}); + }); + + it("should filter by userId when only userId is provided", () => { + const userId = 123; + const result = buildWhereClause(userId, [], []); + expect(result).toEqual({ userId: 123 }); + }); + + it("should filter by userIds when userIds array is provided", () => { + const userIds = [1, 2, 3]; + const result = buildWhereClause(null, [], userIds); + expect(result).toEqual({ userId: { in: [1, 2, 3] } }); + }); + + it("should filter by attendee emails when provided", () => { + const attendeeEmails = ["test@example.com", "user@example.com"]; + const result = buildWhereClause(null, attendeeEmails, []); + expect(result).toEqual({ + AND: [ + {}, + { + attendees: { + some: { + email: { in: ["test@example.com", "user@example.com"] }, + }, + }, + }, + ], + }); + }); + + it("should combine userId and attendee email filters", () => { + const userId = 123; + const attendeeEmails = ["test@example.com"]; + const result = buildWhereClause(userId, attendeeEmails, []); + expect(result).toEqual({ + AND: [ + { userId: 123 }, + { + attendees: { + some: { + email: { in: ["test@example.com"] }, + }, + }, + }, + ], + }); + }); + + it("should combine userIds and attendee email filters", () => { + const userIds = [1, 2, 3]; + const attendeeEmails = ["test@example.com"]; + const result = buildWhereClause(null, attendeeEmails, userIds); + expect(result).toEqual({ + AND: [ + { userId: { in: [1, 2, 3] } }, + { + attendees: { + some: { + email: { in: ["test@example.com"] }, + }, + }, + }, + ], + }); + }); +});