Conversation
WalkthroughReplaces a locally defined UpdatePrivateLinkBody DTO in the event-types private-links controller with the exported UpdatePrivateLinkBody from @calcom/platform-types and removes OmitType usage. Adds UpdatePrivateLinkBody to platform types with optional expiresAt and maxUsageCount fields and validations. Introduces an E2E test for organization bookings covering manager and managed organizations scenarios and adds a fixture helper to create managed-organization relations. Adjusts bookings retrieval logic to include organization-scoped memberships when user.orgId is present. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
keithwillcode
added
core
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/14/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/14/25)1 label was added to this PR based on Keith Williams's automation. |
dosubot
bot
added
bookings
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/api/v2/test/fixtures/repository/managed-organizations.repository.fixture.ts (1)
38-45: LGTM: Useful fixture for seeding managed org relationshipsThe helper is straightforward and does exactly what's needed for tests.
Minor nitpick: to align with the "select-only what you need" guideline (even in tests), you can restrict the returned fields.
- async createManagedOrganization(managerOrganizationId: number, managedOrganizationId: number) { - return this.prismaWriteClient.managedOrganization.create({ - data: { - managerOrganizationId, - managedOrganizationId, - }, - }); - } + async createManagedOrganization(managerOrganizationId: number, managedOrganizationId: number) { + return this.prismaWriteClient.managedOrganization.create({ + data: { + managerOrganizationId, + managedOrganizationId, + }, + select: { + managerOrganizationId: true, + managedOrganizationId: true, + }, + }); + }packages/platform/types/event-types/inputs/private-link.input.ts (1)
62-83: Centralizing UpdatePrivateLinkBody in platform-types is the right moveThe exported DTO with validations is clean and reusable. One small Swagger consistency nit: the example shows an ISO string; consider annotating the type as String for better OpenAPI consumers compatibility (same as CreatePrivateLinkInput).
@ApiPropertyOptional({ - description: "New expiration date for time-based links", - type: Date, + description: "New expiration date for time-based links", + type: String, example: "2024-12-31T23:59:59.000Z", })apps/api/v2/src/modules/organizations/bookings/managed-organizations-bookings.controller.e2e-spec.ts (3)
481-499: Ensure OAuth headers are consistently provided (test parity and determinism)In the first and third tests you pass X_CAL_CLIENT_ID and X_CAL_SECRET_KEY; here you don’t. If the endpoint requires client credentials in real deployments, this can hide issues and reduce test fidelity. Recommend adding them for consistency.
return request(app.getHttpServer()) .get(`/v2/organizations/${managerOrganization.id}/bookings?userIds=${managerOrgUser2.id}`) .set(CAL_API_VERSION_HEADER, VERSION_2024_08_13) + .set(X_CAL_CLIENT_ID, oAuthClient.id) + .set(X_CAL_SECRET_KEY, oAuthClient.secret) .expect(200)
541-549: Tighten teardown to avoid cross-test leakageCleanup misses several entities created during setup (managed org, the second org user, managed org user, and their bookings/event types). This can cause flakiness when tests share DB state.
Consider extending afterAll:
afterAll(async () => { await oauthClientRepositoryFixture.delete(oAuthClient.id); - await teamRepositoryFixture.delete(managerOrganization.id); + await teamRepositoryFixture.delete(managerOrganization.id); + await teamRepositoryFixture.delete(managedOrganization.id); await userRepositoryFixture.deleteByEmail(managerOrgUser1.email); + await userRepositoryFixture.deleteByEmail(managerOrgUser2.email); + await userRepositoryFixture.deleteByEmail(managedOrgUser.email); await userRepositoryFixture.deleteByEmail(nonOrgUser1.email); await bookingsRepositoryFixture.deleteAllBookings(managerOrgUser1.id, managerOrgUser1.email); + await bookingsRepositoryFixture.deleteAllBookings(managerOrgUser2.id, managerOrgUser2.email); + await bookingsRepositoryFixture.deleteAllBookings(managedOrgUser.id, managedOrgUser.email); await bookingsRepositoryFixture.deleteAllBookings(nonOrgUser1.id, nonOrgUser1.email); + // Optional: remove event types to ensure no orphaned data if cascading is not configured + // await eventTypesRepositoryFixture.delete(managerOrgEventTypeId); + // await eventTypesRepositoryFixture.delete(nonOrgEventTypeId); + // await eventTypesRepositoryFixture.delete(managedOrgEventTypeId); await app.close(); });
322-325: Good: explicit seeding of managed-organization relationThis ensures the manager→managed linkage exists for the test scenarios that validate scoping. Consider asserting the relation exists (via fixture.getManagedOrganization) to fail fast if seeding breaks in future refactors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/api/v2/src/ee/event-types-private-links/controllers/event-types-private-links.controller.ts(1 hunks)apps/api/v2/src/modules/organizations/bookings/managed-organizations-bookings.controller.e2e-spec.ts(1 hunks)apps/api/v2/test/fixtures/repository/managed-organizations.repository.fixture.ts(1 hunks)packages/platform/types/event-types/inputs/private-link.input.ts(2 hunks)packages/trpc/server/routers/viewer/bookings/get.handler.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/modules/organizations/bookings/managed-organizations-bookings.controller.e2e-spec.tspackages/trpc/server/routers/viewer/bookings/get.handler.tsapps/api/v2/test/fixtures/repository/managed-organizations.repository.fixture.tspackages/platform/types/event-types/inputs/private-link.input.tsapps/api/v2/src/ee/event-types-private-links/controllers/event-types-private-links.controller.ts
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/modules/organizations/bookings/managed-organizations-bookings.controller.e2e-spec.tspackages/trpc/server/routers/viewer/bookings/get.handler.tsapps/api/v2/test/fixtures/repository/managed-organizations.repository.fixture.tspackages/platform/types/event-types/inputs/private-link.input.tsapps/api/v2/src/ee/event-types-private-links/controllers/event-types-private-links.controller.ts
🧠 Learnings (2)
📚 Learning: 2025-07-16T05:10:22.891Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/prisma/schema.prisma:1068-1071
Timestamp: 2025-07-16T05:10:22.891Z
Learning: In PR #22304 for Cal.com private link expiration features, the `maxUsageCount` field was intentionally set to default to 1 (non-nullable) as a breaking change, making all existing private links single-use after migration. This was a deliberate design decision by alishaz-polymath.
Applied to files:
packages/platform/types/event-types/inputs/private-link.input.ts
📚 Learning: 2025-07-16T06:42:27.024Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
Applied to files:
packages/platform/types/event-types/inputs/private-link.input.ts
🧬 Code Graph Analysis (1)
apps/api/v2/src/modules/organizations/bookings/managed-organizations-bookings.controller.e2e-spec.ts (11)
apps/api/v2/test/fixtures/repository/bookings.repository.fixture.ts (1)
BookingsRepositoryFixture(8-44)apps/api/v2/test/fixtures/repository/event-types.repository.fixture.ts (1)
EventTypesRepositoryFixture(9-62)apps/api/v2/test/fixtures/repository/oauth-client.repository.fixture.ts (1)
OAuthClientRepositoryFixture(8-64)apps/api/v2/test/fixtures/repository/membership.repository.fixture.ts (1)
MembershipRepositoryFixture(6-38)apps/api/v2/test/fixtures/repository/hosts.repository.fixture.ts (1)
HostsRepositoryFixture(7-27)apps/api/v2/test/fixtures/repository/profiles.repository.fixture.ts (1)
ProfileRepositoryFixture(6-30)apps/api/v2/test/fixtures/repository/managed-organizations.repository.fixture.ts (1)
ManagedOrganizationsRepositoryFixture(5-46)apps/api/v2/src/app.ts (1)
bootstrap(27-89)apps/api/v2/test/utils/randomString.ts (1)
randomString(3-6)packages/platform/constants/api.ts (5)
CAL_API_VERSION_HEADER(72-72)VERSION_2024_08_13(59-59)X_CAL_CLIENT_ID(50-50)X_CAL_SECRET_KEY(49-49)SUCCESS_STATUS(9-9)packages/platform/types/bookings/2024-08-13/outputs/booking.output.ts (3)
BookingOutput_2024_08_13(280-306)RecurringBookingOutput_2024_08_13(308-323)GetSeatedBookingOutput_2024_08_13(325-331)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Atoms E2E Tests
🔇 Additional comments (4)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)
102-113: Approve: admin/owner membership scoping is correctThe OR clause in packages/trpc/server/routers/viewer/bookings/get.handler.ts correctly limits admin/owner memberships to the user’s organization (teamId === orgId or team.parentId === orgId) and matches the PR objective. I also verified getBookings/getAllUserBookings are exercised by organization booking controllers and e2e tests (apps/api/v2/... and packages/trpc/server/...), so the path is covered.
Optional hardening — exclude pending/invited memberships by requiring accepted: true:
const membershipIdsWhereUserIsAdminOwner = ( await prisma.membership.findMany({ where: { userId: user.id, + accepted: true, role: { in: ["ADMIN", "OWNER"], }, ...(user.orgId && { OR: [ { teamId: user.orgId, }, { team: { parentId: user.orgId, }, }, ], }), }, select: { id: true, }, }) ).map((membership) => membership.id);
- File to note: packages/trpc/server/routers/viewer/bookings/get.handler.ts (membership.findMany block above).
packages/platform/types/event-types/inputs/private-link.input.ts (1)
21-22: Doc wording aligns with intended default behaviorThe description clarifies the default single-use behavior when omitted, matching prior decisions noted in PR #22304. Good consistency.
apps/api/v2/src/ee/event-types-private-links/controllers/event-types-private-links.controller.ts (1)
7-18: Import cleanup and type source-of-truth migration look goodSwitching to UpdatePrivateLinkBody from platform-types and removing OmitType usage simplifies the controller and reduces duplication. No functional changes introduced; signatures remain stable.
apps/api/v2/src/modules/organizations/bookings/managed-organizations-bookings.controller.e2e-spec.ts (1)
75-112: Test bootstrap and guard overrides are appropriateThe usage of withApiAuth, guard overrides, and late app.init() is standard for this suite. Good separation of seeding helpers before app initialization.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
apps/api/v2/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Atoms E2E Tests
supalarry
deleted the
lauris/cal-6250-fix-managed-organization-bookings
branch
E2E results are ready! |
3 participants
Linear CAL-6250