Skip to content

Conversation

@eunjae-lee
Copy link
Contributor

What does this PR do?

Fixes integration test failures by correcting the Prisma filter syntax in the isAdmin guard for checking organization admin permissions.

The issue was that the guard was filtering on the one-to-one organizationSettings relation without using the required is wrapper, causing the query to return no matches even when valid org admins with isAdminAPIEnabled: true existed.

Root cause: Prisma requires the is wrapper when filtering on one-to-one relations. The query was using:

organizationSettings: {
  isAdminAPIEnabled: true,
}

But should be:

organizationSettings: {
  is: {
    isAdminAPIEnabled: true,
  },
}

Impact: This fixes 5 failing integration tests:

  • apps/api/v1/test/lib/utils/isAdmin.integration-test.ts - Org admin recognition
  • apps/api/v1/test/lib/utils/retrieveScopedAccessibleUsers.integration-test.ts - User scoping (2 tests)
  • apps/api/v1/test/lib/bookings/_get.integration-test.ts - Org admin booking access
  • apps/api/v1/test/lib/bookings/[id]/_patch.integration-test.ts - Booking modification permissions

Link to Devin run: https://app.devin.ai/sessions/fe59aee7f5494dc888ecbd3299e65b63
Requested by: eunjae@cal.com (@eunjae-lee)

How should this be tested?

Run the integration tests to verify they now pass:

TZ=UTC yarn test --project=IntegrationTests

Specifically check that these tests pass:

  • isAdmin guard correctly recognizes org admins when isAdminAPIEnabled: true
  • retrieveScopedAccessibleUsers returns expected user counts for org admins
  • Org admins can access and modify bookings within their organization

Mandatory Tasks

  • I have self-reviewed the code
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change - N/A (internal bug fix, no API changes)
  • I confirm automated tests are in place that prove my fix is effective - Yes, the failing integration tests will verify the fix

Checklist

  • My code follows the style guidelines of this project
  • I have checked if my changes generate no new warnings (verified via type-check, same pre-existing errors on main)

Important Notes for Reviewers

⚠️ I was unable to run the integration tests locally due to vitest configuration issues, so this fix is based on:

  1. Analysis of the Prisma schema confirming organizationSettings is a one-to-one relation
  2. Understanding that Prisma requires the is wrapper for one-to-one relation filters
  3. Verification that the test setup correctly configures isAdminAPIEnabled: true in fixtures

Please verify:

  • All 5 integration tests now pass in CI
  • The Prisma filter syntax with is wrapper is correct for one-to-one relations
  • No regressions in other admin permission checks

The isAdmin guard was filtering on the one-to-one organizationSettings relation
without using the 'is' wrapper, which caused the query to not match any records.
This fix adds the proper Prisma filter syntax for one-to-one relations.

Fixes integration tests:
- isAdmin.integration-test.ts
- retrieveScopedAccessibleUsers.integration-test.ts
- bookings/_get.integration-test.ts
- bookings/[id]/_patch.integration-test.ts

Co-Authored-By: eunjae@cal.com <hey@eunjae.dev>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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

Labels

consumer core area: core, team members only size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants