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 <keith@cal.com> * fix: use SchedulingType enum instead of string for schedulingType Co-Authored-By: keith@cal.com <keith@cal.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com <keith@cal.com> * test: add test to ensure no duplicate bookings when merging query results Co-Authored-By: keith@cal.com <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 <keith@cal.com> Co-authored-by: Keith Williams <keithwillcode@gmail.com> * fix: deduplicate bookings when merging results from multiple queries Co-Authored-By: keith@cal.com <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 <keith@cal.com> --------- Co-authored-by: Tushar Bhatt <95581504+TusharBhatt1@users.noreply.github.com> Co-authored-by: Tushar <tusharbhatt0135@gmail.com> Co-authored-by: sean-brydon <sean@cal.com> Co-authored-by: Peer Richelsen <peeroke@gmail.com> Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: keith@cal.com <keith@cal.com>
2025-05-07 11:44:56 -07:00
1 parent 1c088f65c5
commit 594bb606b8
@@ -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;
+}
@@ -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)) };
 }

@@ -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<CustomNextApiRequest, CustomNextApiResponse>({
+        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<CustomNextApiRequest, CustomNextApiResponse>({
+        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<CustomNextApiRequest, CustomNextApiResponse>({
+        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<CustomNextApiRequest, CustomNextApiResponse>({
+        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<CustomNextApiRequest, CustomNextApiResponse>({
+        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<CustomNextApiRequest, CustomNextApiResponse>({
+        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<number, number>);
+
+        const duplicates = Object.entries(counts)
+          .filter(([_, count]) => count > 1)
+          .map(([id]) => id);
+
+        console.log(`Found duplicate booking IDs: ${duplicates.join(", ")}`);
+      }
+    });
+  });
 });
@@ -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"] },
+            },
+          },
+        },
+      ],
+    });
+  });
+});