refactor: Booking list items actions UI #22540
Conversation
|
""" WalkthroughThe changes refactor the Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Estimated code review effort3 (~45 minutes) Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/components/booking/bookingActions.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-playwright". (The package "eslint-plugin-playwright" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-playwright" was referenced from the config file in ".eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Files:
🧠 Learnings (1)apps/web/components/booking/bookingActions.ts (1)Learnt from: eunjae-lee 🧬 Code Graph Analysis (1)apps/web/components/booking/bookingActions.ts (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{js,jsx,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Files:
🧠 Learnings (1)apps/web/components/booking/bookingActions.ts (1)Learnt from: eunjae-lee 🧬 Code Graph Analysis (1)apps/web/components/booking/bookingActions.ts (2)
⏰ 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). (1)
🔇 Additional comments (9)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
|
✅ No security or compliance issues detected. Reviewed everything up to de176b2. Security Overview
Detected Code Changes
Reply to this PR with |
| } | ||
| }); | ||
| } | ||
| const cancelEventAction: ActionType = { |
There was a problem hiding this comment.
on recurring tab we just display cancel action and nothing else
| label: t("view_session_details"), | ||
| onClick: () => setMeetingSessionDetailsDialogIsOpen(true), | ||
| icon: "info", | ||
| disabled: !(isBookingInPast && isConfirmed && isCalVideoLocation), |
There was a problem hiding this comment.
Only display when meeting has passed
| /> | ||
| )} | ||
| {isPending && isUpcoming && !isCancelled && <TableActions actions={pendingActions} />} | ||
| {!isPending && !(isTabRecurring && isRecurring) && !isCancelled && <BookingActions />} |
There was a problem hiding this comment.
All actions is added here now.
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/16/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (07/21/25)1 label was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/components/booking/BookingListItem.tsx(5 hunks)apps/web/public/static/locales/en/common.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
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.
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>
⏰ 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). (4)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
- GitHub Check: Security Check
🔇 Additional comments (3)
apps/web/public/static/locales/en/common.json (1)
52-53: Add corresponding keys in non-EN locale files
edit_eventandview_session_detailswere added only to the English bundle.
If the project relies on key-parity across locale files, remember to add placeholders (or translations) to the other language JSONs now—otherwise runtime look-ups may fall back to the key string or crash depending on the i18n setup.Nothing else to change in this snippet.
apps/web/components/booking/BookingListItem.tsx (2)
248-257: Good extraction of cancel action with proper styling and logicThe separation of the cancel action with destructive color and comprehensive disabled logic is well implemented.
740-742: Clean refactoring of action rendering logicThe simplified conditional rendering logic is much clearer and correctly handles all booking states as intended.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Okay. I was planning to add this in a follow up PR. I'll just add it now |
|
Past unconfirmed bookings will not show any actions. Want to confirm if that is fine? Looks fine to me though. |
|
Except for the tests, it LGTM !! |
Yes as booking is not confirmed |
|
It was getting difficult writing unit tests for BookingListItem so i just refactored the code and created separate file for handling booking actions |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/components/booking/bookingActions.ts (1)
108-171: Consider extracting the reschedule disabled logic to reduce duplication.The disabled condition for both reschedule actions (lines 126-128 and 134-136) is identical. Consider extracting it to a variable for better maintainability.
export function getEditEventActions(context: BookingActionContext): ActionType[] { const { booking, isBookingInPast, isDisabledRescheduling, isBookingFromRoutingForm, getSeatReferenceUid, t, } = context; + const isRescheduleDisabled = + (isBookingInPast && !booking.eventType.allowReschedulingPastBookings) || isDisabledRescheduling; + const actions: (ActionType | null)[] = [ { id: "reschedule", icon: "clock", label: t("reschedule_booking"), href: `/reschedule/${booking.uid}${ booking.seatsReferences.length ? `?seatReferenceUid=${getSeatReferenceUid()}` : "" }`, - disabled: - (isBookingInPast && !booking.eventType.allowReschedulingPastBookings) || isDisabledRescheduling, + disabled: isRescheduleDisabled, }, { id: "reschedule_request", icon: "send", iconClassName: "rotate-45 w-[16px] -translate-x-0.5 ", label: t("send_reschedule_request"), - disabled: - (isBookingInPast && !booking.eventType.allowReschedulingPastBookings) || isDisabledRescheduling, + disabled: isRescheduleDisabled, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/components/booking/BookingListItem.tsx(7 hunks)apps/web/components/booking/bookingActions.test.ts(1 hunks)apps/web/components/booking/bookingActions.ts(1 hunks)apps/web/playwright/booking-pages.e2e.ts(3 hunks)apps/web/playwright/bookings-list.e2e.ts(1 hunks)apps/web/playwright/dynamic-booking-pages.e2e.ts(2 hunks)apps/web/playwright/reschedule.e2e.ts(3 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:
apps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/reschedule.e2e.tsapps/web/playwright/booking-pages.e2e.tsapps/web/playwright/bookings-list.e2e.tsapps/web/components/booking/bookingActions.test.tsapps/web/components/booking/bookingActions.ts
🧬 Code Graph Analysis (1)
apps/web/components/booking/bookingActions.test.ts (2)
apps/web/components/booking/bookingActions.ts (11)
BookingActionContext(6-34)getPendingActions(36-62)getCancelEventAction(64-87)getVideoOptionsActions(89-106)getEditEventActions(108-171)getAfterEventActions(173-196)shouldShowPendingActions(198-201)shouldShowEditActions(203-206)shouldShowRecurringCancelAction(208-211)isActionDisabled(213-231)getActionLabel(233-252)packages/platform/libraries/index.ts (1)
SchedulingType(30-30)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/components/booking/BookingListItem.tsx
🧰 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:
apps/web/playwright/dynamic-booking-pages.e2e.tsapps/web/playwright/reschedule.e2e.tsapps/web/playwright/booking-pages.e2e.tsapps/web/playwright/bookings-list.e2e.tsapps/web/components/booking/bookingActions.test.tsapps/web/components/booking/bookingActions.ts
🧬 Code Graph Analysis (1)
apps/web/components/booking/bookingActions.test.ts (2)
apps/web/components/booking/bookingActions.ts (11)
BookingActionContext(6-34)getPendingActions(36-62)getCancelEventAction(64-87)getVideoOptionsActions(89-106)getEditEventActions(108-171)getAfterEventActions(173-196)shouldShowPendingActions(198-201)shouldShowEditActions(203-206)shouldShowRecurringCancelAction(208-211)isActionDisabled(213-231)getActionLabel(233-252)packages/platform/libraries/index.ts (1)
SchedulingType(30-30)
⏰ 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: Security Check
🔇 Additional comments (22)
apps/web/playwright/dynamic-booking-pages.e2e.ts (2)
39-41: Test correctly updated for dropdown UI pattern.The test properly reflects the UI refactor where booking actions are now accessed through a dropdown menu. The added step to click the ellipsis menu button before selecting reschedule is correct and maintains the test's effectiveness.
59-61: Consistent test update for cancel action.The cancel test follows the same pattern as reschedule, correctly implementing the dropdown interaction pattern. This maintains consistency across all booking action tests.
apps/web/playwright/bookings-list.e2e.ts (1)
366-369: Test improvement for dropdown interaction verification.The test now properly opens the dropdown menu before verifying the cancel option is visible, making it more accurate and specific to the actual user interaction pattern. This is an improvement over the previous approach of just checking for text presence on the page.
apps/web/playwright/booking-pages.e2e.ts (3)
158-159: Reschedule test properly updated for dropdown UI.The test correctly implements the new interaction pattern by opening the dropdown menu before selecting the reschedule option, maintaining the test's effectiveness while adapting to the UI refactor.
209-211: Cancel test consistently updated with dropdown pattern.The test properly implements the dropdown interaction pattern for cancellation, maintaining consistency with other booking action tests and ensuring compatibility with the refactored UI.
244-247: Additional cancel test updated for dropdown consistency.This test follows the same dropdown interaction pattern as other booking action tests, ensuring all cancellation scenarios work correctly with the new UI structure.
apps/web/playwright/reschedule.e2e.ts (4)
36-37: Reschedule request test updated for dropdown UI.The test correctly opens the dropdown menu before selecting the reschedule request option, maintaining test functionality while adapting to the refactored UI structure.
79-80: Past booking reschedule test adapted for dropdown menu.The test properly opens the dropdown to verify reschedule options are available for past bookings when explicitly allowed, ensuring the test works with the new UI pattern.
96-97: Dropdown interaction added for disabled state verification.The test correctly opens the dropdown menu to verify the state of reschedule options after disabling rescheduling for past bookings, ensuring proper testing of the disabled functionality.
99-103: Improved UX behavior: disabled actions remain visible.The test now correctly verifies that reschedule options are visible but disabled when rescheduling past bookings is disallowed, rather than being hidden entirely. This provides better user experience by showing available actions even when they're not currently usable.
apps/web/components/booking/bookingActions.test.ts (6)
19-130: Well-structured test setup with comprehensive mock context.The test setup provides a robust foundation with a realistic mock context that covers all required properties. The overrides pattern allows for flexible test variations while maintaining good defaults. The mock translation function is appropriately simple for testing purposes.
133-173: Comprehensive testing of pending actions logic.The tests thoroughly cover the getPendingActions function, verifying correct action generation for pending vs non-pending bookings, proper labeling for recurring scenarios, and appropriate action properties. Good test coverage ensures the centralized logic works correctly.
175-218: Thorough testing of cancel action logic.The tests properly verify the getCancelEventAction function across various scenarios, including disabled states, recurring bookings, and edge cases like past pending bookings. The URL construction and label logic are well tested.
220-259: Video options actions properly tested.The tests effectively verify the getVideoOptionsActions function, covering the various conditions that enable/disable video-related actions like recordings and session details. The logic for Cal video locations and booking states is well tested.
261-323: Edit event actions comprehensively tested.The tests thoroughly verify the getEditEventActions function, covering all conditional logic for actions like reroute (routing forms), reassign (round robin), and guest management. The disabled state handling for rescheduling restrictions is properly tested.
325-462: Complete test coverage for all helper functions.The remaining tests provide excellent coverage for after-event actions, boolean helper functions, disabled state logic, and label generation. The tests for no-show handling, charge card actions, and various conditional display logic ensure the helper functions work correctly across all booking scenarios.
apps/web/components/booking/bookingActions.ts (6)
36-62: LGTM! Well-structured pending action generation.The function correctly implements the business logic for pending bookings, with appropriate conditional logic for payment-enabled bookings.
64-87: LGTM! Comprehensive cancel action implementation.The function properly handles all edge cases including recurring bookings, seat references, and past pending bookings.
89-106: LGTM! Clear video action conditions.The function correctly differentiates between recordings (requiring isRecorded flag) and session details availability.
173-196: LGTM! Good reuse of video actions.The function properly combines video actions with post-event specific actions, with appropriate conditional logic for payment holds.
198-211: LGTM! Clear visibility condition functions.These helper functions provide clean, testable logic for determining action visibility.
233-252: LGTM! Well-organized label generation.The function provides consistent, context-aware labels for all action types.
What does this PR do?
Follow up:- Add UI tests
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?