refactor: circular deps between app store and lib [1]#23653
refactor: circular deps between app store and lib [1]#23653keithwillcode merged 22 commits intomainfrom
Conversation
WalkthroughThis PR updates module import paths across web, features, app-store, emails, and platform packages. Event naming utilities (EventNameObjectType, getEventName, validateCustomEventName, nameObjectSchema) are moved to @calcom/features/eventtypes/lib/eventNaming. Booking calendar link utilities are moved to @calcom/features/bookings/lib/getCalendarLinks. Daily API fetcher imports are redirected to local modules in the dailyvideo app and referenced by tasker. Routing-forms modules consolidate to local or @calcom/app-store/routing-forms paths. Tests are updated for new paths and minor typing changes. Platform libraries re-export validateCustomEventName and getCalendarLinks from the new feature modules. No functional logic changes. 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 |
hbjORbj
changed the title
@calcom/app-store
dosubot
bot
added
app-store
hbjORbj
changed the title
@calcom/app-store
| @@ -1,6 +1,7 @@ | |||
| import { getDailyAppKeys } from "@calcom/app-store/dailyvideo/lib/getDailyAppKeys"; | |||
There was a problem hiding this comment.
Violation: lib was importing app-store
hbjORbj
force-pushed
the
refactor/move-app-store-utils-to-app-store
branch
from
0cd7ddd to
b96ef76
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| import AssignmentReasonRecorder, { | ||
| RRReassignmentType, | ||
| } from "@calcom/features/ee/round-robin/assignmentReason/AssignmentReasonRecorder"; | ||
| import { BookingLocationService } from "@calcom/features/ee/round-robin/lib/bookingLocationService"; |
There was a problem hiding this comment.
BookingLocationService was only consumed in Features package, so there's no reason for this to live in @calcom/lib + BookingLocationService was importing app store (which is a violation)
|
@CodeRabbit Add a PR description. Focus on what migrations happened. The reason is to fix circular dependencies since packages/lib shouldn't import packages/app-store. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 578 🏁 Script executed: Length of output: 1321 🏁 Script executed: Length of output: 266240 🏁 Script executed: Length of output: 1793 🏁 Script executed: Unrecoverable error: Sandbox execution failed: max_buffer, terminated 🏁 Script executed: Length of output: 1156 🏁 Script executed: Length of output: 1238 🏁 Script executed: Unrecoverable error: Sandbox execution failed: max_buffer, terminated 🏁 Script executed: Length of output: 576 @hbjORbj I'll help you add a comprehensive PR description focusing on the migrations that happened to fix the circular dependencies. Based on my analysis of the changes, here's a detailed PR description for your refactor: refactor: circular deps between app store <-> lib [1]What does this PR do?This PR addresses circular dependency issues between Problem StatementPreviously, there were circular dependencies between:
This violates the intended architecture where Migrations Performed1. Payment App Data Migration
2. Usage UpdatesThe following files were updated to use the new import path:
Current State & Next StepsNote: This is part 1 of the circular dependency refactor. The
These remaining dependencies will be addressed in subsequent PRs to fully resolve the circular dependency chain. Benefits
Testing
Mandatory Tasks (DO NOT REMOVE)
|
hbjORbj
changed the title
|
@calcom/platform this PR requires publishing a new version of platform libraries |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
packages/app-store/routing-forms/cron/queuedFormResponseCleanup.ts (1)
10-18: Harden auth check against unset/empty env vars.Avoid accidentally accepting "Bearer undefined" or similar when env is missing; fail closed.
Apply:
-function validateRequest(request: NextRequest) { - const apiKey = request.headers.get("authorization") || request.nextUrl.searchParams.get("apiKey"); - - if (![process.env.CRON_API_KEY, `Bearer ${process.env.CRON_SECRET}`].includes(`${apiKey}`)) { +function validateRequest(request: NextRequest) { + const headerAuth = request.headers.get("authorization")?.trim() ?? null; + const queryApiKey = request.nextUrl.searchParams.get("apiKey") ?? null; + const allowed = new Set( + [process.env.CRON_API_KEY, process.env.CRON_SECRET ? `Bearer ${process.env.CRON_SECRET}` : null].filter( + Boolean + ) as string[] + ); + if (allowed.size === 0 || !(headerAuth ? allowed.has(headerAuth) : queryApiKey ? allowed.has(queryApiKey) : false)) { return NextResponse.json({ message: "Not authenticated" }, { status: 401 }); } return null; }packages/app-store/routing-forms/api/responses/[formId].ts (1)
21-29: Replace skip/take with cursor pagination; add select and orderBy.
- skip/take without orderBy can return nondeterministic pages and is O(n) on large offsets.
- Prisma guideline: select only needed columns; avoid returning extra data.
Apply this diff to getResponses:
@@ - let responses; - let skip = 0; + let responses; + let lastId: number | undefined; @@ - while ( - (responses = await prisma.app_RoutingForms_FormResponse.findMany({ - where: { - formId, - }, - take: take, - skip: skip, - })) && - responses.length - ) { + while (true) { + responses = await prisma.app_RoutingForms_FormResponse.findMany({ + where: { formId }, + take, + ...(lastId ? { cursor: { id: lastId }, skip: 1 } : {}), + orderBy: { id: "asc" }, + select: { id: true, response: true, createdAt: true }, + }); + if (!responses.length) break; @@ - skip += take; + lastId = responses[responses.length - 1].id; yield csv.join("\n"); }Also applies to: 31-49
packages/features/ee/round-robin/roundRobinReassignment.ts (2)
265-272: Avoid non-null assertion on previousRRHostAttendee.If the attendee isn’t present,
previousRRHostAttendee!.idwill throw at runtime.- await prisma.attendee.update({ - where: { - id: previousRRHostAttendee!.id, - }, + if (!previousRRHostAttendee) { + logger.warn("Previous RR host attendee not found; skipping attendee update"); + } else { + await prisma.attendee.update({ + where: { id: previousRRHostAttendee.id }, data: { name: reassignedRRHost.name || "", email: reassignedRRHost.email, timeZone: reassignedRRHost.timeZone, locale: reassignedRRHost.locale, }, - }); + }); + }
341-352: Drop Prisma include; fetch only what you need.
include: { user: { select: { email: true } } }violates the “no include; prefer select” guidance and the nested user email isn’t used here (organizer already has email).- const credentials = await prisma.credential.findMany({ - where: { - userId: organizer.id, - }, - include: { - user: { - select: { - email: true, - }, - }, - }, - }); + const credentials = await prisma.credential.findMany({ + where: { userId: organizer.id }, + });packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
312-315: Prisma queries must use select instead of include and return only needed fields
- credentials.findMany (lines 312–315): replace
includewithselectlisting only the credential fields you use plususer: { select: { email: true } }.- destinationCalendar.findFirst (lines 321–325): add a
selectclause for only the properties consumed byhandleRescheduleEventManager.- workflow.findMany (around lines 549–556): replace
include: { steps: … }withselect: { steps: { select: { … } } }, choosing the minimal fields needed.packages/app-store/dailyvideo/lib/dailyApiFetcher.ts (1)
1-16: Remove all remaining@calcom/app-storeimports frompackages/lib
Therg -nP '@calcom/app-store' packages/libsearch still shows numerous imports (e.g., invideoClient.ts,server/getDefaultLocations.ts,server/getLuckyUser.ts, and many more) that reintroduce the lib→app-store cycle. Remove or replace these imports to fully break the dependency cycle.packages/features/bookings/lib/getCalendarLinks.ts (3)
83-86: Encode eventName for Google Calendar link.Unencoded text can break the link with spaces or special chars.
- const googleCalendarLink = `https://calendar.google.com/calendar/r/eventedit?dates=${startTimeInUtcFormat}/${endTimeInUtcFormat}&text=${eventName}&details=${description}${ + const googleCalendarLink = `https://calendar.google.com/calendar/r/eventedit?dates=${startTimeInUtcFormat}/${endTimeInUtcFormat}&text=${encodeURIComponent(eventName)}&details=${description}${ location ? `&location=${location}` : "" }${recurrence ? `&recur=${recurrence}` : ""}`;
103-112: Encode subject for Microsoft Office link.Subject is not encoded; align with description/location.
- const description = encodeURIComponent(eventDescription ?? ""); + const description = encodeURIComponent(eventDescription ?? ""); + const subject = encodeURIComponent(eventName); ... - const microsoftOfficeLink = `https://outlook.office.com/calendar/0/deeplink/compose?body=${description}&enddt=${endTimeInUtcFormat}&path=%2Fcalendar%2Faction%2Fcompose&rru=addevent&startdt=${startTimeInUtcFormat}&subject=${eventName}${ + const microsoftOfficeLink = `https://outlook.office.com/calendar/0/deeplink/compose?body=${description}&enddt=${endTimeInUtcFormat}&path=%2Fcalendar%2Faction%2Fcompose&rru=addevent&startdt=${startTimeInUtcFormat}&subject=${subject}${ location ? `&location=${location}` : "" }`;
128-136: Build Outlook link with per-param encoding (avoid encodeURI on the whole URL).encodeURI won’t encode reserved characters in values; encode each param.
- const microsoftOutlookLink = - encodeURI( - `https://outlook.live.com/calendar/0/deeplink/compose?body=${eventDescription}&enddt=${endTimeInUtcFormat}&path=%2Fcalendar%2Faction%2Fcompose&rru=addevent&startdt=${startTimeInUtcFormat}&subject=${eventName}` - ) + (location ? `&location=${location}` : ""); + const description = encodeURIComponent(eventDescription ?? ""); + const subject = encodeURIComponent(eventName); + const microsoftOutlookLink = + `https://outlook.live.com/calendar/0/deeplink/compose?body=${description}&enddt=${endTimeInUtcFormat}&path=%2Fcalendar%2Faction%2Fcompose&rru=addevent&startdt=${startTimeInUtcFormat}&subject=${subject}` + + (location ? `&location=${location}` : "");packages/app-store/salesforce/lib/CrmService.ts (1)
159-167: Prevent SOQL injection by replacing raw string-interpolated queriesAll methods in CrmService.ts embed user-controlled values (email, userId, recordType, etc.) directly into SOQL strings via template interpolation (e.g., getSalesforceUserIdFromEmail, getSalesforceUserFromUserId, findContactByEmail, findLeadByEmail, and many others). This poses a critical SOQL injection risk. Replace each raw conn.query(...) call with the platform’s safe query builder (e.g., conn.sobject(...).find({...})) or use an escaping helper.
Example fix for getSalesforceUserIdFromEmail:
-const query = await conn.query( - `SELECT Id, Email FROM User WHERE Email = '${email}' AND IsActive = true LIMIT 1` -); +const records = await conn + .sobject("User") + .find({ Email: email, IsActive: true }) + .select(["Id", "Email"]) + .limit(1); +if (records.length > 0) return (records[0] as { Id: string }).Id;
🧹 Nitpick comments (18)
packages/app-store/routing-forms/cron/queuedFormResponseCleanup.ts (1)
41-46: Use the shared logger instead of console.error.Keeps logging consistent and structured.
Apply:
- console.error("Error during queued form response cleanup:", error); + logger.error({ err: error }, "Error during queued form response cleanup");packages/app-store/routing-forms/lib/queuedFormResponse/PrismaQueuedFormResponseRepository.integration-test.ts (1)
388-396: Minor: explicit response overrides are redundant.The builders already default response values; you can drop these fields to reduce noise.
Example:
-const response1 = await createQueuedResponse(testForm.id, { - response: { field1: "value1" }, -}); +const response1 = await createQueuedResponse(testForm.id);Also applies to: 421-423, 434-436
packages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.ts (1)
26-29: Add deterministic ordering to batched queries.Ordering avoids inconsistent batches under concurrent writes.
Apply:
return this.deps.queuedFormResponseRepo.findMany({ where: { actualResponseId: null, createdAt: { lt: cutoffTime, }, }, params: { take: batchSize, + orderBy: { createdAt: "asc" }, }, });packages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.test.ts (2)
10-10: Prefer Vitest’s Mocked type over Jest typings.Removes implicit dependency on @types/jest.
Apply:
-import type logger from "@calcom/lib/logger"; +import type logger from "@calcom/lib/logger"; +import type { Mocked } from "vitest";- let mockRepo: jest.Mocked<QueuedFormResponseRepositoryInterface>; + let mockRepo: Mocked<QueuedFormResponseRepositoryInterface>;- } as unknown as jest.Mocked<QueuedFormResponseRepositoryInterface>; + } as unknown as Mocked<QueuedFormResponseRepositoryInterface>;
133-133: Avoidas anyin mocks; keep type-safety.Cast to the repository’s actual findMany return type.
Apply:
- mockRepo.findMany.mockResolvedValueOnce([{ id: "1" }, { id: "2" }, { id: "3" }] as any); + mockRepo.findMany.mockResolvedValueOnce( + [{ id: "1" }, { id: "2" }, { id: "3" }] as unknown as FindManyResult + );Add this helper type near the top (after imports):
type FindManyResult = Awaited<ReturnType<QueuedFormResponseRepositoryInterface["findMany"]>>;packages/app-store/routing-forms/api/responses/[formId].ts (1)
100-104: Sanitize CSV header labels.Header cell text may contain commas/quotes; sanitize like values to keep CSV valid.
- `${headerFields - .map((field) => `${field.label}${field.deleted ? "(Deleted)" : ""}`) + `${headerFields + .map((field) => sanitizeValue(`${field.label}${field.deleted ? "(Deleted)" : ""}`)) .concat(["Submission Time"]) .join(",")}\n`packages/features/ee/round-robin/roundRobinReassignment.ts (1)
469-469: Prefer named export over default.Switching to
export { roundRobinReassignment }eases refactors and improves tree-shaking.-export default roundRobinReassignment; +export { roundRobinReassignment };packages/features/bookings/lib/handleNewBooking.ts (2)
1166-1174: Select only required columns in Prisma query.Only
organizationIdis used; avoid fetching full Profile rows.- const organizerOrganizationProfile = await prisma.profile.findFirst({ - where: { - userId: organizerUser.id, - username: dynamicUserList[0], - }, - }); + const organizerOrganizationProfile = await prisma.profile.findFirst({ + where: { userId: organizerUser.id, username: dynamicUserList[0] }, + select: { organizationId: true }, + });
2377-2377: Prefer named export over default.Optional, but aligns with repo guidance for better tree-shaking.
-export default handler; +export { handler };packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
571-571: Prefer named export over default export.Improves tree-shaking and refactoring across the monorepo.
-export default roundRobinManualReassignment; +export { roundRobinManualReassignment };packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (1)
215-223: Localize the aria-label.Frontend guideline: use t() for user-facing strings.
- aria-label="edit custom name" + aria-label={t("edit_custom_name")}apps/web/modules/test-setup.ts (1)
116-122: Remove duplicate mock block.There are two identical mocks for @calcom/lib/recurringStrings; keep one.
vi.mock("@calcom/lib/recurringStrings", () => { return {}; }); - -vi.mock("@calcom/lib/recurringStrings", () => { - return {}; -});packages/app-store/dailyvideo/lib/VideoApiAdapter.ts (2)
455-457: Remove unnecessary Promise.all on strings.processTranscriptsInBatches already returns string[].
- const allTranscriptsAccessLink = await processTranscriptsInBatches(allTranscriptsIds); - const accessLinks = await Promise.all(allTranscriptsAccessLink); - - return Promise.resolve(accessLinks); + const accessLinks = await processTranscriptsInBatches(allTranscriptsIds); + return Promise.resolve(accessLinks);Also applies to: 473-474
540-540: Prefer named export over default export.Aligns with repo guidance for better tree-shaking.
-export default DailyVideoApiAdapter; +export { DailyVideoApiAdapter };packages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.ts (1)
5-7: Encode roomName in query param to avoid malformed URLs.If roomName contains spaces or special chars, the request may break. Encode it.
- return fetcher(`/meetings?room=${roomName}`).then(triggerNoShowPayloadSchema.parse); + return fetcher(`/meetings?room=${encodeURIComponent(roomName)}`).then( + triggerNoShowPayloadSchema.parse + );packages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.ts (1)
15-18: Nit: make log prefix match the class for easier tracing.- prefix: ["[routingFormFieldService]", { bookingUid }], + prefix: ["[RoutingFormResponseDataFactory]", { bookingUid }], ... - prefix: ["[routingFormFieldService]", { responseId }], + prefix: ["[RoutingFormResponseDataFactory]", { responseId }],Also applies to: 30-33
packages/features/bookings/lib/getCalendarLinks.ts (1)
108-111: Resolve the TODO by adopting URLSearchParams.Using URLSearchParams removes ambiguity about when to encode.
I can provide a follow-up refactor using URL/URLSearchParams for all three builders if you want.
packages/app-store/salesforce/lib/CrmService.ts (1)
80-86: Prefer named export over default to align with repo guidelines.Switching to a named export improves tree-shaking and refactors. Consider an interim re-export to avoid breaking changes.
-export default class SalesforceCRMService implements CRM { +export class SalesforceCRMService implements CRM {Optionally add:
// temporary shim export { SalesforceCRMService as default } from "./CrmService";
📜 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 (29)
apps/web/modules/bookings/views/bookings-single-view.tsx(1 hunks)apps/web/modules/test-setup.ts(1 hunks)packages/app-store/dailyvideo/lib/VideoApiAdapter.ts(1 hunks)packages/app-store/dailyvideo/lib/dailyApiFetcher.ts(1 hunks)packages/app-store/routing-forms/api/responses/[formId].ts(1 hunks)packages/app-store/routing-forms/cron/queuedFormResponseCleanup.ts(1 hunks)packages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.test.ts(1 hunks)packages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.ts(1 hunks)packages/app-store/routing-forms/lib/findFieldValueByIdentifier.test.ts(1 hunks)packages/app-store/routing-forms/lib/findFieldValueByIdentifier.ts(1 hunks)packages/app-store/routing-forms/lib/queuedFormResponse/PrismaQueuedFormResponseRepository.integration-test.ts(6 hunks)packages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.test.ts(2 hunks)packages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.ts(1 hunks)packages/app-store/routing-forms/lib/responseData/getHumanReadableFieldResponseValue.test.ts(1 hunks)packages/app-store/routing-forms/lib/substituteVariables.ts(1 hunks)packages/app-store/salesforce/lib/CrmService.ts(2 hunks)packages/emails/email-manager.ts(1 hunks)packages/features/bookings/lib/BookingEmailSmsHandler.ts(1 hunks)packages/features/bookings/lib/getCalendarLinks.ts(1 hunks)packages/features/bookings/lib/handleNewBooking.ts(1 hunks)packages/features/ee/round-robin/roundRobinManualReassignment.ts(1 hunks)packages/features/ee/round-robin/roundRobinReassignment.ts(1 hunks)packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx(1 hunks)packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx(1 hunks)packages/features/eventtypes/lib/eventNaming.test.ts(1 hunks)packages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.ts(1 hunks)packages/platform/atoms/event-types/hooks/useEventTypeForm.ts(1 hunks)packages/platform/libraries/event-types.ts(1 hunks)packages/platform/libraries/index.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/app-store/routing-forms/cron/queuedFormResponseCleanup.tspackages/app-store/routing-forms/lib/findFieldValueByIdentifier.test.tspackages/app-store/dailyvideo/lib/VideoApiAdapter.tspackages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/eventtypes/lib/eventNaming.test.tspackages/app-store/routing-forms/api/responses/[formId].tspackages/features/bookings/lib/handleNewBooking.tspackages/app-store/routing-forms/lib/responseData/getHumanReadableFieldResponseValue.test.tspackages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.tspackages/platform/libraries/event-types.tspackages/app-store/routing-forms/lib/findFieldValueByIdentifier.tspackages/app-store/dailyvideo/lib/dailyApiFetcher.tsapps/web/modules/test-setup.tspackages/app-store/routing-forms/lib/substituteVariables.tspackages/emails/email-manager.tspackages/features/bookings/lib/getCalendarLinks.tspackages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.test.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/roundRobinReassignment.tspackages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.test.tspackages/app-store/routing-forms/lib/queuedFormResponse/PrismaQueuedFormResponseRepository.integration-test.tspackages/features/ee/round-robin/roundRobinManualReassignment.tspackages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.tspackages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.tspackages/app-store/salesforce/lib/CrmService.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/app-store/routing-forms/cron/queuedFormResponseCleanup.tspackages/app-store/routing-forms/lib/findFieldValueByIdentifier.test.tspackages/app-store/dailyvideo/lib/VideoApiAdapter.tspackages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsxpackages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/eventtypes/lib/eventNaming.test.tspackages/app-store/routing-forms/api/responses/[formId].tspackages/features/bookings/lib/handleNewBooking.tspackages/app-store/routing-forms/lib/responseData/getHumanReadableFieldResponseValue.test.tspackages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.tspackages/platform/libraries/event-types.tspackages/app-store/routing-forms/lib/findFieldValueByIdentifier.tspackages/app-store/dailyvideo/lib/dailyApiFetcher.tsapps/web/modules/test-setup.tspackages/app-store/routing-forms/lib/substituteVariables.tspackages/emails/email-manager.tspackages/features/bookings/lib/getCalendarLinks.tspackages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.test.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/roundRobinReassignment.tspackages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.test.tspackages/app-store/routing-forms/lib/queuedFormResponse/PrismaQueuedFormResponseRepository.integration-test.tspackages/features/ee/round-robin/roundRobinManualReassignment.tspackages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.tspackages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.tsapps/web/modules/bookings/views/bookings-single-view.tsxpackages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsxpackages/app-store/salesforce/lib/CrmService.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/app-store/routing-forms/cron/queuedFormResponseCleanup.tspackages/app-store/routing-forms/lib/findFieldValueByIdentifier.test.tspackages/app-store/dailyvideo/lib/VideoApiAdapter.tspackages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsxpackages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/eventtypes/lib/eventNaming.test.tspackages/app-store/routing-forms/api/responses/[formId].tspackages/features/bookings/lib/handleNewBooking.tspackages/app-store/routing-forms/lib/responseData/getHumanReadableFieldResponseValue.test.tspackages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.tspackages/platform/libraries/event-types.tspackages/app-store/routing-forms/lib/findFieldValueByIdentifier.tspackages/app-store/dailyvideo/lib/dailyApiFetcher.tsapps/web/modules/test-setup.tspackages/app-store/routing-forms/lib/substituteVariables.tspackages/emails/email-manager.tspackages/features/bookings/lib/getCalendarLinks.tspackages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.test.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/roundRobinReassignment.tspackages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.test.tspackages/app-store/routing-forms/lib/queuedFormResponse/PrismaQueuedFormResponseRepository.integration-test.tspackages/features/ee/round-robin/roundRobinManualReassignment.tspackages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.tspackages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.tsapps/web/modules/bookings/views/bookings-single-view.tsxpackages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsxpackages/app-store/salesforce/lib/CrmService.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsxapps/web/modules/bookings/views/bookings-single-view.tsxpackages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx
**/*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/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.tspackages/app-store/salesforce/lib/CrmService.ts
🧠 Learnings (10)
📓 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.
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/app-store/dailyvideo/lib/VideoApiAdapter.tspackages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.ts
📚 Learning: 2025-09-01T10:25:51.923Z
Learnt from: nangelina
PR: calcom/cal.com#23486
File: packages/app-store/kyzonspacevideo/package.json:7-12
Timestamp: 2025-09-01T10:25:51.923Z
Learning: In Cal.com's monorepo, app-store packages don't need to declare `zod` as a direct dependency in their package.json files. The monorepo uses yarn workspaces with dependency hoisting, where `zod` is available through workspace-level dependency management. Most app-store packages successfully import zod without declaring it as a dependency, following the established monorepo pattern.
Applied to files:
packages/app-store/dailyvideo/lib/VideoApiAdapter.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.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:
packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsxpackages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/eventtypes/lib/eventNaming.test.tspackages/features/bookings/lib/handleNewBooking.tsapps/web/modules/test-setup.tspackages/features/bookings/lib/getCalendarLinks.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/roundRobinManualReassignment.tspackages/platform/atoms/event-types/hooks/useEventTypeForm.tsapps/web/modules/bookings/views/bookings-single-view.tsxpackages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx
📚 Learning: 2025-08-29T22:57:30.382Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/form-builder/FormBuilder.tsx:11-11
Timestamp: 2025-08-29T22:57:30.382Z
Learning: FormBuilder.tsx in packages/features/form-builder/ does not have "use client" directive at the top despite using client-side React hooks and event handlers, which suggests it should be a client component.
Applied to files:
packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx
📚 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:
packages/app-store/routing-forms/lib/findFieldValueByIdentifier.ts
📚 Learning: 2025-08-13T12:36:23.649Z
Learnt from: hariombalhara
PR: calcom/cal.com#17870
File: packages/app-store/routing-forms/lib/substituteVariables.ts:40-40
Timestamp: 2025-08-13T12:36:23.649Z
Learning: In packages/app-store/routing-forms/lib/substituteVariables.ts, the current replace() implementation works correctly for multiple variable occurrences because the variables array contains duplicate entries for repeated variables, and the forEach loop processes each occurrence. The user hariombalhara acknowledged it's not efficient but doesn't want to focus on optimizing this aspect.
Applied to files:
packages/app-store/routing-forms/lib/substituteVariables.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/platform/libraries/index.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/platform/libraries/index.ts
📚 Learning: 2025-07-22T11:42:47.623Z
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
packages/features/ee/round-robin/roundRobinManualReassignment.ts
⏰ 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). (8)
- GitHub Check: Production builds / Build Web App
- GitHub Check: Linters / lint
- 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: Type check / check-types
🔇 Additional comments (34)
packages/app-store/routing-forms/cron/queuedFormResponseCleanup.ts (1)
7-9: Approve: Import path refactor complete
Verified no leftover alias imports forPrismaQueuedFormResponseRepositoryorQueuedFormResponseServicefrom@calcom/app-storeor@calcom/libacross the repository.packages/app-store/routing-forms/lib/queuedFormResponse/PrismaQueuedFormResponseRepository.integration-test.ts (3)
1-1: Type-only Prisma import for JSON payloads — good call.Matches Prisma’s JSON create semantics.
36-37: Switch to Prisma.InputJsonValue is correct here.Create inputs should use InputJsonValue.
52-56: Consistent use of Prisma.InputJsonValue for queued responses — LGTM.Keeps builders aligned with Prisma types.
packages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.ts (1)
3-3: Import path cleanup — approved.Local interface co-location improves maintainability and avoids deep relative paths.
packages/app-store/routing-forms/lib/queuedFormResponse/QueuedFormResponseService.test.ts (1)
5-5: Import path update — good.Keeps tests in lockstep with the service refactor.
packages/app-store/routing-forms/api/responses/[formId].ts (1)
10-10: Path update LGTM.Localizing the import removes the cross-package edge; consistent with the migration intent.
packages/features/ee/round-robin/roundRobinReassignment.ts (1)
19-19: Path update LGTM.Importing from features/eventtypes removes the lib↔app-store coupling.
packages/features/eventtypes/lib/eventNaming.test.ts (1)
4-5: Test import path update LGTM.Matches the new module location; no behavior change.
packages/features/eventtypes/components/tabs/advanced/CustomEventTypeModal.tsx (1)
7-9: Path updates LGTM.Types and functions sourced from the new eventNaming module; UI logic unchanged.
packages/features/bookings/lib/handleNewBooking.ts (2)
26-26: Path updates LGTM.Consolidates event naming utilities under features/eventtypes.
26-26: Remove all@calcom/app-storeimports frompackages/libMultiple files under
packages/libstill import from@calcom/app-store(e.g.
packages/lib/getCalendarsEvents.ts: import { getCalendar } from "@calcom/app-store/_utils/getCalendar"
packages/lib/delegationCredential/server.ts: import { metadata as googleCalendarMetadata } from "@calcom/app-store/googlecalendar/_metadata", etc.). Replace these with the new feature-aligned modules (e.g.@calcom/features/...) or core utilities as intended.⛔ Skipped due to learnings
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.Learnt from: nangelina PR: calcom/cal.com#23486 File: packages/app-store/kyzonspacevideo/package.json:7-12 Timestamp: 2025-09-01T10:25:51.923Z Learning: In Cal.com's monorepo, app-store packages don't need to declare `zod` as a direct dependency in their package.json files. The monorepo uses yarn workspaces with dependency hoisting, where `zod` is available through workspace-level dependency management. Most app-store packages successfully import zod without declaring it as a dependency, following the established monorepo pattern.packages/emails/email-manager.ts (1)
7-9: Path updates LGTM.Email titles now use the centralized eventNaming util; behavior unchanged.
packages/app-store/routing-forms/lib/responseData/getHumanReadableFieldResponseValue.test.ts (1)
3-3: Local type import LGTM.Keeps tests within the app-store package boundary; no runtime effect.
packages/features/bookings/lib/BookingEmailSmsHandler.ts (1)
20-20: Type import path update LGTM.No behavioral impact; aligns with the migration.
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
16-16: Import rewiring looks correct.Paths now point to features/eventtypes and round-robin libs, aligning with the dep-cleanup goal.
Also applies to: 22-22
packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (1)
23-25: Import migration LGTM.Switch to eventNaming module is consistent with the PR’s dependency cleanup.
packages/app-store/routing-forms/lib/substituteVariables.ts (1)
5-5: Import path fix is correct.Keeps routing-forms utilities self-contained after the move.
packages/platform/atoms/event-types/hooks/useEventTypeForm.ts (2)
7-7: Import updates LGTM.validateCustomEventName now correctly sourced from eventNaming; prompt constants consolidation is fine.
Also applies to: 10-10
24-36: Confirm package version bump. VerifiedvalidateCustomEventNameis re-exported in packages/platform/libraries/index.ts (line 89) and packages/platform/libraries/event-types.ts (line 21); ensure you’ve bumped the version in the relevant package.json as noted in the PR.apps/web/modules/test-setup.ts (1)
38-40: Mock path update LGTM.Keeps tests aligned with the eventNaming relocation.
packages/app-store/routing-forms/lib/findFieldValueByIdentifier.test.ts (1)
4-4: Test import path update LGTM.Matches the new location of responseData/types.
packages/app-store/dailyvideo/lib/VideoApiAdapter.ts (1)
18-18: Fetcher import migration LGTM.Using the local dailyApiFetcher removes the lib↔app-store cycle.
packages/app-store/dailyvideo/lib/dailyApiFetcher.ts (1)
3-3: Import relocation LGTM.getDailyAppKeys is now local; resolves the prior “lib importing app-store” violation.
packages/features/tasker/tasks/triggerNoShow/getMeetingSessionsFromRoomName.ts (1)
1-1: No residual old imports found
rg -nP "@calcom/lib/dailyApiFetcher"returned no matches—old import isn’t used anywhere.packages/app-store/routing-forms/lib/findFieldValueByIdentifier.ts (1)
1-2: LGTM on import path consolidation.Type-only import and local path look correct; no behavior change.
packages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.ts (1)
2-2: Good: type-only import avoids runtime dep on @calcom/lib.This helps keep app-store -> lib dependency compile-time only.
packages/platform/libraries/event-types.ts (1)
21-21: LGTM: re-export from features/eventtypes.Keeps platform API stable while decoupling from @calcom/lib/event.
packages/app-store/salesforce/lib/CrmService.ts (2)
6-6: LGTM: routing-forms factory import path updated.
22-22: LGTM: findFieldValueByIdentifier path updated to new app-store module.packages/app-store/routing-forms/lib/RoutingFormResponseDataFactory.test.ts (1)
3-4: LGTM: type-only import path updated; tests remain valid.packages/platform/libraries/index.ts (2)
89-89: LGTM: validateCustomEventName re-export moved to features/eventtypes.
123-124: LGTM: getCalendarLinks re-export moved; no lingering direct imports found. Publishing a new platform version is required for downstream consumers.apps/web/modules/bookings/views/bookings-single-view.tsx (1)
32-32: Import path change forgetEventNameLGTMAPI usage (
getEventName(eventNameObject, true)) matches prior signature.
| SystemField, | ||
| TITLE_FIELD, | ||
| } from "@calcom/features/bookings/lib/SystemField"; | ||
| import { getCalendarLinks, CalendarLinkType } from "@calcom/features/bookings/lib/getCalendarLinks"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Path migration looks right; verify no stale imports remain
Ensure all imports now point to @calcom/features/bookings/lib/getCalendarLinks and that enum values didn’t change.
🏁 Script executed:
#!/bin/bash
# Find stale paths after migration
rg -n --no-heading -S "@calcom/lib/bookings/getCalendarLinks|@calcom/lib/event\b"
# Quick sanity: ensure the new paths are used
rg -n --no-heading -S "@calcom/features/bookings/lib/getCalendarLinks|@calcom/features/eventtypes/lib/eventNaming"Length of output: 5983
Replace all remaining @calcom/lib/event-types imports with the new feature paths
rg output shows numerous residual imports referencing @calcom/lib/event-types/*; update them to their counterparts under @calcom/features/eventtypes/lib before merging.
🤖 Prompt for AI Agents
In apps/web/modules/bookings/views/bookings-single-view.tsx around line 29,
replace the import that references the old path (@calcom/lib/event-types/...)
with the new feature path under @calcom/features/eventtypes/lib (e.g., update
any imports to @calcom/features/eventtypes/lib/<module> and preserve the exact
named exports), and run a repo-wide search (rg "@calcom/lib/event-types") to
update all remaining occurrences to the corresponding
@calcom/features/eventtypes/lib paths before merging.
| import { getCalendarLinks, CalendarLinkType } from "@calcom/features/bookings/lib/getCalendarLinks"; | ||
| import { RATING_OPTIONS, validateRating } from "@calcom/features/bookings/lib/rating"; | ||
| import { getCalendarLinks, CalendarLinkType } from "@calcom/lib/bookings/getCalendarLinks"; | ||
| import type { nameObjectSchema } from "@calcom/features/eventtypes/lib/eventNaming"; |
There was a problem hiding this comment.
Type-only import breaks z.infer<typeof nameObjectSchema>
import type erases the value symbol, so typeof nameObjectSchema is invalid. Import it as a value (or import a dedicated exported type if available) to fix TS errors.
Apply this diff:
-import type { nameObjectSchema } from "@calcom/features/eventtypes/lib/eventNaming";
+import { nameObjectSchema } from "@calcom/features/eventtypes/lib/eventNaming";If the module exports a dedicated type (e.g., NameObject), prefer:
-import type { nameObjectSchema } from "@calcom/features/eventtypes/lib/eventNaming";
+import type { NameObject } from "@calcom/features/eventtypes/lib/eventNaming";and update usage:
- attendeeName: bookingInfo.responses.name as z.infer<typeof nameObjectSchema> | string,
+ attendeeName: bookingInfo.responses.name as NameObject | string,📝 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.
| import type { nameObjectSchema } from "@calcom/features/eventtypes/lib/eventNaming"; | |
| // apps/web/modules/bookings/views/bookings-single-view.tsx | |
| import { nameObjectSchema } from "@calcom/features/eventtypes/lib/eventNaming"; |
🤖 Prompt for AI Agents
In apps/web/modules/bookings/views/bookings-single-view.tsx around line 31, the
import uses "import type { nameObjectSchema }" which erases the value so
"z.infer<typeof nameObjectSchema>" is invalid; change the import to bring in the
runtime value (remove "type") or, if the module exports a dedicated TypeScript
type (e.g., NameObject), import that type instead and update usages to use
NameObject rather than z.infer<typeof nameObjectSchema>; ensure the schema
symbol is available at runtime for typeof or swap to the exported type
everywhere.
2 participants
What does this PR do?
Migration:
@calcom/lib/dailyApiFetcher -> @calcom/app-store/dailyvideo/lib/dailyApiFetcher@calcom/lib/event -> @calcom/features/eventtypes/lib/eventNaming@calcom/lib/server/service/bookingLocationService -> @calcom/features/ee/round-robin/lib/bookingLocationService@calcom/lib/server/service/routingForm -> @calcom/app-store/routing-forms/lib@calcom/lib/bookings/getCalendarLinks -> @calcom/features/bookings/lib/getCalendarLinksMandatory Tasks (DO NOT REMOVE)
How should this be tested?