Skip to content

fix: reschedule issue when limited booking is enabled in the event settings.#23717

Merged
anikdhabal merged 10 commits intocalcom:mainfrom
ShashwatPS:shashwatps-rescheduleFix
Oct 7, 2025
Merged

fix: reschedule issue when limited booking is enabled in the event settings.#23717
anikdhabal merged 10 commits intocalcom:mainfrom
ShashwatPS:shashwatps-rescheduleFix

Conversation

@ShashwatPS
Copy link
Contributor

@ShashwatPS ShashwatPS commented Sep 9, 2025

What does this PR do?

Visual Demo (For contributors especially)

https://www.loom.com/share/f6ccbf8762c44ff89fe0e4f147c39eff?sid=eb19374d-409b-4f74-86ab-6eec60d650fa

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Set up the project locally.
  • Set the booker’s active booking limit in the Advanced tab for the created event type.
  • Book an event.
  • Then try booking another event of the same type.

Checklist

  • I have read the contributing guide
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked that my changes generate no new warnings

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

Walkthrough

Adds a new setter setRescheduleUid to the Booker store (interface and initializer) and includes the same mock setter in the BookerStore test utilities. Updates the useBookings hook to call setRescheduleUid and setBookingData (per-field setters) instead of a single setState batched update in the BookerLimitExceededReschedule error path. No other state shape, exported signatures, or logic changes were introduced.

Possibly related PRs

Pre-merge checks (5 passed)

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the main change by highlighting the fix for the rescheduling issue under limited booking settings and directly corresponds to the code modifications made in the hook and store. It is concise and clearly informs reviewers of the primary purpose of the pull request.
Linked Issues Check ✅ Passed The code changes introduce a dedicated setter for reschedule identifiers in the store and update the hook to correctly set both the reschedule UID and booking data, thereby restoring the intended “Offer to reschedule last active booking” flow and fulfilling the linked issues' requirements.
Out of Scope Changes Check ✅ Passed All modifications are focused on implementing the rescheduling fix by extending the store API, adjusting the hook logic, and updating tests, with no unrelated or extraneous changes introduced in this pull request.
Description Check ✅ Passed The pull request description clearly outlines the issues being fixed, references the linked issue numbers, provides a visual demonstration and testing instructions that relate directly to the changeset, confirming its relevance to the modifications made.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed The code changes introduce a dedicated setter for reschedule identifiers in the store and update the hook to correctly set both the reschedule UID and booking data, thereby restoring the intended “Offer to reschedule last active booking” flow and fulfilling the linked issues' requirements.
Out of Scope Changes Check ✅ Passed All modifications are focused on implementing the rescheduling fix by extending the store API, adjusting the hook logic, and updating tests, with no unrelated or extraneous changes introduced in this pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title clearly and succinctly summarizes the primary change by indicating a fix for the reschedule issue in the context of limited booking settings, matching the pull request’s focus.
Description Check ✅ Passed The description clearly outlines the issues being fixed, references related tickets, provides a demo link, lists testing steps and mandatory tasks, and directly relates to the changes in the pull request.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Sep 9, 2025
@graphite-app graphite-app bot requested a review from a team September 9, 2025 15:01
@vercel
Copy link

vercel bot commented Sep 9, 2025

@ShashwatPS is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working labels Sep 9, 2025
@github-actions github-actions bot added the Low priority Created by Linear-GitHub Sync label Sep 9, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

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

⚠️ Outside diff range comments (1)
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)

349-359: Remove duplicate rescheduleUid store update
Drop the direct useBookerStore.setState({ rescheduleUid }) call—only invoke setRescheduleUid(error.data?.rescheduleUid) to avoid double updates and extra renders. To prevent transient inconsistent isRescheduling, consider adding a store action that sets both rescheduleUid and bookingData together atomically.

-        useBookerStore.setState({
-          rescheduleUid: error.data?.rescheduleUid,
-        });
-        setRescheduleUid(error.data?.rescheduleUid);
+        setRescheduleUid(error.data?.rescheduleUid);
🧹 Nitpick comments (4)
packages/features/bookings/Booker/__tests__/test-utils.tsx (1)

54-55: Make the mock setter mutate state to better reflect real behavior.

Right now setRescheduleUid is a no-op vi.fn(). If tests assert on store state after the error path, they may miss regressions. Mirror the pattern used for setMonth.

-    setRescheduleUid: vi.fn(),
+    setRescheduleUid: vi.fn((uid: string | null) => {
+      state.rescheduleUid = uid;
+    }),
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)

354-358: Avoid unsafe type coercion; pass a typed shape or relax the setter type.

as unknown as GetBookingType defeats type safety and may hide shape mismatches. Either accept Partial<GetBookingType> in the store setter or construct a properly typed minimal object.

Example minimal change (store API):

// store.ts
setBookingData: (bookingData: Partial<GetBookingType> | null | undefined) => void;

Then remove the unknown as cast here.

packages/features/bookings/Booker/store.ts (2)

140-144: Clarify the docstring to match the field name.

“ReferenceID” is vague. Use “Reschedule UID” to align with the state key and domain language.

-  /**
-   * Method used to set ReferenceID
-   */
+  /**
+   * Set the reschedule UID used during rescheduling flows.
+   */

300-401: Optional: provide an atomic updater for reschedule context.

To prevent intermediate UI states when both rescheduleUid and bookingData must change together, consider a helper like setRescheduleContext(uid, data) that updates both in a single set call.

Example:

// Add to BookerStore
setRescheduleContext: (uid: string, data: Partial<GetBookingType>) => void;

// In createBookerStore:
setRescheduleContext: (uid, data) => {
  set({ rescheduleUid: uid ?? null, bookingData: (data as GetBookingType) ?? null });
},

Then replace the two separate calls in the hook with one.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d908403 and 07b138d.

📒 Files selected for processing (3)
  • packages/features/bookings/Booker/__tests__/test-utils.tsx (1 hunks)
  • packages/features/bookings/Booker/components/hooks/useBookings.ts (2 hunks)
  • packages/features/bookings/Booker/store.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts

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

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

Files:

  • packages/features/bookings/Booker/components/hooks/useBookings.ts
  • packages/features/bookings/Booker/store.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:

  • packages/features/bookings/Booker/components/hooks/useBookings.ts
  • packages/features/bookings/Booker/store.ts
  • packages/features/bookings/Booker/__tests__/test-utils.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/hooks/useBookings.ts
  • packages/features/bookings/Booker/store.ts
  • packages/features/bookings/Booker/__tests__/test-utils.tsx
**/*.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/__tests__/test-utils.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-29T22:57:31.407Z
Learnt from: bandhan-majumder
PR: calcom/cal.com#23454
File: packages/features/bookings/Booker/components/EventMeta.tsx:16-16
Timestamp: 2025-08-29T22:57:31.407Z
Learning: In Cal.com's Booker architecture, components become client-side through BookerStoreProvider.tsx which has "use client". Any component using useBookerStoreContext automatically runs on the client and should use client-appropriate utilities like markdownToSafeHTMLClient, regardless of whether they have explicit "use client" directives.

Applied to files:

  • packages/features/bookings/Booker/components/hooks/useBookings.ts
📚 Learning: 2025-07-15T12:59:34.389Z
Learnt from: eunjae-lee
PR: calcom/cal.com#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.

Applied to files:

  • packages/features/bookings/Booker/components/hooks/useBookings.ts
🧬 Code graph analysis (1)
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)
packages/features/bookings/Booker/BookerStoreProvider.tsx (1)
  • useBookerStoreContext (24-35)
⏰ 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: Detect changes
🔇 Additional comments (2)
packages/features/bookings/Booker/components/hooks/useBookings.ts (1)

148-150: Good move: consume dedicated setters via context.

Selecting setBookingData and setRescheduleUid from the store improves cohesion and testability.

packages/features/bookings/Booker/store.ts (1)

413-415: Setter implementation looks correct.

Normalizing undefined to null avoids tri-state issues and keeps selectors simple.

kart1ka
kart1ka previously approved these changes Sep 9, 2025
Copy link
Contributor

@kart1ka kart1ka left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for the PR.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

E2E results are ready!

Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

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

Don't get the fix. I don’t see in the attached video that you rescheduled a booking

@ShashwatPS
Copy link
Contributor Author

ShashwatPS commented Sep 10, 2025

Don't get the fix. I don’t see in the attached video that you rescheduled a booking

@anikdhabal Actually, a booking was already there for the same event type. I just made another booking for the same event type. The warning that pops up ( attached a ss showing the same ) when I try to make the booking basically shows whether I’m crossing the per-person limit for that event type or not. Then, when I click confirm, it reschedules the last meeting.

Screenshot 2025-09-10 at 7 56 18 AM

@github-actions
Copy link
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Medium priority Created by Linear-GitHub Sync label Oct 7, 2025
@pull-request-size pull-request-size bot added size/M and removed size/S labels Oct 7, 2025
Copy link
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

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

Looks good

@anikdhabal anikdhabal enabled auto-merge (squash) October 7, 2025 17:34
@anikdhabal anikdhabal merged commit 3c0fc2e into calcom:main Oct 7, 2025
50 of 59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bookings area: bookings, availability, timezones, double booking 🐛 bug Something isn't working community Created by Linear-GitHub Sync Low priority Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync ready-for-e2e size/M Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rescheduling with booker active booking limit not working Issue when rescheduling

4 participants