Skip to content

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

Open
tomerqodo wants to merge 2 commits into
sentry_full_base_chore_booking_cancellation_refactor_-_2_inject_repositories_and_use_them_instead_of_prisma_in_cancellation_flow_pr3from
sentry_full_head_chore_booking_cancellation_refactor_-_2_inject_repositories_and_use_them_instead_of_prisma_in_cancellation_flow_pr3
Open

chore: [Booking Cancellation Refactor - 2] Inject repositories and use them instead of Prisma in cancellation flow#3
tomerqodo wants to merge 2 commits into
sentry_full_base_chore_booking_cancellation_refactor_-_2_inject_repositories_and_use_them_instead_of_prisma_in_cancellation_flow_pr3from
sentry_full_head_chore_booking_cancellation_refactor_-_2_inject_repositories_and_use_them_instead_of_prisma_in_cancellation_flow_pr3

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#3

Comment on lines +513 to 518
const allUpdatedBookings = await bookingRepository.findManyIncludeWorkflowRemindersAndReferences({
where: {
recurringEventId: bookingToDelete.recurringEventId,
startTime: {
gte: new Date(),
},
},
select: {
id: true,
startTime: true,
endTime: true,
references: {
select: {
uid: true,
type: true,
externalCalendarId: true,
credentialId: true,
},
gte: bookingToDelete.startTime,
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: When cancelling subsequent recurring bookings, the findMany query for reminder cleanup uses a different gte time filter than the updateMany query that performs the cancellation.
Severity: MEDIUM

Suggested Fix

The findMany operation should use the same gte variable that was calculated for and used by the updateMany operation. This ensures that reminder cleanup is performed on the exact same set of bookings that were cancelled.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/features/bookings/lib/handleCancelBooking.ts#L513-L518

Potential issue: When cancelling subsequent recurring bookings where
`cancelSubsequentBookings` is true, the logic to cancel bookings and the logic to clean
up their reminders use different time filters. The cancellation query (`updateMany`)
uses `startTime: { gte: new Date() }`, while the subsequent query to find bookings for
reminder deletion (`findMany`) uses `startTime: { gte: bookingToDelete.startTime }`.
This mismatch can cause workflow reminders to be deleted for bookings that were not
actually cancelled, specifically when cancelling a booking that has already started but
not yet ended.

Did we get this right? 👍 / 👎 to inform future reviews.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant