feat: Allow to remove seats from a booking#21232
feat: Allow to remove seats from a booking#21232kart1ka wants to merge 25 commits intocalcom:mainfrom
Conversation
|
@kart1ka is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add community label" took an action on this PR • (05/11/25)1 label was added to this PR based on Keith Williams's automation. |
| cancellationReason: z.string().optional(), | ||
| seatReferenceUid: z.string().optional(), | ||
| seatReferenceUid: z.union([z.string(), z.array(z.string())]).optional(), | ||
| cancelledBy: z.string().email({ message: "Invalid email" }).optional(), |
There was a problem hiding this comment.
IG we should ideally make it only z.array(z.string()) and alter code accordingly
There was a problem hiding this comment.
I left the z.string() option for backward compatibility.
There was a problem hiding this comment.
What does it mean exactly
TusharBhatt1
left a comment
There was a problem hiding this comment.
Can you add tests for it , making it draft until then
Tests added. |
|
This PR is being marked as stale due to inactivity. |
|
|
||
| // For seated events, require host permissions when canceling multiple seats or the entire booking | ||
| // Only individual attendees can cancel their own seats without being a host | ||
| if (bookingToDelete.eventType?.seatsPerTimeSlot && seatsToCancel.length !== 1) { |
There was a problem hiding this comment.
no seats provided: seatsToCancel === 0 should be better than seatsToCancel.length !== 1)here ?
There was a problem hiding this comment.
seatsToCancel.length !== 1 is used to skip the host check when an attendee is cancelling their own seat.
There was a problem hiding this comment.
I don't think we need this check. We also don't authrize the host for other bookings, attendee's shouldn't have other seatReferenceUids so they can't do this call anyways
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts
Outdated
Show resolved
Hide resolved
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts
Outdated
Show resolved
Hide resolved
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts
Outdated
Show resolved
Hide resolved
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis change set introduces the ability for authorized users to remove one or more seats from a booking in a multi-attendee (seated) event. A new dialog component allows selection of seats to remove, capturing the cancellation reason and performing the removal via an API call. Authorization checks are implemented to restrict this action to booking owners, hosts, organization admins, or team admins/owners. The backend logic for seat cancellation is refactored to support multiple seat references in a single operation, including validation, database updates, and sending appropriate email notifications (with new logic to distinguish host-initiated removals). The booking retrieval and attendee visibility logic are also updated to support these features, and new localization strings are added for the UI. Comprehensive tests are included for the new seat cancellation scenarios. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)
560-569: Authorization logic looks correct.The visibility check properly restricts seat attendee information to booking owners and the attendees themselves.
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts (1)
112-118: Clear and correct integration update logic.The code properly filters out cancelled attendees and updates integrations with the remaining attendee list. The variable naming is descriptive.
apps/web/components/booking/BookingListItem.tsx (1)
191-216: Comprehensive authorization implementation.The authorization checks properly handle all user roles (owner, host, org admin, team admin) with safe optional chaining. This addresses the previous review comment about allowing team admins to make changes.
🧹 Nitpick comments (7)
packages/features/bookings/lib/handleCancelBooking.ts (1)
123-123: Remove unnecessary.flat()call for clarity.Since
seatReferenceUidis a single string value, the.flat()method is redundant when wrapping it in an array.- const seatsToCancel = seatReferenceUid ? [seatReferenceUid].flat() : []; + const seatsToCancel = seatReferenceUid ? [seatReferenceUid] : [];apps/web/public/static/locales/en/common.json (1)
646-649: Add strings to all locale files, not justen.Nice addition, but remember: adding keys only to the English bundle will throw missing-translation warnings for every other locale.
Replicateremove_seats,seats_removed,error_removing_seats, andselect_seatsin the rest of thestatic/locales/<lang>/common.jsonfiles (even if the value is the English fallback) to keep i18n builds clean.packages/prisma/zod-utils.ts (1)
303-303: Consider using consistent union syntax.While functionally equivalent, this uses
.or()syntax whereas line 290 usesz.union(). For consistency and readability, consider using the same pattern in both schemas.- seatReferenceUid: z.string().or(z.array(z.string())), + seatReferenceUid: z.union([z.string(), z.array(z.string())]),apps/web/components/dialog/RemoveBookingSeatsDialog.tsx (2)
43-76: Consider using TRPC mutation for consistency.The API integration works correctly, but consider using a TRPC mutation instead of raw fetch for consistency with the rest of the codebase. The current implementation is functional but mixing API patterns could lead to maintenance issues.
The error handling and state management are well implemented.
46-46: Consider adding validation for referenceUid filtering.The
filter(Boolean)correctly removes null/undefined values, but consider being more explicit for clarity:-const seatReferenceUids = selectedOptions.map((option) => option.data.referenceUid).filter(Boolean); +const seatReferenceUids = selectedOptions + .map((option) => option.data.referenceUid) + .filter((uid): uid is string => uid !== null && uid !== undefined);This makes the filtering intent clearer and provides better type safety.
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts (1)
85-87: Consider using a more descriptive variable name.The filtered attendees represent those being cancelled, so a name like
cancelledAttendeeswould be clearer than justattendees.- const attendees = bookingToDelete?.attendees.filter((attendee) => + const cancelledAttendees = bookingToDelete?.attendees.filter((attendee) => seatReferences.some((reference) => reference?.attendeeId === attendee.id) );Note: You'll need to update all references to
attendeestocancelledAttendeesthroughout the function.apps/web/components/booking/BookingListItem.tsx (1)
583-603: Remove duplicate authorization check.The authorization check here is redundant since the button to open this dialog is already conditionally rendered based on the same check (lines 336-356). The dialog will only open if the user is authorized.
- {checkIfUserIsAuthorizedToCancelSeats( - { user: booking.user, eventType: booking.eventType }, - { - userId: booking.loggedInUser.userId, - userIsOrgAdminOrOwner: booking.loggedInUser.userIsOrgAdminOrOwner, - teamsWhereUserIsAdminOrOwner: booking.loggedInUser.teamsWhereUserIsAdminOrOwner, - } - ) && ( <RemoveBookingSeatsDialog isOpenDialog={isOpenRemoveSeatsDialog} setIsOpenDialog={setIsOpenRemoveSeatsDialog} bookingUid={booking.uid} attendees={booking.seatsReferences .map((seat) => ({ email: seat.attendee?.email || "", name: seat.attendee?.name || null, referenceUid: seat.referenceUid, })) .filter((attendee) => attendee.email)} /> - )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/web/components/booking/BookingListItem.tsx(5 hunks)apps/web/components/dialog/RemoveBookingSeatsDialog.tsx(1 hunks)apps/web/modules/bookings/views/bookings-listing-view.tsx(1 hunks)apps/web/public/static/locales/en/common.json(2 hunks)packages/emails/email-manager.ts(1 hunks)packages/emails/src/templates/AttendeeCancelledSeatEmail.tsx(1 hunks)packages/emails/src/templates/AttendeeScheduledEmail.tsx(1 hunks)packages/emails/src/templates/OrganizerAttendeeCancelledSeatEmail.tsx(1 hunks)packages/emails/src/templates/OrganizerScheduledEmail.tsx(3 hunks)packages/emails/templates/attendee-cancelled-seat-email.ts(1 hunks)packages/emails/templates/attendee-scheduled-email.ts(2 hunks)packages/emails/templates/organizer-attendee-cancelled-seat-email.ts(1 hunks)packages/emails/templates/organizer-scheduled-email.ts(2 hunks)packages/features/bookings/lib/handleCancelBooking.ts(2 hunks)packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts(5 hunks)packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts(2 hunks)packages/prisma/zod-utils.ts(2 hunks)packages/trpc/server/routers/viewer/bookings/get.handler.ts(3 hunks)packages/trpc/server/routers/viewer/me/get.handler.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
packages/features/bookings/lib/handleCancelBooking.ts (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: alishaz-polymath
PR: calcom/cal.com#22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.001Z
Learning: In the MultiplePrivateLinksController component (packages/features/eventtypes/components/MultiplePrivateLinksController.tsx), the `currentLink.maxUsageCount ?? 1` fallback in the openSettingsDialog function is intentional. Missing maxUsageCount values indicate old/legacy private links that existed before the expiration feature was added, and they should default to single-use behavior (1) for backward compatibility.
apps/web/modules/bookings/views/bookings-listing-view.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
apps/web/components/dialog/RemoveBookingSeatsDialog.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
packages/trpc/server/routers/viewer/bookings/get.handler.ts (2)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
Learnt from: vijayraghav-io
PR: calcom/cal.com#21072
File: packages/prisma/schema.prisma:891-891
Timestamp: 2025-07-18T08:47:01.255Z
Learning: In Cal.com's calendar integration, both Google Calendar and Outlook Calendar are designed to allow multiple eventTypeIds to share the same subscription ID (googleChannelId or outlookSubscriptionId). This is an intentional design pattern to reuse existing subscriptions for efficiency rather than creating separate subscriptions for each event type. Therefore, unique constraints like `@@unique([outlookSubscriptionId, eventTypeId])` should not be added as they would break this subscription sharing functionality.
apps/web/public/static/locales/en/common.json (1)
undefined
<retrieved_learning>
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
</retrieved_learning>
apps/web/components/booking/BookingListItem.tsx (1)
Learnt from: eunjae-lee
PR: calcom/cal.com#22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.341Z
Learning: In the FailedBookingsByField component (packages/features/insights/components/FailedBookingsByField.tsx), although routingFormId is typed as optional in useInsightsParameters, the system automatically enforces a routing form filter, so routingFormId is always present in practice. This means the data always contains only one entry, making the single-entry destructuring approach safe.
🧬 Code Graph Analysis (2)
packages/emails/email-manager.ts (4)
packages/prisma/zod-utils.ts (1)
EventTypeMetadata(146-146)packages/emails/src/templates/AttendeeCancelledSeatEmail.tsx (1)
AttendeeCancelledSeatEmail(3-15)packages/emails/templates/attendee-cancelled-seat-email.ts (1)
AttendeeCancelledSeatEmail(6-24)packages/emails/src/templates/OrganizerAttendeeCancelledSeatEmail.tsx (1)
OrganizerAttendeeCancelledSeatEmail(3-14)
packages/emails/templates/attendee-scheduled-email.ts (1)
packages/types/Calendar.d.ts (2)
CalendarEvent(163-224)Person(33-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (33)
packages/features/bookings/lib/handleCancelBooking.ts (1)
295-302: LGTM: Successfully adapted to multi-seat cancellation API.The change correctly adapts the single-seat cancellation flow to work with the new multi-seat
cancelAttendeeSeatAPI, maintaining backward compatibility while enabling the new seat removal functionality.packages/emails/templates/organizer-attendee-cancelled-seat-email.ts (1)
25-29: LGTM: Enhanced email context with cancellation initiator information.Adding the
isCancelledByHostproperty enables the email template to customize content based on who initiated the cancellation, improving the accuracy and relevance of email notifications.packages/emails/templates/attendee-cancelled-seat-email.ts (1)
16-20: LGTM: Consistent enhancement with organizer email template.The addition of
isCancelledByHostproperty maintains consistency with the organizer email template, ensuring both attendee and organizer notifications have access to the same contextual information about cancellation initiation.apps/web/modules/bookings/views/bookings-listing-view.tsx (1)
255-262: LGTM: Enhanced user context for seat removal authorization.The addition of
teamsWhereUserIsAdminOrOwneranduserIsOrgAdminOrOwnerproperties provides the necessary authorization context to BookingListItem components, enabling proper permission checks for the new seat removal feature. The optional chaining safely handles cases where user data might not be available.packages/emails/src/templates/AttendeeScheduledEmail.tsx (1)
5-11: LGTM: Consistent interface enhancement for email templates.Adding the optional
isCancelledByHostprop maintains consistency with other email template components and ensures compatibility with the broader email notification system. The optional nature preserves backward compatibility while enabling future utilization of this contextual information.packages/emails/src/templates/AttendeeCancelledSeatEmail.tsx (1)
3-15: Well-implemented conditional email content based on cancellation source.The conditional logic correctly differentiates between host-initiated and attendee-initiated cancellations, providing appropriate messaging for each scenario. The component properly leverages the existing
AttendeeScheduledEmailcomponent while customizing the title and other props.apps/web/public/static/locales/en/common.json (1)
2035-2035: Key placement & naming look good – LGTMThe new
attendee_removed_subtitlefollows the existing naming pattern (attendee_*_subtitle) and placeholder style. No issues spotted.packages/emails/src/templates/OrganizerAttendeeCancelledSeatEmail.tsx (1)
11-11: LGTM! Correct logic for distinguishing cancellation context.The negation
!props.isCancelledByHostcorrectly setsattendeeCancelledtotruewhen the host did NOT initiate the cancellation (i.e., attendee-initiated), which aligns with the semantic meaning of the prop.packages/trpc/server/routers/viewer/me/get.handler.ts (1)
109-123: LGTM! Well-structured query for team-based authorization.The new query correctly fetches team memberships where the user has admin/owner roles, specifically filtering for teams (not organizations). This extends the existing boolean flag with detailed membership information that will support authorization decisions for features like seat removal.
The query structure is efficient and follows established patterns in the codebase.
packages/emails/src/templates/OrganizerScheduledEmail.tsx (3)
14-14: LGTM! Proper addition of optional prop.The
isCancelledByHostprop is correctly typed as optional boolean and integrates well with the existing prop structure.
42-46: LGTM! Clear and logical subtitle determination.The subtitle logic correctly handles three distinct scenarios:
- Attendee-initiated cancellation: "attendee_no_longer_attending_subtitle"
- Host-initiated cancellation: "attendee_removed_subtitle"
- Normal case: empty string
The conditional structure is clear and maintains proper precedence between the different cancellation contexts.
58-58: LGTM! Proper fallback handling.The subtitle assignment correctly prioritizes explicit
props.subtitleover the computed subtitle, preserving existing behavior while enabling the new cancellation context logic.packages/emails/email-manager.ts (3)
373-374: LGTM! Proper parameter addition for cancellation context.The optional
isCancelledByHostparameter is correctly added to support distinguishing host-initiated vs attendee-initiated cancellations in email notifications.
381-385: LGTM! Correct parameter threading to attendee email.The
isCancelledByHostparameter is properly passed as the fourth argument to theAttendeeCancelledSeatEmailconstructor, maintaining the existing parameter order withundefinedfor the third parameter.
387-396: LGTM! Correct parameter threading to organizer email.The
isCancelledByHostparameter is properly included in the options object passed toOrganizerAttendeeCancelledSeatEmail, enabling proper cancellation context in organizer notifications.packages/emails/templates/attendee-scheduled-email.ts (3)
18-18: LGTM! Proper property addition for cancellation context.The optional
isCancelledByHostproperty is correctly typed and follows the established pattern for email template classes.
20-25: LGTM! Clean constructor signature update.The constructor properly accepts the optional
isCancelledByHostparameter as the fourth argument, maintaining backward compatibility while enabling the new functionality.
36-36: LGTM! Straightforward property assignment.The
isCancelledByHostparameter is correctly assigned to the instance property, making it available for email rendering logic.packages/prisma/zod-utils.ts (1)
290-290: LGTM! Good backward compatibility consideration.The union type correctly supports both single seat and multi-seat cancellation scenarios while maintaining backward compatibility with existing single-string usage.
packages/features/bookings/lib/handleSeats/test/handleSeats.test.ts (4)
1839-1967: LGTM! Comprehensive permission check test.This test case properly verifies that non-host users cannot cancel multiple seats. The test setup is correct, using a different user ID (999) than the organizer (101), and it properly validates both the error message and that the booking remains unchanged.
2799-2916: LGTM! Correctly tests full booking cancellation logic.This test properly verifies that when all seats in a booking are cancelled, the entire booking status is updated to CANCELLED. The test setup and assertions are appropriate for validating this important business rule.
2918-3060: LGTM! Excellent validation and atomicity test.This test properly verifies the atomic nature of seat cancellation operations. When any invalid seat references are provided, the entire operation fails and no changes are made. The test correctly validates both the error message and that the booking state remains unchanged.
3062-3219: LGTM! Comprehensive partial cancellation test.This test thoroughly validates the partial seat cancellation functionality. It correctly verifies that only the specified seats are removed while the booking remains active, and properly checks both attendee and booking seat data integrity.
apps/web/components/dialog/RemoveBookingSeatsDialog.tsx (2)
78-90: LGTM! Clean data transformation with proper fallbacks.The attendee-to-option mapping properly handles optional fields and provides sensible fallbacks. The early return for empty options prevents unnecessary rendering.
92-164: LGTM! Well-structured UI with excellent UX.The dialog implementation follows good UI/UX practices with proper validation, loading states, and accessibility attributes. The form validation correctly disables the submit button until both seats are selected and a cancellation reason is provided.
packages/trpc/server/routers/viewer/bookings/get.handler.ts (3)
491-503: LGTM! Well-structured query additions.The addition of
hostsandparentfields to the event type selection is implemented correctly using Kysely's JSON helpers and follows the existing patterns in the codebase.
558-558: Good addition for UI display.Adding the attendee name alongside email in seat references improves the user experience without any negative impact.
668-672: Proper enhancement to attendee visibility rules.The additional check ensuring booking owners can always see all attendees is a good authorization improvement that aligns with the expected behavior.
packages/features/bookings/lib/handleSeats/cancel/cancelAttendeeSeat.ts (3)
24-24: Good backward-compatible type enhancement.The parameter type change to
string | string[]properly supports multi-seat cancellation while maintaining compatibility with existing single-seat cancellation calls.
62-68: Proper validation of seat references.The validation ensures all requested seats belong to the booking before proceeding, preventing invalid cancellation attempts.
71-83: Efficient bulk delete implementation.The use of
deleteManywith concurrent execution properly handles multiple seat cancellations in a single database transaction.apps/web/components/booking/BookingListItem.tsx (2)
63-75: Well-structured type definition for authorization.The
LoggedInUsertype comprehensively captures all necessary user permission data with proper typing.
336-356: Properly gated remove seats action.The action is correctly conditioned on seat availability, booking timing, and user authorization, maintaining consistency with other booking actions.
Addressed the comments. |
There was a problem hiding this comment.
The email for the host only mentions that one attendee was removed when removing more: 
Title: 'Some attendees are no longer attending your event'
Subtile for two: 'test1 and test2 were removed. ...' --> if this makes things complex we can also keep it simple and say 'Some attendees'
Subtitle for more than two: 'Some attendees were removed. ...' -
| translate: await getTranslation(attendee.locale ?? "en", "common"), | ||
| locale: attendee.locale ?? "en", | ||
| }, | ||
| language: { translate: tAttendees, locale: attendee.locale ?? "en" }, |
There was a problem hiding this comment.
we are using always the first attendee here attendees[0].locale we should use the correct locale to translate
|
Hello @kart1ka, This PR is almost complete, with only a few remaining tasks I guess. Could you please resolve the merge conflicts? Also, have you addressed CarinaWolli’s two review comments? If you have done so, could you please retest after the changes, push the updates, and ensure that CI (tests) passes? Please let me know if you are busy and unable to complete this in the near future. |
|
@dhairyashiil this PR has been untouched for a month and open for half a year. I have expressed the importance of this feature for my company. I am willing to contribute if necessary, but as far as I know this PR is essentially complete. Could I get a status update if you have one? |
okay I understand, thank you for the comment, I will personally look into this. |
|
@corbin-ward-q I’ve raised a new PR and linked it to issue. I’ll try to get it reviewed as soon as possible. |
What does this PR do?
Visual Demo (For contributors especially)
https://www.loom.com/share/f45c79f75e534abb8ac98bb5f8b099c8?sid=0a6687b5-9033-4a68-955c-34d875fdd0f7
Mandatory Tasks (DO NOT REMOVE)
Summary by mrge
Added the ability for users to remove seats from an existing booking, allowing hosts to cancel individual attendees.