Skip to content

fix: rescheduling last active booking#23601

Closed
hemantmm wants to merge 12 commits intocalcom:mainfrom
hemantmm:reschedule-last-booking
Closed

fix: rescheduling last active booking#23601
hemantmm wants to merge 12 commits intocalcom:mainfrom
hemantmm:reschedule-last-booking

Conversation

@hemantmm
Copy link
Contributor

@hemantmm hemantmm commented Sep 4, 2025

closes: #23588

What does this PR do?

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):

Before:

Screen.Recording.2025-09-04.at.23.55.01.mov

After:

Screen.Recording.2025-09-09.at.10.59.47.mov
Screen.Recording.2025-09-09.at.10.57.07.mov

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

@hemantmm hemantmm requested a review from a team as a code owner September 4, 2025 18:36
@vercel
Copy link

vercel bot commented Sep 4, 2025

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

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 4, 2025

Walkthrough

The PR reworks how booker active-booking limits are enforced. If maxActiveBookingsPerBooker is falsy the check returns early. Otherwise it fetches active bookings (statuses ACCEPTED or PENDING) for the given eventTypeId and bookerEmail, ordered by startTime desc, and compares the count to the limit. If the limit is exceeded and offerToRescheduleLastBooking is true, it selects the booking at index maxActiveBookingsPerBooker - 1, updates it to CANCELLED with rescheduled: true, and throws ErrorWithCode BookerLimitExceededReschedule including previousDate and rescheduleUid. If reschedule is not offered it logs a warning and throws BookerLimitExceeded. The prior checkActiveBookingsLimitAndOfferReschedule flow was removed and checkActiveBookingsLimit was simplified to a count-based check for future ACCEPTED bookings with the booker as attendee. No public API signatures in the bookings lib were changed.

Possibly related PRs

Pre-merge checks (5 passed)

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title concisely and accurately summarizes the primary change of fixing the rescheduling of the last active booking and is directly related to the core functionality addressed by the pull request without extraneous detail.
Linked Issues Check ✅ Passed The pull request implements the backend logic to cancel and reschedule the previous booking with the correct error code and payload and updates the frontend form to handle and display the reschedule flow, fully addressing the objectives described in issues #23588 and CAL-6370.
Out of Scope Changes Check ✅ Passed All modifications in this pull request pertain to the booking limit and rescheduling functionality, and there are no unrelated changes introduced beyond minor cleanups that remain within the scope of fixing the rescheduling issue.
Description Check ✅ Passed The description clearly references the linked issues, explains that the pull request fixes the rescheduling flow, includes context, demonstration links, mandatory checklists, and testing guidance, making it relevant and on-topic.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
		  - name: "Undocumented Breaking Changes"
			  mode: "warning"
			  instructions: |
				  Flag potential breaking changes that are not documented:
				  1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
				  2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
				  3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • 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 4, 2025
@graphite-app graphite-app bot requested a review from a team September 4, 2025 18:36
@github-actions github-actions bot added bookings area: bookings, availability, timezones, double booking Low priority Created by Linear-GitHub Sync 🐛 bug Something isn't working labels Sep 4, 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: 2

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

📥 Commits

Reviewing files that changed from the base of the PR and between e02108d and 4a01421.

📒 Files selected for processing (1)
  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.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/lib/handleNewBooking/checkActiveBookingsLimitForBooker.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:

  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
🧠 Learnings (1)
📚 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:

  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
⏰ 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

Comment on lines 24 to 48
if (offerToRescheduleLastBooking) {
await checkActiveBookingsLimitAndOfferReschedule({
const lastActiveBooking = await prisma.booking.findFirst({
where: {
eventTypeId,
status: { in: [BookingStatus.ACCEPTED, BookingStatus.PENDING] },
attendees: {
some: {
email: bookerEmail,
},
},
},
orderBy: { startTime: "desc" },
});

if (lastActiveBooking) {
await prisma.booking.update({
where: { id: lastActiveBooking.id },
data: {
status: BookingStatus.CANCELLED,
rescheduled: true,
},
});
}

return await checkActiveBookingsLimitAndOfferReschedule({
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Don’t preemptively cancel bookings; gate by limit, align filters to “active”, and avoid canceling PENDING/past meetings.

As written, any time offerToRescheduleLastBooking is true you cancel the most recent booking—even if the booker is under the limit. You also:

  • include PENDING in the cancel set, while the limit checks only consider ACCEPTED → inconsistent and potentially destructive
  • don’t restrict by startTime >= now, so you may cancel a past booking
  • cancel before the new booking is created, risking data loss if the subsequent flow fails
  • fetch full rows instead of a minimal select

Refactor to:

  • only cancel when at/over maxActiveBookingsPerBooker
  • cancel only future ACCEPTED bookings
  • minimize data fetched
  • drop redundant return await

Apply this diff:

-    const lastActiveBooking = await prisma.booking.findFirst({
-      where: {
-        eventTypeId,
-        status: { in: [BookingStatus.ACCEPTED, BookingStatus.PENDING] },
-        attendees: {
-          some: {
-            email: bookerEmail,
-          },
-        },
-      },
-      orderBy: { startTime: "desc" },
-    });
-
-    if (lastActiveBooking) {
-      await prisma.booking.update({
-        where: { id: lastActiveBooking.id },
-        data: {
-          status: BookingStatus.CANCELLED,
-          rescheduled: true,
-        },
-      });
-    }
-
-    return await checkActiveBookingsLimitAndOfferReschedule({
+    // Cancel only when at/over limit; restrict to future ACCEPTED bookings.
+    const activeAcceptedCount = await prisma.booking.count({
+      where: {
+        eventTypeId,
+        startTime: { gte: new Date() },
+        status: BookingStatus.ACCEPTED,
+        attendees: { some: { email: bookerEmail } },
+      },
+    });
+
+    if (activeAcceptedCount >= maxActiveBookingsPerBooker) {
+      const lastActiveBooking = await prisma.booking.findFirst({
+        where: {
+          eventTypeId,
+          startTime: { gte: new Date() },
+          status: BookingStatus.ACCEPTED,
+          attendees: { some: { email: bookerEmail } },
+        },
+        orderBy: { startTime: "desc" },
+        select: { id: true },
+      });
+
+      if (lastActiveBooking) {
+        await prisma.booking.updateMany({
+          where: { id: lastActiveBooking.id, status: BookingStatus.ACCEPTED },
+          data: { status: BookingStatus.CANCELLED, rescheduled: true },
+        });
+      }
+    }
+
+    return checkActiveBookingsLimitAndOfferReschedule({
       eventTypeId,
       maxActiveBookingsPerBooker,
       bookerEmail,
     });
📝 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.

Suggested change
if (offerToRescheduleLastBooking) {
await checkActiveBookingsLimitAndOfferReschedule({
const lastActiveBooking = await prisma.booking.findFirst({
where: {
eventTypeId,
status: { in: [BookingStatus.ACCEPTED, BookingStatus.PENDING] },
attendees: {
some: {
email: bookerEmail,
},
},
},
orderBy: { startTime: "desc" },
});
if (lastActiveBooking) {
await prisma.booking.update({
where: { id: lastActiveBooking.id },
data: {
status: BookingStatus.CANCELLED,
rescheduled: true,
},
});
}
return await checkActiveBookingsLimitAndOfferReschedule({
if (offerToRescheduleLastBooking) {
// Cancel only when at/over limit; restrict to future ACCEPTED bookings.
const activeAcceptedCount = await prisma.booking.count({
where: {
eventTypeId,
startTime: { gte: new Date() },
status: BookingStatus.ACCEPTED,
attendees: { some: { email: bookerEmail } },
},
});
if (activeAcceptedCount >= maxActiveBookingsPerBooker) {
const lastActiveBooking = await prisma.booking.findFirst({
where: {
eventTypeId,
startTime: { gte: new Date() },
status: BookingStatus.ACCEPTED,
attendees: { some: { email: bookerEmail } },
},
orderBy: { startTime: "desc" },
select: { id: true },
});
if (lastActiveBooking) {
await prisma.booking.updateMany({
where: { id: lastActiveBooking.id, status: BookingStatus.ACCEPTED },
data: { status: BookingStatus.CANCELLED, rescheduled: true },
});
}
}
return checkActiveBookingsLimitAndOfferReschedule({
eventTypeId,
maxActiveBookingsPerBooker,
bookerEmail,
});
}
🤖 Prompt for AI Agents
In
packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
around lines 24-48, remove the preemptive cancellation and instead: first call
the existing checkActiveBookingsLimitAndOfferReschedule logic to determine if
the booker is at/over the limit; only when that check indicates we must
reschedule, query for the last active booking using filters status = ACCEPTED
and startTime >= now, selecting only the id; then perform the update to mark it
CANCELLED/rescheduled; ensure we do not include PENDING or past meetings in the
cancel query and avoid using redundant "return await".

Comment on lines 38 to 46
if (lastActiveBooking) {
await prisma.booking.update({
where: { id: lastActiveBooking.id },
data: {
status: BookingStatus.CANCELLED,
rescheduled: true,
},
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Avoid bypassing cancellation side‑effects; use the booking cancellation/reschedule service with hooks.

Direct prisma.booking.update skips notifications, calendar sync, webhooks, refunds, etc. Route cancellation through the existing domain service (e.g., a “cancelBooking/reschedule” service) or emit the same hooks that normal cancellations do. Ideally, perform “cancel old + create new” in one orchestrated path/transaction.

Run this to locate the canonical cancellation/reschedule utilities to call instead of raw updates:


🏁 Script executed:

#!/bin/bash
# Find booking cancellation/reschedule helpers/services
rg -nP -C3 '(cancelBooking|CancelBooking|bookingCancel|reschedul(e|ed|ing).*(service|handler)|Booking.*cancel)' --type=ts --type=tsx

Length of output: 162


🏁 Script executed:

#!/bin/bash
# Re-run search for booking cancellation/reschedule utilities in TS/TSX files using globs
rg -nP -C3 '(cancelBooking|CancelBooking|bookingCancel|reschedul(e|ed|ing).*(service|handler)|Booking.*cancel)' -g '*.ts' -g '*.tsx'

Length of output: 142389


Use the domain-level cancellation service, not prisma.booking.update.
packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts:38–46 – direct updates bypass all of your cancellation side-effects (webhooks, notifications, calendar sync, refunds, etc.). Import and invoke the canonical handler (e.g. handleCancelBooking from @calcom/features/bookings/lib/handleCancelBooking) so “cancel old + create new” runs in one orchestrated path.

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.

This is not the solution we are looking for. You are just rescheduling the existing booking without informing the user about it. Also what happens if there are more than one existing bookings?

@kart1ka kart1ka marked this pull request as draft September 5, 2025 03:34
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.

Agree with @kart1ka here, also the issue is related to reschedule not normal booking

@hemantmm hemantmm requested a review from kart1ka September 9, 2025 05:42
@hemantmm hemantmm marked this pull request as ready for review September 9, 2025 05:42
@hemantmm
Copy link
Contributor Author

hemantmm commented Sep 9, 2025

This is not the solution we are looking for. You are just rescheduling the existing booking without informing the user about it. Also what happens if there are more than one existing bookings?

@kart1ka, I have updated the fix. You can have a look at the video above.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)
packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx (1)

160-173: Localize the button label inside ServerTrans.

Direct English text violates our TSX i18n guideline. Use t() for the label so it can be translated.

-                    <button
+                    <button
                       key="please-select-a-new-time-button"
                       type="button"
                       className="underline"
                       onClick={onCancel}>
-                      Please select a new time
+                      {t("please_select_a_new_time")}
                     </button>,
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts (1)

39-42: Do not bypass domain cancellation side-effects; guard against races.

Direct prisma.booking.update skips notifications, calendar sync, webhooks, refunds, etc., and is racy between read and write. Route through the canonical cancellation/reschedule service (e.g., handleCancelBooking/booking cancellation domain handler) or at minimum add status/time guards and use updateMany.

-    await prisma.booking.update({
-      where: { id: bookingToReschedule.id },
-      data: { status: BookingStatus.CANCELLED, rescheduled: true },
-    });
+    // Prefer: call the domain-level cancellation/reschedule service here.
+    // Fallback hardening:
+    await prisma.booking.updateMany({
+      where: {
+        id: bookingToReschedule.id,
+        status: BookingStatus.ACCEPTED,
+        startTime: { gte: now },
+      },
+      data: { status: BookingStatus.CANCELLED, rescheduled: true },
+    });
🧹 Nitpick comments (5)
packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts (3)

1-1: Remove unnecessary ESLint disable.

No unused vars in this file; keep the rule active.

-/* eslint-disable @typescript-eslint/no-unused-vars */

44-51: Avoid coupling UI to literal message strings; rely on code + data.

The UI already keys off ErrorCode.BookerLimitExceededReschedule. Keep message generic and let translations derive text; keep previousDate/rescheduleUid in data.

-    throw new ErrorWithCode(
-      ErrorCode.BookerLimitExceededReschedule,
-      "Previous booking has been rescheduled",
-      {
-        previousDate: bookingToReschedule.startTime.toISOString(),
-        rescheduleUid: bookingToReschedule.uid,
-      }
-    );
+    throw new ErrorWithCode(ErrorCode.BookerLimitExceededReschedule, "booker_limit_exceeded_reschedule", {
+      previousDate: bookingToReschedule.startTime.toISOString(),
+      rescheduleUid: bookingToReschedule.uid,
+    });

68-79: Unify “active” filters and simplify status condition.

Use the same where-clause as above (future + ACCEPTED) for consistency; replace in: [ACCEPTED] with equality; also consider extracting the filter to a shared helper to avoid drift.

-      startTime: { gte: new Date() },
-      status: { in: [BookingStatus.ACCEPTED] },
+      startTime: { gte: new Date() },
+      status: BookingStatus.ACCEPTED,
       attendees: { some: { email: bookerEmail } },
packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx (2)

314-329: Key off error.code only; don’t match literal messages.

String comparisons are brittle and break i18n. The server already sends ErrorCode.BookerLimitExceededReschedule with data.previousDate; rely on that.

-  if (
-    error?.code === ErrorCode.BookerLimitExceededReschedule ||
-    error?.message === "Previous booking has been rescheduled"
-  ) {
+  if (error?.code === ErrorCode.BookerLimitExceededReschedule) {

331-335: Avoid t(error.message) for arbitrary server strings; map known codes to keys.

If message isn’t an i18n key, t() won’t translate it. Prefer a small map from server ErrorCode → translation key, with a safe fallback.

-      {responseVercelIdHeader ?? ""} {t(error.message)}
+      {responseVercelIdHeader ?? ""} {t(error.code ?? "can_you_try_again")}

Or introduce a helper:

const errorKeyByCode: Record<string, string> = {
  [ErrorCode.BookerLimitExceeded]: "maximum_booking_limit_reached",
  [ErrorCode.BookerLimitExceededReschedule]: "your_previous_booking_has_been_rescheduled",
};
📜 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 4a01421 and fae9627.

📒 Files selected for processing (2)
  • packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx (7 hunks)
  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts (3 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/features/bookings/Booker/components/BookEventForm/BookEventForm.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/BookEventForm.tsx
  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.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:

  • packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx
  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
**/*.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/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
🧠 Learnings (4)
📚 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/BookEventForm.tsx
📚 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/BookEventForm/BookEventForm.tsx
📚 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:

  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
📚 Learning: 2025-08-27T12:15:43.830Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/trpc/server/routers/viewer/aiVoiceAgent/testCall.handler.ts:41-44
Timestamp: 2025-08-27T12:15:43.830Z
Learning: In calcom/cal.com, the AgentService.getAgent() method in packages/features/calAIPhone/providers/retellAI/services/AgentService.ts does NOT include authorization checks - it only validates the agentId parameter and directly calls the repository without verifying user/team access. This contrasts with other methods like getAgentWithDetails() which properly use findByIdWithUserAccessAndDetails() for authorization. When reviewing updateToolsFromAgentId() calls, always verify both agent ownership and eventType ownership are checked.

Applied to files:

  • packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
⏰ 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). (4)
  • GitHub Check: Linters / lint
  • GitHub Check: Tests / Unit
  • GitHub Check: Type check / check-types
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts (3)

21-22: Early return looks good.

Avoids unnecessary DB calls when limit is disabled.


32-38: Confirm selection of booking index to reschedule.

You select activeBookings[maxActiveBookingsPerBooker - 1] in a list ordered by startTime desc. This targets the Nth-most-recent future booking, not necessarily the “last created” or “oldest future” booking. Please confirm product spec; otherwise we may need earliest future (asc + first) or most recent (desc + first).


58-80: Remove or export checkActiveBookingsLimit
No call sites found across .ts/.tsx files—delete this dead helper or, if it’s meant to be used elsewhere, export it and add corresponding tests.

packages/features/bookings/Booker/components/BookEventForm/BookEventForm.tsx (1)

243-257: UX change: confirm CTA hidden when multiple active bookings.

Hiding the confirm button blocks progression from this screen. Confirm this matches expected flow (e.g., user must first resolve bookings list), or provide a dedicated “Go to last booking”/“Manage bookings” action.

Comment on lines +23 to +31
const activeBookings = await prisma.booking.findMany({
where: {
eventTypeId,
status: { in: [BookingStatus.ACCEPTED, BookingStatus.PENDING] },
attendees: { some: { email: bookerEmail } },
},
orderBy: { startTime: "desc" },
});

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Scope the Prisma query to “active” future ACCEPTED bookings and fetch only needed fields.

Current query:

  • includes PENDING and past bookings (inconsistent with checkActiveBookingsLimit below)
  • fetches entire rows
  • scans all matches

Refactor to align filters, reduce IO, and cap results at the limit.

-  const activeBookings = await prisma.booking.findMany({
-    where: {
-      eventTypeId,
-      status: { in: [BookingStatus.ACCEPTED, BookingStatus.PENDING] },
-      attendees: { some: { email: bookerEmail } },
-    },
-    orderBy: { startTime: "desc" },
-  });
+  const now = new Date();
+  const activeBookings = await prisma.booking.findMany({
+    where: {
+      eventTypeId,
+      startTime: { gte: now },
+      status: BookingStatus.ACCEPTED,
+      attendees: { some: { email: bookerEmail } },
+    },
+    orderBy: { startTime: "desc" },
+    select: { id: true, startTime: true, uid: true },
+    take: maxActiveBookingsPerBooker, // we only need up to the Nth most recent
+  });
📝 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.

Suggested change
const activeBookings = await prisma.booking.findMany({
where: {
eventTypeId,
status: { in: [BookingStatus.ACCEPTED, BookingStatus.PENDING] },
attendees: { some: { email: bookerEmail } },
},
orderBy: { startTime: "desc" },
});
// Only consider future ACCEPTED bookings and fetch minimal fields up to the limit
const now = new Date();
const activeBookings = await prisma.booking.findMany({
where: {
eventTypeId,
startTime: { gte: now },
status: BookingStatus.ACCEPTED,
attendees: { some: { email: bookerEmail } },
},
orderBy: { startTime: "desc" },
select: { id: true, startTime: true, uid: true },
take: maxActiveBookingsPerBooker, // only need the N most recent
});
🤖 Prompt for AI Agents
In
packages/features/bookings/lib/handleNewBooking/checkActiveBookingsLimitForBooker.ts
around lines 23 to 31, the Prisma query currently returns PENDING and past
bookings and selects full rows which contradicts the active-bookings limit
logic; change the where clause to only ACCEPTED bookings with startTime in the
future (e.g., startTime: { gt: new Date() }), restrict the select to only the
minimal fields you need (like id and startTime or just id), add take: limit to
cap results at the configured limit, and keep orderBy as needed (e.g., startTime
desc) so the query is efficient and aligned with the limit check.

@kart1ka
Copy link
Contributor

kart1ka commented Sep 9, 2025

Thank you for the PR. But closing in favour of #23717.

@kart1ka kart1ka closed this Sep 9, 2025
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 size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue when rescheduling

4 participants