Skip to content

feat: convert InsightsBookingService to use Prisma.sql raw queries#5

Open
everettbu wants to merge 1 commit into
insights-query-foundationfrom
insights-performance-optimization
Open

feat: convert InsightsBookingService to use Prisma.sql raw queries#5
everettbu wants to merge 1 commit into
insights-query-foundationfrom
insights-performance-optimization

Conversation

@everettbu

@everettbu everettbu commented Jul 28, 2025

Copy link
Copy Markdown

Test 5

…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>
@lizard-boy

Copy link
Copy Markdown

cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bugbot free trial expires on August 11, 2025
Learn more in the Cursor dashboard.

return filterConditions;
} else {
return NOTHING_CONDITION;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug

The InsightsBookingService has two distinct issues:

  1. Type Mismatch & Validation: The constructor's options parameter (InsightsBookingServicePublicOptions) allows teamId to be optional, but the internal insightsBookingServiceOptionsSchema used for runtime validation requires teamId for team scope and disallows it for user/org scopes. This creates a type safety regression, allowing TypeScript to accept invalid configurations (e.g., missing teamId for team scope) that fail runtime validation. When validation fails, this.options is set to null, causing all authorization methods to return NOTHING_CONDITION, leading to silent failures (no results) instead of proper error handling.
  2. Unreachable Code: In getBaseConditions(), the else if (filterConditions) and final else branches are unreachable. This is because getAuthorizationConditions() always returns a non-null Prisma.Sql object, making authConditions always truthy, which means only the first two if/else if conditions are ever evaluated.
Locations (1)
Fix in Cursor Fix in Web

@github-actions

Copy link
Copy Markdown
Contributor

This PR is being marked as stale due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants