Code Review Bench PR #24159 - chore: [Booking Cancellation Refactor - 2] Inject repositories and use them instead of Prisma in cancellation flow

[ketkarameya]

Code Review Bench PR calcom#24159 - chore: [Booking Cancellation Refactor - 2] Inject repositories and use them instead of Prisma in cancellation flow

Benchmark PR for Gitar evaluation

Original PR: agentic-review-benchmarks#3

chore: [Booking Cancellation Refactor - 2] Inject repositories and use them instead of Prisma in cancellation flow

---

Summary by Gitar

• Repository abstraction layer: Introduced five repository interfaces (IUserRepository, IBookingRepository, IProfileRepository, IBookingReferenceRepository, IBookingAttendeeRepository) to replace direct Prisma client access in the booking cancellation flow
• Refactored cancellation service: Updated BookingCancelService constructor to accept repository dependencies instead of raw Prisma client, enabling better testability through dependency injection while maintaining backward compatibility via optional injection
• DI system expansion: Extended dependency injection configuration with new module loaders, tokens, and NestJS provider bindings for all five repositories across packages/features/di/, apps/api/v2/src/lib/modules/, and apps/api/v2/src/lib/repositories/
• Test coverage: Added test case validating booking reference cleanup during cancellation with multiple reference types (daily_video, google_calendar)

_{This will update automatically on new commits.}

[gitar-bot-staging]

🚨 Bug: Removing locale from userSelect breaks organizer translation

The locale field was removed from userSelect in UserRepository.ts, but organizer.locale is still accessed in handleCancelBooking.ts at lines 337 and 395. Since findByIdOrThrow uses userSelect to determine which fields to fetch, locale will no longer be returned by Prisma. This means organizer.locale will be undefined, and the translation will always fall back to "en", breaking localized cancellation emails for non-English users.

This is a data regression — every cancellation notification will now be sent in English regardless of the organizer's locale preference.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	completedOnboarding: true,
	completedOnboarding: true,
	locale: true,
	timeFormat: true,

• Apply suggested fix

[gitar-bot-staging]

⚠️ Bug: updateMany return type mismatch with IBookingRepository

The IBookingRepository interface declares updateMany as returning Promise<{ count: number }>, but the implementation in BookingRepository.updateMany (line 1502) uses await without return, making it effectively return Promise<void>.

Prisma's updateMany returns Prisma.BatchPayload (which has a count field), but since the result is discarded with await (no return), any consumer relying on the interface contract to get the count back will get undefined at runtime.

While the current callers in handleCancelBooking don't use the return value, this violates the interface contract and will break any future consumer that does.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	async updateMany({ where, data }: { where: BookingWhereInput; data: BookingUpdateData }) {
	async updateMany({ where, data }: { where: BookingWhereInput; data: BookingUpdateData }) {
	return await this.prismaClient.booking.updateMany({
	where: where,
	data,
	});
	}

• Apply suggested fix

[gitar-bot-staging]

⚠️ Bug: updateIncludeWorkflowRemindersAndReferences missing from interface

The handleCancelBooking.ts handler calls bookingRepository.updateIncludeWorkflowRemindersAndReferences() (line 546), and BookingRepository implements it (line 1520), but this method is not declared in the IBookingRepository interface.

This means:

1. The interface is incomplete — anyone implementing IBookingRepository for testing or alternative backends won't know they need this method.
2. If the Dependencies type or BookingCancelService is ever updated to use IBookingRepository instead of the concrete BookingRepository, TypeScript won't catch the missing method, and it will fail at runtime.

The method should be added to IBookingRepository to maintain the interface contract.

_{Was this helpful? React with 👍 / 👎}

• Apply suggested fix

[gitar-bot-staging]

⚠️ Bug: Behavioral change: findMany gte filter uses wrong date

In the recurring booking cancellation path, the findManyIncludeWorkflowRemindersAndReferences query now uses gte: bookingToDelete.startTime (line 517) instead of the original gte: new Date().

For the allRemainingBookings case (not cancelSubsequentBookings), the updateMany correctly cancels bookings with gte: new Date(), but the subsequent findMany uses gte: bookingToDelete.startTime, which could be in the past. This means the findMany will also fetch bookings that have already occurred (between bookingToDelete.startTime and now) and were NOT cancelled by the updateMany. This will cause workflow reminder cleanup to run on bookings that shouldn't have been touched, potentially deleting reminders for already-completed bookings.

The original code correctly used new Date() for both the updateMany and the findMany in the allRemainingBookings path.

_{Was this helpful? React with 👍 / 👎}

Suggested change

	gte: bookingToDelete.startTime,
	const allUpdatedBookings = await bookingRepository.findManyIncludeWorkflowRemindersAndReferences({
	where: {
	recurringEventId: bookingToDelete.recurringEventId,
	startTime: {
	gte,
	},
	},
	});

• Apply suggested fix

[gitar-bot-staging]

⚠️ This PR is blocked due to unresolved code review findings.

_{Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.}

[gitar-bot-staging]

Code Review 🚫 Blocked 0 resolved / 5 findings

Repository pattern refactor has a critical bug (removing locale from user select breaks cancellation emails) and several important contract/behavioral issues in the new repository layer.

🚨 Bug: Removing locale from userSelect breaks organizer translation

📄 packages/features/users/repositories/UserRepository.ts:100

The locale field was removed from userSelect in UserRepository.ts, but organizer.locale is still accessed in handleCancelBooking.ts at lines 337 and 395. Since findByIdOrThrow uses userSelect to determine which fields to fetch, locale will no longer be returned by Prisma. This means organizer.locale will be undefined, and the translation will always fall back to "en", breaking localized cancellation emails for non-English users.

This is a data regression — every cancellation notification will now be sent in English regardless of the organizer's locale preference.

Suggested fix

  completedOnboarding: true,
  locale: true,
  timeFormat: true,

⚠️ Bug: updateMany return type mismatch with IBookingRepository

📄 packages/features/bookings/repositories/BookingRepository.ts:1502 📄 packages/lib/server/repository/dto/IBookingRepository.ts:48

The IBookingRepository interface declares updateMany as returning Promise<{ count: number }>, but the implementation in BookingRepository.updateMany (line 1502) uses await without return, making it effectively return Promise<void>.

Prisma's updateMany returns Prisma.BatchPayload (which has a count field), but since the result is discarded with await (no return), any consumer relying on the interface contract to get the count back will get undefined at runtime.

While the current callers in handleCancelBooking don't use the return value, this violates the interface contract and will break any future consumer that does.

Suggested fix

async updateMany({ where, data }: { where: BookingWhereInput; data: BookingUpdateData }) {
    return await this.prismaClient.booking.updateMany({
      where: where,
      data,
    });
  }

⚠️ Bug: updateIncludeWorkflowRemindersAndReferences missing from interface

📄 packages/lib/server/repository/dto/IBookingRepository.ts:46

The handleCancelBooking.ts handler calls bookingRepository.updateIncludeWorkflowRemindersAndReferences() (line 546), and BookingRepository implements it (line 1520), but this method is not declared in the IBookingRepository interface.

This means:

1. The interface is incomplete — anyone implementing IBookingRepository for testing or alternative backends won't know they need this method.
2. If the Dependencies type or BookingCancelService is ever updated to use IBookingRepository instead of the concrete BookingRepository, TypeScript won't catch the missing method, and it will fail at runtime.

The method should be added to IBookingRepository to maintain the interface contract.

⚠️ Bug: Behavioral change: findMany gte filter uses wrong date

📄 packages/features/bookings/lib/handleCancelBooking.ts:517

In the recurring booking cancellation path, the findManyIncludeWorkflowRemindersAndReferences query now uses gte: bookingToDelete.startTime (line 517) instead of the original gte: new Date().

For the allRemainingBookings case (not cancelSubsequentBookings), the updateMany correctly cancels bookings with gte: new Date(), but the subsequent findMany uses gte: bookingToDelete.startTime, which could be in the past. This means the findMany will also fetch bookings that have already occurred (between bookingToDelete.startTime and now) and were NOT cancelled by the updateMany. This will cause workflow reminder cleanup to run on bookings that shouldn't have been touched, potentially deleting reminders for already-completed bookings.

The original code correctly used new Date() for both the updateMany and the findMany in the allRemainingBookings path.

Suggested fix

    const allUpdatedBookings = await bookingRepository.findManyIncludeWorkflowRemindersAndReferences({
      where: {
        recurringEventId: bookingToDelete.recurringEventId,
        startTime: {
          gte,
        },
      },
    });

💡 Quality: Dependencies type uses concrete classes instead of interfaces

📄 packages/features/bookings/lib/handleCancelBooking.ts:89

The Dependencies type in handleCancelBooking.ts (line 89) uses concrete classes (UserRepository, BookingRepository, ProfileRepository, BookingReferenceRepository, PrismaBookingAttendeeRepository) instead of the newly-created interfaces (IBookingRepository, IProfileRepository, IBookingReferenceRepository, IBookingAttendeeRepository).

This undermines the purpose of the repository pattern refactor — consumers are still coupled to the concrete Prisma implementations. Using interfaces would allow test doubles, alternative backends, or decorators to be injected cleanly without depending on Prisma at all.

Rules 🎸 2 actions taken

Gitar Rules

🎸 Linear Auto-Linking: Searched author's recent Linear issues; no matches with sufficient confidence (<40%) for auto-linking

🎸 Summary Enhancement: Appended technical summary with 4 bullet points covering repository abstraction, service refactoring, DI expansion, and test coverage

_{4 rules not applicable. Show all rules by commenting gitar display:verbose.}

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[kayagokalp]

Benchmark Evaluation Results

Ground Truth Bugs (Recall: 4/6 = 66.7%)

ID	Severity	Title	Found?
GT-3-001	Critical	Locale field removed from userSelect breaks organizer transl	Yes
GT-3-002	Critical	Date filter mismatch in recurring booking cancellation	Yes
GT-3-003	Important	updateMany returns void instead of count, violating interfac	Yes
GT-3-004	Important	updateIncludeWorkflowRemindersAndReferences method missing f	Yes
GT-3-005	Critical	Invalid module import path will break at build time	No
GT-3-006	Important	Incorrect import path for BookingReferenceRepository	No

Extra Gitar Findings

File	Line	Valid Bug?	Notes
packages/features/bookings/lib/handleCan	89	No	Dependencies type uses concrete classes instead of

Summary

• Recall: 66.7% (4/6)
• Precision: 80.0% (4/5)
• Precision counts ground truth matches + valid extra findings as true positives