feat: convert InsightsBookingService to use Prisma.sql raw queries#8
feat: convert InsightsBookingService to use Prisma.sql raw queries#8ShashankFC wants to merge 1 commit into
Conversation
…22345) * fix: use raw query at InsightsBookingService * feat: convert InsightsBookingService to use Prisma.sql raw queries - Convert auth conditions from Prisma object notation to Prisma.sql - Convert filter conditions from Prisma object notation to Prisma.sql - Update return types from Prisma.BookingTimeStatusDenormalizedWhereInput to Prisma.Sql - Fix type error in isOrgOwnerOrAdmin method - Follow same pattern as InsightsRoutingService conversion Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * feat: convert InsightsBookingService to use Prisma.sql raw queries - Convert auth conditions from Prisma object notation to Prisma.sql - Convert filter conditions from Prisma object notation to Prisma.sql - Update return types from Prisma.BookingTimeStatusDenormalizedWhereInput to Prisma.Sql - Fix type error in isOrgOwnerOrAdmin method - Follow same pattern as InsightsRoutingService conversion Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix: update InsightsBookingService integration tests for Prisma.sql format - Replace Prisma object notation expectations with Prisma.sql template literals - Add NOTHING_CONDITION constant for consistency with InsightsRoutingService - Update all test cases to use direct Prisma.sql comparisons - Use $queryRaw for actual database integration testing - Follow same testing patterns as InsightsRoutingService Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix: exclude intentionally skipped jobs from required CI check failure - Remove 'skipped' from failure condition in pr.yml and all-checks.yml - Allow E2E jobs to be skipped without failing the required check - Only actual failures and cancelled jobs will cause required check to fail Co-Authored-By: eunjae@cal.com <hey@eunjae.dev> * fix tests * Revert "fix: exclude intentionally skipped jobs from required CI check failure" This reverts commit 6ff44fc9a8f14ad657f7bba7c2e454e192b66c8f. * clean up tests * address feedback --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the InsightsBookingService to use Prisma's raw SQL queries (Prisma.sql) instead of Prisma's ORM query builder for constructing authorization and filter conditions. The refactoring improves performance by generating more efficient SQL queries while maintaining the same authorization logic.
Changes:
- Converted authorization and filter condition building from Prisma ORM objects to raw SQL queries using
Prisma.sql - Replaced the
findManymethod withgetBaseConditionsthat returns raw SQL conditions - Updated the
NOTHINGconstant toNOTHING_CONDITIONwith a SQL expression1=0
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/lib/server/service/insightsBooking.ts | Refactored service to use Prisma.sql for building authorization and filter conditions, replacing ORM-based query construction with raw SQL |
| packages/lib/server/service/tests/insightsBooking.integration-test.ts | Updated test expectations to match new Prisma.sql return types and replaced findMany tests with getBaseConditions tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return conditions.reduce((acc, condition, index) => { | ||
| if (index === 0) return condition; | ||
| return Prisma.sql`(${acc}) AND (${condition})`; | ||
| }); |
There was a problem hiding this comment.
The reduce function lacks an initial value, which will cause an error when the conditions array is empty. Although there's an early return at line 114-116 when conditions.length === 0, the reduce function should still have an initial value for type safety and clarity. Add an initial value parameter to the reduce call.
| }); | |
| }, Prisma.sql``); |
| return conditions.reduce((acc, condition, index) => { | ||
| if (index === 0) return condition; | ||
| return Prisma.sql`(${acc}) OR (${condition})`; | ||
| }); |
There was a problem hiding this comment.
The reduce function lacks an initial value. While the conditions array is guaranteed to have at least one element (line 164), explicitly providing an initial value improves code robustness and makes the intention clearer.
| return conditions.reduce((acc, condition, index) => { | ||
| if (index === 0) return condition; | ||
| return Prisma.sql`(${acc}) OR (${condition})`; | ||
| }); |
There was a problem hiding this comment.
The reduce function lacks an initial value. While the conditions array is guaranteed to have at least one element (line 196-198), explicitly providing an initial value improves code robustness and makes the intention clearer.
| return conditions.reduce((acc, condition, index) => { | |
| if (index === 0) return condition; | |
| return Prisma.sql`(${acc}) OR (${condition})`; | |
| }); | |
| return conditions.reduce<Prisma.Sql>( | |
| (acc, condition) => Prisma.sql`(${acc}) OR (${condition})`, | |
| NOTHING_CONDITION | |
| ); |
Test 5nn
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
nn---n*Replicated from [ai-code-review-evaluation/cal.com-coderabbit#5](https://github.com/ai-code-review-evaluation/cal.com-coderabbit/pull/5)*