fix: including Prisma.sql for Prisma v6 upgrade#23775
Conversation
WalkthroughThis change refactors SQL construction across InsightsBookingBaseService.ts and InsightsRoutingBaseService.ts by replacing inline prisma.$queryRaw Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
Nice one man! oversight of a merge 🙏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/lib/server/service/InsightsBookingBaseService.ts (3)
171-181: Type mismatch: hour should be number, not string.You cast to
::intin SQL but typehour: stringin TS. Tighten typing to prevent unnecessaryNumber(...)conversions.- const results = await this.prisma.$queryRaw< - Array<{ - hour: string; - count: number; - }> - >(query); + const results = await this.prisma.$queryRaw< + Array<{ + hour: number; + count: number; + }> + >(query);Also applies to: 182-187
456-461: CSV query refactor looks good; two small checks.
- Selecting explicit columns: great. Consider aligning
uidnullability with the view (schema says non-null). Ifuidcan be null, updatebookingDataSchemaor remove the filter-on-null path.- Minor: you can drop the interim array type’s nullable
uidif it’s guaranteed.Also applies to: 462-462, 467-488, 490-509
1083-1115: Cast avg_rating to float for consistency with decoder.
AVG(...)without cast may come back as string (numeric) from pg. Align with other queries that use::float.- AVG(CASE WHEN "rating" IS NOT NULL THEN "rating" END) as avg_rating, + AVG(CASE WHEN "rating" IS NOT NULL THEN "rating" END)::float as avg_rating,Also applies to: 1116-1129
packages/lib/server/service/InsightsRoutingBaseService.ts (1)
186-206: Safer status check: cast enum to text before string comparison.If
bookingStatusis an enum, compare via::textto avoid implicit casts.- COUNT(CASE WHEN "bookingStatus" NOT IN ('cancelled', 'rejected') THEN 1 END) as "acceptedBookings" + COUNT(CASE WHEN "bookingStatus"::text NOT IN ('cancelled', 'rejected') THEN 1 END) as "acceptedBookings"Also applies to: 208-216
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/lib/server/service/InsightsBookingBaseService.ts(20 hunks)packages/lib/server/service/InsightsRoutingBaseService.ts(13 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/lib/server/service/InsightsRoutingBaseService.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/lib/server/service/InsightsRoutingBaseService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/lib/server/service/InsightsRoutingBaseService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/lib/server/service/InsightsRoutingBaseService.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
📚 Learning: 2025-08-22T16:38:00.225Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-07-24T08:39:06.185Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (17)
packages/lib/server/service/InsightsBookingBaseService.ts (9)
16-16: Prisma v6-safe import confirmed.Switch to
import { Prisma }aligns with Prisma v6$queryRaw(Prisma.Sql)usage.
171-181: Accepted-only filter: verify product requirement.Adding
AND "status" = 'accepted'narrows previous behavior. Confirm UI/metrics expect excluding non-accepted bookings for this chart.
221-226: Good v6 migration toPrisma.sql+$queryRaw(query).Pattern is correct and future-proof for Prisma v6.
Also applies to: 227-228
652-690: CTE split for host/guest stats LGTM.Joins and casts are safe; counts are correctly narrowed.
Also applies to: 691-699
790-800: Popular events query migration LGTM.Casts and top-N ordering preserved; execution path is v6-safe.
Also applies to: 801-807
881-890: Members count query migration LGTM; keep prior learning in mind.No extra
endTime <= now()filter added—good (per prior learning).Also applies to: 892-898
944-954: Average rating query migration LGTM.Type cast to
::floatensures numeric decode; consistent with downstream usage.Also applies to: 955-961
1007-1016: Recent ratings query migration LGTM.Ordering and limit preserved; safe for v6.
Also applies to: 1018-1025
1159-1195: Recent no-show guests query migration LGTM.Accepted-only plus HAVING logic preserved; windowed row selection is correct.
Also applies to: 1197-1205
packages/lib/server/service/InsightsRoutingBaseService.ts (8)
270-275: Total count query migration LGTM.Bigint handling retained; conversion occurs later.
Also applies to: 276-277
282-346: Paginated table data query migration LGTM.JSON aggregation subqueries and ordering preserved; v6-safe pattern.
Also applies to: 348-349
368-373: Routing form stats queries migration LGTM.Splitting total/without-booking is clear; filters preserved.
Also applies to: 374-375, 378-385
452-467: Users query (DISTINCT ON) migration LGTM.
ORDER BY "bookingUserId", "createdAt" DESCmatches DISTINCT intent; optionalLIMITis safe.Also applies to: 469-477
495-555: Period stats CTEs migration LGTM.Parameterized period granularity is handled correctly with
date_truncand window bounds.Also applies to: 556-563
565-577: Whole-period stats query migration LGTM.Aggregations and ordering kept; used only for percentile-ish metrics.
Also applies to: 578-584
646-649: CSV map guard LGTM.
userStatnull-check avoids undefined writes; minimal and safe.
964-1009: Failed bookings by field: migration LGTM.CTEs and JSONB unnest logic preserved; integer cast on counts retained.
Also applies to: 1011-1021
E2E results are ready! |
3 participants
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Prompt used to fix this problem
After upgrading Prisma to v6, things are broken. Fix all the $queryRaw usages from
to
We're basically extracting the whole query as Prisma.sql, and pass the query to queryRaw.
But this needs to be fixed only when the $queryRaw
.....includes a parameter that is an instance of Prisma.sql. So no need to fix something like this, because the parameterized values (someString) is primitive values, and not an instance of Prisma.sql....