Skip to content

[CLEAN] Synthetic Benchmark PR #23475 - refactor: remove circular dependency between prisma and app-store packages#87

Open
tomerqodo wants to merge 1 commit intobase_pr_23475_20251204_9511from
clean_pr_23475_20251204_9511
Open

[CLEAN] Synthetic Benchmark PR #23475 - refactor: remove circular dependency between prisma and app-store packages#87
tomerqodo wants to merge 1 commit intobase_pr_23475_20251204_9511from
clean_pr_23475_20251204_9511

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR calcom#23475

Type: Clean (correct implementation)

Original PR Title: refactor: remove circular dependency between prisma and app-store packages
Original PR Description: ## What does this PR do?

Removes the circular dependency between @calcom/prisma and @calcom/app-store packages by eliminating schema exports from the prisma package and moving validation closer to consuming code.

Key Changes:

  • Removed EventTypeAppMetadataSchema and eventTypeAppMetadataOptionalSchema exports from @calcom/prisma/zod-utils
  • Eliminated problematic import: import { appDataSchemas } from "@calcom/app-store/apps.schemas.generated" from prisma package
  • Updated 14 consuming files to import appDataSchemas directly and create local typed schemas using z.object(appDataSchemas).partial().optional() pattern
  • Added type casting for dynamic property access: appSlug as keyof typeof apps

Requested by: @keithwillcode
Devin Session: https://app.devin.ai/sessions/e26d7a824c6d4c9d8ea0803a3714ce33

Visual Demo (For contributors especially)

N/A - This is an internal refactoring with no user-facing changes.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

Critical Testing Areas:

  1. Payment flows - Verify payment processing still works correctly with the new schema validation
  2. Event type management - Test creating/updating event types with app configurations
  3. Credential management - Test app credential deletion and updates
  4. CRM integrations - Verify CRM round-robin functionality works
  5. Booking flows - Test booking creation, confirmation, and seat management

Environment Setup:

  • Standard cal.com development environment
  • Test with various app integrations enabled (payment apps, CRM apps, etc.)

Expected Behavior:

  • All existing functionality should work identically
  • No circular dependency errors during build
  • TypeScript compilation should pass
  • All app metadata validation should behave the same as before

Checklist

⚠️ High Risk Areas for Review:

  • Type Safety: Verify type casting operations (appSlug as keyof typeof apps) handle edge cases safely
  • Schema Consistency: Ensure all local schema definitions use identical patterns across files
  • Circular Dependency: Confirm the dependency cycle is actually broken by checking import graphs
  • Validation Behavior: Test that app metadata validation works identically to before the refactor
  • Edge Cases: Test with invalid/missing app keys to ensure graceful error handling
  • Payment Integration: Thoroughly test payment flows as these use dynamic app property access

Files with Highest Risk:


PR Type

Enhancement


Description

  • Move app metadata schemas from prisma to app-store package

  • Eliminate circular dependency between @calcom/prisma and @calcom/app-store

  • Update 40+ files to import schemas from new location

  • Create new packages/app-store/zod-utils.ts with typed app schemas


Diagram Walkthrough

flowchart LR
  A["@calcom/prisma/zod-utils"] -->|previously exported| B["EventTypeAppMetadataSchema<br/>eventTypeAppMetadataOptionalSchema<br/>eventTypeMetaDataSchemaWithTypedApps"]
  C["@calcom/app-store/zod-utils"] -->|now exports| B
  D["40+ consuming files"] -->|import from| C
  E["@calcom/app-store/apps.schemas.generated"] -->|used by| C
  A -->|no longer exports app schemas| F["Circular dependency broken"]
Loading

File Walkthrough

Relevant files
Enhancement
42 files
zod-utils.ts
New file with typed app metadata schemas                                 
+17/-0   
zod-utils.ts
Remove app metadata schema exports                                             
+0/-11   
queries.ts
Import schema from app-store package                                         
+1/-1     
test-setup.ts
Update mock imports for new schema location                           
+5/-2     
integrations-stripe.e2e.ts
Import schema from app-store package                                         
+1/-1     
CRMRoundRobinSkip.ts
Import schema from app-store package                                         
+1/-1     
getEventTypeAppData.ts
Import schema from app-store package                                         
+1/-1     
checkForMultiplePaymentApps.ts
Import schema from app-store package                                         
+1/-1     
handlePaymentSuccess.ts
Import schema from app-store package                                         
+1/-1     
eventTypeService.ts
Import schema from app-store package                                         
+2/-1     
routingFormBookingFormHandler.ts
Update import path for EventTypeService                                   
+1/-1     
getAllCredentials.ts
Import schema from app-store package                                         
+2/-2     
handleCancelBooking.ts
Import schema from app-store package                                         
+2/-5     
handleConfirmation.ts
Import schema from app-store package                                         
+2/-1     
handleNewBooking.ts
Import schemas from app-store package                                       
+3/-5     
handlePayment.ts
Import schema from app-store package                                         
+1/-1     
createNewSeat.ts
Import schema from app-store package                                         
+1/-1     
handleDeleteCredential.ts
Import schemas from app-store package                                       
+7/-3     
handleChildrenEventTypes.ts
Import schema from app-store package                                         
+1/-1     
webhook.ts
Import schema from app-store package                                         
+1/-1     
roundRobinManualReassignment.ts
Import schema from app-store package                                         
+1/-1     
roundRobinReassignment.ts
Import schema from app-store package                                         
+1/-1     
getPublicEvent.ts
Import schema from app-store package                                         
+1/-1     
types.ts
Import schema from app-store package                                         
+10/-9   
handleSendingAttendeeNoShowDataToApps.ts
Import schema from app-store package                                         
+2/-1     
EventManager.ts
Replace typed schema with generic Record type                       
+3/-4     
defaultEvents.ts
Import schema from app-store package                                         
+1/-1     
getEventTypeById.ts
Import schema from app-store package                                         
+2/-1     
getPaymentAppData.ts
Import schema from app-store package                                         
+1/-1     
handleNoShowFee.ts
Import schema from app-store package                                         
+1/-1     
processNoShowFeeOnCancellation.ts
Import schema from app-store package                                         
+1/-1     
shouldChargeNoShowCancellationFee.test.ts
Import schema from app-store package                                         
+1/-1     
shouldChargeNoShowCancellationFee.ts
Import schema from app-store package                                         
+1/-1     
update.handler.ts
Import schema from app-store package                                         
+2/-2     
getAllActiveWorkflows.handler.ts
Import schema from app-store package                                         
+1/-1     
step-view.tsx
Import schema from app-store package                                         
+1/-1     
bookings-single-view.getServerSideProps.tsx
Import schema from app-store package                                         
+2/-1     
bookings-single-view.tsx
Import schema from app-store package                                         
+2/-1     
EventTypeDescription.tsx
Import schema from app-store package                                         
+1/-1     
EventInstantTab.tsx
Import schema from app-store package                                         
+1/-1     
EventRecurringTab.tsx
Import schema from app-store package                                         
+1/-1     
useTabsNavigations.tsx
Import schema from app-store package                                         
+1/-1     

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "[CLEAN] Synthetic Benchmark PR #23475 - refactor: remove circular dependency between prisma and app-store packages". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Payment logging: The payment webhook handler imports and processes payment success without any visible
additions to structured audit logging of critical actions in the new code lines.

Referred Code
import { handlePaymentSuccess } from "@calcom/app-store/_utils/payments/handlePaymentSuccess";
import { eventTypeMetaDataSchemaWithTypedApps } from "@calcom/app-store/zod-utils";
import { sendAttendeeRequestEmailAndSMS, sendOrganizerRequestEmail } from "@calcom/emails";

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Typing loosened: The constructor and properties changed from typed schemas to Record<string, any>
which may weaken validation and error handling for edge cases.

Referred Code
  user: EventManagerUser;
  eventTypeAppMetadata?: Record<string, any>;
};

export default class EventManager {
  calendarCredentials: CredentialForCalendarService[];
  videoCredentials: CredentialForCalendarService[];
  crmCredentials: CredentialForCalendarService[];
  appOptions?: Record<string, any>;
  /**
   * Takes an array of credentials and initializes a new instance of the EventManager.
   *
   * @param user
   */
  constructor(user: EventManagerUser, eventTypeAppMetadata?: Record<string, any>) {
    log.silly("Initializing EventManager", safeStringify({ user: getPiiFreeUser(user) }));
    const appCredentials = getApps(user.credentials, true).flatMap((app) =>

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Payment log context: New imports and usage around payment flows do not show structured logging or explicit
redaction for sensitive payment metadata in the added lines.

Referred Code
import prisma from "@calcom/prisma";
import type { Prisma } from "@calcom/prisma/client";
import { BookingStatus } from "@calcom/prisma/enums";
import type { EventTypeMetadata } from "@calcom/prisma/zod-utils";

const log = logger.getSubLogger({ prefix: ["[handlePaymentSuccess]"] });
export async function handlePaymentSuccess(paymentId: number, bookingId: number) {
  log.debug(`handling payment success for bookingId ${bookingId}`);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Schema changes: Validation schemas for event type app metadata moved packages and redefined; added
nullable merges should be reviewed to ensure equivalent validation strength across
consuming code.

Referred Code
export const EventTypeAppMetadataSchema = z.object(appDataSchemas).partial();
export const eventTypeAppMetadataOptionalSchema = EventTypeAppMetadataSchema.optional();

export const eventTypeMetaDataSchemaWithTypedApps = eventTypeMetaDataSchemaWithoutApps
  .unwrap()
  .merge(
    z.object({
      apps: eventTypeAppMetadataOptionalSchema,
    })
  )
  .nullable();

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Avoid using Record<string, any>

The EventManager class now uses Record<string, any> for app metadata, which
weakens type safety. To fix this, make the EventManager class generic to allow
consumers to specify the metadata type, preserving type safety without causing
circular dependencies.

Examples:

packages/lib/EventManager.ts [132-145]
  eventTypeAppMetadata?: Record<string, any>;
};

export default class EventManager {
  calendarCredentials: CredentialForCalendarService[];
  videoCredentials: CredentialForCalendarService[];
  crmCredentials: CredentialForCalendarService[];
  appOptions?: Record<string, any>;
  /**
   * Takes an array of credentials and initializes a new instance of the EventManager.

 ... (clipped 4 lines)

Solution Walkthrough:

Before:

// packages/lib/EventManager.ts

export type EventManagerInitParams = {
  user: EventManagerUser;
  eventTypeAppMetadata?: Record<string, any>;
};

export default class EventManager {
  appOptions?: Record<string, any>;

  constructor(user: EventManagerUser, eventTypeAppMetadata?: Record<string, any>) {
    this.appOptions = eventTypeAppMetadata;
    // ...
  }
  // ...
}

After:

// packages/lib/EventManager.ts

export type EventManagerInitParams<TAppMetadata> = {
  user: EventManagerUser;
  eventTypeAppMetadata?: TAppMetadata;
};

export default class EventManager<TAppMetadata = Record<string, unknown>> {
  appOptions?: TAppMetadata;

  constructor(user: EventManagerUser, eventTypeAppMetadata?: TAppMetadata) {
    this.appOptions = eventTypeAppMetadata;
    // ...
  }
  // ...
}
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a significant regression in type safety in the core EventManager class and proposes a robust, architecturally sound solution using generics to fix it without re-introducing dependency issues.

High
Possible issue
Restore type safety for metadata

Restore type safety for eventTypeAppMetadata in EventManagerInitParams by using
the EventTypeAppMetadataSchema from @calcom/app-store/zod-utils instead of
Record<string, any>.

packages/lib/EventManager.ts [130-133]

+import type { EventTypeAppMetadataSchema } from "@calcom/app-store/zod-utils";
+
 export type EventManagerInitParams = {
   user: EventManagerUser;
-  eventTypeAppMetadata?: Record<string, any>;
+  eventTypeAppMetadata?: z.infer<typeof EventTypeAppMetadataSchema>;
 };
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a loss of type safety introduced in the PR by changing a specific Zod schema type to Record<string, any>. Restoring the specific type improves maintainability and prevents potential runtime errors.

Medium
Maintain typed constructor parameters

Restore type safety for the eventTypeAppMetadata parameter in the EventManager
constructor by using the EventTypeAppMetadataSchema from
@calcom/app-store/zod-utils instead of Record<string, any>.

packages/lib/EventManager.ts [145-149]

-constructor(user: EventManagerUser, eventTypeAppMetadata?: Record<string, any>) {
+import type { EventTypeAppMetadataSchema } from "@calcom/app-store/zod-utils";
+
+constructor(user: EventManagerUser, eventTypeAppMetadata?: z.infer<typeof EventTypeAppMetadataSchema>) {
   log.silly("Initializing EventManager", safeStringify({ user: getPiiFreeUser(user) }));
   const appCredentials = getApps(user.credentials, true).flatMap((app) =>
     app.credentials.map((creds) => ({ ...creds, appName: app.name }))
   );

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a loss of type safety in the EventManager constructor, which was changed to use Record<string, any>. Reverting to a Zod-inferred type improves code quality and prevents potential runtime errors.

Medium
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant