Skip to content

fix: Make BOOKING_CANCELLED` webhook payload consistent for regular cancellation and request-reschedule flow #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tomerqodo wants to merge 3 commits into
base: qodo_action_req_1_base_fix_make_booking_cancelled_webhook_payload_consistent_for_regular_cancellation_and_request-reschedule_flow_pr11
Choose a base branch
from
Open

fix: Make BOOKING_CANCELLED` webhook payload consistent for regular cancellation and request-reschedule flow #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
91644a1
init
alishaz-polymath Jan 13, 2026
8c828ea
type and test fix
alishaz-polymath Jan 13, 2026
f1abf5d
update pr
tomerqodo Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/developing/guides/automation/webhooks.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,8 @@ Select a version and trigger event to view the example payload:
"requiresConfirmation": true,
"price": null,
"currency": "usd",
"status": "CANCELLED"
"status": "CANCELLED",
"requestReschedule": false
}
}
```
Expand Down
53 changes: 35 additions & 18 deletions packages/features/bookings/lib/handleCancelBooking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ import { getBookingToDelete } from "./getBookingToDelete";
import { handleInternalNote } from "./handleInternalNote";
import cancelAttendeeSeat from "./handleSeats/cancel/cancelAttendeeSeat";
import type { IBookingCancelService } from "./interfaces/IBookingCancelService";
import { buildActorEmail, getUniqueIdentifier, makeGuestActor, makeUserActor } from "@calcom/features/booking-audit/lib/makeActor";
import {
buildActorEmail,
getUniqueIdentifier,
makeGuestActor,
makeUserActor,
} from "@calcom/features/booking-audit/lib/makeActor";
import type { Actor } from "@calcom/features/booking-audit/lib/dto/types";

const log = logger.getSubLogger({ prefix: ["handleCancelBooking"] });
Expand Down Expand Up @@ -102,12 +107,17 @@ function getAuditActor({
})
);
// Having fallback prefix makes it clear that we created guest actor from fallback logic
actorEmail = buildActorEmail({ identifier: getUniqueIdentifier({ prefix: "fallback" }), actorType: "guest" });
}
else {
actorEmail = buildActorEmail({
identifier: getUniqueIdentifier({ prefix: "fallback" }),
actorType: "guest",
});
} else {
// We can't trust cancelledByEmail and thus can't reuse it as is because it can be set anything by anyone. If we use that as guest actor, we could accidentally attribute the action to the wrong guest actor.
// Having param prefix makes it clear that we created guest actor from query param and we still don't use the email as is.
actorEmail = buildActorEmail({ identifier: getUniqueIdentifier({ prefix: "param" }), actorType: "guest" });
actorEmail = buildActorEmail({
identifier: getUniqueIdentifier({ prefix: "param" }),
actorType: "guest",
});
}

return makeGuestActor({ email: actorEmail, name: null });
Expand Down Expand Up @@ -141,10 +151,13 @@ async function handler(input: CancelBookingInput) {
// Extract action source once for reuse
const actionSource = input.actionSource ?? "UNKNOWN";
if (actionSource === "UNKNOWN") {
log.warn("Booking cancellation with unknown actionSource", safeStringify({
bookingUid: bookingToDelete.uid,
userUuid,
}));
log.warn(
"Booking cancellation with unknown actionSource",
safeStringify({
bookingUid: bookingToDelete.uid,
userUuid,
})
);
}

const actorToUse = getAuditActor({
Expand Down Expand Up @@ -358,12 +371,12 @@ async function handler(input: CancelBookingInput) {
cancellationReason: cancellationReason,
...(teamMembers &&
teamId && {
team: {
name: bookingToDelete?.eventType?.team?.name || "Nameless",
members: teamMembers,
id: teamId,
},
}),
team: {
name: bookingToDelete?.eventType?.team?.name || "Nameless",
members: teamMembers,
id: teamId,
},
}),
seatsPerTimeSlot: bookingToDelete.eventType?.seatsPerTimeSlot,
seatsShowAttendees: bookingToDelete.eventType?.seatsShowAttendees,
iCalUID: bookingToDelete.iCalUID,
Expand Down Expand Up @@ -397,20 +410,24 @@ async function handler(input: CancelBookingInput) {
message: "Attendee successfully removed.",
} satisfies HandleCancelBookingResponse;

const promises = webhooks.map((webhook) =>
// Only send webhooks if enabled in environment
const webhooksEnabled = process.env.ENABLE_WEBHOOKS !== "false";

const promises = webhooksEnabled ? webhooks.map((webhook) =>
Comment on lines +413 to +416
Copy link

@qodo-code-review qodo-code-review bot Jan 29, 2026

Choose a reason for hiding this comment

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

Action required

1. process.env in handlecancelbooking 📘 Rule violation ⛯ Reliability

• The cancellation handler reads process.env.ENABLE_WEBHOOKS directly inside business logic to
  decide whether to emit webhooks.
• This violates the requirement to limit direct environment access to configuration/environment
  utilities, reducing testability and increasing risk of runtime misconfiguration.
• The webhook enablement flag should be injected via a config object or read through a dedicated
  configuration abstraction.
Agent prompt 
## Issue description
`handleCancelBooking` directly reads `process.env.ENABLE_WEBHOOKS` inside business logic, which violates the rule that environment variables must only be accessed in configuration/environment-detection code.

## Issue Context
Webhook emission should be controlled via an injected/configured flag (e.g., `deps.config.webhooksEnabled`) or a dedicated config module that encapsulates `process.env` reads.

## Fix Focus Areas
- packages/features/bookings/lib/handleCancelBooking.ts[413-430]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

sendPayload(webhook.secret, eventTrigger, new Date().toISOString(), webhook, {
...evt,
...eventTypeInfo,
status: "CANCELLED",
smsReminderNumber: bookingToDelete.smsReminderNumber || undefined,
cancelledBy: cancelledBy,
requestReschedule: false,
}).catch((e) => {
logger.error(
`Error executing webhook for event: ${eventTrigger}, URL: ${webhook.subscriberUrl}, bookingId: ${evt.bookingId}, bookingUid: ${evt.uid}`,
safeStringify(e)
);
})
);
) : [];
Comment on lines +413 to +430
Copy link

@qodo-code-review qodo-code-review bot Jan 29, 2026

Choose a reason for hiding this comment

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

Action required

3. Webhook gate incomplete 🐞 Bug ⛯ Reliability

• ENABLE_WEBHOOKS gating was added only to the main cancellation handler; other cancellation paths
  still emit webhooks unconditionally.
• This creates unreliable operational behavior: setting ENABLE_WEBHOOKS=false will not actually
  disable all cancellation-related webhooks, leading to partial/accidental emissions.
Agent prompt 
### Issue description
`ENABLE_WEBHOOKS` is only checked in one cancellation code path, so webhooks are still emitted by other cancellation flows even when operators believe webhooks are disabled.

### Issue Context
There is already an env-driven decision point in `sendOrSchedulePayload` (`TASKER_ENABLE_WEBHOOKS`) which is a more central place to enforce webhook-level gating.

### Fix Focus Areas
- packages/features/bookings/lib/handleCancelBooking.ts[413-431]
- packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts[170-184]
- packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts[305-314]
- packages/features/webhooks/lib/sendOrSchedulePayload.ts[6-10]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

await Promise.all(promises);

const workflows = await getAllWorkflowsFromEventType(bookingToDelete.eventType, bookingToDelete.userId);
Expand Down Expand Up @@ -714,7 +731,7 @@ type BookingCancelServiceDependencies = {
* Handles both individual booking cancellations and bulk cancellations for recurring events.
*/
export class BookingCancelService implements IBookingCancelService {
constructor(private readonly deps: BookingCancelServiceDependencies) { }
constructor(private readonly deps: BookingCancelServiceDependencies) {}

async cancelBooking(input: { bookingData: CancelRegularBookingData; bookingMeta?: CancelBookingMeta }) {
const cancelBookingInput: CancelBookingInput = {
Expand Down
1 change: 1 addition & 0 deletions packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ async function cancelAttendeeSeat(
...eventTypeInfo,
status: "CANCELLED",
smsReminderNumber: bookingToDelete.smsReminderNumber || undefined,
requestReschedule: true,
};

const promises = webhooks.map((webhook) =>
Expand Down
71 changes: 40 additions & 31 deletions packages/features/bookings/repositories/BookingRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ const buildWhereClauseForActiveBookings = ({
},
...(!includeNoShowInRRCalculation
? {
OR: [{ noShowHost: false }, { noShowHost: null }],
}
OR: [{ noShowHost: false }, { noShowHost: null }],
}
: {}),
},
{
Expand All @@ -159,24 +159,24 @@ const buildWhereClauseForActiveBookings = ({
...(startDate || endDate
? rrTimestampBasis === RRTimestampBasis.CREATED_AT
? {
createdAt: {
...(startDate ? { gte: startDate } : {}),
...(endDate ? { lte: endDate } : {}),
},
}
createdAt: {
...(startDate ? { gte: startDate } : {}),
...(endDate ? { lte: endDate } : {}),
},
}
: {
startTime: {
...(startDate ? { gte: startDate } : {}),
...(endDate ? { lte: endDate } : {}),
},
}
startTime: {
...(startDate ? { gte: startDate } : {}),
...(endDate ? { lte: endDate } : {}),
},
}
: {}),
...(virtualQueuesData
? {
routedFromRoutingFormReponse: {
chosenRouteId: virtualQueuesData.chosenRouteId,
},
}
routedFromRoutingFormReponse: {
chosenRouteId: virtualQueuesData.chosenRouteId,
},
}
: {}),
});

Expand Down Expand Up @@ -325,7 +325,7 @@ const selectStatementToGetBookingForCalEventBuilder = {
};

export class BookingRepository {
constructor(private prismaClient: PrismaClient) { }
constructor(private prismaClient: PrismaClient) {}

/**
* Gets the fromReschedule field for a booking by UID
Expand Down Expand Up @@ -656,20 +656,20 @@ export class BookingRepository {

const currentBookingsAllUsersQueryThree = eventTypeId
? this.prismaClient.booking.findMany({
where: {
startTime: { lte: endDate },
endTime: { gte: startDate },
eventType: {
id: eventTypeId,
requiresConfirmation: true,
requiresConfirmationWillBlockSlot: true,
},
status: {
in: [BookingStatus.PENDING],
where: {
startTime: { lte: endDate },
endTime: { gte: startDate },
eventType: {
id: eventTypeId,
requiresConfirmation: true,
requiresConfirmationWillBlockSlot: true,
},
status: {
in: [BookingStatus.PENDING],
},
},
},
select: bookingsSelect,
})
select: bookingsSelect,
})
: [];

const [resultOne, resultTwo, resultThree] = await Promise.all([
Expand Down Expand Up @@ -1659,6 +1659,8 @@ export class BookingRepository {
teamId: true,
parentId: true,
slug: true,
title: true,
length: true,
hideOrganizerEmail: true,
customReplyToEmail: true,
bookingFields: true,
Expand All @@ -1683,6 +1685,7 @@ export class BookingRepository {
workflowReminders: true,
responses: true,
iCalUID: true,
iCalSequence: true,
},
});
}
Expand Down Expand Up @@ -1913,7 +1916,13 @@ export class BookingRepository {
});
}

async updateRecordedStatus({ bookingUid, isRecorded }: { bookingUid: string; isRecorded: boolean }): Promise<void> {
async updateRecordedStatus({
bookingUid,
isRecorded,
}: {
bookingUid: string;
isRecorded: boolean;
}): Promise<void> {
await this.prismaClient.booking.update({
where: { uid: bookingUid },
data: { isRecorded },
Expand Down
3 changes: 3 additions & 0 deletions packages/features/webhooks/lib/dto/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ export interface BookingCancelledDTO extends BaseEventDTO {
eventTypeId: number | null;
userId: number | null;
smsReminderNumber?: string | null;
iCalSequence?: number | null;
};
cancelledBy?: string;
cancellationReason?: string;
requestReschedule?: boolean;
}

export interface BookingRejectedDTO extends BaseEventDTO {
Expand Down Expand Up @@ -582,6 +584,7 @@ export type EventPayloadType = CalendarEvent &
rescheduledBy?: string;
cancelledBy?: string;
paymentData?: Record<string, unknown>;
requestReschedule?: boolean;
};

// dto/types.ts
Expand Down
6 changes: 5 additions & 1 deletion packages/features/webhooks/lib/factory/base/BaseBookingPayloadBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import type { IBookingPayloadBuilder } from "../versioned/PayloadBuilderFactory"
*/
export type BookingExtraDataMap = {
[WebhookTriggerEvents.BOOKING_CREATED]: null;
[WebhookTriggerEvents.BOOKING_CANCELLED]: { cancelledBy?: string; cancellationReason?: string };
[WebhookTriggerEvents.BOOKING_CANCELLED]: {
cancelledBy?: string;
cancellationReason?: string;
requestReschedule?: boolean;
};
[WebhookTriggerEvents.BOOKING_REQUESTED]: null;
[WebhookTriggerEvents.BOOKING_REJECTED]: null;
[WebhookTriggerEvents.BOOKING_RESCHEDULED]: {
Expand Down
1 change: 1 addition & 0 deletions packages/features/webhooks/lib/factory/versioned/v2021-10-20/BookingPayloadBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class BookingPayloadBuilder extends BaseBookingPayloadBuilder {
extra: {
cancelledBy: dto.cancelledBy,
cancellationReason: dto.cancellationReason,
requestReschedule: dto.requestReschedule ?? true,
Copy link

@qodo-code-review qodo-code-review bot Jan 29, 2026

Choose a reason for hiding this comment

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

Action required

2. Wrong reschedule default 🐞 Bug ✓ Correctness

• Webhook v2021-10-20 builder defaults requestReschedule to true when omitted, which will mark
  *all* BOOKING_CANCELLED webhooks as reschedule-related for subscribers on that version.
• This contradicts the docs and other cancellation senders in this PR that use/assume false, and
  can break downstream automations that rely on this flag.
Agent prompt 
### Issue description
`packages/features/webhooks` v2021-10-20 payload builder currently defaults `requestReschedule` to `true` for BOOKING_CANCELLED when the DTO doesn't set it, which mislabels normal cancellations as reschedule requests.

### Issue Context
Docs and other cancellation senders in this PR treat `requestReschedule` as `false` for standard cancellations.

### Fix Focus Areas
- packages/features/webhooks/lib/factory/versioned/v2021-10-20/BookingPayloadBuilder.ts[37-50]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

},
});

Expand Down
1 change: 1 addition & 0 deletions packages/features/webhooks/lib/sendPayload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export type EventPayloadType = CalendarEvent &
rescheduledBy?: string;
cancelledBy?: string;
paymentData?: PaymentData;
requestReschedule?: boolean;
};

export type WebhookPayloadType =
Expand Down
1 change: 1 addition & 0 deletions packages/features/webhooks/lib/service/BookingWebhookService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export class BookingWebhookService implements IBookingWebhookService {
booking: params.booking,
cancelledBy: params.cancelledBy,
cancellationReason: params.cancellationReason,
requestReschedule: params.requestReschedule,
};

await this.webhookNotifier.emitWebhook(dto, params.isDryRun);
Expand Down
2 changes: 2 additions & 0 deletions packages/features/webhooks/lib/types/params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface BookingCancelledParams {
eventTypeId: number | null;
userId: number | null;
smsReminderNumber?: string | null;
iCalSequence?: number | null;
};
eventType: {
id: number;
Expand All @@ -57,6 +58,7 @@ export interface BookingCancelledParams {
};
cancelledBy?: string;
cancellationReason?: string;
requestReschedule?: boolean;
teamId?: number | null;
orgId?: number | null;
platformClientId?: string;
Expand Down
11 changes: 11 additions & 0 deletions packages/lib/server/service/BookingWebhookFactory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ interface BaseWebhookPayload {
interface CancelledEventPayload extends BaseWebhookPayload {
cancelledBy: string;
cancellationReason: string;
eventTypeId?: number | null;
length?: number | null;
iCalSequence?: number | null;
eventTitle?: string | null;
requestReschedule?: boolean;
}

export class BookingWebhookFactory {
Expand Down Expand Up @@ -122,6 +127,12 @@ export class BookingWebhookFactory {
...basePayload,
cancelledBy: params.cancelledBy,
cancellationReason: params.cancellationReason,
status: "CANCELLED" as const,
eventTypeId: params.eventTypeId ?? null,
length: params.length ?? null,
iCalSequence: params.iCalSequence ?? null,
eventTitle: params.eventTitle ?? null,
requestReschedule: params.requestReschedule ?? false,
};
}
}
6 changes: 6 additions & 0 deletions packages/lib/server/service/__tests__/BookingWebhookFactory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ describe("BookingWebhookFactory", () => {
"description",
"customInputs",
"responses",
"eventTitle",
"eventTypeId",
"length",
"requestReschedule",
"iCalSequence",
"userFieldsResponses",
"startTime",
"endTime",
Expand All @@ -104,6 +109,7 @@ describe("BookingWebhookFactory", () => {
"smsReminderNumber",
"cancellationReason",
"cancelledBy",
"status",
];
const actualKeys = Object.keys(payload).sort();
expect(actualKeys).toEqual(expectedKeys.sort());
Expand Down
10 changes: 6 additions & 4 deletions packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ export const requestRescheduleHandler = async ({ ctx, input, source }: RequestRe
throw new TRPCError({ code: "FORBIDDEN", message: "User isn't owner of the current booking" });
}

let event: Partial<EventType> = {};
if (bookingToReschedule.eventType) {
event = bookingToReschedule.eventType;
}
const event: Partial<EventType> = bookingToReschedule.eventType ?? {};
await bookingRepository.updateBookingStatus({
bookingId: bookingToReschedule.id,
status: BookingStatus.CANCELLED,
Expand Down Expand Up @@ -274,6 +271,11 @@ export const requestRescheduleHandler = async ({ ctx, input, source }: RequestRe
smsReminderNumber: bookingToReschedule.smsReminderNumber,
}),
cancelledBy: user.email,
eventTypeId: bookingToReschedule.eventTypeId,
length: bookingToReschedule.eventType?.length ?? null,
iCalSequence: (bookingToReschedule.iCalSequence ?? 0) + 1,
eventTitle: bookingToReschedule.eventType?.title ?? null,
requestReschedule: true,
Comment on lines +274 to +278
Copy link

@qodo-code-review qodo-code-review bot Jan 29, 2026

Choose a reason for hiding this comment

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

Action required

4. Icalsequence not persisted 🐞 Bug ⛯ Reliability

• The request-reschedule flow includes an incremented iCalSequence in the webhook payload, but
  does not persist the increment to the booking record.
• This can cause inconsistencies between webhook data and the booking source-of-truth
  (booking.iCalSequence), and diverges from the standard cancellation flow which persists
  iCalSequence when cancelling.
Agent prompt 
### Issue description
The request-reschedule flow increments `iCalSequence` for the webhook payload but does not update the booking record. This can create inconsistency between stored booking data and emitted webhook data.

### Issue Context
Standard cancellation persists iCalSequence on cancellation; request-reschedule should likely do the same if it emits the incremented value.

### Fix Focus Areas
- packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts[106-113]
- packages/trpc/server/routers/viewer/bookings/requestReschedule.handler.ts[248-279]
- packages/features/bookings/repositories/BookingRepository.ts[1693-1720]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

});

// Send webhook
Expand Down
Loading