Skip to content

Comments

fix: reschedule flow is broken for seated booking#23987

Merged
anikdhabal merged 5 commits intomainfrom
broken-seated-booking
Sep 23, 2025
Merged

fix: reschedule flow is broken for seated booking#23987
anikdhabal merged 5 commits intomainfrom
broken-seated-booking

Conversation

@anikdhabal
Copy link
Contributor

@anikdhabal anikdhabal commented Sep 22, 2025

What does this PR do?

Found that the reschedule flow for seated events is broken when trying to reschedule from the /upcoming: Page not found when an attendee tries to reschedule and When the host tries to reschedule, the rescheduleUid is set to the first attendee’s seatRefUid

@anikdhabal anikdhabal requested a review from a team as a code owner September 22, 2025 17:12
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 22, 2025

Walkthrough

BookingListItem now finds a userSeat by matching booking.seatsReferences attendees to the current user and sets isAttendee; getSeatReferenceUid returns only userSeat?.referenceUid (no fallback). BookingActionContext now includes isAttendee and bookingActions conditionally include seatReferenceUid in generated links only when isAttendee is true. getServerSideProps for the reschedule route prefers a provided seatReferenceUid over bookingUid. The viewer bookings handler removed the Attendee.email = user.email filter from SeatsReferences. E2E tests for rescheduling seated bookings were added.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "fix: reschedule flow is broken for seated booking" concisely and accurately summarizes the primary change — fixing the reschedule flow for seated bookings — and aligns with the code modifications and PR objectives.
Description Check ✅ Passed The PR description describes the core bug addressed (attendee reschedule yields "Page not found" and host reschedule uses the first attendee's seatRefUid), which is directly related to the changes in seatReferenceUid handling, attendee gating, and added tests.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch broken-seated-booking

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 03657cc and 00078ee.

📒 Files selected for processing (1)
  • apps/web/components/booking/BookingListItem.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/components/booking/BookingListItem.tsx

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@graphite-app graphite-app bot requested a review from a team September 22, 2025 17:12
@vercel
Copy link

vercel bot commented Sep 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 23, 2025 2:41pm
cal-eu Ignored Ignored Sep 23, 2025 2:41pm

@keithwillcode keithwillcode added the core area: core, team members only label Sep 22, 2025
@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Sep 22, 2025
label: t("reschedule_booking"),
href: `/reschedule/${booking.uid}${
booking.seatsReferences.length ? `?seatReferenceUid=${getSeatReferenceUid()}` : ""
booking.seatsReferences.length && isAttendee ? `?seatReferenceUid=${getSeatReferenceUid()}` : ""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the currentUser is a host or admin/pwner don't include seatRefuid

Comment on lines -226 to -229
if (!booking.seatsReferences[0]) {
return undefined;
}
return booking.seatsReferences[0].referenceUid;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was always returning the first attendee's seat

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/lib/reschedule/[uid]/getServerSideProps.ts (1)

89-90: Narrow Prisma selection for booking.user.

Selecting user: true fetches all columns. Only id is needed for enrichUserWithItsProfile. Reduce surface per our guideline “only select what you need”.

Apply:

-      user: true,
+      user: {
+        select: { id: true },
+      },
🧹 Nitpick comments (3)
apps/web/lib/reschedule/[uid]/getServerSideProps.ts (1)

148-173: Access control for seated bookings without seatReferenceUid looks right; tiny polish.

Use .some(...) for boolean intent instead of .find(...) + truthiness.

-    const userIsHost = booking?.eventType.hosts.find((host) => {
-      if (host.user.id === userId) return true;
-    });
+    const userIsHost = booking?.eventType.hosts.some((host) => host.user.id === userId);

Please confirm there’s no code path where a non‑host/owner without a valid seatReferenceUid should be allowed through.

apps/web/components/booking/bookingActions.ts (1)

66-74: Follow-up: cancel action still appends seatReferenceUid unconditionally for seated events.

This relies on getSeatReferenceUid() returning a string; see bug in BookingListItem fallback that can yield an object. Once fixed, consider mirroring the attendee-gate here if product wants hosts to cancel entire booking rather than a seat.

apps/web/components/booking/BookingListItem.tsx (1)

187-188: Make attendee detection case-insensitive and explicit.

Email comparisons should be case-insensitive. Also prefer .some() for intent.

-  const isAttendee = !!booking.attendees.find((attendee) => attendee.email === userEmail);
+  const isAttendee =
+    !!userEmail &&
+    booking.attendees.some((a) => a.email?.toLowerCase() === userEmail.toLowerCase());
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 55df0e3 and 3de58d0.

📒 Files selected for processing (5)
  • apps/web/components/booking/BookingListItem.tsx (3 hunks)
  • apps/web/components/booking/bookingActions.ts (3 hunks)
  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts (2 hunks)
  • apps/web/playwright/booking-seats.e2e.ts (1 hunks)
  • packages/trpc/server/routers/viewer/bookings/get.handler.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • packages/trpc/server/routers/viewer/bookings/get.handler.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/web/components/booking/bookingActions.ts
  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts
  • apps/web/playwright/booking-seats.e2e.ts
**/*.{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/bookingActions.ts
  • apps/web/components/booking/BookingListItem.tsx
  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts
  • apps/web/playwright/booking-seats.e2e.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/bookingActions.ts
  • apps/web/components/booking/BookingListItem.tsx
  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts
  • apps/web/playwright/booking-seats.e2e.ts
**/*.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
🧠 Learnings (6)
📓 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.
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.
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.

Applied to files:

  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts
📚 Learning: 2025-08-20T17:34:35.004Z
Learnt from: mdm317
PR: calcom/cal.com#23221
File: packages/features/schedules/components/NewScheduleButton.tsx:1-1
Timestamp: 2025-08-20T17:34:35.004Z
Learning: In the Cal.com codebase, server actions like `revalidateAvailabilityList()` from `app/(use-page-wrapper)/(main-nav)/availability/actions` are imported and called directly in client components within mutation `onSuccess` callbacks. This pattern is consistently used across availability-related components (both in availability-view.tsx for updates and NewScheduleButton.tsx for creates) to ensure the availability list cache is fresh after mutations.

Applied to files:

  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts
📚 Learning: 2025-08-19T09:47:49.478Z
Learnt from: eunjae-lee
PR: calcom/cal.com#23166
File: packages/prisma/migrations/20250818151914_routing_form_response_denormalized_backfill2/migration.sql:65-66
Timestamp: 2025-08-19T09:47:49.478Z
Learning: The Booking table has a unique constraint and index on the uid column (defined as `uid String unique` in schema.prisma), so JOINs on Booking.uid have optimal performance and don't require additional indexing.

Applied to files:

  • apps/web/lib/reschedule/[uid]/getServerSideProps.ts
📚 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/playwright/booking-seats.e2e.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/playwright/booking-seats.e2e.ts
🧬 Code graph analysis (1)
apps/web/playwright/booking-seats.e2e.ts (1)
apps/web/playwright/lib/testUtils.ts (3)
  • createUserWithSeatedEventAndAttendees (373-391)
  • selectFirstAvailableTimeSlotNextMonth (109-117)
  • confirmReschedule (510-514)
🔇 Additional comments (5)
apps/web/lib/reschedule/[uid]/getServerSideProps.ts (1)

42-42: Good: prefer seatReferenceUid when present.

This fixes the seated flow by resolving the booking via seat when provided.

apps/web/components/booking/bookingActions.ts (1)

100-118: Gate seatReferenceUid on attendee status for reschedule — good.

Prevents hosts from leaking attendee seat refs in reschedule links.

apps/web/playwright/booking-seats.e2e.ts (2)

461-507: Test: host reschedule keeps booking uid — nice coverage.

Asserts rescheduleUid equals booking.uid from /upcoming. Looks good.


509-583: Test: attendee-specific reschedule uses correct seatReferenceUid and pre-fills — solid.

Deterministic ordering with orderBy: { id: "asc" } avoids flakes.

Consider adding a negative test for a host cancel path ensuring no [object Object] leaks in seatReferenceUid (ties to fix in BookingListItem).

apps/web/components/booking/BookingListItem.tsx (1)

255-260: Propagating isAttendee into action context — good.

This enables correct gating in bookingActions.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 23, 2025

E2E results are ready!

@anikdhabal anikdhabal enabled auto-merge (squash) September 23, 2025 13:23
@vercel vercel bot temporarily deployed to Preview – dev September 23, 2025 15:40 Inactive
Copy link
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

Let's get this merged, happy with this.

@anikdhabal anikdhabal merged commit bcc433b into main Sep 23, 2025
62 of 66 checks passed
@anikdhabal anikdhabal deleted the broken-seated-booking branch September 23, 2025 21:05
saurabhraghuvanshii pushed a commit to saurabhraghuvanshii/cal.com that referenced this pull request Sep 24, 2025
* fix: reschedule flow is broken for seated booking

* Update BookingListItem.tsx

* Update

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

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working core area: core, team members only High priority Created by Linear-GitHub Sync ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants