fix: event host unable to cancel event when Disable Cancelling is enabled.#24021
fix: event host unable to cancel event when Disable Cancelling is enabled.#24021GurneeshBudhiraja wants to merge 16 commits intocalcom:mainfrom
Disable Cancelling is enabled.#24021Conversation
…Rescheduling` is enabled
…ts can cancel event when cancellation is disabled
|
|
WalkthroughAdds host detection in BookingListItem and passes an optional isHost flag through BookingActionContext. BookingActionContext’s attendee item shape is expanded (id, noShow, phoneNumber). bookingActions now permits cancel when isHost is true by short-circuiting cancel-disabled checks; a minor variable rename in getActionLabel was made. Server-side cancellation logic (handleCancelBooking) is updated to compute isCancellationUserHost earlier and allow host-initiated cancellations for event types that disable guest cancellations. Tests were added to assert host-can-cancel and non-host-cannot for disabled-cancellation events; two duplicate test pairs were introduced. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.ts📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx,js,jsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2025-09-12T11:23:34.158ZApplied to files:
📚 Learning: 2025-08-27T13:32:46.887ZApplied to files:
📚 Learning: 2025-08-21T13:55:23.470ZApplied to files:
📚 Learning: 2025-08-21T12:28:42.018ZApplied to files:
🧬 Code graph analysis (1)packages/features/bookings/lib/handleCancelBooking/test/handleCancelBooking.test.ts (1)
⏰ 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). (1)
🔇 Additional comments (5)
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 |
github-actions
bot
added
the
bookings
|
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: |
|
@GurneeshBudhiraja is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
GurneeshBudhiraja
changed the title
Disable Cancelling is enabled.Disable Cancelling is enabled.
GurneeshBudhiraja
changed the title
Disable Cancelling is enabled.Disable Cancelling is enabled.
GurneeshBudhiraja
changed the title
Disable Cancelling is enabled.Disable Cancelling is enabled.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/components/booking/bookingActions.ts (1)
26-33: Consider making attendee list type more explicit.The inline type definition for
attendeeListcould be extracted into a named type for better maintainability and reusability.+export type BookingAttendee = { + name: string; + email: string; + id: number; + noShow: boolean; + phoneNumber: string | null; +}; + export interface BookingActionContext { booking: BookingItemProps; // ... other fields ... isHost?: boolean; - attendeeList: Array<{ - name: string; - email: string; - id: number; - noShow: boolean; - phoneNumber: string | null; - }>; + attendeeList: Array<BookingAttendee>; getSeatReferenceUid: () => string | undefined; t: (key: string) => string; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/components/booking/BookingListItem.tsx(2 hunks)apps/web/components/booking/bookingActions.test.ts(2 hunks)apps/web/components/booking/bookingActions.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:
apps/web/components/booking/BookingListItem.tsx
**/*.{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:
apps/web/components/booking/BookingListItem.tsxapps/web/components/booking/bookingActions.test.tsapps/web/components/booking/bookingActions.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:
apps/web/components/booking/BookingListItem.tsxapps/web/components/booking/bookingActions.test.tsapps/web/components/booking/bookingActions.ts
**/*.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:
apps/web/components/booking/bookingActions.test.tsapps/web/components/booking/bookingActions.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23736
File: packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.ts:73-84
Timestamp: 2025-09-12T11:23:34.158Z
Learning: In the Cal.com codebase, the forceRescheduleForCancelledBooking flag historically affects both CANCELLED and REJECTED booking statuses, despite its name suggesting it should only affect cancelled bookings. This behavior existed before PR #23736 and was preserved during the refactoring.
📚 Learning: 2025-09-12T11:23:34.158Z
Learnt from: hariombalhara
PR: calcom/cal.com#23736
File: packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.ts:73-84
Timestamp: 2025-09-12T11:23:34.158Z
Learning: The test file packages/features/bookings/lib/reschedule/determineReschedulePreventionRedirect.test.ts explicitly documents on line 236 that the current behavior of forceRescheduleForCancelledBooking affecting both CANCELLED and REJECTED bookings is known to be incorrect, but is preserved as "Current Behavior" for backward compatibility. The test comment states the expected behavior should be that REJECTED bookings redirect to booking details even when forceRescheduleForCancelledBooking=true.
Applied to files:
apps/web/components/booking/bookingActions.test.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:
apps/web/components/booking/bookingActions.test.ts
🧬 Code graph analysis (1)
apps/web/components/booking/bookingActions.test.ts (1)
apps/web/components/booking/bookingActions.ts (1)
isActionDisabled(207-234)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (7)
apps/web/components/booking/bookingActions.test.ts (2)
506-514: LGTM! Test validates the fix for issue #24017.The test correctly verifies that hosts can cancel bookings even when the "Disable Cancelling" feature is enabled, which directly addresses the bug fix objective.
47-57: LGTM! Type safety improvement.Removing the type assertion improves type safety without changing functionality.
apps/web/components/booking/bookingActions.ts (2)
207-224: Excellent implementation of the host override logic.The implementation correctly grants hosts the ability to cancel events regardless of the
isDisabledCancellingsetting, fixing the reported issue. The early return pattern is clean and efficient.
237-237: Good practice: Prefixing unused variables.Using the underscore prefix for
_bookingcorrectly indicates it's intentionally unused while avoiding linter warnings.apps/web/components/booking/BookingListItem.tsx (3)
192-196: Host detection logic correctly implemented.The host detection properly checks both the booking's user ID and the logged-in user's ID to determine host status. The null checks ensure safety.
137-145: Attendee list mapping aligns with updated type definition.The attendee list correctly includes all required fields (id, noShow, phoneNumber) as defined in the BookingActionContext interface.
239-266: Context object properly includes isHost flag.The BookingActionContext is correctly constructed with the new
isHostproperty, enabling the host-specific cancellation logic in the action handlers.
Devanshusharma2005
left a comment
Devanshusharma2005
left a comment
There was a problem hiding this comment.
Lets add loom to show the changes.
| bookingId: 1, | ||
| noShow: false, | ||
| } as any, | ||
| }, |
There was a problem hiding this comment.
any was giving the type issue at the time of commit.
I have added the Loom link in the PR description |
…the event is disabled for cancellation
|
This PR is being marked as stale due to inactivity. |
pallava-joshi
left a comment
pallava-joshi
left a comment
There was a problem hiding this comment.
looks good, can you please resolve the merge conflicts here.
|
I'm marking this draft until then. feel free to tag me here; happy to help :) |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…conflicts Co-Authored-By: unknown <>
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…conflicts Co-Authored-By: unknown <>
|
@romitg2 I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/components/booking/actions/bookingActions.ts">
<violation number="1" location="apps/web/components/booking/actions/bookingActions.ts:267">
P2: Host override skips cancelled/rejected checks, enabling cancel for already cancelled/rejected bookings.</violation>
<violation number="2" location="apps/web/components/booking/actions/bookingActions.ts:267">
P2: `isHost` is never provided by the action context builder, so the new cancel override always sees `undefined` and the fix has no effect.</violation>
</file>
<file name="packages/features/bookings/lib/handleCancelBooking.ts">
<violation number="1" location="packages/features/bookings/lib/handleCancelBooking.ts:222">
P3: Duplicated host-authorization logic triggers the org-admin DB lookup twice for seated events, adding redundant query work and risking drift between the two authorization checks. Consider reusing the earlier computed result (e.g., cache the org-admin check or reuse isCancellationUserHost) instead of repeating the same call.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| isWithinMinimumNotice | ||
| ); | ||
| case "cancel": | ||
| if (isHost && !isBookingInPast) return false; |
There was a problem hiding this comment.
P2: isHost is never provided by the action context builder, so the new cancel override always sees undefined and the fix has no effect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/components/booking/actions/bookingActions.ts, line 267:
<comment>`isHost` is never provided by the action context builder, so the new cancel override always sees `undefined` and the fix has no effect.</comment>
<file context>
@@ -263,6 +264,7 @@ export function isActionDisabled(actionId: string, context: BookingActionContext
isWithinMinimumNotice
);
case "cancel":
+ if (isHost && !isBookingInPast) return false;
return isDisabledCancelling || isBookingInPast || isCancelled || isRejected;
case "view_recordings":
</file context>
| isCancellationUserHost = true; | ||
| } | ||
| else if ( | ||
| await PrismaOrgMembershipRepository.isLoggedInUserOrgAdminOfBookingHost( |
There was a problem hiding this comment.
P3: Duplicated host-authorization logic triggers the org-admin DB lookup twice for seated events, adding redundant query work and risking drift between the two authorization checks. Consider reusing the earlier computed result (e.g., cache the org-admin check or reuse isCancellationUserHost) instead of repeating the same call.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/bookings/lib/handleCancelBooking.ts, line 222:
<comment>Duplicated host-authorization logic triggers the org-admin DB lookup twice for seated events, adding redundant query work and risking drift between the two authorization checks. Consider reusing the earlier computed result (e.g., cache the org-admin check or reuse isCancellationUserHost) instead of repeating the same call.</comment>
<file context>
@@ -207,16 +207,35 @@ async function handler(input: CancelBookingInput, dependencies?: Dependencies) {
+ isCancellationUserHost = true;
+ }
+ else if (
+ await PrismaOrgMembershipRepository.isLoggedInUserOrgAdminOfBookingHost(
+ userId,
+ bookingToDelete.userId
</file context>
Devin AI is completing this stale PRThis PR by @GurneeshBudhiraja has been marked as stale. A Devin session has been created to complete the remaining work. Devin will review the PR, address any feedback, and push updates to complete this PR. |
Address review feedback from cubic-dev-ai: - Add checks for isCancelled and isRejected in the isHost bypass - Add tests to verify hosts cannot cancel already cancelled or rejected bookings Co-authored-by: GurneeshBudhiraja <gurneeshbudhiraja@gmail.com> Co-Authored-By: unknown <>
Summary of ChangesI've completed this PR by addressing the review feedback from @cubic-dev-ai. Changes MadeFixed the The original implementation allowed hosts to cancel bookings even when "Disable Cancelling" was enabled, but it didn't check if the booking was already cancelled or rejected. This could have allowed hosts to attempt cancelling already cancelled/rejected bookings. Code change in case "cancel":
- if (isHost && !isBookingInPast) return false;
+ if (isHost && !isBookingInPast && !isCancelled && !isRejected) return false;
return isDisabledCancelling || isBookingInPast || isCancelled || isRejected;Added tests to verify:
Verification
Note on Other Review Comments
Completed by Devin AI on behalf of @GurneeshBudhiraja |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/components/booking/actions/bookingActions.ts">
<violation number="1" location="apps/web/components/booking/actions/bookingActions.ts:267">
P2: The updated cancel bypass relies on `isHost`, but the action context never supplies `isHost`, so hosts still can’t bypass `disableCancelling` as intended.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
The isHost flag was computed but not included in the actionContext, causing the host bypass for disableCancelling to not work in the dropdown component. Co-authored-by: GurneeshBudhiraja <gurneeshbudhiraja@gmail.com> Co-Authored-By: unknown <>
|
@romitg2 I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/components/booking/BookingListItem.tsx">
<violation number="1" location="apps/web/components/booking/BookingListItem.tsx:189">
P2: isHost is computed using only userId equality, which doesn’t account for team host assignments that depend on attendee email. This diverges from server-side host determination and can block legitimate hosts from canceling/rescheduling when Disable Cancelling is enabled.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const isAttendee = !!userSeat; | ||
|
|
||
| // Checks if the logged in user is the host of the booking | ||
| const isHost = |
There was a problem hiding this comment.
P2: isHost is computed using only userId equality, which doesn’t account for team host assignments that depend on attendee email. This diverges from server-side host determination and can block legitimate hosts from canceling/rescheduling when Disable Cancelling is enabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/components/booking/BookingListItem.tsx, line 189:
<comment>isHost is computed using only userId equality, which doesn’t account for team host assignments that depend on attendee email. This diverges from server-side host determination and can block legitimate hosts from canceling/rescheduling when Disable Cancelling is enabled.</comment>
<file context>
@@ -185,6 +185,12 @@ function BookingListItem(booking: BookingItemProps) {
const isAttendee = !!userSeat;
+ // Checks if the logged in user is the host of the booking
+ const isHost =
+ booking.user?.id != null &&
+ booking.loggedInUser.userId != null &&
</file context>
8 participants
What does this PR do?
The PR fixes the bug that was preventing the hosts to cancel their own events when
Disable Cancellingis enabled.Updates since last revision
Addressed review feedback and fixed additional issues:
Fixed cancel action logic (
bookingActions.ts): The host bypass now also checksisCancelledandisRejectedstates to prevent hosts from attempting to cancel already cancelled/rejected bookings.Fixed missing
isHostin dropdown (BookingActionsDropdown.tsx): TheisHostflag was computed but not included in theactionContextobject, causing the host bypass to not work in the dropdown component. This was the root cause of the fix not working properly.Added test cases: Two new tests verify that hosts cannot cancel already cancelled or rejected bookings.
Visual Demo (For contributors especially)
Video Demo (if applicable):
The below video demo shows the updated functionality for both the host account(on the right) and the attendee account(on the left).
Loom Video Link
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
The hosts should be able to cancel the event even when
Disable Cancellingis enabled. On the other hand, for somebody viewing the event other than the host, they would not be able to cancel it.Test scenarios:
Disable Cancellingenabled, verify you CAN cancel an upcoming bookingDisable Cancellingenabled, verify you CANNOT cancel the bookingChecklist for human review
isHostis correctly computed in bothBookingListItem.tsx(line 189-192) andBookingActionsDropdown.tsx(line 201)bookingActions.ts(line 267) correctly handles the host bypass with cancelled/rejected checksLink to Devin run: https://app.devin.ai/sessions/1f26c48a6898417188d7d856880e3f8b
Requested by: unknown ()