feat: convert InsightsBookingService to use Prisma.sql raw queries

[akshayutture-augment]

Test 5

Summary by CodeRabbit

Release Notes

• Refactor
  • Improved internal booking insights service architecture with optimized data handling for better performance and reliability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

---

Replicated from ai-code-review-evaluation/cal.com-coderabbit#5

[akshayutture-augment]

@coderabbitai review

[akshayutture-augment]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

Refactors InsightsBookingService to replace Prisma object-based where conditions with raw SQL fragments. Introduces new public options type with constructor validation. All condition builders rewritten to return Prisma.sql expressions. Corresponding test suite updated to validate SQL-based conditions and utilize new getBaseConditions method.

Changes

Cohort / File(s)	Summary
Service Implementation packages/lib/server/service/insightsBooking.ts	Migrated condition building from BookingTimeStatusDenormalizedWhereInput objects to Prisma.sql fragments. Added InsightsBookingServicePublicOptions type and constructor validation schema. Replaced findMany entry point with getBaseConditions returning SQL. Refactored buildAuthorizationConditions, buildFilterConditions, and related helpers to compose SQL expressions using AND/OR operators. Updated internal properties to store Prisma.sql conditions.
Test Suite packages/lib/server/service/__tests__/insightsBooking.integration-test.ts	Added Prisma import and NOTHING_CONDITION constant (Prisma.sql`1=0`). Updated test expectations to compare against Prisma.sql expressions instead of object literals. Refactored tests to use getBaseConditions and prisma.$queryRaw. Introduced test data factory for constructing richer fixtures (org, team, eventType, bookings, memberships) with cleanup routine.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• SQL composition logic: Verify correctness of AND/OR joining of multiple conditions in condition builders
• Constructor validation: Review insightsBookingServiceOptionsSchema implementation and error handling for public options
• Test expectations: Confirm Prisma.sql comparisons accurately reflect intended query behavior
• Migration completeness: Ensure all legacy object-based condition references have been replaced with SQL expressions

Poem

🐰 From nested objects to SQL we hop,
Prisma fragments make conditions pop!
Raw queries dance where conditions once stood,
Service reborn—cleaner, stronger, good! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main change: converting InsightsBookingService to use Prisma.sql raw queries instead of Prisma object structures, which is reflected throughout both the service and test file modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch insights-performance-optimization

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[akshayutture-augment]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/lib/server/service/insightsBooking.ts (1)

145-175: Possible under-authorization when org has no child teams

In buildOrgAuthorizationCondition, userIdsFromOrg is only populated when teamsFromOrg.length > 0:

const userIdsFromOrg =
  teamsFromOrg.length > 0
    ? (await MembershipRepository.findAllByTeamIds({ teamIds, select: { userId: true } })).map(
        (m) => m.userId
      )
    : [];

But teamIds always includes options.orgId itself. If an organization has no child teams yet (only the org “team”), this branch will skip the membership lookup entirely, so userIdsFromOrg is empty and you never add the "userId" = ANY(...) AND "isTeamBooking" = false condition. That means non-team bookings for org members would be excluded for such orgs.

Unless you can guarantee that every org always has at least one child team, this is a real edge-case bug.

You can fix it by always querying memberships using teamIds and then deduplicating:

-    const userIdsFromOrg =
-      teamsFromOrg.length > 0
-        ? (await MembershipRepository.findAllByTeamIds({ teamIds, select: { userId: true } })).map(
-            (m) => m.userId
-          )
-        : [];
+    const userIdsFromOrg = (
+      await MembershipRepository.findAllByTeamIds({
+        teamIds,
+        select: { userId: true },
+      })
+    ).map((m) => m.userId);

The later if (userIdsFromOrg.length > 0) guard still ensures you only add the userId condition when there are actual memberships.

🧹 Nitpick comments (2)

packages/lib/server/service/insightsBooking.ts (2)

29-36: Tighten InsightsBookingServicePublicOptions to mirror the zod discriminated union

Right now InsightsBookingServicePublicOptions allows invalid combinations like scope: "team" without teamId at compile time; these are only rejected at runtime and then silently treated as options = null, yielding NOTHING_CONDITION. That can hide misconfigurations and be hard to debug.

Consider making the public options type a discriminated union matching insightsBookingServiceOptionsSchema so invalid shapes are rejected by TypeScript instead of only by zod:

-export type InsightsBookingServicePublicOptions = {
-  scope: "user" | "org" | "team";
-  userId: number;
-  orgId: number;
-  teamId?: number;
-};
+export type InsightsBookingServicePublicOptions =
+  | {
+      scope: "user";
+      userId: number;
+      orgId: number;
+    }
+  | {
+      scope: "org";
+      userId: number;
+      orgId: number;
+    }
+  | {
+      scope: "team";
+      userId: number;
+      orgId: number;
+      teamId: number;
+    };

This keeps the runtime validation but also gives callers stronger static guarantees.

Also applies to: 52-66

---

68-81: Simplify getBaseConditions since auth conditions are always present

getAuthorizationConditions() always returns a Prisma.Sql (either real conditions or NOTHING_CONDITION), so authConditions is never falsy. The last else branch is therefore unreachable, and the second else if (authConditions) is redundant.

You can simplify the control flow to make the intent clearer:

-  async getBaseConditions(): Promise<Prisma.Sql> {
-    const authConditions = await this.getAuthorizationConditions();
-    const filterConditions = await this.getFilterConditions();
-
-    if (authConditions && filterConditions) {
-      return Prisma.sql`(${authConditions}) AND (${filterConditions})`;
-    } else if (authConditions) {
-      return authConditions;
-    } else if (filterConditions) {
-      return filterConditions;
-    } else {
-      return NOTHING_CONDITION;
-    }
-  }
+  async getBaseConditions(): Promise<Prisma.Sql> {
+    const authConditions = await this.getAuthorizationConditions();
+    const filterConditions = await this.getFilterConditions();
+
+    if (!filterConditions) {
+      return authConditions;
+    }
+
+    return Prisma.sql`(${authConditions}) AND (${filterConditions})`;
+  }

Behavior stays the same while reducing branches.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 202d248 and 56ff75f.

📒 Files selected for processing (2)

• packages/lib/server/service/__tests__/insightsBooking.integration-test.ts (10 hunks)
• packages/lib/server/service/insightsBooking.ts (6 hunks)

🔇 Additional comments (7)

packages/lib/server/service/insightsBooking.ts (4)

97-123: Filter condition builder is correct and safely parameterized

The buildFilterConditions implementation correctly:

• Returns null when no filters are provided or no conditions are added.
• Builds eventTypeId conditions as eventTypeId = ? OR eventParentId = ?.
• Builds memberUserId conditions as "userId" = ?.
• Joins multiple filters with AND while wrapping each fragment in parentheses.

All values are injected via Prisma.sql template parameters, so you get proper parameter binding and avoid SQL injection pitfalls.

---

125-143: Authorization builder correctly normalizes invalid/unauthorized cases to NOTHING_CONDITION

buildAuthorizationConditions centralizes the checks for:

• Invalid options (failed zod parse ⇒ this.options is null).
• Non-owner/non-admin users (via isOrgOwnerOrAdmin).
• Scope-specific SQL fragments for "user", "org", and "team".

In all non-authorized cases you consistently return NOTHING_CONDITION, which keeps downstream combining logic simple and safe. This looks good.

---

177-208: Team-scope authorization SQL composition looks correct

For team scope:

• You first assert that the given team is actually a child of the org (findByIdAndParentId), falling back to NOTHING_CONDITION otherwise.
• You include team bookings for that team ("teamId" = ? AND "isTeamBooking" = true).
• You include non-team bookings for users belonging to that team ("userId" = ANY(?) AND "isTeamBooking" = false).
• You OR these clauses together with proper parentheses.

The use of Prisma.sql keeps parameters bound safely, and the early NOTHING_CONDITION when childTeamOfOrg is missing protects against cross-org access.

---

210-218: isOrgOwnerOrAdmin simplification is clear and precise

The role check now directly compares membership.role to MembershipRole.OWNER and MembershipRole.ADMIN with an accepted guard, which is simple and unambiguous. Returning a boolean via Boolean(...) is fine here and keeps the intent obvious.

packages/lib/server/service/__tests__/insightsBooking.integration-test.ts (3)

2-11: Using Prisma.sql + NOTHING_CONDITION in tests aligns well with the service behavior

Importing Prisma and defining NOTHING_CONDITION as Prisma.sql\1=0`` lets the tests assert exactly the same SQL fragments the service produces, including for invalid options and non-owner/admin users.

The expectations:

expect(conditions).toEqual(NOTHING_CONDITION);

are a good way to ensure the service normalizes “no access” correctly at the SQL level, not just structurally.

Also applies to: 210-211, 245-246

---

273-274: Authorization and filter condition expectations thoroughly cover SQL output

The updated expectations that compare against Prisma.sql fragments for:

• User scope: "userId" = ? AND "teamId" IS NULL.
• Team scope: OR between team bookings and team-user non-team bookings using ANY([...]).
• Org scope: OR between teamId = ANY([...]) and userId = ANY([...]).
• Filter conditions for eventTypeId, memberUserId, and their combination.

provide strong coverage that the refactored service builds the right SQL. This is especially helpful now that the implementation has moved away from object-shaped where clauses.

Also applies to: 295-299, 332-343, 384-386, 407-407, 429-431

---

437-492: getBaseConditions integration test is a solid end-to-end verification

The new getBaseConditions test:

• Creates a user-scoped event type and booking.
• Instantiates InsightsBookingService with matching auth + filter options.
• Uses baseConditions directly in a $queryRaw against "BookingTimeStatusDenormalized" and asserts only the expected booking ID is returned.

This is an effective end-to-end check that auth and filter fragments are combined correctly and that they behave as intended when applied to the actual denormalized view.