fix: Disable cancel button for past bookings#22667
Conversation
|
@husniabad is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughUI and backend now prevent cancelling past bookings. UI: bookings-single-view hides the "or" separator and Cancel button when a booking is in the past; bookingActions disables the cancel action for past bookings. Backend: handleCancelBooking rejects cancellation requests for bookings whose endTime is before now with a 400 HttpError. Tests: unit tests for bookingActions, a unit test for handleCancelBooking blocking past cancellations, and a Playwright E2E asserting cancel controls are not available for past bookings. Assessment against linked issues
Out-of-scope changesNo out-of-scope or unrelated functional code changes were identified in the provided diffs. Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
github-actions
bot
added
consumer
linear
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/22/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (07/22/25)1 label was added to this PR based on Keith Williams's automation. |
dosubot
bot
added
bookings
![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)
packages/features/bookings/lib/handleCancelBooking.ts (1)
123-129: Consider using native Date comparison for better performance.The server-side validation logic is correct and essential for preventing past booking cancellations. However, according to the coding guidelines, Day.js methods like
.isAfter()can be slow in performance-critical code. Consider using native Date comparison for better performance:- if (bookingToDelete.endTime && dayjs().isAfter(bookingToDelete.endTime)) { + if (bookingToDelete.endTime && new Date() > new Date(bookingToDelete.endTime)) {This maintains the same functionality while potentially improving performance in this critical validation path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/components/booking/BookingListItem.tsx(1 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(2 hunks)apps/web/playwright/booking-pages.e2e.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(1 hunks)packages/platform/examples/base/src/pages/[bookingUid].tsx(2 hunks)
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
packages/platform/examples/base/src/pages/[bookingUid].tsxapps/web/components/booking/BookingListItem.tsxpackages/features/bookings/lib/handleCancelBooking.tsapps/web/playwright/booking-pages.e2e.tsapps/web/modules/bookings/views/bookings-single-view.tsx
🧠 Learnings (3)
packages/platform/examples/base/src/pages/[bookingUid].tsx (4)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.343Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.539Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
apps/web/components/booking/BookingListItem.tsx (1)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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/modules/bookings/views/bookings-single-view.tsx (3)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.343Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
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.
🧬 Code Graph Analysis (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
packages/platform/libraries/index.ts (1)
HttpError(49-49)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code. Functions like .add, .diff, .isBefore, and .isAfter are slow, especially in timezone mode. Prefer .utc() for better performance. Where possible, replace with native Date and direct .valueOf() comparisons for faster execution. Recommend using native methods or Day.js .utc() consistently in hot paths like loops.
Files:
packages/platform/examples/base/src/pages/[bookingUid].tsxapps/web/components/booking/BookingListItem.tsxpackages/features/bookings/lib/handleCancelBooking.tsapps/web/playwright/booking-pages.e2e.tsapps/web/modules/bookings/views/bookings-single-view.tsx
🧠 Learnings (3)
packages/platform/examples/base/src/pages/[bookingUid].tsx (4)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.343Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
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.
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/RoutingFunnel.tsx:15-17
Timestamp: 2025-07-15T12:58:40.539Z
Learning: In the insights routing funnel component (packages/features/insights/components/RoutingFunnel.tsx), the useColumnFilters exclusions are intentionally different from the general useInsightsParameters exclusions. RoutingFunnel specifically excludes only ["createdAt"] while useInsightsParameters excludes ["bookingUserId", "formId", "createdAt", "eventTypeId"]. This difference is by design.
apps/web/components/booking/BookingListItem.tsx (1)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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/modules/bookings/views/bookings-single-view.tsx (3)
Learnt from: eunjae-lee
PR: #22106
File: packages/features/insights/components/FailedBookingsByField.tsx:65-71
Timestamp: 2025-07-15T12:59:34.389Z
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: Anshumancanrock
PR: #22570
File: apps/web/modules/signup-view.tsx:253-253
Timestamp: 2025-07-21T21:33:23.343Z
Learning: In signup-view.tsx, when checking if redirectUrl contains certain strings, using explicit && checks (redirectUrl && redirectUrl.includes()) is preferred over optional chaining (redirectUrl?.includes()) to ensure the result is always a boolean rather than potentially undefined. This approach provides cleaner boolean contracts for downstream conditional logic.
Learnt from: alishaz-polymath
PR: #22304
File: packages/features/eventtypes/components/MultiplePrivateLinksController.tsx:92-94
Timestamp: 2025-07-16T06:42:27.024Z
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.
🧬 Code Graph Analysis (1)
packages/features/bookings/lib/handleCancelBooking.ts (1)
packages/platform/libraries/index.ts (1)
HttpError(49-49)
⏰ 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). (3)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Security Check
🔇 Additional comments (7)
apps/web/components/booking/BookingListItem.tsx (1)
372-374: LGTM! Simplified logic correctly prevents cancellation of past bookings.The condition has been appropriately simplified to remove the cancel action for any past booking, regardless of its confirmation status. This aligns with the PR objective and provides a cleaner, more consistent user experience.
packages/platform/examples/base/src/pages/[bookingUid].tsx (2)
30-30: Good choice using native Date comparison for performance.The use of native
Dateobjects for the time comparison is appropriate and aligns with the coding guidelines that recommend avoiding Day.js methods like.isBefore()in performance-critical code.
134-165: LGTM! Correctly hides reschedule and cancel options for past bookings.The conditional rendering properly restricts access to booking modification options for past bookings, ensuring consistency with the overall PR objective. The logic is clear and the implementation is correct.
apps/web/playwright/booking-pages.e2e.ts (1)
759-784: Excellent E2E test coverage for past booking restrictions.The test comprehensively verifies that both the cancel button and the "Need to make a change?" text are properly hidden for past bookings. The test setup correctly creates a past booking scenario and the assertions ensure the UI behaves as expected according to the PR requirements.
apps/web/modules/bookings/views/bookings-single-view.tsx (3)
817-818: LGTM: Conditional logic correctly prevents past booking cancellationThe updated condition properly restricts reschedule/cancel options for past bookings while preserving the ability to reschedule past bookings when explicitly configured via
eventType.allowReschedulingPastBookings.
843-843: Good UX improvement: Hide "or" separator for past bookingsThis prevents displaying the "or" text when the cancel button is hidden for past bookings, maintaining clean UI consistency.
849-849: Core feature implemented correctly: Cancel button hidden for past bookingsThis change successfully implements the PR objective by preventing the cancel button from appearing for bookings that have already ended, based on the booking's end time.
husniabad
force-pushed
the
fix/remove-cancel-button-for-past-bookings
branch
from
8545ad1 to
8aadb25
Compare
|
This PR is being marked as stale due to inactivity. |
|
@husniabad pls fix the conflicts |
husniabad
force-pushed
the
fix/remove-cancel-button-for-past-bookings
branch
from
8aadb25 to
909efa5
Compare
|
Hi @anikdhabal I have fixed the conflicts and updated the PR to fit the latest design, there could be some failed tests but none of them related to my PR |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/components/booking/BookingListItem.tsx (1)
272-288: Initialize edit-actions handlers after hook declarations to avoid TDZ surprisesThese onClick handlers reference state setters declared later (Lines 305-309). While closures make this work at runtime, it’s easy to trip eslint “no-use-before-define” and it’s less readable.
Consider moving this block below the useState declarations (Lines 305-309), or memoize it after those hooks:
// After lines 309 (state hooks) const editEventActions: ActionType[] = getEditEventActions(actionContext).map((action) => ({ ...action, onClick: action.id === "reschedule_request" ? () => setIsOpenRescheduleDialog(true) : action.id === "reroute" ? () => setRerouteDialogIsOpen(true) : action.id === "change_location" ? () => setIsOpenLocationDialog(true) : action.id === "add_members" ? () => setIsOpenAddGuestsDialog(true) : action.id === "reassign" ? () => setIsOpenReassignDialog(true) : undefined, }));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/booking/BookingListItem.tsx(2 hunks)apps/web/components/booking/bookingActions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/booking/bookingActions.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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.tsx
🧬 Code Graph Analysis (1)
apps/web/components/booking/BookingListItem.tsx (2)
apps/web/components/booking/bookingActions.ts (1)
getEditEventActions(98-162)packages/ui/components/dropdown/Dropdown.tsx (1)
DropdownItem(161-181)
⏰ 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: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
E2E results are ready! |
husniabad
force-pushed
the
fix/remove-cancel-button-for-past-bookings
branch
from
d7d834a to
c9b1978
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/components/booking/BookingListItem.tsx (1)
272-288: Flatten the nested ternary chain with a handler mapA small refactor improves readability and makes adding actions trivial.
Apply this diff:
- const baseEditEventActions = getEditEventActions(actionContext); - const editEventActions: ActionType[] = baseEditEventActions.map((action) => ({ - ...action, - onClick: - action.id === "reschedule_request" - ? () => setIsOpenRescheduleDialog(true) - : action.id === "reroute" - ? () => setRerouteDialogIsOpen(true) - : action.id === "change_location" - ? () => setIsOpenLocationDialog(true) - : action.id === "add_members" - ? () => setIsOpenAddGuestsDialog(true) - : action.id === "reassign" - ? () => setIsOpenReassignDialog(true) - : undefined, - })) as ActionType[]; + const baseEditEventActions = getEditEventActions(actionContext); + const handlerById: Record<string, () => void> = { + reschedule_request: () => setIsOpenRescheduleDialog(true), + reroute: () => setRerouteDialogIsOpen(true), + change_location: () => setIsOpenLocationDialog(true), + add_members: () => setIsOpenAddGuestsDialog(true), + reassign: () => setIsOpenReassignDialog(true), + }; + const editEventActions: ActionType[] = baseEditEventActions.map((action) => ({ + ...action, + onClick: handlerById[action.id], + })) as ActionType[];Nit: consider aligning naming for the location dialog booleans (isOpenSetLocationDialog vs setIsOpenLocationDialog) for symmetry in a follow-up.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/web/components/booking/BookingListItem.tsx(2 hunks)apps/web/components/booking/bookingActions.ts(1 hunks)apps/web/modules/bookings/views/bookings-single-view.tsx(2 hunks)apps/web/playwright/booking-pages.e2e.ts(1 hunks)packages/features/bookings/lib/handleCancelBooking.ts(1 hunks)packages/platform/examples/base/src/pages/[bookingUid].tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/platform/examples/base/src/pages/[bookingUid].tsx
- apps/web/playwright/booking-pages.e2e.ts
- apps/web/components/booking/bookingActions.ts
- packages/features/bookings/lib/handleCancelBooking.ts
- apps/web/modules/bookings/views/bookings-single-view.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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.tsx
**/*.{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.tsx
🧬 Code graph analysis (1)
apps/web/components/booking/BookingListItem.tsx (2)
apps/web/components/booking/bookingActions.ts (1)
getEditEventActions(98-162)packages/ui/components/dropdown/Dropdown.tsx (1)
DropdownItem(161-181)
⏰ 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)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
apps/web/components/booking/BookingListItem.tsx (2)
272-288: LGTM: Centralized edit actions wiring is consistent and readableMapping getEditEventActions to dialog openers is correct, ids match the action catalog, and state setters exist. Nice consolidation.
686-726: Nice a11y pattern: keeps menu item focusable with tooltip while blocking activationThis matches the earlier guidance: wrapper stays interactive, tooltip works, and activation is safely prevented via onSelect/onClick. Good localization use with t("cannot_cancel_booking").
husniabad
force-pushed
the
fix/remove-cancel-button-for-past-bookings
branch
from
c9b1978 to
be83756
Compare
anikdhabal
changed the title
| const month = dayjs(date).format("MMMM"); | ||
| const isBookingInPast = booking?.end ? dayjs(booking?.end).isBefore(dayjs()) : false; |
There was a problem hiding this comment.
Is it related? Let tackle it later
| }); | ||
| } | ||
|
|
||
| // Block cancellation attempts for bookings that have already ended |
There was a problem hiding this comment.
Remove Commet, as it is understandable from the code
| ((!isBookingInPast && canCancelOrReschedule) || | ||
| ((!isBookingInPast || eventType.allowReschedulingPastBookings) && canReschedule)) && |
There was a problem hiding this comment.
It's not needed; our aim is to hide the cancel button. And we already tackling it
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/playwright/booking-pages.e2e.ts (4)
778-781: Make “past” timestamps robust against TZ/DST to avoid flakesSubtracting one calendar day can straddle DST or locale midnights. Use a fixed offset from now so endTime is guaranteed to be in the past on all environments.
- const pastDate = new Date(); - pastDate.setDate(pastDate.getDate() - 1); - const endDate = new Date(pastDate.getTime() + 30 * 60 * 1000); + // Keep endTime safely in the past across TZ/DST (end ≈ 90 mins ago) + const pastDate = new Date(Date.now() - 2 * 60 * 60 * 1000); // 2h ago + const endDate = new Date(pastDate.getTime() + 30 * 60 * 1000); // +30m
789-792: Wait for bookings list and avoid nth selectors to reduce flakinessEnsure the list has rendered before interacting and prefer first() over nth(0). Optionally assert the created booking title is present.
- await page.goto("/bookings/past"); - await page.locator('[data-testid="booking-actions-dropdown"]').nth(0).click(); + await page.goto("/bookings/past"); + await page.waitForSelector('[data-testid="bookings"]'); + await expect(page.locator('[data-testid="bookings"]')).toContainText("Past Meeting"); + await page.locator('[data-testid="booking-actions-dropdown"]').first().click();
793-795: Guard for page readiness on booking detailsAdd a lightweight readiness wait to avoid occasional race conditions after navigation.
- await page.goto(`/booking/${booking.uid}`); + await page.goto(`/booking/${booking.uid}`); + await page.waitForLoadState("domcontentloaded");
770-796: Optional: assert server-side guard via API for completenessConsider a small companion check that direct POST /api/cancel for this past booking returns 400. This complements unit tests and hardens the e2e signal.
// Add within this describe as a separate test test("Cancel API should be blocked for past bookings", async ({ page }) => { const csrfToken = (await (await page.request.get("/api/csrf")).json()).csrfToken; const resp = await page.request.post("/api/cancel", { data: { uid: booking.uid, csrfToken }, headers: { "Content-Type": "application/json" }, }); expect(resp.status()).toBe(400); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/playwright/booking-pages.e2e.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/playwright/booking-pages.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/playwright/booking-pages.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/playwright/booking-pages.e2e.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: bandhan-majumder
PR: calcom/cal.com#23192
File: packages/lib/server/service/InsightsBookingBaseService.ts:814-816
Timestamp: 2025-08-22T16:38:00.225Z
Learning: In the InsightsBookingBaseService (packages/lib/server/service/InsightsBookingBaseService.ts), when filtering for "accepted" bookings in getMembersStatsWithCount(), using `endTime <= now()` in the SQL condition should be avoided as it conflicts with existing date filtering logic. The components already handle completion filtering by setting `endDate: currentTime` in their query parameters, making additional SQL-level endTime filtering unnecessary and potentially problematic.
🔇 Additional comments (1)
apps/web/playwright/booking-pages.e2e.ts (1)
770-796: E2E coverage for past booking cancel UX — looks goodSolid end-to-end assertion that cancel is disabled in the list and hidden on the booking page for past events. Matches the UI behavior described in the PR.
4 participants
What does this PR do?
This PR prevents users from canceling past bookings by hiding cancel buttons in the UI and adding server-side checks to block API bypass attempts. It keeps the existing cancellation flow and user experience unchanged for valid future bookings.
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?