feat: convert InsightsBookingService to use Prisma.sql raw queries

[zaibkhan]

No description provided.

[codoki-pr-intelligence]

Codoki PR Review

Summary: Fix org member fetch gating, prevent missing user bookings
What’s good: Clear separation of auth vs filter conditions, consistent caching, and safe parameterization with Prisma.sql; tests now execute a real $queryRaw path for end-to-end validation.
Review Status: ❌ Requires changes
Overall Priority: High

Issues (Critical & High only)

Severity	Issue	Why it matters
High	Correctness — Org scope misses user bookings when org has no child teams	…/service/insightsBooking.ts Org scope incorrectly skips fetching org-level members when there are no child teams, causing user bookings (isTeamBooking=false) to be omitted. Always fetch memberships for teamIds (which already include orgId) to cover both org-only and child-team scenarios.

Showing top 1 issues. Critical: 0, High: 1. See inline suggestions for more.

Key Feedback (click to expand)

• Needs improvement: In org scope, user membership fetch is gated by presence of child teams, which omits org-level members when there are no child teams; remove the gate to always include org members.
• Testing: Add an integration test for org scope where the organization has zero child teams but has org-level members, asserting user bookings (isTeamBooking=false) are included; this would catch the current omission.
• Documentation: Consider a short JSDoc on getBaseConditions describing its return type (Prisma.Sql) and expected usage with $queryRaw to avoid misuse.
• Compatibility: Direct SQL with quoted identifiers ('"userId"', '"teamId"') and ANY() binds the implementation to Postgres semantics; if multi-database support is a goal, consider keeping a model-level wrapper or using IN with Prisma.join for broader compatibility.
• Performance: The OR between team bookings and user bookings can inhibit index usage; if performance becomes an issue at scale, consider using UNION ALL on two targeted queries instead of an OR.
• Security: Use of Prisma.sql with interpolated values is parameterized and avoids injection; keep avoiding Prisma.raw for user-sourced values. Filters are not validated via zod—if filters become user-facing, add validation to align with the options schema.
• Open questions: Do we intend this service to be Postgres-specific long-term? If not, should we expose a model-aware API (or keep a findMany wrapper) to avoid leaking SQL dialect assumptions to callers?

Confidence: 3/5 — Needs work before merge (1 high · status: Requires changes)

Sequence Diagram

sequenceDiagram
    participant Caller
    participant InsightsBookingService
    participant TeamRepository
    participant MembershipRepository
    Caller->>InsightsBookingService: getBaseConditions()
    InsightsBookingService->>InsightsBookingService: getAuthorizationConditions()
    alt options invalid or not owner/admin
        InsightsBookingService-->>Caller: NOTHING_CONDITION
    else user scope
        InsightsBookingService-->>Caller: ("userId" = ? AND "teamId" IS NULL)
    else org scope
        InsightsBookingService->>TeamRepository: findAllByParentId(orgId)
        TeamRepository-->>InsightsBookingService: teamsFromOrg
        InsightsBookingService->>MembershipRepository: findAllByTeamIds(teamIds)
        MembershipRepository-->>InsightsBookingService: userIdsFromOrg
        InsightsBookingService-->>Caller: team OR user conditions
    else team scope
        InsightsBookingService->>TeamRepository: findByIdAndParentId(teamId, orgId)
        TeamRepository-->>InsightsBookingService: childTeamOfOrg
        InsightsBookingService->>MembershipRepository: findAllByTeamIds([teamId])
        MembershipRepository-->>InsightsBookingService: userIdsFromTeam
        InsightsBookingService-->>Caller: team OR user conditions
    end
    Caller->>InsightsBookingService: getFilterConditions()
    InsightsBookingService-->>Caller: filterSql|null
    Caller-->>Caller: Combine with AND when both present

Loading

React with 👍 or 👎 if you found this review useful.