Skip to content

[MSD benchmark] feat: convert InsightsBookingService to use Prisma.sql raw queries#4

Open
khaliqgant wants to merge 12 commits into
msd-benchmark-base/cal_dot_com/pr-22345from
msd-benchmark/cal_dot_com/pr-22345
Open

[MSD benchmark] feat: convert InsightsBookingService to use Prisma.sql raw queries#4
khaliqgant wants to merge 12 commits into
msd-benchmark-base/cal_dot_com/pr-22345from
msd-benchmark/cal_dot_com/pr-22345

Conversation

@khaliqgant

Copy link
Copy Markdown

MSD benchmark PR recreated from Martian Code Review Bench.

Source PR: calcom#22345
Dataset: cal_dot_com
Original title: feat: convert InsightsBookingService to use Prisma.sql raw queries

Fix: Update InsightsBookingService integration tests for Prisma.sql format

Summary

This PR updates the InsightsBookingService integration tests to work with the new Prisma.sql format that was previously converted in the service implementation. The changes follow the exact same testing patterns established in the InsightsRoutingService integration tests.

Key Changes:

  • Replace Prisma object notation expectations with direct Prisma.sql template literal comparisons
  • Add NOTHING_CONDITION = Prisma.sql1=0`` constant for consistency with other services
  • Update all authorization and filter condition tests to expect raw SQL instead of object structures
  • Modify the final integration test to use $queryRaw for actual database querying
  • Remove complex object notation assertions in favor of direct SQL comparisons

Review & Testing Checklist for Human

⚠️ HIGH RISK - This involves SQL query construction and authorization logic

  • Run integration tests manually - I encountered test runner configuration issues, so please verify the tests actually pass: TZ=UTC yarn test packages/lib/server/service/__tests__/insightsBooking.integration-test.ts
  • Review SQL query construction - Carefully examine the authorization conditions in the service, especially complex team/org scope logic with AND/OR combinations
  • Verify authorization security - Test with different user roles (owner, admin, member) to ensure no privilege escalation bugs
  • Check parameter binding - Ensure all user inputs in the SQL templates are properly parameterized (no injection risks)
  • Test edge cases - Verify behavior with empty teams, missing users, null values, and boundary conditions

Recommended Test Plan:

  1. Run the integration tests locally to verify they pass
  2. Test the service with various user roles and team configurations
  3. Verify the $queryRaw integration test actually queries the database correctly
  4. Check that authorization conditions properly restrict access based on user permissions

Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    Service["packages/lib/server/service/<br/>insightsBooking.ts"]:::context
    Tests["packages/lib/server/service/__tests__/<br/>insightsBooking.integration-test.ts"]:::major-edit
    RoutingTests["InsightsRoutingService<br/>integration tests<br/>(attachment example)"]:::context
    
    Service --> Tests
    RoutingTests --> Tests
    
    Tests --> AuthTests["Authorization Condition Tests<br/>- NOTHING_CONDITION<br/>- User/Team/Org scope"]:::major-edit
    Tests --> FilterTests["Filter Condition Tests<br/>- Event type filtering<br/>- Member user filtering"]:::major-edit
    Tests --> IntegrationTest["Database Integration Test<br/>- $queryRaw usage<br/>- Real query execution"]:::major-edit
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • This conversion was based on the InsightsRoutingService integration test patterns provided in the user's attachment
  • The test runner had configuration issues during development, so manual verification of test execution is critical
  • All test expectations now use direct Prisma.sql comparisons instead of inspecting .strings and .values properties
  • The NOTHING_CONDITION constant ensures consistency across different insight services

Session Info: Requested by @eunjae-lee | Session: https://app.devin.ai/sessions/a5e216ec6c364679a220f799e2abc700

eunjae-lee and others added 12 commits July 9, 2025 11:14
- 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>
- 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>
…ormat

- 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>
- 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>
…ithub.com:calcom/cal.com into devin/convert-insights-booking-service-1752054886
@khaliqgant khaliqgant changed the base branch from main to msd-benchmark-base/cal_dot_com/pr-22345 May 15, 2026 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants