perf: move to disable prisma client extension inference#23692
Conversation
WalkthroughThe PR standardizes Prisma usage and JSON typing across the repo: switches default Prisma imports to named Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
dosubot
bot
added
the
🧹 Improvements
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (16)
packages/lib/server/repository/delegationCredential.ts (2)
230-263: Useselect(neverinclude) to comply with Prisma query guidelinesSwitch this query to explicit
selectand reuse the existing safe select to avoid overfetching.- const delegationCredentials = await prisma.delegationCredential.findMany({ - where: - { enabled: true }, - include: { - workspacePlatform: { select: { slug: true } }, - organization: { - include: { - members: { - select: { - id: true, - userId: true, - user: { select: { id: true, email: true } }, - accepted: true, - }, - where: { accepted: true }, - }, - }, - }, - }, - }); + const delegationCredentials = await prisma.delegationCredential.findMany({ + where: { enabled: true }, + select: { + ...delegationCredentialSafeSelect, + organization: { + select: { + members: { + select: { + id: true, + userId: true, + user: { select: { id: true, email: true } }, + accepted: true, + }, + where: { accepted: true }, + }, + }, + }, + }, + });
284-316: ReplaceincludewithselectindelegationCredential.findMany
Use the existingdelegationCredentialSafeSelectfor top-level fields and add an explicitorganization.selectto limit the payload:const delegationCredentials = await prisma.delegationCredential.findMany({ where: { enabled: false }, - include: { - workspacePlatform: { select: { slug: true } }, - organization: { - include: { - members: { - select: { - id: true, - userId: true, - user: { select: { id: true, email: true } }, - accepted: true, - }, - where: { accepted: true }, - }, - }, - }, - }, + select: { + ...delegationCredentialSafeSelect, + organization: { + select: { + members: { + select: { + id: true, + userId: true, + user: { select: { id: true, email: true } }, + accepted: true, + }, + where: { accepted: true }, + }, + }, + }, + }, });Also, two other Prisma calls still use
includein:
- apps/web/app/api/auth/two-factor/totp/setup/route.ts
- apps/web/app/api/auth/two-factor/totp/disable/route.ts
Convert those to explicit
selectas well to comply with the repository’s “noinclude” rule.packages/app-store/nextcloudtalk/lib/VideoApiAdapter.ts (1)
75-83: Fix impossible status-range condition.
(response.status < 200 && response.status >= 300)is always false. Either rely on!response.okor correct the range check.- if (!response.ok || (response.status < 200 && response.status >= 300)) { + if (!response.ok || (response.status < 200 || response.status >= 300)) {Also applies to: 88-96
packages/lib/server/repository/membership.ts (4)
166-221: Replace deep include chains with select to reduce payload and follow guidelines.Per repo guidance, avoid
include. Convert toselectat each level.- return await prisma.membership.findMany({ - where: prismaWhere, - include: { - team: { - include: { + return await prisma.membership.findMany({ + where: prismaWhere, + select: { + team: { + select: { members: { select: membershipSelect, }, parent: { select: teamParentSelect, }, eventTypes: { - select: { + select: { ...eventTypeSelect, hashedLink: true, users: { select: userSelect }, children: { - include: { - users: { select: userSelect }, - }, + select: { users: { select: userSelect } }, }, hosts: { - include: { - user: { select: userSelect }, - }, + select: { user: { select: userSelect } }, }, team: { - select: { + select: { id: true, members: { - select: { - user: { - select: { - timeZone: true, - }, - }, - }, + select: { user: { select: { timeZone: true } } }, take: 1, }, }, }, }, orderBy: [ { position: "desc" }, { id: "asc" }, ], }, }, }, }, });
292-304: Use select instead of include for team.- return await prisma.membership.findMany({ - where: prismaWhere, - include: { - team: { - include: { - parent: { select: teamParentSelect }, - }, - }, - }, - }); + return await prisma.membership.findMany({ + where: prismaWhere, + select: { + team: { select: { parent: { select: teamParentSelect } } }, + }, + });
317-330: Switch top-level include to select.- const memberships = await prisma.membership.findMany({ - where: { teamId }, - include: { - user: { - select: { - credentials: { select: credentialForCalendarServiceSelect }, - ...availabilityUserSelect, - }, - }, - }, - }); + const memberships = await prisma.membership.findMany({ + where: { teamId }, + select: { + user: { + select: { + credentials: { select: credentialForCalendarServiceSelect }, + ...availabilityUserSelect, + }, + }, + }, + });
429-445: Use select for user fields in recent-members query.- return prisma.membership.findMany({ + return prisma.membership.findMany({ where: { teamId: organizationId, createdAt: { gt: time }, accepted: true, }, - include: { - user: { - select: { email: true, name: true, id: true }, - }, - }, + select: { + user: { select: { email: true, name: true, id: true } }, + }, });packages/prisma/extensions/usage-tracking.ts (1)
18-35: RenameusageTrackingExtentiontousageTrackingExtension
- In
packages/prisma/extensions/usage-tracking.ts, update the export signature.- In
packages/prisma/index.ts, update the import at line 8 and the two calls at lines 43 and 59.packages/features/insights/server/trpc-router.ts (4)
672-683: Replace include with select to comply with our Prisma guideline (no include).Using include inflates payloads. Select only what’s needed from the relation.
- include: { - team: { - select: { - id: true, - name: true, - logoUrl: true, - slug: true, - metadata: true, - }, - }, - }, + select: { + team: { + select: { + id: true, + name: true, + logoUrl: true, + slug: true, + metadata: true, + }, + }, + },
706-720: Use select instead of include when fetching users.Keep responses minimal per repo guideline.
- include: { - user: { - select: userSelect, - }, - }, + select: { + user: { + select: userSelect, + }, + },
733-738: Use select over include for membership lookup.We only need the user fields for the caller.
- include: { - user: { - select: userSelect, - }, - }, + select: { + user: { + select: userSelect, + }, + },
753-759: Use select over include when listing users in a team.- include: { - user: { - select: userSelect, - }, - }, + select: { + user: { + select: userSelect, + }, + },packages/lib/server/repository/booking.ts (4)
220-233: Select only attendee.email to reduce payload.attendees: true returns full rows; you only need email for filtering/removal.
- attendees: true, + attendees: { + select: { + email: true, + }, + },
342-350: Narrow relation selection and avoid selecting entire JSON blobs by default.Only chosenRouteId and response are used; also narrow attendees to email only.
- select: { - id: true, - attendees: true, - userId: true, - createdAt: true, - status: true, - startTime: true, - routedFromRoutingFormReponse: true, - }, + select: { + id: true, + attendees: { select: { email: true } }, + userId: true, + createdAt: true, + status: true, + startTime: true, + routedFromRoutingFormReponse: { + select: { + chosenRouteId: true, + response: true, + }, + }, + },
358-377: Fix unsafe access and order-sensitive array comparison in routing-form filtering.
- Potential runtime crash if response or field is missing.
- Array equality currently depends on order; use set equality.
- if (virtualQueuesData) { - queueBookings = allBookings.filter((booking) => { - const responses = booking.routedFromRoutingFormReponse; - const fieldId = virtualQueuesData.fieldOptionData.fieldId; - const selectedOptionIds = virtualQueuesData.fieldOptionData.selectedOptionIds; - - const response = responses?.response as FormResponse; - - const responseValue = response[fieldId].value; - - if (Array.isArray(responseValue) && Array.isArray(selectedOptionIds)) { - //check if all values are the same (this only support 'all in' not 'any in') - return ( - responseValue.length === selectedOptionIds.length && - responseValue.every((value, index) => value === selectedOptionIds[index]) - ); - } else { - return responseValue === selectedOptionIds; - } - }); - } + if (virtualQueuesData) { + queueBookings = allBookings.filter((booking) => { + const responses = booking.routedFromRoutingFormReponse; + const { fieldId, selectedOptionIds } = virtualQueuesData.fieldOptionData; + const response = (responses?.response as FormResponse | undefined) || undefined; + const field = response?.[fieldId]; + if (!field) return false; + const responseValue = field.value; + + if (Array.isArray(responseValue)) { + const selected = Array.isArray(selectedOptionIds) ? selectedOptionIds : [selectedOptionIds]; + const a = new Set(responseValue.map(String)); + const b = new Set(selected.map(String)); + if (a.size !== b.size) return false; + for (const v of a) if (!b.has(v)) return false; + return true; + } + return String(responseValue) === String(selectedOptionIds); + }); + }
455-460: Avoid selecting user.credentials with true; never risk exposing credential.key.Reuse credentialForCalendarServiceSelect to whitelist fields.
- user: { - select: { - credentials: true, - }, - }, + user: { + select: { + credentials: { select: credentialForCalendarServiceSelect }, + }, + },
🧹 Nitpick comments (29)
apps/web/test/lib/plainTiers.test.ts (1)
79-81: Plan Prisma 6.7 upgrade to migrate ts-expect-error mocks to mockDeep
Suppressions remain in several tests under apps/api and apps/web. Once we upgrade to Prisma 6.7.0, replace these ad-hocts-expect-errormocks with a shared DeepMocked (viamockDeep) and remove the suppressions to keep tests type-safe and avoid drift.apps/api/v1/test/lib/event-types/[id]/_get.test.ts (1)
87-93: Queue removal of ts-expect-error after Prisma 6.7 mockDeep upgrade
Suppressing the type error is acceptable now; once Prisma 6.7 introducesmockDeep, replace these with typed deep mocks.
Occurrences:
- apps/api/v1/test/lib/event-types/[id]/_get.test.ts:87
- apps/api/v1/test/lib/event-types/[id]/_delete.test.ts:53
apps/api/v1/test/lib/attendees/_post.test.ts (1)
80-81: Prefer a typed shape overanyfor mocked bookingUse a minimal typed shape to guard against accidental shape drift while keeping the mock flexible.
+import type { Booking } from "@prisma/client"; @@ - const userBooking = { id: 1 }; + const userBooking = { id: 1 } satisfies Partial<Booking>; - prismaMock.booking.findFirst.mockResolvedValue(userBooking as any); + prismaMock.booking.findFirst.mockResolvedValue(userBooking as any); // keep until mockDeep is inpackages/lib/server/repository/delegationCredential.ts (1)
100-102: Avoidunknown as Prisma.InputJsonValue; make encryption output typedReduce casts by returning Prisma.InputJsonValue from the encryption util (or assert with a narrow guard).
Apply one of:
- Preferred (in serviceAccountKey module outside this file):
// serviceAccountKey.ts import type { Prisma } from "@prisma/client"; export type EncryptedServiceAccountKey = Prisma.InputJsonValue; export function encryptServiceAccountKey(/* ... */): EncryptedServiceAccountKey { /* ... */ }
- Local guard (keeps this file self-contained):
- // z.passthrough() is not allowed in Prisma, but we know this is trusted. - serviceAccountKey: encryptedKey as unknown as Prisma.InputJsonValue, + // encryption returns a JSON-serializable object + serviceAccountKey: encryptedKey as Prisma.InputJsonValue,If encryption can yield non-object JSON (edge), add a runtime assert to ensure it’s JSON-serializable before write.
apps/api/v1/test/lib/event-types/[id]/_delete.test.ts (1)
53-58: Temporary ts-expect-error is acceptable; plan to remove with typed deep mocksKeep a follow-up to migrate this to a typed mock and delete the suppression.
apps/api/v1/test/lib/bookings/[id]/_delete.test.ts (1)
23-29: Minor: prefer a typed mock toas anyNon-blocking. Consider
satisfies Partial<User>(from @prisma/client) to keep shape-checked while you wait for mockDeep.apps/api/v1/test/lib/bookings/[id]/_patch.test.ts (1)
18-24: Same note: avoidanyin mocks where feasibleUse a minimal typed shape (
satisfies Partial<User>) and drop theanyonce mockDeep arrives.packages/app-store/nextcloudtalk/lib/VideoApiAdapter.ts (1)
100-109: Cast to Prisma.InputJsonValue is fine; consider validating JSON-serializability.The double-cast is acceptable, but a small helper to assert serializable JSON would prevent accidental Dates/Maps from slipping in.
packages/lib/server/repository/membership.ts (1)
488-490: Prefer enum values over string literals.Use
MembershipRole.ADMINandMembershipRole.OWNERfor consistency and type safety.- role: { - in: ["ADMIN", "OWNER"], - }, + role: { in: [MembershipRole.ADMIN, MembershipRole.OWNER] },packages/app-store/_utils/oauth/updateTokenObject.ts (1)
21-28: Unify casts: drop the intermediate unknown.Other code in this file casts directly to
Prisma.InputJsonValue. Mirror that here.- data: { - key: tokenObject as unknown as Prisma.InputJsonValue, - }, + data: { key: tokenObject as Prisma.InputJsonValue },packages/prisma/extensions/usage-tracking.ts (1)
2-2: Consider using the unified Prisma type import path.For consistency with the rest of the repo, import
PrismaClientfrom@calcom/prisma/client.apps/api/v1/test/lib/bookings/_get.test.ts (3)
28-34: Type loosening in mocks is acceptable as a stopgap.Track a follow-up to remove
as anyonce the Prisma 6.7.0 mock upgrade lands.
37-45: ts-expect-error is fine here; keep it temporary.Please add a TODO with the PR/issue reference for removal post-migration.
47-53: Same here: minimizeas anyusage in mocks over time.packages/features/insights/server/trpc-router.ts (3)
209-211: Avoid fetching entire membership rows; select only id.We only check existence; cut unused columns.
- const membership = await ctx.insightsDb.membership.findFirst({ - where: membershipWhereConditional, - }); + const membership = await ctx.insightsDb.membership.findFirst({ + where: membershipWhereConditional, + select: { id: true }, + });
220-225: Select only id for isChildTeamOfOrg check.- const isChildTeamOfOrg = await ctx.insightsDb.team.findFirst({ - where: { - id: parse.data.teamId, - parentId: ctx.user.organizationId, - }, - }); + const isChildTeamOfOrg = await ctx.insightsDb.team.findFirst({ + where: { + id: parse.data.teamId, + parentId: ctx.user.organizationId, + }, + select: { id: true }, + });
231-240: Select only id when verifying org membership role.- const membershipOrg = await ctx.insightsDb.membership.findFirst({ + const membershipOrg = await ctx.insightsDb.membership.findFirst({ where: { userId: ctx.user.id, teamId: ctx.user.organizationId, accepted: true, role: { in: ["OWNER", "ADMIN"], }, }, - }); + select: { id: true }, + });packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (1)
428-429: Prefer named exports over default exports.Improves tree-shaking and refactors.
-export default ZoomVideoApiAdapter; +export { ZoomVideoApiAdapter };apps/api/v1/pages/api/credential-sync/_patch.ts (1)
6-8: LGTM; minimal select avoids leaking credential.key.Casting key to Prisma.InputJsonValue is fine. Consider centralizing this cast in a small helper to DRY across endpoints.
Also applies to: 74-75
packages/lib/server/repository/booking.ts (1)
616-658: Consider returning minimal fields (or bookingMinimalSelect) for team booking queries.findMany without select returns full Booking rows; prefer bookingMinimalSelect or explicit selects for perf.
Would you like me to propose concrete selects for these calls based on their current consumers?
Also applies to: 793-814
apps/api/v1/test/lib/bookings/_post.test.ts (1)
473-476: Use BookingStatus enum instead of string literal for consistency.- prismaMock.booking.findUnique.mockResolvedValue({ - ...originalBooking, - status: "CANCELLED", - }); + prismaMock.booking.findUnique.mockResolvedValue({ + ...originalBooking, + status: BookingStatus.CANCELLED, + });Also applies to: 540-541
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts (1)
8-8: Remove unused Prisma import.Keeps helpers lean.
-import { Prisma } from "@calcom/prisma/client";packages/prisma/index.ts (7)
13-15: Make global typing accurate (optional)
baseClientcan be undefined on first load; type it as optional and preferglobalThisfor portability.Apply this diff:
-const globalForPrisma = global as unknown as { - baseClient: PrismaClient; -}; +const globalForPrisma = globalThis as unknown as { + baseClient?: PrismaClient; +};
38-40: Nit: prefer nullish coalescing for readability
Equivalent behavior, slightly clearer.-const baseClient = - globalForPrisma.baseClient || new PrismaClient(prismaOptions); +const baseClient = + globalForPrisma.baseClient ?? new PrismaClient(prismaOptions);
41-48: DRY the extension pipeline and centralize the cast
The $extends chain and theas unknown as PrismaClientcast are duplicated. Extract a helper to avoid drift and keep the “disable inference” cast in one place.+const applyExtensions = (c: PrismaClient): PrismaClient => + (c + .$extends(usageTrackingExtention(baseClient)) + .$extends(excludeLockedUsersExtension()) + .$extends(excludePendingPaymentsExtension()) + .$extends(bookingIdempotencyKeyExtension()) + .$extends(disallowUndefinedDeleteUpdateManyExtension())) as unknown as PrismaClient; + -export const customPrisma = (options?: Prisma.PrismaClientOptions) => - new PrismaClient({ ...prismaOptions, ...options }) - .$extends(usageTrackingExtention(baseClient)) - .$extends(excludeLockedUsersExtension()) - .$extends(excludePendingPaymentsExtension()) - .$extends(bookingIdempotencyKeyExtension()) - .$extends(disallowUndefinedDeleteUpdateManyExtension()) as unknown as PrismaClient; +export const customPrisma = (options?: Prisma.PrismaClientOptions) => + applyExtensions(new PrismaClient({ ...prismaOptions, ...options })); -export const prisma: PrismaClient = baseClient - .$extends(usageTrackingExtention(baseClient)) - .$extends(excludeLockedUsersExtension()) - .$extends(excludePendingPaymentsExtension()) - .$extends(bookingIdempotencyKeyExtension()) - .$extends(disallowUndefinedDeleteUpdateManyExtension()) as unknown as PrismaClient; +export const prisma: PrismaClient = applyExtensions(baseClient);Also applies to: 58-64
41-44: Confirm usage tracking on secondary clients
customPrismaalso getsusageTrackingExtention(baseClient). IfcustomPrismapoints to INSIGHTS/readonly DB, accidental writes would still record usage against the primary DB, even if the write fails. Confirm this is intended; otherwise consider disabling usage tracking forreadonlyPrisma.
51-51: Migrate bookingReferenceMiddleware to $extends
Align with the extension pipeline for consistent ordering and easier typing.
83-87: Prefer Prisma.TransactionClient over Omit<> shim
Use the official transaction client type to avoid drift and improve accuracy.Apply this diff:
-export type { - OmitPrismaClient as PrismaTransaction, - // we re-export the native PrismaClient type for backwards-compatibility. - PrismaClient -}; +// we re-export the native PrismaClient type for backwards-compatibility. +export type { PrismaClient }; +export type PrismaTransaction = Prisma.TransactionClient;Additionally remove the now-unused alias:
-type OmitPrismaClient = Omit< - PrismaClient, - "$connect" | "$disconnect" | "$on" | "$transaction" | "$use" | "$extends" ->;
92-92: Avoid default export
Our guidelines prefer named exports. Consider dropping the default export and updating imports.If you want, I can generate a repo-wide search to list default-import call sites for
@calcom/prisma.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (25)
apps/api/v1/pages/api/credential-sync/_patch.ts(2 hunks)apps/api/v1/pages/api/credential-sync/_post.ts(2 hunks)apps/api/v1/test/lib/attendees/_post.test.ts(1 hunks)apps/api/v1/test/lib/bookings/[id]/_delete.test.ts(1 hunks)apps/api/v1/test/lib/bookings/[id]/_patch.test.ts(1 hunks)apps/api/v1/test/lib/bookings/_get.test.ts(2 hunks)apps/api/v1/test/lib/bookings/_post.test.ts(11 hunks)apps/api/v1/test/lib/event-types/[id]/_delete.test.ts(1 hunks)apps/api/v1/test/lib/event-types/[id]/_get.test.ts(1 hunks)apps/web/playwright/lib/test-helpers/routingFormHelpers.ts(4 hunks)apps/web/test/lib/plainTiers.test.ts(1 hunks)packages/app-store/_utils/oauth/updateTokenObject.ts(2 hunks)packages/app-store/nextcloudtalk/lib/VideoApiAdapter.ts(2 hunks)packages/app-store/zoomvideo/lib/VideoApiAdapter.ts(2 hunks)packages/features/flags/features.repository.ts(0 hunks)packages/features/insights/server/trpc-router.ts(4 hunks)packages/lib/server/repository/booking.ts(4 hunks)packages/lib/server/repository/delegationCredential.ts(1 hunks)packages/lib/server/repository/membership.ts(1 hunks)packages/lib/server/repository/workspacePlatform.ts(3 hunks)packages/lib/server/service/InsightsBookingBaseService.ts(3 hunks)packages/lib/server/service/InsightsRoutingBaseService.ts(3 hunks)packages/lib/server/service/InsightsRoutingDIService.ts(2 hunks)packages/prisma/extensions/usage-tracking.ts(2 hunks)packages/prisma/index.ts(4 hunks)
💤 Files with no reviewable changes (1)
- packages/features/flags/features.repository.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/lib/server/repository/delegationCredential.tsapps/api/v1/test/lib/attendees/_post.test.tsapps/api/v1/pages/api/credential-sync/_post.tspackages/app-store/zoomvideo/lib/VideoApiAdapter.tsapps/web/test/lib/plainTiers.test.tsapps/api/v1/test/lib/event-types/[id]/_get.test.tsapps/api/v1/test/lib/event-types/[id]/_delete.test.tspackages/app-store/nextcloudtalk/lib/VideoApiAdapter.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/prisma/extensions/usage-tracking.tsapps/api/v1/pages/api/credential-sync/_patch.tspackages/lib/server/service/InsightsRoutingBaseService.tsapps/api/v1/test/lib/bookings/[id]/_delete.test.tspackages/lib/server/service/InsightsBookingBaseService.tsapps/api/v1/test/lib/bookings/[id]/_patch.test.tspackages/app-store/_utils/oauth/updateTokenObject.tsapps/api/v1/test/lib/bookings/_get.test.tspackages/lib/server/repository/workspacePlatform.tspackages/features/insights/server/trpc-router.tsapps/api/v1/test/lib/bookings/_post.test.tspackages/lib/server/repository/booking.tspackages/lib/server/repository/membership.tspackages/prisma/index.tsapps/web/playwright/lib/test-helpers/routingFormHelpers.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/lib/server/repository/delegationCredential.tsapps/api/v1/test/lib/attendees/_post.test.tsapps/api/v1/pages/api/credential-sync/_post.tspackages/app-store/zoomvideo/lib/VideoApiAdapter.tsapps/web/test/lib/plainTiers.test.tsapps/api/v1/test/lib/event-types/[id]/_get.test.tsapps/api/v1/test/lib/event-types/[id]/_delete.test.tspackages/app-store/nextcloudtalk/lib/VideoApiAdapter.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/prisma/extensions/usage-tracking.tsapps/api/v1/pages/api/credential-sync/_patch.tspackages/lib/server/service/InsightsRoutingBaseService.tsapps/api/v1/test/lib/bookings/[id]/_delete.test.tspackages/lib/server/service/InsightsBookingBaseService.tsapps/api/v1/test/lib/bookings/[id]/_patch.test.tspackages/app-store/_utils/oauth/updateTokenObject.tsapps/api/v1/test/lib/bookings/_get.test.tspackages/lib/server/repository/workspacePlatform.tspackages/features/insights/server/trpc-router.tsapps/api/v1/test/lib/bookings/_post.test.tspackages/lib/server/repository/booking.tspackages/lib/server/repository/membership.tspackages/prisma/index.tsapps/web/playwright/lib/test-helpers/routingFormHelpers.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/server/repository/delegationCredential.tsapps/api/v1/test/lib/attendees/_post.test.tsapps/api/v1/pages/api/credential-sync/_post.tspackages/app-store/zoomvideo/lib/VideoApiAdapter.tsapps/web/test/lib/plainTiers.test.tsapps/api/v1/test/lib/event-types/[id]/_get.test.tsapps/api/v1/test/lib/event-types/[id]/_delete.test.tspackages/app-store/nextcloudtalk/lib/VideoApiAdapter.tspackages/lib/server/service/InsightsRoutingDIService.tspackages/prisma/extensions/usage-tracking.tsapps/api/v1/pages/api/credential-sync/_patch.tspackages/lib/server/service/InsightsRoutingBaseService.tsapps/api/v1/test/lib/bookings/[id]/_delete.test.tspackages/lib/server/service/InsightsBookingBaseService.tsapps/api/v1/test/lib/bookings/[id]/_patch.test.tspackages/app-store/_utils/oauth/updateTokenObject.tsapps/api/v1/test/lib/bookings/_get.test.tspackages/lib/server/repository/workspacePlatform.tspackages/features/insights/server/trpc-router.tsapps/api/v1/test/lib/bookings/_post.test.tspackages/lib/server/repository/booking.tspackages/lib/server/repository/membership.tspackages/prisma/index.tsapps/web/playwright/lib/test-helpers/routingFormHelpers.ts
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/lib/server/service/InsightsRoutingDIService.tspackages/lib/server/service/InsightsRoutingBaseService.tspackages/lib/server/service/InsightsBookingBaseService.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
📚 Learning: 2025-09-01T08:56:14.071Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/lib/tokenManager.ts:25-31
Timestamp: 2025-09-01T08:56:14.071Z
Learning: In token refresh utilities like tokenManager.ts, accessing credential.key from Prisma is legitimate and necessary for OAuth token refresh flows. These internal utilities need stored credentials to refresh tokens and don't expose them in API responses.
Applied to files:
packages/lib/server/repository/delegationCredential.tsapps/api/v1/pages/api/credential-sync/_post.tspackages/app-store/zoomvideo/lib/VideoApiAdapter.tspackages/app-store/nextcloudtalk/lib/VideoApiAdapter.tsapps/api/v1/pages/api/credential-sync/_patch.tspackages/app-store/_utils/oauth/updateTokenObject.tspackages/lib/server/repository/workspacePlatform.ts
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : Ensure the `credential.key` field is never returned from tRPC endpoints or APIs
Applied to files:
packages/lib/server/repository/delegationCredential.tsapps/api/v1/pages/api/credential-sync/_post.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
apps/api/v1/test/lib/attendees/_post.test.tsapps/api/v1/test/lib/event-types/[id]/_get.test.tsapps/api/v1/test/lib/bookings/[id]/_delete.test.tsapps/api/v1/test/lib/bookings/[id]/_patch.test.tsapps/api/v1/test/lib/bookings/_get.test.tsapps/api/v1/test/lib/bookings/_post.test.tsapps/web/playwright/lib/test-helpers/routingFormHelpers.ts
📚 Learning: 2025-09-01T07:31:00.963Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.963Z
Learning: In Cal.com's video integration system, credential types (e.g., "kyzon-space_video") are transformed to app names by removing underscores using `cred.type.split("_").join("")` in videoClient.ts line 28. This means the key in packages/app-store/index.ts should match the underscore-removed version (e.g., "kyzon-spacevideo") rather than the original type name.
Applied to files:
packages/app-store/zoomvideo/lib/VideoApiAdapter.tspackages/app-store/nextcloudtalk/lib/VideoApiAdapter.ts
📚 Learning: 2025-09-01T07:31:00.963Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzon-space/config.json:5-5
Timestamp: 2025-09-01T07:31:00.963Z
Learning: In Cal.com's video integration system, credential types (e.g., "kyzon-space_video") are transformed to app names by removing underscores using `cred.type.split("_").join("")` in videoClient.ts line 28. This means the key in packages/app-store/index.ts should match the underscore-removed version (e.g., "kyzon-spacevideo") rather than the original type name. This same pattern is used in other parts of the system like getCalendar.ts.
Applied to files:
packages/app-store/zoomvideo/lib/VideoApiAdapter.tspackages/app-store/nextcloudtalk/lib/VideoApiAdapter.ts
📚 Learning: 2025-07-24T08:39:06.185Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22702
File: packages/lib/server/service/insightsBooking.ts:120-124
Timestamp: 2025-07-24T08:39:06.185Z
Learning: In the InsightsBookingService (packages/lib/server/service/insightsBooking.ts), the constructor stores null for invalid options or filters but this is handled safely through null checks in buildFilterConditions() and buildAuthorizationConditions() methods. The service uses defensive programming to return safe fallback conditions (null or NOTHING_CONDITION) rather than throwing errors on invalid inputs.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.ts
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/lib/server/service/InsightsBookingBaseService.tspackages/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.
Applied to files:
packages/features/insights/server/trpc-router.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking fields are restricted structures containing only specific fields (id, eventTypeId, userId, and sometimes additional fields like startTime or smsReminderNumber) rather than full database booking objects, so there are no security or PII leakage concerns when using these booking objects in webhook payloads.
Applied to files:
apps/api/v1/test/lib/bookings/_post.test.ts
📚 Learning: 2025-08-21T13:55:23.470Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/service/FormWebhookService.ts:0-0
Timestamp: 2025-08-21T13:55:23.470Z
Learning: In the new webhook architecture for Cal.com, schedulePayload is considered legacy code that doesn't fit the clean architecture. The new architecture keeps webhook scheduling logic within the Service layer, specifically through a new method WebhookService.scheduleDelayedWebhooks, rather than using the old centralized schedulePayload helper.
Applied to files:
apps/api/v1/test/lib/bookings/_post.test.ts
📚 Learning: 2025-08-21T12:28:42.018Z
Learnt from: alishaz-polymath
PR: calcom/cal.com#23247
File: packages/features/webhooks/lib/factory/WebhookPayloadFactory.ts:274-282
Timestamp: 2025-08-21T12:28:42.018Z
Learning: In BookingPaymentInitiatedDTO and other webhook DTOs in packages/features/webhooks/lib/dto/types.ts, the booking field is a restricted structure containing only specific fields (id, eventTypeId, userId) rather than the full database booking object, so there are no security or PII leakage concerns when passing the booking object to buildEventPayload.
Applied to files:
apps/api/v1/test/lib/bookings/_post.test.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/lib/server/repository/booking.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma include uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/lib/server/repository/booking.ts
📚 Learning: 2025-09-03T11:54:05.409Z
Learnt from: supalarry
PR: calcom/cal.com#23514
File: apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts:579-582
Timestamp: 2025-09-03T11:54:05.409Z
Learning: In calcom/cal.com bookings repository methods, when Prisma select uses `eventType: true`, all eventType fields including seatsShowAttendees are automatically included in the selection. Explicit field selection is not required when using `true` for nested relations.
Applied to files:
packages/lib/server/repository/booking.tspackages/lib/server/repository/membership.ts
📚 Learning: 2025-09-08T07:27:42.863Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.863Z
Learning: In the Cal.com codebase, some calendar integrations like google-calendar already use SelectedCalendarRepository.create for selectedCalendar creation, which automatically triggers reconnection logic, while others like feishucalendar use direct prisma.selectedCalendar.create calls that bypass the repository hooks.
Applied to files:
packages/lib/server/repository/membership.ts
📚 Learning: 2025-09-08T07:27:42.863Z
Learnt from: vijayraghav-io
PR: calcom/cal.com#16878
File: packages/app-store/feishucalendar/api/callback.ts:72-79
Timestamp: 2025-09-08T07:27:42.863Z
Learning: Four calendar integrations in Cal.com still use direct prisma.selectedCalendar.create instead of SelectedCalendarRepository.create: feishucalendar, zohocalendar, office365calendar, and larkcalendar. These bypass repository hooks and won't trigger reconnection logic for BookingReferences.
Applied to files:
packages/lib/server/repository/membership.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts
🧬 Code graph analysis (7)
packages/lib/server/service/InsightsRoutingDIService.ts (1)
packages/prisma/index.ts (1)
PrismaClient(86-86)
packages/prisma/extensions/usage-tracking.ts (1)
packages/features/ee/common/server/LicenseKeyService.ts (2)
incrementUsage(90-105)incrementUsage(127-130)
packages/lib/server/service/InsightsRoutingBaseService.ts (1)
packages/prisma/index.ts (1)
PrismaClient(86-86)
packages/lib/server/service/InsightsBookingBaseService.ts (1)
packages/prisma/index.ts (1)
PrismaClient(86-86)
packages/features/insights/server/trpc-router.ts (1)
packages/prisma/index.ts (1)
PrismaClient(86-86)
packages/prisma/index.ts (1)
packages/prisma/extensions/usage-tracking.ts (1)
usageTrackingExtention(18-35)
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts (1)
packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Production builds / Build Web App
- GitHub Check: Production builds / Build Atoms
- GitHub Check: Production builds / Build API v2
- GitHub Check: Production builds / Build Docs
- GitHub Check: Tests / Unit
- GitHub Check: Production builds / Build API v1
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (20)
packages/app-store/nextcloudtalk/lib/VideoApiAdapter.ts (1)
8-8: Type-only Prisma import is correct.Matches the repo-wide pattern and avoids bundling the client at runtime.
apps/api/v1/pages/api/credential-sync/_post.ts (2)
9-11: Prisma import changes look good.Switch to named prisma and type-only Prisma aligns with the new client setup.
92-99: Safe JSON typing on credential.key.Casting to
Prisma.InputJsonValueis consistent with the new typing strategy. You also avoid returningcredential.keyin the API response, which satisfies the security guideline.packages/lib/server/repository/membership.ts (3)
1-1: Good: unified prisma import with transaction type.
3-3: Type-only import cleanup is correct.
240-276: Nice: this path already uses select.Keeping
selecthere is aligned with the guideline.packages/app-store/_utils/oauth/updateTokenObject.ts (1)
5-7: Prisma import updates are consistent with the new client.packages/lib/server/service/InsightsRoutingDIService.ts (2)
1-1: Correct: interface now depends on PrismaClient type.
10-11: LGTM.Public contract now uses
PrismaClient; construction remains unchanged.packages/lib/server/repository/workspacePlatform.ts (2)
2-2: Good: use @calcom/prisma/client types.
42-48: Create: correct handling of null vs undefined JSON.Casting to
InputJsonValuewithPrisma.JsonNullfallback is correct.packages/prisma/extensions/usage-tracking.ts (1)
8-9: Signature change to PrismaClient is correct.Works with extension inference disabled; dependency injection remains clear.
packages/features/insights/server/trpc-router.ts (1)
772-788: No issue: ctx.prisma is exposed in this router
createContextInnerreturns bothprismaandinsightsDb, andcreateContextspreads its result into the TRPC context, soctx.prismais always defined.packages/app-store/zoomvideo/lib/VideoApiAdapter.ts (1)
320-322: Good: JSON type cast is explicit and narrow.Casting newTokenObject to Prisma.InputJsonValue is appropriate here, given trusted OAuth manager input.
packages/lib/server/service/InsightsRoutingBaseService.ts (1)
15-16: Type-only migration to PrismaClient looks good.No runtime changes; aligns with the repo-wide typing refactor.
Also applies to: 139-149
packages/lib/server/repository/booking.ts (2)
12-21: FormResponse type addition is sensible.Clearer typing for routing form responses.
668-702: Replace Prismaincludewith explicitselect. Avoid loading entire related entities; confirm which relation fields callers actually use and update the query to select only those.apps/web/playwright/lib/test-helpers/routingFormHelpers.ts (1)
320-360: Routes/fields cloning approach looks good and the options now required aligns with type changes.No issues spotted with the new RouteConfig/action/queryValue shape.
Also applies to: 388-498, 499-561
packages/prisma/index.ts (2)
2-2: LGTM: switch to named PrismaClient importConsistent with the new pattern and improves clarity.
74-74: LGTM: dev-only global caching
Prevents multiple clients during HMR while keeping production clean.
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | ||
| attendees: true, | ||
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | ||
| references: true, | ||
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | ||
| user: true, | ||
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | ||
| payment: true, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use select instead of include for getValidBookingFromEventTypeForAttendee.
Trim related payloads to what the caller needs.
- include: {
- // eslint-disable-next-line @calcom/eslint/no-prisma-include-true
- attendees: true,
- // eslint-disable-next-line @calcom/eslint/no-prisma-include-true
- references: true,
- // eslint-disable-next-line @calcom/eslint/no-prisma-include-true
- user: true,
- // eslint-disable-next-line @calcom/eslint/no-prisma-include-true
- payment: true,
- },
+ select: {
+ attendees: { select: { id: true, email: true, name: true } },
+ references: { select: { id: true, uid: true, type: true, meetingUrl: true } },
+ user: { select: { id: true } },
+ payment: { select: { id: true } },
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | |
| attendees: true, | |
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | |
| references: true, | |
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | |
| user: true, | |
| // eslint-disable-next-line @calcom/eslint/no-prisma-include-true | |
| payment: true, | |
| }, | |
| select: { | |
| attendees: { select: { id: true, email: true, name: true } }, | |
| references: { select: { id: true, uid: true, type: true, meetingUrl: true } }, | |
| user: { select: { id: true } }, | |
| payment: { select: { id: true } }, | |
| }, |
🤖 Prompt for AI Agents
In packages/lib/server/repository/booking.ts around lines 842 to 850, the query
is using include for attendees, references, user, and payment; change these to
select and only fetch the specific fields the caller needs (e.g., for attendees:
id, email, name; for references: id, type, value; for user: id, email, timezone;
for payment: id, status, amount — adjust to actual required properties). Update
the returned type annotations if necessary and remove the eslint-disable
comments, ensuring the query returns a smaller payload tailored to the caller.
| return await prisma.workspacePlatform.update({ | ||
| where: { id }, | ||
| data, | ||
| data: { | ||
| ...data, | ||
| defaultServiceAccountKey: data.defaultServiceAccountKey as unknown as Prisma.InputJsonValue, | ||
| }, | ||
| select: safeWorkspacePlatformSelect, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid writing undefined for defaultServiceAccountKey on partial updates.
Only set the field when provided; map explicit null to Prisma.JsonNull.
- return await prisma.workspacePlatform.update({
- where: { id },
- data: {
- ...data,
- defaultServiceAccountKey: data.defaultServiceAccountKey as unknown as Prisma.InputJsonValue,
- },
- select: safeWorkspacePlatformSelect,
- });
+ const updateData: any = { ...data };
+ if ("defaultServiceAccountKey" in data) {
+ updateData.defaultServiceAccountKey =
+ data.defaultServiceAccountKey === null
+ ? Prisma.JsonNull
+ : (data.defaultServiceAccountKey as unknown as Prisma.InputJsonValue);
+ }
+ return await prisma.workspacePlatform.update({
+ where: { id },
+ data: updateData,
+ select: safeWorkspacePlatformSelect,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return await prisma.workspacePlatform.update({ | |
| where: { id }, | |
| data, | |
| data: { | |
| ...data, | |
| defaultServiceAccountKey: data.defaultServiceAccountKey as unknown as Prisma.InputJsonValue, | |
| }, | |
| select: safeWorkspacePlatformSelect, | |
| const updateData: any = { ...data }; | |
| if ("defaultServiceAccountKey" in data) { | |
| updateData.defaultServiceAccountKey = | |
| data.defaultServiceAccountKey === null | |
| ? Prisma.JsonNull | |
| : (data.defaultServiceAccountKey as unknown as Prisma.InputJsonValue); | |
| } | |
| return await prisma.workspacePlatform.update({ | |
| where: { id }, | |
| data: updateData, | |
| select: safeWorkspacePlatformSelect, | |
| }); |
🤖 Prompt for AI Agents
In packages/lib/server/repository/workspacePlatform.ts around lines 95 to 101,
the update currently spreads data and always writes defaultServiceAccountKey
(which can become undefined) — change the update payload to only include
defaultServiceAccountKey when it is present on the incoming data: build the data
object so that you spread the other fields, and conditionally add
defaultServiceAccountKey only if Object.prototype.hasOwnProperty.call(data,
'defaultServiceAccountKey'), mapping an explicit null to Prisma.JsonNull and
otherwise casting the value to Prisma.InputJsonValue; this prevents writing
undefined on partial updates and preserves intentional nulls.
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts (6)
32-37: Replace any with a minimal query-AST type; drop eslint disables.Keeps inference off but preserves safety.
Apply within the interface:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - queryValue?: any; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - attributesQueryValue?: any; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - fallbackAttributesQueryValue?: any; + queryValue?: QueryGroup; + attributesQueryValue?: QueryGroup; + fallbackAttributesQueryValue?: QueryGroup;Add near the interfaces (outside this range):
type QueryRule = { type: "rule"; properties: { field: string; value: unknown[] | unknown[][]; operator: string; valueSrc: string[]; valueType: string[]; valueError?: (string | null)[]; }; }; type QueryGroup = { id: string; type: "group"; children1?: Record<string, QueryRule | QueryGroup> };
135-143: Run team event creations in parallel and remove the any map.The map isn’t used; parallelizing reduces test time.
-// eslint-disable-next-line @typescript-eslint/no-explicit-any -const teamEvents: { [key: string]: any } = {}; -for (const eventConfig of teamEventsConfig) { - const eventType = await createRoundRobinTeamEventType({ - teamId, - eventType: { ...eventConfig, length: eventConfig.length || 60 }, - }); - teamEvents[eventConfig.slug] = eventType; -} +await Promise.all( + teamEventsConfig.map((eventConfig) => + createRoundRobinTeamEventType({ + teamId, + eventType: { ...eventConfig, length: eventConfig.length || 60 }, + }) + ) +);
156-158: Avoid non-null assertion on Skills attribute; fail fast with a clear error.Prevents a cryptic runtime throw when config omits “Skills”.
-// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -const skillAttribute = attributes.find((attr) => attr.name === "Skills")!; +const skillAttribute = attributes.find((attr) => attr.name === "Skills"); +if (!skillAttribute) { + throw new Error('Attribute "Skills" not found in attributeRouting.attributes'); +}
321-358: UI copy consistency: capitalize “Skills”.Matches the “Location” label.
- label: "skills", + label: "Skills",
387-497: Minor: rename map var to route and dedupe the repeated field id.Improves readability and avoids string duplication.
Add once near Line 376 (outside this range):
const textFieldUuid = "c4296635-9f12-47b1-8153-c3a854649182";Then:
- routes: ( + routes: ( config.form?.routes || ([ { id: "8a898988-89ab-4cde-b012-31823f708642", action: { type: "eventTypeRedirectUrl", value: "pro/30min" }, queryValue: { id: "8a898988-89ab-4cde-b012-31823f708642", type: "group", children1: { "8988bbb8-0123-4456-b89a-b1823f70c5ff": { type: "rule", properties: { - field: "c4296635-9f12-47b1-8153-c3a854649182", + field: textFieldUuid, value: ["event-routing"], operator: "equal", valueSrc: ["value"], valueType: ["text"], }, }, }, }, }, ... - ] satisfies RouteConfig[]) - ).map((field) => ({ ...field })), + ] satisfies RouteConfig[]) + ).map((route) => ({ ...route })),
498-558: Use the same constant for the Test field id here too.Prevents drift between rules and field definitions.
- { - id: "c4296635-9f12-47b1-8153-c3a854649182", + { + id: textFieldUuid, type: "text", label: "Test field", required: true, },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/web/playwright/lib/test-helpers/routingFormHelpers.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/web/playwright/lib/test-helpers/routingFormHelpers.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: emrysal
PR: calcom/cal.com#23692
File: packages/lib/server/service/InsightsBookingBaseService.ts:16-16
Timestamp: 2025-09-09T03:29:42.984Z
Learning: In the Cal.com codebase, readonlyPrisma is still an instance of PrismaClient, making type changes from `typeof readonlyPrisma` to `PrismaClient` less critical since they are fundamentally compatible types.
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Applied to files:
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts
🧬 Code graph analysis (1)
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts (1)
packages/lib/constants.ts (1)
WEBAPP_URL(12-18)
🔇 Additional comments (1)
apps/web/playwright/lib/test-helpers/routingFormHelpers.ts (1)
216-371: Drop the select suggestion for this test helper
createRoutingForm is used by tests that access form.name, form.userId, form.routes, and form.fields; restricting the returned fields to only id would break existing tests.Likely an incorrect or invalid review comment.
| prismaMock.booking.findUnique.mockResolvedValue({ | ||
| ...originalBooking, | ||
| status: "cancelled", | ||
| status: "CANCELLED", |
There was a problem hiding this comment.
q: would it impact on all queries?
should we use constant BookingStatus.CANCELLED?
| @@ -5,6 +5,7 @@ import { z } from "zod"; | |||
| import logger from "@calcom/lib/logger"; | |||
| import { safeStringify } from "@calcom/lib/safeStringify"; | |||
| import prisma from "@calcom/prisma"; | |||
There was a problem hiding this comment.
can you change it for use the named export?
import { prisma } from "@calcom/prisma";
volnei
left a comment
volnei
left a comment
There was a problem hiding this comment.
nit comments... but looks good to me!
3 participants
What does this PR do?
Spin-off PR from the Prisma 6.7.0 migration work, we discovered a major bottleneck in the typings of the Prisma client extensions, as well as making dependency injection harder as a typed client extension prisma cannot be injected on a client extended prisma with different extensions.
It looks like each client extension definition duplicates the type definitions, so e.g. 6 extensions, duplicate typings of our entire PrismaClient 6x.
Also removes the withAccelerate Client Extensions