feat: allow rescheduling unconfirmed bookings without bypassing confirmation#28926
feat: allow rescheduling unconfirmed bookings without bypassing confirmation#28926SinghaAnirban005 wants to merge 4 commits into
Conversation
|
Welcome to Cal.diy, @SinghaAnirban005! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThe PR adds pending-reschedule handling across UI, booking, and email subsystems. UI edit-action logic and action-disable rules now account for an 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 4/8 reviews remaining, refill in 23 minutes and 36 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/emails/email-manager.ts`:
- Around line 295-309: The pending-reschedule branch bypasses privacy and
recipient controls, logs to console, and sends raw calEvent to attendees while
skipping team-member host notifications; update the branch so it follows the
same reschedule email path as normal reschedules: remove console.log calls,
honor host/attendee email-disable settings before sending, use the scrubbed
payload (call hideCalendarNotes(calEvent, recipient) or the existing scrubber
used for regular reschedules) when constructing AttendeePendingRescheduledEmail
and OrganizerPendingRescheduledEmail, and trigger the same team-member host
notification logic used in the non-pending reschedule flow (reuse the existing
reschedule notification helper or functions) rather than sending directly from
this branch.
In `@packages/emails/templates/attendee-pending-rescheduled-email.ts`:
- Around line 17-28: Update the subject and text-body translation keys to use
pending-specific copy and add the corresponding strings to
packages/i18n/locales/en/common.json; specifically replace the subject call
using
this.attendee.language.translate("event_type_has_been_rescheduled_on_time_date",
{...}) with a new pending-specific key (e.g.
"event_type_pending_has_been_rescheduled_on_time_date") and replace the
getTextBody keys ("event_has_been_rescheduled",
"emailed_you_and_any_other_attendees") with pending variants (e.g.
"pending_event_has_been_rescheduled", "pending_booking_remains_unconfirmed") so
the attendee sees that the booking is still unconfirmed; make the same changes
for the other occurrence referenced (lines 32-37) and ensure the new keys are
added to packages/i18n/locales/en/common.json.
In `@packages/emails/templates/organizer-pending-rescheduled-email.ts`:
- Around line 19-32: The organizer email currently uses the standard rescheduled
subject/body; change it to a pending-specific variant by using a pending
translation key in this.calEvent.organizer.language.translate (e.g., replace
"event_type_has_been_rescheduled_on_time_date" with a pending key like
"event_type_pending_has_been_rescheduled_on_time_date"), and render
pending-specific HTML/text by either passing a pending flag into getHtml (e.g.,
{ ...this.calEvent, attendeeSeatId: undefined, pending: true }) or calling a
dedicated method (e.g., getPendingHtml) and using getTextBody with a pending key
(e.g., "event_pending_has_been_rescheduled"); add the new translation strings to
packages/i18n/locales/en/common.json matching the keys you choose.
In `@packages/features/bookings/lib/service/RegularBookingService.ts`:
- Around line 2197-2207: The pending-reschedule email payload sent in the
non-dry-run branch omits rescheduleReason, so
BookingEmailSmsHandler._handleRequested won't receive it; update the
emailsAndSmsHandler.send call (the branch guarded by isDryRun that sends action
BookingActionMap.requested) to include rescheduleReason inside the data object
alongside evt, eventType, attendees, additionalNotes, and
originalRescheduledBooking so the handler can use the user-provided reason.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 288105a3-3548-4da0-8deb-7e03b33309b5
📒 Files selected for processing (7)
apps/web/components/booking/actions/bookingActions.tspackages/emails/email-manager.tspackages/emails/templates/attendee-pending-rescheduled-email.tspackages/emails/templates/organizer-pending-rescheduled-email.tspackages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking/getRequiresConfirmationFlags.tspackages/features/bookings/lib/service/RegularBookingService.ts
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
cb095f1 to
91f52fd
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/components/booking/actions/bookingActions.ts (1)
109-113:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't key the organizer bypass to the single
booking.user.This file already handles round-robin and managed-child bookings, but
isUserOrganizeronly returnstruewhen the logged-in user matchesbooking.user.id. That means other valid organizers on team bookings can still hit the minimum-notice block, so the new pending-reschedule flow won't work consistently there.If there is a stricter permission flag available upstream, use that instead of comparing against the single assignee.Suggested fix
- const isUserOrganizer = - !isAttendee && - booking.loggedInUser?.userId && - booking.user?.id && - booking.loggedInUser.userId === booking.user.id; + const isUserOrganizer = !isAttendee;Also applies to: 244-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/components/booking/actions/bookingActions.ts` around lines 109 - 113, The current organizer-bypass logic wrongly uses booking.user (see isUserOrganizer) which only matches the single assignee; update isUserOrganizer to detect whether the current user is an organizer via the collective organizer/hosts info or a proper permission flag instead of comparing to booking.user.id. Specifically, replace the single-assignee check in isUserOrganizer with a check against the event-level organizer/hosts list (e.g., hostGroups/members, booking.organizers or booking.organizerIds) or, if available from upstream, use a stricter flag such as booking.userIsOrganizer/booking.hasOrganizerPermission; apply the same change where similar logic appears (the block around lines 244-253 using booking.user) so all round-robin and managed-child booking flows honor real organizer membership/permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/components/booking/actions/bookingActions.ts`:
- Around line 109-113: The current organizer-bypass logic wrongly uses
booking.user (see isUserOrganizer) which only matches the single assignee;
update isUserOrganizer to detect whether the current user is an organizer via
the collective organizer/hosts info or a proper permission flag instead of
comparing to booking.user.id. Specifically, replace the single-assignee check in
isUserOrganizer with a check against the event-level organizer/hosts list (e.g.,
hostGroups/members, booking.organizers or booking.organizerIds) or, if available
from upstream, use a stricter flag such as
booking.userIsOrganizer/booking.hasOrganizerPermission; apply the same change
where similar logic appears (the block around lines 244-253 using booking.user)
so all round-robin and managed-child booking flows honor real organizer
membership/permissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99d09406-aa53-4f57-be28-11a258f888e5
📒 Files selected for processing (8)
apps/web/components/booking/actions/bookingActions.tspackages/emails/email-manager.tspackages/emails/templates/attendee-pending-rescheduled-email.tspackages/emails/templates/organizer-pending-rescheduled-email.tspackages/features/bookings/lib/BookingEmailSmsHandler.tspackages/features/bookings/lib/handleNewBooking/getRequiresConfirmationFlags.tspackages/features/bookings/lib/service/RegularBookingService.tspackages/i18n/locales/en/common.json
✅ Files skipped from review due to trivial changes (3)
- packages/emails/templates/attendee-pending-rescheduled-email.ts
- packages/i18n/locales/en/common.json
- packages/features/bookings/lib/handleNewBooking/getRequiresConfirmationFlags.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/emails/templates/organizer-pending-rescheduled-email.ts
- packages/features/bookings/lib/BookingEmailSmsHandler.ts
- packages/emails/email-manager.ts
- packages/features/bookings/lib/service/RegularBookingService.ts
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
What does this PR do?
Organizers of event types that require confirmation are currently blocked from rescheduling a
PENDINGbooking. The restriction exists to prevent a naive reschedule from producing anACCEPTEDbooking silently bypassing the confirmation workflow.This PR ensures that the organizer can now reschedule a
PENDINGbooking but the resulting booking inheritsPENDINGstatus. The confirmation requirement is fully preserved only the proposed time changes. Both the organizer and attendee receive a notification that the time has changed and that the booking still awaits confirmation.Video Demo (if applicable):
Screencast.from.2026-04-19.02-29-35.webm
Mandatory Tasks (DO NOT REMOVE)