fix: allow org admin to accept/reject booking#23164
Conversation
WalkthroughAdds organization-admin authorization to the booking confirm/decline flow by extending confirm.handler.ts with checks that allow a logged-in user who is OWNER or ADMIN of an organization (root org or its sub-teams) to confirm/decline bookings for users belonging to that org or its teams. Adds helper Prisma queries to resolve org IDs where the actor is admin and to verify booking-user membership. Expands e2e tests to create an org with billing, provision an org-admin managed user, create an event type requiring confirmation, and verify accept/decline behavior and rejection for non-org users. 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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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). (1)
✨ 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
dosubot
bot
added
bookings
…-as-org-admin-managed-user
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/18/25)1 reviewer was added to this PR based on Keith Williams's automation. |
![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)
packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (2)
450-483: Reduce DB round-trips and align Prisma usage with minimal selectsYou can collapse the two membership existence checks into a single query and avoid fetching unnecessary columns by using
select: { id: true }. Also, if available on the Team model, filtering org roots viaisOrganization: trueincreases correctness (prevents personal teams from being treated as org roots).Apply this refactor:
-async function isLoggedInUserOrgAdminOfBookingUser(loggedInUserId: number, bookingUserId: number) { +async function isLoggedInUserOrgAdminOfBookingUser( + loggedInUserId: number, + bookingUserId: number +): Promise<boolean> { const orgIdsWhereLoggedInUserAdmin = await getOrgIdsWhereAdmin(loggedInUserId); if (orgIdsWhereLoggedInUserAdmin.length === 0) { return false; } - const bookingUserOrgMembership = await prisma.membership.findFirst({ - where: { - userId: bookingUserId, - teamId: { - in: orgIdsWhereLoggedInUserAdmin, - }, - team: { - parentId: null, - }, - }, - }); - - if (bookingUserOrgMembership) return true; - - const bookingUserOrgTeamMembership = await prisma.membership.findFirst({ - where: { - userId: bookingUserId, - team: { - parentId: { - in: orgIdsWhereLoggedInUserAdmin, - }, - }, - }, - }); - - return !!bookingUserOrgTeamMembership; + const membership = await prisma.membership.findFirst({ + where: { + userId: bookingUserId, + OR: [ + // Booking user belongs to the same root org + { + teamId: { in: orgIdsWhereLoggedInUserAdmin }, + team: { parentId: null, isOrganization: true }, + }, + // Booking user belongs to a direct sub-team of one of those orgs + { + team: { parentId: { in: orgIdsWhereLoggedInUserAdmin } }, + }, + ], + }, + select: { id: true }, + }); + + return !!membership; }
485-502: Constrain “admin orgs” to actual organizations and annotate return typeGood use of
selectto return onlyteamId. Consider also constraining to org roots explicitly (if supported by schema) and annotate the return type for clarity.-async function getOrgIdsWhereAdmin(loggedInUserId: number) { +async function getOrgIdsWhereAdmin(loggedInUserId: number): Promise<number[]> { const loggedInUserOrgMemberships = await prisma.membership.findMany({ where: { userId: loggedInUserId, role: { in: [MembershipRole.OWNER, MembershipRole.ADMIN], }, team: { - parentId: null, + parentId: null, + isOrganization: true, }, }, select: { teamId: true, }, }); return loggedInUserOrgMemberships.map((m) => m.teamId); }apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (3)
298-309: Event type requiring confirmation setup: consider asserting the flagYou create an event type with
requiresConfirmation: truebut don’t assert it. A tiny assertion would make this intent explicit and protect against fixture changes.Example addition after creation:
- expect(eventTypeRequiresConfirmation.requiresConfirmation).toBe(true)
716-778: Negative path: consider adding decline-unauthorized case as wellYou assert 401 for confirm against a non-org regular user. Consider mirroring for decline to fully cover both endpoints’ authorization behavior.
Would you like me to add a concise test that posts to
/v2/bookings/:uid/declinewith the org-admin token and expects 401?
780-789: Optional: clean up the repo-created bookings to keep DB tidy across runsThe tests create bookings directly via the repository but don’t delete them. While teardown likely cascades via team/user cleanup, explicit deletion reduces coupling and potential leftovers.
Options:
- Delete each created booking at the end of its test:
- await bookingsRepositoryFixture.deleteById(bookingRequiringConfirmation.id)
- Or keep track of created booking IDs and delete them here in
afterAll.If you want, I can draft a small helper to collect created booking IDs and clean them up centrally.
📜 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 sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts(7 hunks)packages/trpc/server/routers/viewer/bookings/confirm.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:
packages/trpc/server/routers/viewer/bookings/confirm.handler.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.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:
packages/trpc/server/routers/viewer/bookings/confirm.handler.tsapps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts
🧬 Code Graph Analysis (2)
packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (1)
packages/platform/libraries/index.ts (2)
TRPCError(56-56)MembershipRole(98-98)
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (6)
apps/api/v2/test/fixtures/repository/bookings.repository.fixture.ts (1)
BookingsRepositoryFixture(8-44)apps/api/v2/test/utils/randomString.ts (1)
randomString(3-6)apps/api/v2/src/modules/oauth-clients/controllers/oauth-client-users/outputs/create-managed-user.output.ts (2)
CreateManagedUserData(9-16)CreateManagedUserOutput(18-31)apps/api/v2/src/modules/users/inputs/create-managed-user.input.ts (1)
CreateManagedUserInput(10-79)packages/platform/constants/api.ts (3)
SUCCESS_STATUS(9-9)CAL_API_VERSION_HEADER(72-72)VERSION_2024_08_13(59-59)packages/platform/types/bookings/2024-08-13/outputs/booking.output.ts (1)
BookingOutput_2024_08_13(280-306)
⏰ 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 (8)
packages/trpc/server/routers/viewer/bookings/confirm.handler.ts (1)
443-445: Org-admin authorization early-exit looks correct and safely placedGood addition. This grants access when the logged-in user is an OWNER/ADMIN of the organization (or its direct sub-team) the booking user belongs to, while preserving existing auth paths and error messaging.
apps/api/v2/src/ee/bookings/2024-08-13/controllers/e2e/managed-user-bookings.e2e-spec.ts (7)
18-18: Repository fixture import is appropriate for direct booking setupSwitching to
BookingsRepositoryFixtureenables precise test setup for confirm/decline flows. Looks good.
56-57: Fixture wiring LGTMInstantiating
BookingsRepositoryFixturealongside the others follows the existing testing pattern.
103-110: Org billing provisioning in fixtures is fineProvisioning
platformBillingas part of org setup keeps tests realistic for platform orgs. No issues.
73-75: Org-admin managed user test identity: good separationDefining a dedicated org-admin managed user keeps test responsibilities clear and isolated from platform admin. LGTM.
258-297: Org-admin managed user creation and membership assignment look correct
- Managed user creation via OAuth flow is validated properly.
- Assigning
ADMINrole to the organization establishes the required authorization state.
611-663: Positive path: org-admin can confirm managed user’s bookingEnd-to-end flow is comprehensive:
- Creates a PENDING booking directly.
- Confirms via API using the org-admin managed user token.
- Asserts API response and persistence layer status.
Nice coverage.
664-715: Positive path: org-admin can reject managed user’s bookingMirrors the confirm case and validates both transport and DB state transitions to REJECTED. Solid.
E2E results are ready! |
ibex088
deleted the
lauris/cal-6272-feat-confirm-decline-booking-as-org-admin-managed-user
branch
4 participants
Linear CAL-6272