refactor: booking-list-state-management#23803
Conversation
|
@hemantmm is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 128 files out of 292 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughAdds a new hook, useBookingItemState, that centralizes ten dialog booleans and a rejectionReason and exposes openDialog/closeDialog/toggleDialog/resetDialogs. BookingListItem was refactored to consume dialogState and use openDialog/closeDialog for all dialog controls instead of many local useState flags; onClick handlers and dialog prop wiring were updated accordingly. Several dialog components (RescheduleDialog, ReassignDialog, EditLocationDialog, AddGuestsDialog, ChargeCardDialog, ViewRecordingsDialog, MeetingSessionDetailsDialog, RerouteDialog, NoShowAttendeesDialog) now receive boolean Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
dosubot
bot
added
the
🧹 Improvements
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/platform/atoms/src/components/ui/dialog.tsx (1)
54-66: enableOverflow is declared but unused and is leaked as an invalid DOM attribute.The prop currently has no effect and is spread onto the DOM (e.g., enableoverflow=""). Implement behavior and avoid leaking the prop.
Apply:
-const DialogContent = React.forwardRef< - React.ElementRef<typeof DialogPrimitives.Content>, - React.ComponentPropsWithoutRef<typeof DialogPrimitives.Content> & { enableOverflow?: boolean } ->(({ className, children, ...props }, ref) => ( +const DialogContent = React.forwardRef< + React.ElementRef<typeof DialogPrimitives.Content>, + React.ComponentPropsWithoutRef<typeof DialogPrimitives.Content> & { enableOverflow?: boolean; closeLabel?: string } +>(({ className, children, enableOverflow, closeLabel, ...props }, ref) => ( <> <DialogPortal> <div className="calcom-atoms"> <DialogOverlay /> <DialogPrimitives.Content ref={ref} className={cn( - "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border p-6 shadow-lg duration-200 sm:rounded-lg", + "bg-background data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] fixed left-[50%] top-[50%] z-50 grid w-full max-w-lg translate-x-[-50%] translate-y-[-50%] gap-4 border p-6 shadow-lg duration-200 sm:rounded-lg", + enableOverflow ? "overflow-visible" : "overflow-y-auto", className )} {...props}> {children} - <DialogPrimitives.Close className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute right-4 top-4 rounded-sm opacity-70 transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-offset-2 disabled:pointer-events-none"> + <DialogPrimitives.Close + aria-label={closeLabel ?? "Close"} + className="ring-offset-background focus:ring-ring data-[state=open]:bg-accent data-[state=open]:text-muted-foreground absolute right-4 top-4 rounded-sm opacity-70 transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-offset-2 disabled:pointer-events-none"> <Icon name="x" className="h-4 w-4" /> - <span className="sr-only">Close</span> + <span className="sr-only">{closeLabel ?? "Close"}</span> </DialogPrimitives.Close> </DialogPrimitives.Content> </div> </DialogPortal> </> ));Follow-up: pass a localized closeLabel from callers if needed (e.g., closeLabel={t("close")}).
apps/web/components/dialog/ChargeCardDialog.tsx (1)
26-35: Localize all user-facing strings and normalize error messages.
- "Charge successful" is hard-coded. Use t().
- Server error messages may be keys; prefer t(error.message) with a sane fallback.
onSuccess: () => { utils.viewer.bookings.invalidate(); setIsOpenDialog(false); setChargeError(null); - showToast("Charge successful", "success"); + showToast(t("charge_successful"), "success"); }, onError: (error) => { - setChargeError(error.message || t("error_charging_card")); + // Try to translate known keys; fall back to raw message or generic error. + const translated = t(error.message as any); + setChargeError(translated !== error.message ? translated : error.message || t("error_charging_card")); },apps/web/components/booking/BookingListItem.tsx (1)
469-475: Localize “(Optional)”.Avoid hard-coded English. Use t("optional") and compose parentheses in JSX.
- label={ - <> - {t("rejection_reason")} - <span className="text-subtle font-normal"> (Optional)</span> - </> - } + label={ + <> + {t("rejection_reason")} + <span className="text-subtle font-normal"> ({t("optional")})</span> + </> + }
🧹 Nitpick comments (5)
apps/web/components/dialog/ChargeCardDialog.tsx (1)
53-58: Add accessible announcement for error block.Improve SR experience when errors appear.
- {chargeError && ( - <div className="mt-4 flex text-red-500"> + {chargeError && ( + <div className="mt-4 flex text-red-500" role="alert" aria-live="polite"> <Icon name="triangle-alert" className="mr-2 h-5 w-5 " aria-hidden="true" /> <p className="text-sm">{chargeError}</p> </div> )}apps/web/components/booking/hooks/useBookingItemState.ts (3)
16-29: Avoid duplicating initial dialog state.Extract a shared constant and reuse it for initialization and reset.
-export function useBookingItemState() { - const [rejectionReason, setRejectionReason] = useState<string>(""); - const [dialogState, setDialogState] = useState<DialogState>({ +const initialDialogState: DialogState = { reschedule: false, reassign: false, editLocation: false, addGuests: false, chargeCard: false, viewRecordings: false, meetingSessionDetails: false, noShowAttendees: false, rejection: false, reroute: false, - }); +}; + +export function useBookingItemState() { + const [rejectionReason, setRejectionReason] = useState<string>(""); + const [dialogState, setDialogState] = useState<DialogState>(initialDialogState);
31-51: Stabilize handlers to reduce re-renders.Wrap openDialog/closeDialog/toggleDialog with useCallback.
-import { useState } from "react"; +import { useCallback, useState } from "react"; ... - const openDialog = (dialog: keyof DialogState) => { + const openDialog = useCallback((dialog: keyof DialogState) => { setDialogState((prevState) => ({ ...prevState, [dialog]: true, })); - }; + }, []); ... - const closeDialog = (dialog: keyof DialogState) => { + const closeDialog = useCallback((dialog: keyof DialogState) => { setDialogState((prevState) => ({ ...prevState, [dialog]: false, })); - }; + }, []); ... - const toggleDialog = (dialog: keyof DialogState) => { + const toggleDialog = useCallback((dialog: keyof DialogState) => { setDialogState((prevState) => ({ ...prevState, [dialog]: !prevState[dialog], })); - }; + }, []);
52-66: Provide a convenience “openExclusiveDialog” helper.Prevents multiple dialogs being open simultaneously with one call site change.
// Helper function to reset all dialogs (useful when one action should close others) const resetDialogs = () => { - setDialogState({ - reschedule: false, - reassign: false, - editLocation: false, - addGuests: false, - chargeCard: false, - viewRecordings: false, - meetingSessionDetails: false, - noShowAttendees: false, - rejection: false, - reroute: false, - }); + setDialogState(initialDialogState); }; + + const openExclusiveDialog = (dialog: keyof DialogState) => { + setDialogState({ ...initialDialogState, [dialog]: true }); + };Export openExclusiveDialog alongside others if useful to callers.
apps/web/components/booking/BookingListItem.tsx (1)
1290-1290: Prefer named export for components.Default export makes refactors and tree-shaking harder. Consider
export { BookingListItem };and update imports.
📜 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.
📒 Files selected for processing (5)
apps/web/components/booking/BookingListItem.tsx(12 hunks)apps/web/components/booking/hooks/useBookingItemState.ts(1 hunks)apps/web/components/dialog/ChargeCardDialog.tsx(1 hunks)packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx(1 hunks)packages/platform/atoms/src/components/ui/dialog.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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:
packages/platform/atoms/src/components/ui/dialog.tsxpackages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsxapps/web/components/dialog/ChargeCardDialog.tsxapps/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:
packages/platform/atoms/src/components/ui/dialog.tsxpackages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsxapps/web/components/booking/hooks/useBookingItemState.tsapps/web/components/dialog/ChargeCardDialog.tsxapps/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:
packages/platform/atoms/src/components/ui/dialog.tsxpackages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsxapps/web/components/booking/hooks/useBookingItemState.tsapps/web/components/dialog/ChargeCardDialog.tsxapps/web/components/booking/BookingListItem.tsx
**/*.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/components/booking/hooks/useBookingItemState.ts
🧠 Learnings (2)
📚 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:
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
🧬 Code graph analysis (1)
apps/web/components/booking/BookingListItem.tsx (6)
apps/web/components/booking/hooks/useBookingItemState.ts (1)
useBookingItemState(16-77)apps/web/components/dialog/ReassignDialog.tsx (1)
ReassignDialog(63-328)apps/web/components/dialog/AddGuestsDialog.tsx (1)
AddGuestsDialog(20-104)apps/web/components/dialog/ChargeCardDialog.tsx (1)
ChargeCardDialog(19-79)packages/features/ee/video/MeetingSessionDetailsDialog.tsx (1)
MeetingSessionDetailsDialog(172-201)packages/features/components/controlled-dialog/index.tsx (1)
Dialog(23-26)
🔇 Additional comments (7)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx (1)
8-8: TooltipProvider migration looks good.Import path switch to the internal UI provider is consistent with the PR direction. No changes needed.
apps/web/components/booking/BookingListItem.tsx (6)
69-70: Centralized state hook import — good move.This aligns with the PR objectives and reduces prop drilling.
131-133: Adoption of useBookingItemState — LGTM.Clear separation of UI and state; API is easy to test.
157-162: Close rejection dialog on successful reject — correct.Matches UX expectations.
260-267: Pending actions wiring — good.Using openDialog("rejection") removes duplicated boolean state.
360-369: Edit actions wiring — good.Consistent pattern across dialogs.
377-393: After-event actions wiring — good.Edge-case for single attendee is handled; multi-attendee flows into dialog.
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/web/components/booking/BookingListItem.tsx (3)
286-292: Bug: mis-destructured useState and stale local dialog state for Edit Location.
const [setIsOpenLocationDialog] = useState(false);assigns the state value to a setter-named var and later calls it like a setter. Also, this local state is obsolete given the centralized dialog state. Remove it and callcloseDialog("editLocation")on success.Apply:
- const [setIsOpenLocationDialog] = useState(false); @@ const setLocationMutation = trpc.viewer.bookings.editLocation.useMutation({ onSuccess: () => { showToast(t("location_updated"), "success"); - setIsOpenLocationDialog(false); + closeDialog("editLocation"); utils.viewer.bookings.invalidate(); },
726-728: Fix: setIsOpenDialog must accept the boolean “open” (RerouteDialog still using zero-arg).Pass the boolean and mirror the pattern used elsewhere to avoid incorrect closes and type mismatches.
- setIsOpenDialog={() => closeDialog("reroute")} + setIsOpenDialog={(open) => (open ? openDialog("reroute") : closeDialog("reroute"))}
724-729: Convert zero-arg dialog handlers to accept and forward the (open: boolean) argumentFound zero-arg handlers — verify which are used as dialog onOpenChange/setters and change them to accept (open: boolean) and pass that value to the state setter or close/open helper:
- packages/app-store/routing-forms/components/_components/TestForm.tsx:161 — const onClose = () => { ... }
- packages/app-store/hitpay/components/HitpayPaymentComponent.tsx:72 — const onClose = () => { ... }
- apps/ui-playground/content/design/components/toast.basic.tsx:39 — const onClose = () => { ... }
- packages/features/ee/organizations/pages/settings/delegationCredential.tsx:361 — const onCreateClick = () => setCreateEditDialog({ isOpen: true, delegation: null });
If any are passed to a Dialog's onOpenChange or similar, change to e.g. const onClose = (open: boolean) => setDialogOpen(open) or forward open to closeDialog.
🧹 Nitpick comments (1)
apps/web/components/booking/BookingListItem.tsx (1)
467-471: Localize “(Optional)”.Per TSX i18n guidelines, avoid hardcoded text. Wrap with
t().- <span className="text-subtle font-normal"> (Optional)</span> + <span className="text-subtle font-normal"> ({t("optional")})</span>
📜 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.
📒 Files selected for processing (1)
apps/web/components/booking/BookingListItem.tsx(12 hunks)
🧰 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
🧠 Learnings (1)
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
🧬 Code graph analysis (1)
apps/web/components/booking/BookingListItem.tsx (6)
apps/web/components/booking/hooks/useBookingItemState.ts (1)
useBookingItemState(16-77)apps/web/components/dialog/ReassignDialog.tsx (1)
ReassignDialog(63-328)apps/web/components/dialog/AddGuestsDialog.tsx (1)
AddGuestsDialog(20-104)apps/web/components/dialog/ChargeCardDialog.tsx (1)
ChargeCardDialog(19-79)packages/features/ee/video/MeetingSessionDetailsDialog.tsx (1)
MeetingSessionDetailsDialog(172-201)packages/features/components/controlled-dialog/index.tsx (1)
Dialog(23-26)
⏰ 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 (3)
apps/web/components/booking/BookingListItem.tsx (3)
68-68: Good move: centralized dialog/form state via useBookingItemState.This reduces prop-drilling and standardizes open/close flows. Nice.
Also applies to: 130-132
259-266: Action handlers now consistently drive dialogs via openDialog — LGTM.The mapping is clear and removes scattered setters.
Also applies to: 355-365, 371-389
399-401: Dialog open/close contract fixed to accept boolean — nice consistency.These updates align with
(open: boolean) => voidexpectations across dialogs and prevent accidental closes on open.Also applies to: 403-411, 415-418, 420-423, 426-431, 436-439, 441-449, 451-457, 459-461
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
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/BookingListItem.tsx (1)
289-296: EditLocationDialog doesn’t close after save (local state is orphaned).
setIsOpenLocationDialog(false)toggles an unused local state; the dialog is driven bydialogState.editLocation. Close it via the central controller and remove the dead state.- const [, setIsOpenLocationDialog] = useState(false); - // const [setIsOpenLocationDialog] = useState(false); + // (removed) local dialog state; use centralized dialog controller instead const setLocationMutation = trpc.viewer.bookings.editLocation.useMutation({ onSuccess: () => { showToast(t("location_updated"), "success"); - setIsOpenLocationDialog(false); + closeDialog("editLocation"); utils.viewer.bookings.invalidate(); },
♻️ Duplicate comments (1)
apps/web/components/booking/BookingListItem.tsx (1)
728-733: Fix RerouteDialog handler signature (uses zero-arg closer).This regresses the earlier fix: children expect
(open: boolean) => void. Passing a zero-arg function can break types and closes on both transitions.- <RerouteDialog - isOpenDialog={dialogState.reroute} - setIsOpenDialog={() => closeDialog("reroute")} - booking={{ ...parsedBooking, eventType: parsedBooking.eventType }} - /> + <RerouteDialog + isOpenDialog={dialogState.reroute} + setIsOpenDialog={(open) => (open ? openDialog("reroute") : closeDialog("reroute"))} + booking={{ ...parsedBooking, eventType: parsedBooking.eventType }} + />
🧹 Nitpick comments (1)
apps/web/components/booking/BookingListItem.tsx (1)
1-3: Drop file-wide eslint disables; fix the root causes instead.These global disables hide real issues. You can remove both by eliminating the
anyusage below and letting Prettier run.Apply:
-/* eslint-disable @typescript-eslint/no-explicit-any */ - -/* eslint-disable prettier/prettier */
📜 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.
📒 Files selected for processing (1)
apps/web/components/booking/BookingListItem.tsx(15 hunks)
🧰 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
🧠 Learnings (1)
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
🧬 Code graph analysis (1)
apps/web/components/booking/BookingListItem.tsx (6)
apps/web/components/booking/hooks/useBookingItemState.ts (1)
useBookingItemState(16-77)apps/web/components/dialog/ReassignDialog.tsx (1)
ReassignDialog(63-328)apps/web/components/dialog/AddGuestsDialog.tsx (1)
AddGuestsDialog(20-104)apps/web/components/dialog/ChargeCardDialog.tsx (1)
ChargeCardDialog(19-79)packages/features/ee/video/MeetingSessionDetailsDialog.tsx (1)
MeetingSessionDetailsDialog(172-201)packages/features/components/controlled-dialog/index.tsx (1)
Dialog(23-26)
⏰ 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). (3)
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
🔇 Additional comments (3)
apps/web/components/booking/BookingListItem.tsx (3)
132-135: Nice: centralized dialog/form state adoption is clean.
useBookingItemStateintegration reads well and simplifies the component.
403-406: LGTM: onOpenChange handlers now accept the boolean “open” argument.The handlers correctly branch on
opento avoid closing during open transitions.Also applies to: 409-414, 419-421, 424-427, 430-435, 440-443, 446-453, 455-461, 463-466
428-436: Confirm gating condition for ChargeCardDialog.It renders only when
booking.paid && booking.payment[0]. If the intended flow is “charge when not yet paid,” this should likely be!booking.paid && booking.payment[0]. IfgetAfterEventActionsalready hides the action once paid, leave as-is.
| .map((date: { toString: () => any }) => date.toString()) | ||
| .includes(recurringDate.toString()) | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid brittle Date string comparisons; use timestamps (also removes any).
Comparing toString() is locale/zone dependent and typed as any here. Compare epoch milliseconds instead.
- .map((date: { toString: () => any }) => date.toString())
- .includes(recurringDate.toString())
+ .map((date: Date) => new Date(date).getTime())
+ .includes(new Date(recurringDate).getTime())- .map((date: { toString: () => any }) => date.toString())
- .includes(aDate.toString());
+ .map((date: Date) => new Date(date).getTime())
+ .includes(new Date(aDate).getTime());Optional: precompute a Set<number> of cancelled timestamps with useMemo to avoid re-mapping in both places.
Also applies to: 841-843
🤖 Prompt for AI Agents
In apps/web/components/booking/BookingListItem.tsx around lines 823-825 (and
similarly at 841-843), replace brittle Date.toString() comparisons with epoch
millisecond comparisons: map the dates to date.getTime() (typed as number)
instead of toString(), and compare recurringDate.getTime() to those numbers; to
avoid repeated mapping and remove the use of any, precompute a Set<number> of
cancelled timestamps using useMemo (dependent on the cancelled dates array) and
then use set.has(recurringDate.getTime()) for membership checks.
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx (5)
14-18: Make the PhoneInput mock resilient to hoisting.Use vi.importActual in the mock factory; avoid referencing a separately imported symbol.
-// Mock PhoneInput to avoid calling the lazy import -vi.mock("@calcom/features/components/phone-input/PhoneInput", () => { - return { - default: PhoneInput, - }; -}); +// Mock PhoneInput with the actual non-lazy module to avoid dynamic import in tests +vi.mock("@calcom/features/components/phone-input/PhoneInput", async () => { + const mod = await vi.importActual< + typeof import("@calcom/features/components/phone-input/PhoneInput") + >("@calcom/features/components/phone-input/PhoneInput"); + return { default: mod.default }; +});Additionally remove the now-unused top-level import:
-import PhoneInput from "@calcom/features/components/phone-input/PhoneInput";
139-147: Escape dynamic labels used as RegExp patterns.Labels like "https://google.com" contain regex metacharacters and can cause unintended matches.
+// escape user-provided strings before constructing RegExp +const escapeRegExp = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const component = { getName: ({ label = "your_name" }: { label?: string } = {}) => screen.getByRole("textbox", { - name: new RegExp(label), + name: new RegExp(escapeRegExp(label), "i"), }) as HTMLInputElement, getEmail: () => screen.getByRole("textbox", { name: /email/i }) as HTMLInputElement, getLocationRadioOption: ({ label }: { label: string }) => - screen.getByRole("radio", { name: new RegExp(label) }) as HTMLInputElement, + screen.getByRole("radio", { name: new RegExp(escapeRegExp(label), "i") }) as HTMLInputElement,
158-169: Avoid brittle placeholder assumptions for location inputs.Make placeholder selection data-driven instead of throwing.
fillRadioInputLocation: ({ label, inputValue }: { label: string; inputValue?: string }) => { fireEvent.click(component.getLocationRadioOption({ label })); if (inputValue) { - let placeholder = label; - if (label === "attendee_phone_number") { - placeholder = "enter_phone_number"; - } else { - // radioInput doesn't have a label, so we need to identify by placeholder - throw new Error("Tell me how to identify the placeholder for this location input"); - } + const placeholders: Record<string, string> = { + attendee_phone_number: "enter_phone_number", + }; + const placeholder = placeholders[label]; + if (!placeholder) { + throw new Error(`Missing placeholder mapping for location label: ${label}`); + } fireEvent.change(component.getLocationRadioInput({ placeholder }), { target: { value: inputValue }, }); } },
204-208: Truthiness check can skip valid "0" values.Be explicit when checking for an entered optionValue.
- if (value.optionValue) { + if (typeof value.optionValue === "string" && value.optionValue.length > 0) { expect(component.getLocationRadioInput({ placeholder: "enter_phone_number" }).value).toEqual( formattedValue ); }
153-176: Prefer userEvent over fireEvent for typing.userEvent better simulates real user input and blur/selection behavior; helpful for inputs with formatting.
If feasible in this repo, replace change calls with userEvent.type and click with userEvent.click.
📜 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.
📒 Files selected for processing (1)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx(1 hunks)
🧰 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:
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.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:
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.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:
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx
🧠 Learnings (1)
📚 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:
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx
🔇 Additional comments (1)
packages/features/bookings/Booker/components/BookEventForm/BookingFields.test.tsx (1)
8-8: Remaining @radix-ui/react-tooltip imports found — confirm intended migrationrg shows many files still import "@radix-ui/react-tooltip" while BookingFields.test.tsx now imports from "@calcom/ui/components/tooltip".
- Examples: apps/web/lib/app-providers.tsx, apps/web/lib/app-providers-app-dir.tsx, packages/features/calendars/tests/DatePicker.test.tsx, packages/features/form-builder/FormBuilder.test.tsx, packages/features/schedules/components/date-override-list.test.tsx, packages/ui/components/form/inputs/input.test.tsx, apps/ui-playground/app/design/layout.tsx.
- Note: packages/platform/atoms/vite.config.ts maps "@radix-ui/react-tooltip" → packages/platform/atoms/src/components/ui/tooltip.tsx, and that file exports TooltipProvider. Decide whether that alias is intentional or whether all imports should be standardized to "@calcom/ui/components/tooltip".
- Action required: either (A) update the remaining imports to "@calcom/ui/components/tooltip", or (B) confirm and document that the vite alias is the intended single source and leave imports as-is.
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
…mantmm/cal.com into booking-list-state-management
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/components/booking/BookingListItem.tsx (2)
289-306: EditLocation dialog doesn’t close after save; leftover local state.
setIsOpenLocationDialog(false)updates an unused local state. Use the centralized state to close.- const [, setIsOpenLocationDialog] = useState(false); + // Local dialog state not needed; use centralized dialog state. const setLocationMutation = trpc.viewer.bookings.editLocation.useMutation({ onSuccess: () => { showToast(t("location_updated"), "success"); - setIsOpenLocationDialog(false); + closeDialog("editLocation"); utils.viewer.bookings.invalidate(); },
329-334: Potential runtime crash: calling .concat on possibly undefined arrays.When
booking.recurringInfois undefined, this throws. Guard and default to empty arrays.- const recurringDates = booking.recurringInfo?.bookings[BookingStatus.ACCEPTED] - .concat(booking.recurringInfo?.bookings[BookingStatus.CANCELLED]) - .concat(booking.recurringInfo?.bookings[BookingStatus.PENDING]) - .sort((date1: Date, date2: Date) => date1.getTime() - date2.getTime()); + const recurringDates = booking.recurringInfo + ? [ + ...(booking.recurringInfo.bookings[BookingStatus.ACCEPTED] ?? []), + ...(booking.recurringInfo.bookings[BookingStatus.CANCELLED] ?? []), + ...(booking.recurringInfo.bookings[BookingStatus.PENDING] ?? []), + ].sort((d1: Date, d2: Date) => d1.getTime() - d2.getTime()) + : undefined;
♻️ Duplicate comments (2)
apps/web/components/booking/BookingListItem.tsx (2)
730-732: RerouteDialog: pass the boolean “open” to handler (one leftover).Align with the pattern used elsewhere to avoid closing on open transitions.
- setIsOpenDialog={() => closeDialog("reroute")} + setIsOpenDialog={(open) => (open ? openDialog("reroute") : closeDialog("reroute"))}
823-826: Avoid brittle Date.toString() comparisons and removeany.Compare epoch milliseconds; also fixes the
anytyping.- !booking.recurringInfo?.bookings[BookingStatus.CANCELLED] - .map((date: { toString: () => any }) => date.toString()) - .includes(recurringDate.toString()) + !booking.recurringInfo?.bookings[BookingStatus.CANCELLED] + .map((date: Date) => new Date(date).getTime()) + .includes(recurringDate.getTime())- booking.recurringInfo?.bookings[BookingStatus.CANCELLED] - .map((date: { toString: () => any }) => date.toString()) - .includes(aDate.toString()); + booking.recurringInfo?.bookings[BookingStatus.CANCELLED] + .map((date: Date) => new Date(date).getTime()) + .includes(aDate.getTime());Optional: precompute a
Set<number>of cancelled timestamps withuseMemoand useset.has(aDate.getTime())to avoid re-mapping on every render.Also applies to: 839-843
🧹 Nitpick comments (5)
apps/web/components/booking/BookingListItem.tsx (5)
1-3: Drop file-wide linter disables; fix the root “any” usages instead.These blanket disables hide real issues. After fixing the Date string comparisons below, they won’t be needed.
-/* eslint-disable @typescript-eslint/no-explicit-any */ - -/* eslint-disable prettier/prettier */
1140-1140: NoShowAttendeesDialog: onOpenChange should accept the boolean parameter.Prevents closing on open transitions and matches the Dialog contract.
- <Dialog open={isOpen} onOpenChange={() => setIsOpen(false)}> + <Dialog open={isOpen} onOpenChange={(open) => setIsOpen(open)}>
524-531: Addnoopenerfor security when using target="_blank".Prevents
window.openerhijacking.- <a + <a href={locationToDisplay} onClick={(e) => e.stopPropagation()} target="_blank" title={locationToDisplay} - rel="noreferrer" + rel="noreferrer noopener" className="text-sm leading-6 text-blue-600 hover:underline dark:text-blue-400">
472-474: Localize “Optional”.Avoid hardcoded English; wrap in
t().- <span className="text-subtle font-normal"> (Optional)</span> + <span className="text-subtle font-normal"> ({t("optional")})</span>
175-178: Minor perf: compute “now” once for time comparisons.Reduces repeated
new Date()calls in a list item render.- const isUpcoming = new Date(booking.endTime) >= new Date(); - const isOngoing = isUpcoming && new Date() >= new Date(booking.startTime); - const isBookingInPast = new Date(booking.endTime) < new Date(); + const now = Date.now(); + const isUpcoming = new Date(booking.endTime).getTime() >= now; + const isOngoing = isUpcoming && now >= new Date(booking.startTime).getTime(); + const isBookingInPast = new Date(booking.endTime).getTime() < now;
📜 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.
📒 Files selected for processing (1)
apps/web/components/booking/BookingListItem.tsx(15 hunks)
🧰 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
🧠 Learnings (3)
📚 Learning: 2025-08-22T16:38:00.225Z
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.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
Applied to files:
apps/web/components/booking/BookingListItem.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
🧬 Code graph analysis (1)
apps/web/components/booking/BookingListItem.tsx (4)
apps/web/components/booking/hooks/useBookingItemState.ts (1)
useBookingItemState(16-77)apps/web/components/dialog/ReassignDialog.tsx (1)
ReassignDialog(63-328)apps/web/components/dialog/ChargeCardDialog.tsx (1)
ChargeCardDialog(19-79)packages/features/ee/video/MeetingSessionDetailsDialog.tsx (1)
MeetingSessionDetailsDialog(172-201)
⏰ 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 (3)
apps/web/components/booking/BookingListItem.tsx (3)
71-71: Centralized dialog state hook integration looks good.Clean replacement of scattered useState with a single source of truth.
Also applies to: 133-135
403-406: Boolean-aware dialog handlers: nice consistency.All these use
(open) => open ? openDialog(...) : closeDialog(...), matching child contracts and preventing accidental closures.Also applies to: 407-415, 419-422, 424-427, 430-435, 439-443, 445-454, 455-462, 463-466
400-436: Scan for zero-arg dialog closers across the repoSandbox ripgrep couldn't search/verify; run these locally and paste results:
rg -nP -C2 --hidden --glob '!node_modules/**' '\b(onOpenChange|setIsOpen[A-Za-z0-9_])\b\s=\s*{\s*(\s*)\s*=>\s*(closeDialog(|set[A-Za-z0-9_]+\s*(\sfalse\s))'
rg -nP -C2 --hidden --glob '!node_modules/**' '(\sopen(?:\s:\sboolean)?\s)\s*=>\s*(closeDialog(|set[A-Za-z0-9_]+\s*(\sfalse\s))'
Check apps/web/components/booking/BookingListItem.tsx lines 400-436 (also 438-466, 728-734, 1101-1169).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/components/booking/BookingListItem.tsx (2)
288-299: Remove leftover local location dialog state; close the centralized dialog on success.
setIsOpenLocationDialog(false)controls an unused local state, so the Edit Location dialog won’t close after success. UsecloseDialog("editLocation")and delete the legacyuseState.Apply:
- const [, setIsOpenLocationDialog] = useState(false); - // const [setIsOpenLocationDialog] = useState(false);- setIsOpenLocationDialog(false); + closeDialog("editLocation");
431-439: Fix ChargeCardDialog render condition; it won’t render when action is available.The action to “charge card” typically appears when payment is pending (i.e.,
!booking.paid). Currently, the dialog is only rendered whenbooking.paid, so clicking the action won’t show anything.Render the dialog when pending payment exists:
- {booking.paid && booking.payment[0] && ( + {showPendingPayment && booking.payment[0] && ( <ChargeCardDialog isOpenDialog={dialogState.chargeCard} setIsOpenDialog={(open) => (open ? openDialog("chargeCard") : closeDialog("chargeCard"))} bookingId={booking.id} paymentAmount={booking.payment[0].amount} paymentCurrency={booking.payment[0].currency} /> )}
🧹 Nitpick comments (1)
apps/web/components/booking/BookingListItem.tsx (1)
1-3: Remove file-wide eslint disables; fix root causes instead.These blanket disables hide real issues. The
no-explicit-anyis only needed due to brittle date string comparisons below; andprettiershould not be disabled repo-wide.Apply:
-/* eslint-disable @typescript-eslint/no-explicit-any */ - -/* eslint-disable prettier/prettier */
📜 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.
📒 Files selected for processing (1)
apps/web/components/booking/BookingListItem.tsx(15 hunks)
🧰 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
🧠 Learnings (3)
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.{ts,tsx} : Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js `.utc()` in hot paths like loops
Applied to files:
apps/web/components/booking/BookingListItem.tsx
📚 Learning: 2025-09-18T13:38:50.936Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.936Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
📚 Learning: 2025-09-08T09:34:46.661Z
Learnt from: CarinaWolli
PR: calcom/cal.com#23421
File: packages/features/ee/workflows/components/AddActionDialog.tsx:171-174
Timestamp: 2025-09-08T09:34:46.661Z
Learning: In packages/features/ee/workflows/components/AddActionDialog.tsx, the actionOptions prop is guaranteed to never be empty according to maintainer CarinaWolli, so defensive checks for empty arrays are not necessary.
Applied to files:
apps/web/components/booking/BookingListItem.tsx
🧬 Code graph analysis (1)
apps/web/components/booking/BookingListItem.tsx (4)
apps/web/components/booking/hooks/useBookingItemState.ts (1)
useBookingItemState(16-77)apps/web/components/dialog/ReassignDialog.tsx (1)
ReassignDialog(63-328)apps/web/components/dialog/ChargeCardDialog.tsx (1)
ChargeCardDialog(19-79)packages/features/ee/video/MeetingSessionDetailsDialog.tsx (1)
MeetingSessionDetailsDialog(172-201)
⏰ 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). (3)
- GitHub Check: Linters / lint
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
🔇 Additional comments (13)
apps/web/components/booking/BookingListItem.tsx (13)
71-71: Nice centralization of dialog state.Import and usage of
useBookingItemStatealigns with PR goals and removes scattereduseState.Also applies to: 133-135
159-163: Correct: close rejection dialog on success.
closeDialog("rejection")on REJECTED is the right cleanup.
262-272: LGTM: pending actions wired to centralized open/confirm.Consistent
openDialog("rejection")and confirm handler looks good.
357-372: LGTM: edit actions mapped to dialog openers.Uniform
openDialog("<id>")usage is clear and consistent.
375-401: LGTM: after-event actions mapped; no-show attendees flow handled.Opens dialogs via centralized API; single-attendee branch directly mutates as expected.
405-409: LGTM: RescheduleDialog setIsOpenDialog uses boolean signature.
410-417: LGTM: ReassignDialog wired with correct boolean onOpenChange signature.
422-424: LGTM: EditLocationDialog uses centralized open/close.
441-446: LGTM: ViewRecordingsDialog boolean handler is correct.
448-456: LGTM: MeetingSessionDetailsDialog gated and wired correctly.
458-464: LGTM: NoShowAttendeesDialog uses(open:boolean) => voidsignature.
466-469: LGTM: Rejection reason dialog uses proper boolean onOpenChange.
826-828: Replace brittle Date.toString() comparisons with epoch milliseconds.
toString()is locale/zone dependent and forcedanytyping. Use timestamps; this also lets you drop the file-levelno-explicit-anydisable.Apply:
- !booking.recurringInfo?.bookings[BookingStatus.CANCELLED] - .map((date: { toString: () => any }) => date.toString()) - .includes(recurringDate.toString()) + !booking.recurringInfo?.bookings[BookingStatus.CANCELLED] + .map((date: Date) => new Date(date).getTime()) + .includes(new Date(recurringDate).getTime())- booking.recurringInfo?.bookings[BookingStatus.CANCELLED] - .map((date: { toString: () => any }) => date.toString()) - .includes(aDate.toString()); + booking.recurringInfo?.bookings[BookingStatus.CANCELLED] + .map((date: Date) => new Date(date).getTime()) + .includes(new Date(aDate).getTime());Optional: precompute a Set of cancelled timestamps with useMemo.
Also applies to: 844-845
| {isBookingFromRoutingForm && ( | ||
| <RerouteDialog | ||
| isOpenDialog={rerouteDialogIsOpen} | ||
| setIsOpenDialog={setRerouteDialogIsOpen} | ||
| isOpenDialog={dialogState.reroute} | ||
| setIsOpenDialog={() => closeDialog("reroute")} | ||
| booking={{ ...parsedBooking, eventType: parsedBooking.eventType }} |
There was a problem hiding this comment.
Use boolean open argument for RerouteDialog’s setIsOpenDialog.
Passing a zero-arg closer breaks the expected signature and can close on open transitions.
Apply:
- <RerouteDialog
- isOpenDialog={dialogState.reroute}
- setIsOpenDialog={() => closeDialog("reroute")}
- booking={{ ...parsedBooking, eventType: parsedBooking.eventType }}
- />
+ <RerouteDialog
+ isOpenDialog={dialogState.reroute}
+ setIsOpenDialog={(open) => (open ? openDialog("reroute") : closeDialog("reroute"))}
+ booking={{ ...parsedBooking, eventType: parsedBooking.eventType }}
+ />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isBookingFromRoutingForm && ( | |
| <RerouteDialog | |
| isOpenDialog={rerouteDialogIsOpen} | |
| setIsOpenDialog={setRerouteDialogIsOpen} | |
| isOpenDialog={dialogState.reroute} | |
| setIsOpenDialog={() => closeDialog("reroute")} | |
| booking={{ ...parsedBooking, eventType: parsedBooking.eventType }} | |
| {isBookingFromRoutingForm && ( | |
| <RerouteDialog | |
| isOpenDialog={dialogState.reroute} | |
| setIsOpenDialog={(open) => (open ? openDialog("reroute") : closeDialog("reroute"))} | |
| booking={{ ...parsedBooking, eventType: parsedBooking.eventType }} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In apps/web/components/booking/BookingListItem.tsx around lines 731 to 735, the
RerouteDialog is currently given a zero-arg closer which breaks the expected
setIsOpenDialog(open: boolean) signature; update the prop to pass a one-arg
function that accepts the boolean open value and forwards it to your dialog
state handler (for example, call your close/open handler with the dialog key
"reroute" and the open boolean) so the dialog opens/closes correctly on state
transitions.
Signed-off-by: Hemant M Mehta <hemant29mehta@gmail.com>
…mantmm/cal.com into booking-list-state-management
|
closing due to upstream issue. |
1 participant
closes: #22214
What does this PR do?
✅ Extracted useDialogState hook
✅ Extracted useBookingFormState hook
✅ Replaced scattered useState calls
✅ Improved separation of concerns
✅ Enhanced testability
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist