fix: booking availability based on limit#23400
fix: booking availability based on limit#23400SinghaAnirban005 wants to merge 15 commits intocalcom:mainfrom
Conversation
|
@SinghaAnirban005 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughUser week start (defaulting to "Monday") is read from user data and threaded through availability and interval-limit logic. weekStart is passed into getBusyTimesFromLimits, LimitManager, and getPeriodStartDatesBetween; LimitManager aligns weekly period starts/ends and busy-time keying. getPeriodStartDatesBetween and related helpers now accept an optional weekStart parameter. getUserAvailability API surface added bookingRepo and redisClient, and busy-times selection logic treats PER_WEEK limits specially when deciding whether to reuse initial busy-times. Possibly related PRs
Pre-merge checks (3 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
bookings
SinghaAnirban005
force-pushed
the
bug/t
branch
2 times, most recently
from
92b199d to
8ac3b23
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/lib/intervalLimits/limitManager.ts (1)
23-26: Weekly keying uses Day.js default week boundaries, causing missed/duplicate busy spans.
isAlreadyBusybuilds keys viastartOf("week")andendOf("week")(Day.js default), whileaddBusyTimestores weekly keys using a custom-aligned start (user weekStart). Keys won’t match when user weekStart ≠ Day.js default, breaking deduplication.Unify key generation to the user’s
weekStartby makingcreateKeyinstance-based and reusing your alignment helpers:- private static createKey(start: Dayjs, unit: IntervalLimitUnit, timeZone?: string): BusyMapKey { - const tzStart = timeZone ? start.tz(timeZone) : start; - return `${unit}-${tzStart.startOf(unit).toISOString()}`; - } + private createKey(start: Dayjs, unit: IntervalLimitUnit, timeZone?: string): BusyMapKey { + let tzStart = timeZone ? start.tz(timeZone) : start; + if (unit === "week") { + tzStart = this.adjustWeekStart(tzStart, timeZone); + } else { + tzStart = tzStart.startOf(unit); + } + return `${unit}-${tzStart.toISOString()}`; + }- if (this.busyMap.has(LimitManager.createKey(start, "year", timeZone))) return true; + if (this.busyMap.has(this.createKey(start, "year", timeZone))) return true; @@ - ((this.busyMap.has(LimitManager.createKey(start, "month", timeZone)) && - this.busyMap.has(LimitManager.createKey(start.endOf("week"), "month", timeZone))) || - this.busyMap.has(LimitManager.createKey(start, "week", timeZone))) + ((this.busyMap.has(this.createKey(start, "month", timeZone)) && + this.busyMap.has( + this.createKey(this.getPeriodEnd(this.adjustWeekStart(start, timeZone), "week", timeZone), "month", timeZone) + )) || + this.busyMap.has(this.createKey(start, "week", timeZone)))- this.busyMap.set(`${unit}-${adjustedStart.toISOString()}`, { + this.busyMap.set(this.createKey(adjustedStart, unit, timeZone), { start: adjustedStart.toISOString(), end: periodEnd.toISOString(), });Also applies to: 56-66, 70-74, 97-100
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
146-147: Weekly periodEnd should honor user weekStart (not Day.js endOf('week')).For non-default weekStart,
endOf("week")cuts the period short. Compute end via +6 days from aligned start.- const periodEnd = periodStart.endOf(unit); + const periodEnd = + unit === "week" ? periodStart.add(6, "day").endOf("day") : periodStart.endOf(unit);
268-281: Team limits ignore weekStart. Propagate it for consistency.
_getBusyTimesFromTeamLimitsbuilds weekly ranges with defaults (and constructsLimitManager()without weekStart). PassweekStartthrough and use it.Apply these changes (and update callers):
-const _getBusyTimesFromTeamLimits = async ( +const _getBusyTimesFromTeamLimits = async ( user: { id: number; email: string }, bookingLimits: IntervalLimit, dateFrom: Dayjs, dateTo: Dayjs, teamId: number, includeManagedEvents: boolean, timeZone: string, - rescheduleUid?: string + rescheduleUid?: string, + weekStart?: string ) => { @@ - const limitManager = new LimitManager(); + const limitManager = new LimitManager(weekStart); @@ await getBusyTimesFromBookingLimits({ bookings: busyTimes, bookingLimits, dateFrom, dateTo, limitManager, rescheduleUid, teamId, user, includeManagedEvents, timeZone, + weekStart, });And in
getUserAvailability.ts(team path), passuser.weekStart:busyTimesFromTeamLimits = await getBusyTimesFromTeamLimits( user, teamBookingLimits, dateFrom.tz(finalTimezone), dateTo.tz(finalTimezone), teamForBookingLimits.id, teamForBookingLimits.includeManagedEventsInLimits, finalTimezone, initialData?.rescheduleUid ?? undefined, user.weekStart );
🧹 Nitpick comments (2)
packages/lib/intervalLimits/limitManager.ts (1)
12-12: Prefer named exports over default.Converting
export default class LimitManagerto a named export improves tree-shaking and refactors.Apply this diff:
-export default class LimitManager { +export class LimitManager {And update imports accordingly:
- import LimitManager from "../limitManager"; + import { LimitManager } from "../limitManager";packages/lib/getUserAvailability.ts (1)
410-414: Tighten weekly-limit detection.Key existence can be true even if value is
undefined. Use a value check.- function hasWeeklyLimits(bookingLimits: BookingLimits) { - if (!bookingLimits) return false; - return "PER_WEEK" in bookingLimits; - } + function hasWeeklyLimits(bookingLimits: BookingLimits) { + return bookingLimits?.PER_WEEK != null; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/lib/getUserAvailability.ts(6 hunks)packages/lib/intervalLimits/limitManager.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(6 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/limitManager.tspackages/prisma/selects/user.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/lib/intervalLimits/limitManager.tspackages/prisma/selects/user.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/lib/intervalLimits/limitManager.tspackages/prisma/selects/user.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧠 Learnings (1)
📚 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:
packages/lib/intervalLimits/limitManager.ts
🧬 Code graph analysis (2)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
packages/lib/sentryWrapper.ts (1)
withReporting(107-139)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
packages/lib/intervalLimits/limitManager.ts (1)
LimitManager(12-120)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
getPeriodStartDatesBetween(49-52)
🔇 Additional comments (8)
packages/prisma/selects/user.ts (1)
9-9: Expose weekStart for availability: LGTMSelecting
weekStarthere is correct and aligns with the downstream usage. No leakage of sensitive fields.packages/lib/intervalLimits/limitManager.ts (1)
28-51: Week alignment helpers: LGTM
adjustWeekStartandgetPeriodEndcorrectly align to user weekStart and produce 7-day windows.packages/lib/getUserAvailability.ts (4)
159-163: Type for booking limits: LGTMThe
BookingLimitshelper type is fine for lightweight checks.
325-327: Ensure consistent default weekStart when user value is absent.You set
this.weekStart = user?.weekStart(possibly undefined). LimitManager currently defaults to Sunday, while the period util defaults to Monday. Confirm unified default or always pass an explicit default.If unifying to Monday, also pass
this.weekStart || "Monday"downstream where needed.
423-435: Threading weekStart into limit checks: LGTMPassing
user.weekStartintogetBusyTimesFromLimitsis correct.
622-626: Forwarding weekStart to period-start utility: LGTMThis keeps weekly windows aligned with user preference.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
33-33: Constructing LimitManager with weekStart: LGTMInstantiating with
weekStartkeeps internal keys aligned if the class uses it consistently.
111-117: Weekly period starts with weekStart: LGTMUsing
getPeriodStartDatesBetween(..., weekStart)here is correct.
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
Outdated
Show resolved
Hide resolved
SinghaAnirban005
force-pushed
the
bug/t
branch
from
d531124 to
c189ab8
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/lib/getUserAvailability.ts (2)
410-414: Prefer hasOwn over "in" for detecting PER_WEEK presenceAvoid prototype-chain surprises; presence should be about own keys, even if value is
undefined.- function hasWeeklyLimits(bookingLimits: BookingLimits) { - if (!bookingLimits) return false; - return "PER_WEEK" in bookingLimits; - } + function hasWeeklyLimits(bookingLimits: BookingLimits) { + if (!bookingLimits) return false; + return Object.hasOwn(bookingLimits, "PER_WEEK"); + }
415-419: Guard cache reuse by matching eventTypeEnsure
initialData.busyTimesFromLimitscorresponds to the sameeventTypeto avoid cross-ET contamination.- if ( - initialData?.busyTimesFromLimits && - initialData?.eventTypeForLimits && - !hasWeeklyLimits(bookingLimits) - ) { + if ( + initialData?.busyTimesFromLimits && + initialData?.eventTypeForLimits && + (!eventType || initialData.eventTypeForLimits.id === eventType.id) && + !hasWeeklyLimits(bookingLimits) + ) {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/lib/getUserAvailability.ts(6 hunks)packages/lib/intervalLimits/limitManager.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(6 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/prisma/selects/user.ts
- packages/lib/intervalLimits/limitManager.ts
- packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
- packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/getUserAvailability.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/lib/getUserAvailability.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/lib/getUserAvailability.ts
🔇 Additional comments (1)
packages/lib/getUserAvailability.ts (1)
423-434: The scripts have been submitted for execution.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
36-44: End-bound misaligned for weeks; uses Day.js locale week, not user weekStart.
This can drop or over-add a weekly period and ignores timeZone for dateTo. Align the loop end with the same weekStart/timeZone logic as the start and normalize weekly increments to startOf("day") to avoid DST drift.- while (currentDate.isBefore(dateTo) || currentDate.isSame(dateTo, period)) { - dates.push(currentDate); - - if (period === "week") { - currentDate = currentDate.add(1, "week"); - } else { - currentDate = currentDate.add(1, period).startOf(period); - } - } + // Normalize end bound with same semantics as the start + const tzDateTo = timeZone ? dayjs(dateTo).tz(timeZone) : dayjs(dateTo); + const lastStart = + period === "week" + ? (() => { + const weekStartName = weekStart || "Sunday"; + const weekIdx = weekStartNum(weekStartName); + const currentDayIdx = tzDateTo.day(); + const daysToSubtract = (currentDayIdx - weekIdx + 7) % 7; + return tzDateTo.subtract(daysToSubtract, "day").startOf("day"); + })() + : tzDateTo.startOf(period); + + while (currentDate.isBefore(lastStart) || currentDate.isSame(lastStart)) { + dates.push(currentDate); + + if (period === "week") { + currentDate = currentDate.add(1, "week").startOf("day"); + } else { + currentDate = currentDate.add(1, period).startOf(period); + } + }
🧹 Nitpick comments (3)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (3)
12-14: Strengthen the weekStart type.
Use a literal union (or a shared exported type) for weekStart to prevent invalid values.-import { weekStartNum } from "@calcom/lib/weekstart"; +import { weekStartNum, type WeekStartName } from "@calcom/lib/weekstart"- timeZone?: string, - weekStart?: string + timeZone?: string, + weekStart?: WeekStartName
15-17: Type the accumulator.
Make the return type explicit on the local array for clarity and stricter TS checking.- const dates = []; + const dates: Dayjs[] = [];
18-22: Optional: prefer UTC when no timeZone to avoid host TZ/DST surprises.
When timeZone is omitted, using .utc() avoids environment-dependent behavior in loops.- if (timeZone) { - currentDate = dayjs(dateFrom).tz(timeZone); - } else { - currentDate = dayjs(dateFrom); - } + if (timeZone) { + currentDate = dayjs(dateFrom).tz(timeZone); + } else { + currentDate = dayjs(dateFrom).utc(); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.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/lib/intervalLimits/utils/getPeriodStartDatesBetween.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/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧠 Learnings (1)
📚 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:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧬 Code graph analysis (1)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
packages/lib/sentryWrapper.ts (1)
withReporting(107-139)
🔇 Additional comments (2)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (2)
4-4: LGTM: importing weekStartNum is appropriate.
No issues with this import; it matches usage below.
49-52: LGTM: observability wrapper.
Good use of withReporting with a stable span name.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/lib/getUserAvailability.ts (1)
418-429: Propagate weekStart to team limits as well; team PER_WEEK constraints may be misaligned.
getBusyTimesFromTeamLimits currently lacks weekStart; weekly team limits will compute with default semantics.Proposed change in call site (and update function signature accordingly in its module):
- busyTimesFromTeamLimits = await getBusyTimesFromTeamLimits( + busyTimesFromTeamLimits = await getBusyTimesFromTeamLimits( user, teamBookingLimits, dateFrom.tz(finalTimezone), dateTo.tz(finalTimezone), teamForBookingLimits.id, teamForBookingLimits.includeManagedEventsInLimits, - finalTimezone, - initialData?.rescheduleUid ?? undefined + finalTimezone, + initialData?.rescheduleUid ?? undefined, + weekStart );
♻️ Duplicate comments (1)
packages/lib/intervalLimits/limitManager.ts (1)
16-18: Default weekStart should be "Sunday" to match product default; "Monday" causes misaligned weekly limits when unset.
Align with cal.com’s default to avoid off-by-one availability. Also update utils to the same default.- constructor(weekStart?: string) { - this.weekStart = weekStart || "Monday"; - } + constructor(weekStart?: string) { + this.weekStart = weekStart || "Sunday"; + }
🧹 Nitpick comments (3)
packages/lib/intervalLimits/limitManager.ts (1)
12-13: Prefer named export over default for libraries.
Improves tree-shaking and refactors.-export default class LimitManager { +export class LimitManager {(Remember to update imports.)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (2)
15-17: Type the accumulator to avoid any[].- const dates = []; + const dates: Dayjs[] = [];
24-47: Minor DRY/perf: compute weekStartIndex once.- const weekStartName = weekStart || "Sunday"; - const weekStartIndex = weekStartNum(weekStartName); + const weekStartName = weekStart || "Sunday"; + const weekStartIndex = weekStartNum(weekStartName); ... - const lastStart = - period === "week" - ? (() => { - const weekStartName = weekStart || "Sunday"; - const weekStartIndex = weekStartNum(weekStartName); + const lastStart = + period === "week" + ? (() => { const currentDayIndex = tzDateTo.day(); const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; return tzDateTo.subtract(daysToSubtract, "day").startOf("day"); })()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/lib/getUserAvailability.ts(5 hunks)packages/lib/intervalLimits/limitManager.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(8 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/getUserAvailability.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/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/getUserAvailability.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/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/getUserAvailability.ts
🧠 Learnings (2)
📚 Learning: 2025-08-31T19:20:20.795Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/limitManager.ts:16-18
Timestamp: 2025-08-31T19:20:20.795Z
Learning: cal.com defaults to Sunday as the week start setting, so components should align with this default for consistency.
Applied to files:
packages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
📚 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:
packages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧬 Code graph analysis (1)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
packages/lib/sentryWrapper.ts (1)
withReporting(107-139)
⏰ 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 (12)
packages/lib/intervalLimits/limitManager.ts (3)
2-2: Import is appropriate for week-start alignment.
28-39: End-of-week computation is fine, but verify DST and month-boundary cases.
Because week-end is computed via add(6 days).endOf("day"), add unit tests for weeks crossing DST and month edges.Would you like me to add tests covering DST-crossing weeks (America/Los_Angeles) and month boundaries?
41-52: Week-start adjustment looks correct.
Adjusts by user’s weekStart consistently.packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (4)
4-4: Import is correct to support week-start alignment.
12-14: Signature extension looks good.
Adding timeZone and weekStart is appropriate.
48-56: Loop logic is correct and inclusive of lastStart.
61-64: Wrapper export change is fine.packages/lib/getUserAvailability.ts (5)
159-160: BookingLimits typing fix LGTM.
Accurately represents optional map or null.
405-409: Weekly-limits check is fine.
410-418: Reuse guard is correct; only reuse when no PER_WEEK limit.
617-621: Wrapper now forwards weekStart; good.
Consider using this consistently in internal callers to avoid accidental Monday defaults.
161-166: DI wiring covers all IUserAvailabilityService dependencies. Confirmed thatgetUserAvailabilityModulebindsoooRepo,bookingRepo,eventTypeRepo, andredisClient, and no manualnew UserAvailabilityServicecalls exist.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/lib/getUserAvailability.ts (2)
405-409: Tighten weekly-limit detection to avoid false positivesUse a numeric check so a present-but-zero/undefined PER_WEEK value doesn’t force the slow path unnecessarily.
- function hasWeeklyLimits(bookingLimits: BookingLimits) { - if (!bookingLimits) return false; - return "PER_WEEK" in bookingLimits; - } + function hasWeeklyLimits(bookingLimits: BookingLimits) { + if (!bookingLimits) return false; + const v = (bookingLimits as Record<string, unknown>)["PER_WEEK"]; + return typeof v === "number" && v > 0; + }
410-415: Avoid reusing initialData when weekly duration limits existIf duration limits can also be weekly, the reuse path should be gated on both booking and duration weekly limits to ensure weekStart-aware recomputation.
- if ( - initialData?.busyTimesFromLimits && - initialData?.eventTypeForLimits && - !hasWeeklyLimits(bookingLimits) - ) { + // Treat weekly duration limits like weekly booking limits for reuse gating + const hasWeeklyDuration = + !!durationLimits && "PER_WEEK" in (durationLimits as Record<string, unknown>); + if ( + initialData?.busyTimesFromLimits && + initialData?.eventTypeForLimits && + !hasWeeklyLimits(bookingLimits) && + !hasWeeklyDuration + ) {Also consider verifying the cached
eventTypeForLimits?.idmatches the currenteventType?.idto prevent reuse across different event types. Want a small follow-up diff?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lib/getUserAvailability.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/getUserAvailability.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/lib/getUserAvailability.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/lib/getUserAvailability.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:11:58.484Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/getUserAvailability.ts
📚 Learning: 2025-08-31T21:16:24.376Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/getUserAvailability.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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (4)
packages/lib/getUserAvailability.ts (4)
159-160: LGTM: BookingLimits type fixed as intendedThe alias now correctly models a partial map or null, matching prior feedback.
321-321: Explicit Monday fallback for weekStartGood call making the fallback explicit; aligns with the stated consistency goal.
417-429: Week-start now threaded into getBusyTimesFromLimitsPassing
weekStarthere addresses the core bug (weekly windows honoring the configured start-of-week). Nice. One follow-up: team limits path below doesn’t takeweekStart; ensure team-level weekly limits, if any, are aligned similarly.
618-620: Omit explicitweekStartaudit The wrapper’s default fallback of Monday is intentional and consistent across the codebase (see long-term defaults ingetPeriodStartDatesBetweenUtilandgetUserAvailability), so call sites don’t need an explicitweekStartfor weekly periods.Likely an incorrect or invalid review comment.
SinghaAnirban005
force-pushed
the
bug/t
branch
from
9afae29 to
40e1f0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/lib/getUserAvailability.ts (1)
405-409: hasWeeklyLimits should check value presence, not just key.
Using the in-operator can mis-detect when keys exist but values are undefined.Apply:
- function hasWeeklyLimits(bookingLimits: BookingLimits) { - if (!bookingLimits) return false; - return "PER_WEEK" in bookingLimits; - } + function hasWeeklyLimits(bookingLimits: BookingLimits) { + return bookingLimits?.PER_WEEK != null; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/lib/getUserAvailability.ts(5 hunks)packages/lib/intervalLimits/limitManager.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(8 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/prisma/selects/user.ts
- packages/lib/intervalLimits/limitManager.ts
- packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
- packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/getUserAvailability.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/lib/getUserAvailability.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/lib/getUserAvailability.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:11:58.484Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/getUserAvailability.ts
📚 Learning: 2025-08-31T21:16:24.376Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/getUserAvailability.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). (3)
- GitHub Check: Linters / lint
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
🔇 Additional comments (6)
packages/lib/getUserAvailability.ts (6)
159-160: BookingLimits type fixed and precise. LGTM.
Null union moved outside Partial and value type simplified; matches intended shape.
321-321: Explicit Monday fallback for weekStart—good.
Aligns with repo-wide consistency decision.
410-418: Reuse gate for initialData when no PER_WEEK limits—LGTM.
The short-circuit avoids unnecessary recomputation when week alignment is irrelevant.
427-429: Threading rescheduleUid and weekStart into getBusyTimesFromLimits—LGTM.
Ensures correct weekly window alignment and reschedule handling.
618-620: getPeriodStartDatesBetween now forwards weekStart—LGTM.
Interfaces stay clean; alignment delegated to the util.
441-454: Confirm team weekly limits also honor weekStart.
If team limits include PER_WEEK, ensure getBusyTimesFromTeamLimits applies the same week boundary; otherwise results may misalign.If supported by the cal.com API, pass weekStart through:
- busyTimesFromTeamLimits = await getBusyTimesFromTeamLimits( + busyTimesFromTeamLimits = await getBusyTimesFromTeamLimits( user, teamBookingLimits, dateFrom.tz(finalTimezone), dateTo.tz(finalTimezone), teamForBookingLimits.id, teamForBookingLimits.includeManagedEventsInLimits, finalTimezone, - initialData?.rescheduleUid ?? undefined + initialData?.rescheduleUid ?? undefined, + /* weekStart */ user.weekStart ?? "Monday" );
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/lib/intervalLimits/limitManager.ts (1)
71-76: Fix: endOf('week') ignores custom weekStart; align with adjustWeekStart.Using
start.endOf("week")relies on locale week semantics and can disagree with the manager’sweekStart. Compute aligned week start/end usingadjustWeekStartto ensure consistent keys and containment checks.- } else if ( - unit === "week" && - // weeks can be part of two months - ((this.busyMap.has(this.createKey(start, "month", timeZone)) && - this.busyMap.has(this.createKey(start.endOf("week"), "month", timeZone))) || - this.busyMap.has(this.createKey(start, "week", timeZone))) - ) { + } else if (unit === "week") { + // weeks can be part of two months; align week to custom weekStart + const alignedStart = this.adjustWeekStart(start, timeZone); + const alignedEnd = alignedStart.add(6, "days").endOf("day"); + if ( + (this.busyMap.has(this.createKey(alignedStart, "month", timeZone)) && + this.busyMap.has(this.createKey(alignedEnd, "month", timeZone))) || + this.busyMap.has(this.createKey(alignedStart, "week", timeZone)) + ) { + return true; + } + } else if (
🧹 Nitpick comments (4)
packages/lib/intervalLimits/limitManager.ts (4)
16-18: Confirm default weekStart; consider centralizing the fallback.Code defaults to "Monday". Retrieved learnings indicate other modules also fallback to Monday, but past discussion suggested "Sunday" as product default. Pick one fallback and centralize it (e.g., exported constant), or ensure callers always pass
weekStart.Possible tweak if Monday is the canonical fallback:
- constructor(weekStart?: string) { - this.weekStart = weekStart || "Monday"; - } + constructor(weekStart?: string) { + // TODO: import DEFAULT_WEEK_START from a single source if available + this.weekStart = weekStart ?? "Monday"; + }Would you like me to scan the repo for other implicit defaults and open a follow-up PR to unify them?
53-60: Week alignment helper is sound; consider reusing in createKey to DRY.Minor:
createKeyre-implements the same math. Optionally calladjustWeekStartinsidecreateKeyfor theweekbranch.- if (unit === "week") { - const weekStartIndex = weekStartNum(this.weekStart); - const currentDayIndex = tzStart.day(); - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; - const alignedStart = tzStart.subtract(daysToSubtract, "days").startOf("day"); + if (unit === "week") { + const alignedStart = this.adjustWeekStart(tzStart, undefined); return `${unit}-${alignedStart.toISOString()}`;
12-13: Prefer named export over default export.Project guideline suggests avoiding default exports for library modules to aid tree-shaking and refactors.
-export default class LimitManager { +export class LimitManager {This is breaking for import sites. If desired, I can generate a codemod to update imports across the repo.
94-111: AddBusyTime flow is consistent; small micro-optimization optional.
createKeyrecomputes TZ/alignment even whenadjustedStartis already aligned. Not harmful; consider documenting the idempotency for future maintainers.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lib/intervalLimits/limitManager.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/limitManager.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/lib/intervalLimits/limitManager.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/lib/intervalLimits/limitManager.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:16:24.376Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/intervalLimits/limitManager.ts
📚 Learning: 2025-08-31T21:11:58.484Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/intervalLimits/limitManager.ts
📚 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:
packages/lib/intervalLimits/limitManager.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
🔇 Additional comments (2)
packages/lib/intervalLimits/limitManager.ts (2)
23-35: Key generation logic for 'week' looks correct and idempotent.Alignment respects
timeZoneandweekStart; passing an already adjusted start keepsdaysToSubtractat 0. Good.
40-48: Weekly period end calculation aligns with custom weekStart.Using
+6 daysfrom aligned start is correct and avoids locale-dependentendOf('week').
SinghaAnirban005
force-pushed
the
bug/t
branch
from
9bf803f to
0cec474
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/lib/intervalLimits/limitManager.ts (1)
71-76: Fix locale-dependent endOf('week') check; align to custom weekStart + timeZone.Using endOf("week") ignores the configured weekStart and may misclassify weeks across month boundaries. Compute aligned weekly start/end via adjustWeekStart/getPeriodEnd before month-key checks.
- } else if ( - unit === "week" && - // weeks can be part of two months - ((this.busyMap.has(this.createKey(start, "month", timeZone)) && - this.busyMap.has(this.createKey(start.endOf("week"), "month", timeZone))) || - this.busyMap.has(this.createKey(start, "week", timeZone))) - ) { - return true; + } else if (unit === "week") { + const alignedStart = this.adjustWeekStart(start, timeZone); + const alignedEnd = this.getPeriodEnd(alignedStart, "week", timeZone); + const crossesMonths = + this.busyMap.has(this.createKey(alignedStart, "month", timeZone)) && + this.busyMap.has(this.createKey(alignedEnd, "month", timeZone)); + if (crossesMonths || this.busyMap.has(this.createKey(alignedStart, "week", timeZone))) { + return true; + }#!/bin/bash # Find remaining locale-based week boundaries in interval limits code rg -nP -C2 --type=ts '\.(startOf|endOf)\(\s*["'\'']week["'\'']\s*\)' packages/lib/intervalLimits
🧹 Nitpick comments (3)
packages/lib/intervalLimits/limitManager.ts (1)
23-35: DRY: reuse adjustWeekStart() inside createKey for weekly keys.Avoid duplicating the week-alignment logic and ensure identical behavior everywhere.
- private createKey(start: Dayjs, unit: IntervalLimitUnit, timeZone?: string): BusyMapKey { - const tzStart = timeZone ? start.tz(timeZone) : start; - - if (unit === "week") { - const weekStartIndex = weekStartNum(this.weekStart); - const currentDayIndex = tzStart.day(); - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; - const alignedStart = tzStart.subtract(daysToSubtract, "days").startOf("day"); - return `${unit}-${alignedStart.toISOString()}`; - } else { - return `${unit}-${tzStart.startOf(unit).toISOString()}`; - } - } + private createKey(start: Dayjs, unit: IntervalLimitUnit, timeZone?: string): BusyMapKey { + const tzStart = timeZone ? start.tz(timeZone) : start; + if (unit === "week") { + const alignedStart = this.adjustWeekStart(start, timeZone); + return `${unit}-${alignedStart.toISOString()}`; + } + return `${unit}-${tzStart.startOf(unit).toISOString()}`; + }packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
36-47: Minor: compute weekStartName/index once to avoid duplication.Small readability/maintenance win.
- const lastStart = - period === "week" - ? (() => { - const weekStartName = weekStart || "Monday"; - const weekStartIndex = weekStartNum(weekStartName); + const weekStartName = weekStart || "Monday"; + const weekStartIndex = period === "week" ? weekStartNum(weekStartName) : 0; + const lastStart = + period === "week" + ? (() => { const currentDayIndex = tzDateTo.day(); const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; return tzDateTo.subtract(daysToSubtract, "day").startOf("day"); })() : tzDateTo.startOf(period);packages/lib/getUserAvailability.ts (1)
418-429: Thread weekStart through all weekly limit paths (team limits too?).You pass weekStart to getBusyTimesFromLimits; confirm whether getBusyTimesFromTeamLimits also supports weekStart. If yes, pass weekStart there as well to avoid misaligned weekly windows for team-level PER_WEEK limits (see Lines 444-454).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/lib/getUserAvailability.ts(5 hunks)packages/lib/intervalLimits/limitManager.ts(3 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(8 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/prisma/selects/user.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/getUserAvailability.tspackages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/lib/getUserAvailability.tspackages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/lib/getUserAvailability.tspackages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:11:58.484Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.484Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/getUserAvailability.tspackages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 Learning: 2025-08-31T21:16:24.376Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.376Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/getUserAvailability.tspackages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 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:
packages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
📚 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/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧬 Code graph analysis (2)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
packages/lib/sentryWrapper.ts (1)
withReporting(107-139)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
packages/lib/intervalLimits/limitManager.ts (1)
LimitManager(12-130)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
getPeriodStartDatesBetween(61-64)
⏰ 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 (13)
packages/lib/intervalLimits/limitManager.ts (2)
16-18: Default fallback to Monday looks consistent (per retrieved learnings).Using "Monday" as the fallback aligns with getUserAvailability and getPeriodStartDatesBetween decisions noted in learnings. Please ensure callers don't assume "Sunday".
93-111: LGTM: addBusyTime aligns weekly starts and computes week-aware period ends.Keying and stored ranges now respect weekStart and timeZone.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (5)
27-34: Threading weekStart end-to-end is correct.Instantiating LimitManager with weekStart and forwarding it into booking-limits path looks good.
Also applies to: 47-48
112-119: Good: period enumeration respects weekStart and timeZone.Passing weekStart into getPeriodStartDatesBetween prevents misaligned windows.
147-147: Correct weekly periodEnd computation.Using add(6, "day").endOf("day") avoids locale week semantics and matches LimitManager.getPeriodEnd.
173-175: Duration-limits: weekStart-aware start dates.Consistent with booking-limits path.
Also applies to: 181-181
211-211: Duration-limits: correct weekly periodEnd.Matches booking-limits and LimitManager behavior.
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (3)
24-35: Start alignment for weekly periods is correct (Monday fallback per learnings).TimeZone-aware alignment + Monday fallback matches the chosen convention.
48-56: LGTM: inclusive loop to lastStart with week-aware increment.Clean and predictable.
61-64: withReporting wrapper application looks fine.Name provided; will no-op when Sentry env is absent.
packages/lib/getUserAvailability.ts (3)
159-160: Type fix looks correct.The BookingLimits alias now accurately models a partial map or null.
321-321: Explicit Monday fallback for weekStart — good.This aligns with the chosen default and avoids undefined propagation.
618-620: Wrapper signature extension is sensible.Optional weekStart at the end preserves back-compat and makes alignment explicit.
SinghaAnirban005
force-pushed
the
bug/t
branch
from
0cec474 to
c375053
Compare
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)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
234-243: Thread weekStart through team-limits path for consistency.Team limits still instantiate LimitManager without weekStart and don’t pass weekStart to getBusyTimesFromBookingLimits. This can reintroduce misaligned weekly windows when user preference is not Sunday.
Apply:
-const _getBusyTimesFromTeamLimits = async ( +const _getBusyTimesFromTeamLimits = async ( user: { id: number; email: string }, bookingLimits: IntervalLimit, dateFrom: Dayjs, dateTo: Dayjs, teamId: number, includeManagedEvents: boolean, - timeZone: string, - rescheduleUid?: string + timeZone: string, + rescheduleUid?: string, + weekStart?: string ) => { @@ - const limitManager = new LimitManager(); + const limitManager = new LimitManager(weekStart); @@ await getBusyTimesFromBookingLimits({ bookings: busyTimes, bookingLimits, dateFrom, dateTo, limitManager, rescheduleUid, teamId, user, includeManagedEvents, timeZone, + weekStart, });Note: update call sites accordingly.
Also applies to: 269-282
78-163: Replace hardcoded end-of-week with week-aware period end in LimitManager
Inpackages/lib/intervalLimits/limitManager.tsat line 74,start.endOf("week")still ignores theweekStartoverride. Change that call to:this.busyMap.has( this.createKey( this.getPeriodEnd(start, "week", timeZone), "month", timeZone ) )so weekly boundaries respect the configured
weekStart.
🧹 Nitpick comments (3)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (1)
121-121: Pass timeZone into LimitManager checks to avoid key skew.LimitManager.createKey re-normalizes by timeZone when provided. Passing the same timeZone here removes any chance of mismatched keys if a non-tz Dayjs slips in.
Apply:
- if (limitManager.isAlreadyBusy(periodStart, unit)) continue; + if (limitManager.isAlreadyBusy(periodStart, unit, timeZone || undefined)) continue; @@ - if (periodStartDates.every((start) => limitManager.isAlreadyBusy(start, unit))) { + if (periodStartDates.every((start) => limitManager.isAlreadyBusy(start, unit, timeZone || undefined))) { return; } @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone || undefined); @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone || undefined);And in duration-limits:
- if (limitManager.isAlreadyBusy(periodStart, unit)) continue; + if (limitManager.isAlreadyBusy(periodStart, unit, timeZone)) continue; @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone); @@ - if (periodStartDates.every((start) => limitManager.isAlreadyBusy(start, unit))) { + if (periodStartDates.every((start) => limitManager.isAlreadyBusy(start, unit, timeZone))) { return; } @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone);Also applies to: 139-141, 157-159, 184-185, 190-191, 203-206, 221-223
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (2)
18-22: Nit: small micro-allocations in the weekly loop.Cache weekStartIndex once to avoid recomputing it for lastStart; minor but cheap.
- if (period === "week") { - const weekStartName = weekStart || "Monday"; - const weekStartIndex = weekStartNum(weekStartName); + if (period === "week") { + const weekStartName = weekStart || "Monday"; + const weekStartIndex = weekStartNum(weekStartName); @@ - const lastStart = - period === "week" - ? (() => { - const weekStartName = weekStart || "Monday"; - const weekStartIndex = weekStartNum(weekStartName); + const lastStart = + period === "week" + ? (() => { const currentDayIndex = tzDateTo.day(); const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; return tzDateTo.subtract(daysToSubtract, "day").startOf("day"); })() : tzDateTo.startOf(period);Also applies to: 36-47, 51-55
48-56: Add DST-crossing tests for weekly ranges.Ensure loops spanning DST transitions still yield midnight-aligned starts in the target timeZone.
I can add tests for a zone like America/New_York with dateFrom/dateTo around March/November transitions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/lib/getUserAvailability.ts(5 hunks)packages/lib/intervalLimits/limitManager.ts(3 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(8 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/prisma/selects/user.ts
- packages/lib/intervalLimits/limitManager.ts
- packages/lib/getUserAvailability.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.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/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.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/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:16:24.415Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
📚 Learning: 2025-08-31T21:11:58.503Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/intervalLimits/server/getBusyTimesFromLimits.tspackages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
📚 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:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧬 Code graph analysis (2)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
packages/lib/intervalLimits/limitManager.ts (1)
LimitManager(12-130)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
getPeriodStartDatesBetween(61-64)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
packages/lib/sentryWrapper.ts (1)
withReporting(107-139)
🔇 Additional comments (7)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (5)
27-34: Good: weekStart plumbed into the top-level and LimitManager.Threading weekStart through the entry point and constructing LimitManager with it is the right call. This is necessary for consistent weekly windows.
112-119: Correct: weekly-aware period starts now respect weekStart and timeZone.Passing weekStart into getPeriodStartDatesBetween fixes the original bug vector.
147-147: Fix: week periodEnd no longer uses locale week semantics.Using periodStart.add(6, "day").endOf("day") avoids endOf("week") mismatches.
173-176: Consistent weekly boundary handling in duration-limits path.Mirrors the booking-limits fix; keeps start/end aligned to weekStart.
Also applies to: 181-181, 211-211
78-163: Check duration-limit inequality matches product semantics.Code uses > for both initial selectedDuration and running sum comparisons. If “limit” is intended as an inclusive cap (at most X minutes), this is correct; if it’s exclusive (< X), it should be >=. Please confirm.
Would you like me to add unit tests covering both branches (== and >) for week/month/year to lock intent?
Also applies to: 165-227
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (2)
24-34: Week boundaries now correctly honor weekStart at both ends.Start and end alignment logic is consistent and timeZone-aware; this addresses the root cause.
Also applies to: 36-47, 48-56
8-15: API surface change looks good; default fallback “Monday” matches repo convention.Signature additions (timeZone, weekStart) and withReporting wrapper remain clean.
Also applies to: 61-64
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/features/bookings/lib/handleNewBooking.ts (1)
711-716: Ensure default choice matches LimitManager/check*Limits defaults.If you adopt Monday elsewhere, switch the fallback to "Monday" here. Inconsistent fallbacks will shift weekly windows and reintroduce the bug this PR fixes.
#!/bin/bash rg -nP 'weekStartNum\([^)]*\?\? +"(?i:sunday|monday)"' --type=ts --type=tsx -C2
🧹 Nitpick comments (10)
packages/lib/intervalLimits/limitManager.ts (4)
23-35: Deduplicate week alignment logic.createKey re-implements the alignment done in adjustWeekStart. Use adjustWeekStart for the weekly branch to keep a single source of truth.
Apply:
- if (unit === "week") { - const weekStartIndex = weekStartNum(this.weekStart); - const currentDayIndex = tzStart.day(); - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; - const alignedStart = tzStart.subtract(daysToSubtract, "days").startOf("day"); - return `${unit}-${alignedStart.toISOString()}`; - } else { + if (unit === "week") { + const alignedStart = this.adjustWeekStart(tzStart, timeZone); + return `${unit}-${alignedStart.toISOString()}`; + } else { return `${unit}-${tzStart.startOf(unit).toISOString()}`; }
53-60: Tiny perf/readability nit: inline local vars.You can inline currentDayIndex to shorten the function; behavior unchanged.
- const currentDayIndex = tzStart.day(); - - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; + const daysToSubtract = (tzStart.day() - weekStartIndex + 7) % 7;
65-87: Week spanning two months logic is sound; consider short-circuit on month busy.If alignedStart and alignedEnd are in the same month, the double-month check reduces to a single month check. Minor micro-opt: early-return if weekStartMonth === weekEndMonth.
- // Check if week spans two months and both months are busy - const weekStartMonth = this.createKey(alignedStart, "month", timeZone); - const weekEndMonth = this.createKey(alignedEnd, "month", timeZone); + const weekStartMonth = this.createKey(alignedStart, "month", timeZone); + const weekEndMonth = this.createKey(alignedEnd, "month", timeZone); + if (weekStartMonth === weekEndMonth && this.busyMap.has(weekStartMonth)) { + return true; + }
12-12: Prefer named exports over default exports.Default exports hinder tree-shaking and refactors. Consider
export class LimitManager {}and update imports.packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts (1)
15-16: Strongly type weekStartDay as a day index (0-6).Prevents accidental out-of-range values and clarifies intent.
-type InputProps = { +type DayIndex = 0 | 1 | 2 | 3 | 4 | 5 | 6; +type InputProps = { eventType: EventType; reqBodyStart: string; reqBodyRescheduleUid?: string; - weekStartDay?: number; + weekStartDay?: DayIndex; };packages/features/bookings/lib/handleNewBooking.ts (2)
711-716: Derive weekStart from the primary/organizer user and add a safe fallback.Using users[0] can be wrong for team events; prefer the event owner/organizer or the user matching eventType.userId. Also guard against undefined weekStart.
- weekStartDay: weekStartNum(eventType.users[0].weekStart), + weekStartDay: weekStartNum( + (eventType.users.find((u) => u.id === eventType.userId) ?? eventType.users[0])?.weekStart ?? "Sunday" + ),
1-1: Prefer named exports over default exports.Consider
export function handler(...)and update imports. Low priority, follow-up acceptable.packages/lib/intervalLimits/server/checkBookingLimits.ts (3)
82-88: Normalize weekStartDay input and guard against invalid valuesClamp to [0–6] and fall back to Monday if undefined/NaN to avoid edge-case regressions.
- // Custom week calculation based on user's week start day + // Custom week calculation based on user's week start day + const normalizeWeekStart = (val: number | undefined): WeekStartDay => { + const n = Number(val); + return Number.isFinite(n) ? (((n % 7) + 7) % 7) as WeekStartDay : 1; + };
93-102: Use normalized week start and reduce duplication riskCompute a normalized index once; also consider extracting these helpers to a shared util already used by getPeriodStartDatesBetween/LimitManager to keep week-boundary logic consistent.
- let startDate: Date; - let endDate: Date; + let startDate: Date; + let endDate: Date; if (unit === "week") { - startDate = getCustomWeekStart(eventDateInOrganizerTz, weekStartDay).toDate(); - endDate = getCustomWeekEnd(eventDateInOrganizerTz, weekStartDay).toDate(); + const ws = normalizeWeekStart(weekStartDay); + startDate = getCustomWeekStart(eventDateInOrganizerTz, ws).toDate(); + endDate = getCustomWeekEnd(eventDateInOrganizerTz, ws).toDate(); } else { startDate = dayjs(eventDateInOrganizerTz).startOf(unit).toDate(); endDate = dayjs(eventDateInOrganizerTz).endOf(unit).toDate(); }
24-26: Constrain weekStartDay to valid values 0–6
Definetype WeekStartDay = 0 | 1 | 2 | 3 | 4 | 5 | 6;and replace every
weekStartDay?: numberparameter inpackages/lib/intervalLimits/server/checkBookingLimits.tswithweekStartDay?: WeekStartDay. Then update all callers (e.g. infeatures/bookings/lib/handleNewBooking.tsandcheckBookingAndDurationLimits.ts) to pass aWeekStartDay(for example, castingweekStartNum(...) as WeekStartDay).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/features/bookings/lib/handleNewBooking.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts(3 hunks)packages/lib/getUserAvailability.ts(5 hunks)packages/lib/intervalLimits/limitManager.ts(2 hunks)packages/lib/intervalLimits/server/checkBookingLimits.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/lib/getUserAvailability.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleNewBooking.tspackages/lib/intervalLimits/limitManager.tspackages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.tspackages/lib/intervalLimits/server/checkBookingLimits.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.tspackages/lib/intervalLimits/limitManager.tspackages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.tspackages/lib/intervalLimits/server/checkBookingLimits.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.tspackages/lib/intervalLimits/limitManager.tspackages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.tspackages/lib/intervalLimits/server/checkBookingLimits.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:11:58.503Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/features/bookings/lib/handleNewBooking.tspackages/lib/intervalLimits/limitManager.tspackages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.tspackages/lib/intervalLimits/server/checkBookingLimits.ts
📚 Learning: 2025-08-31T21:16:24.415Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/features/bookings/lib/handleNewBooking.tspackages/lib/intervalLimits/limitManager.tspackages/lib/intervalLimits/server/checkBookingLimits.ts
📚 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:
packages/lib/intervalLimits/limitManager.ts
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts
🧬 Code graph analysis (1)
packages/lib/intervalLimits/limitManager.ts (1)
packages/types/Calendar.d.ts (1)
IntervalLimitUnit(138-138)
⏰ 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 (5)
packages/lib/intervalLimits/limitManager.ts (2)
40-48: LGTM: period end for weeks now respects custom week alignment.The 6-day span with endOf("day") matches the aligned start; DST is handled by dayjs-timezone.
97-115: LGTM: addBusyTime now aligns starts and computes period ends consistently.Key generation and stored ranges look coherent with isAlreadyBusy.
packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts (2)
30-35: LGTM: threading weekStartDay through the internal method.
47-50: Use nullish coalescing for weekStartDay default
In the call tocheckBookingLimits, replaceweekStartDay || 0withweekStartDay ?? 0so that a valid0(Sunday) isn’t treated as falsy. Theundefinedargument correctly aligns with theincludeManagedEventsparameter.- eventType.schedule?.timeZone, - undefined, - weekStartDay || 0 + eventType.schedule?.timeZone, + undefined, + weekStartDay ?? 0packages/features/bookings/lib/handleNewBooking.ts (1)
82-82: LGTM: importing weekStartNum where used.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/lib/defaultEvents.ts (1)
215-215: Prefer named export over default exportRepo guideline favors named exports for better tree-shaking and refactors. Consider removing the default export.
Apply:
-export default defaultEvents;Then update consumers:
import { defaultEvents } from "@calcom/lib/defaultEvents";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/lib/defaultEvents.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/defaultEvents.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/lib/defaultEvents.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/lib/defaultEvents.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:11:58.503Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/defaultEvents.ts
📚 Learning: 2025-08-31T21:16:24.415Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/defaultEvents.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). (3)
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
|
@kart1ka PTAL !! |
SinghaAnirban005
force-pushed
the
bug/t
branch
from
4644a0a to
aff74fa
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/lib/intervalLimits/limitManager.ts (5)
14-18: Strongly type weekStart + confirm default alignment across callers
- Prefer a constrained type (e.g., WeekStartName) over string to prevent invalid inputs.
- Defaulting to "Monday" matches the retrieved learnings for getUserAvailability and getPeriodStartDatesBetween; please verify all call sites share the same fallback to avoid divergent behavior when user.weekStart is undefined.
We used your retrieved learnings for this recommendation.
Apply:
-import { weekStartNum } from "@calcom/lib/weekstart"; +import { weekStartNum } from "@calcom/lib/weekstart"; +import type { WeekStartName } from "@calcom/lib/weekstart"; - private weekStart: string; + private weekStart: WeekStartName; - constructor(weekStart?: string) { + constructor(weekStart?: WeekStartName) { this.weekStart = weekStart || "Monday"; }
23-35: Deduplicate weekly alignment logic by reusing adjustWeekStartcreateKey re-implements the same alignment done in adjustWeekStart. Centralize to avoid drift.
- if (unit === "week") { - const weekStartIndex = weekStartNum(this.weekStart); - const currentDayIndex = tzStart.day(); - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; - const alignedStart = tzStart.subtract(daysToSubtract, "days").startOf("day"); - return `${unit}-${alignedStart.toISOString()}`; - } else { - return `${unit}-${tzStart.startOf(unit).toISOString()}`; - } + if (unit === "week") { + const alignedStart = this.adjustWeekStart(start, timeZone); + return `${unit}-${alignedStart.toISOString()}`; + } + return `${unit}-${tzStart.startOf(unit).toISOString()}`;
37-48: Harden getPeriodEnd for weekly: align internallyDefensive alignment helps if future call sites forget to pass an already-aligned start.
- if (unit === "week") { - return tzStart.add(6, "days").endOf("day"); - } else { - return tzStart.endOf(unit); - } + if (unit === "week") { + const aligned = this.adjustWeekStart(start, timeZone); + return aligned.add(6, "day").endOf("day"); + } + return tzStart.endOf(unit);
65-92: isAlreadyBusy: good coverage; add minor consistency checks
- Logic for cross-month weekly windows is solid. Consider computing keys once to avoid repeated tz/alignment work.
- Ensure all callers pass the same timeZone consistently; mixed TZs would create different keys for the same instant.
Optional micro-refactor:
- const weekStartMonth = this.createKey(alignedStart, "month", timeZone); - const weekEndMonth = this.createKey(alignedEnd, "month", timeZone); + const monthKeyStart = this.createKey(alignedStart, "month", timeZone); + const monthKeyEnd = this.createKey(alignedEnd, "month", timeZone); ... - (this.busyMap.has(weekStartMonth) && this.busyMap.has(weekEndMonth)) || + (this.busyMap.has(monthKeyStart) && this.busyMap.has(monthKeyEnd)) ||
97-115: addBusyTime: correct normalization + storage; add small cleanup
- Flow is correct. Minor: getPeriodEnd re-applies tz() even when adjustedStart is already TZ’d; harmless but avoidable.
- const periodEnd = this.getPeriodEnd(adjustedStart, unit, timeZone); + const periodEnd = this.getPeriodEnd(adjustedStart, unit, undefined);Also consider asserting that managers merged via mergeBusyTimes share the same weekStart/timeZone policy to avoid mixed-key semantics.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
packages/features/bookings/lib/handleNewBooking.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts(3 hunks)packages/lib/defaultEvents.ts(1 hunks)packages/lib/getUserAvailability.ts(5 hunks)packages/lib/intervalLimits/limitManager.ts(2 hunks)packages/lib/intervalLimits/server/checkBookingLimits.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(8 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/lib/defaultEvents.ts
- packages/prisma/selects/user.ts
- packages/features/bookings/lib/handleNewBooking.ts
- packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
- packages/lib/intervalLimits/server/checkBookingLimits.ts
- packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
- packages/lib/getUserAvailability.ts
- packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/limitManager.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/lib/intervalLimits/limitManager.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/lib/intervalLimits/limitManager.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:16:24.415Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/intervalLimits/limitManager.ts
📚 Learning: 2025-08-31T21:11:58.503Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/intervalLimits/limitManager.ts
📚 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:
packages/lib/intervalLimits/limitManager.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). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
- GitHub Check: Linters / lint
🔇 Additional comments (2)
packages/lib/intervalLimits/limitManager.ts (2)
2-2: LGTM: bringing in weekStartNumImport is correct and scoped. No issues.
50-60: LGTM: week-start adjustment logicClear and correct alignment to custom weekStart with TZ awareness.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/features/bookings/lib/handleNewBooking.ts (2)
720-733: Derive weekStart from stable owner/host, not array[0]Using the first user/host is order-sensitive. Prefer event owner; then any host with weekStart; then fallback.
Apply:
- const getWeekStartFromEventType = (eventType: EventType): string => { - // For individual user events - if (eventType.users && eventType.users.length > 0 && eventType.users[0].weekStart) { - return eventType.users[0].weekStart; - } - // For team events - if (eventType.hosts && eventType.hosts.length > 0 && eventType.hosts[0].user?.weekStart) { - return eventType.hosts[0].user.weekStart; - } - return "Monday"; - }; + const getWeekStartFromEventType = (eventType: EventType): string => { + // Prefer the event owner (stable) + const owner = eventType.users?.find((u) => u.id === eventType.userId); + if (owner?.weekStart) return owner.weekStart; + // For team events, pick the first host that actually has weekStart set + const hostWithWeekStart = eventType.hosts?.find((h) => h.user?.weekStart)?.user; + if (hostWithWeekStart?.weekStart) return hostWithWeekStart.weekStart; + // Fallback: any user with weekStart + const anyUserWithWeekStart = eventType.users?.find((u) => !!u.weekStart); + if (anyUserWithWeekStart?.weekStart) return anyUserWithWeekStart.weekStart; + return "Monday"; + };
2536-2536: Prefer named export over defaultThis is a library module; named exports ease refactors and tree-shaking.
-export default handler; +export { handler };packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
112-119: Pass timeZone to LimitManager checks to avoid subtle tz/DST drift
isAlreadyBusy/addBusyTimeaccepttimeZone; pass it for consistency with tz-aligned period starts.- if (limitManager.isAlreadyBusy(periodStart, unit)) continue; + if (limitManager.isAlreadyBusy(periodStart, unit, timeZone || undefined)) continue; @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone || undefined); @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone || undefined);Also applies to: 147-159
181-182: Do the same tz pass-through in duration-limits pathMinor consistency fix; prevents edge cases around local vs tz math.
- const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit, timeZone, weekStart); + const periodStartDates = getPeriodStartDatesBetween(dateFrom, dateTo, unit, timeZone, weekStart); @@ - if (limitManager.isAlreadyBusy(periodStart, unit)) continue; + if (limitManager.isAlreadyBusy(periodStart, unit, timeZone)) continue; @@ - limitManager.addBusyTime(periodStart, unit); + limitManager.addBusyTime(periodStart, unit, timeZone); @@ - if (totalDuration > limit) { - limitManager.addBusyTime(periodStart, unit); + if (totalDuration > limit) { + limitManager.addBusyTime(periodStart, unit, timeZone);Also applies to: 210-214, 219-221
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/bookings/lib/handleNewBooking.ts(3 hunks)packages/lib/getUserAvailability.ts(6 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/bookings/lib/handleNewBooking.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:11:58.503Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/features/bookings/lib/handleNewBooking.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 Learning: 2025-08-31T21:16:24.415Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/features/bookings/lib/handleNewBooking.tspackages/lib/getUserAvailability.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 Learning: 2025-08-21T13:44:06.805Z
Learnt from: supalarry
PR: calcom/cal.com#23217
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/services/output-event-types.service.ts:93-94
Timestamp: 2025-08-21T13:44:06.805Z
Learning: In apps/api/v2/src/ee/event-types/event-types_2024_06_14/event-types.repository.ts, repository functions that use explicit Prisma select clauses (like getEventTypeWithSeats) are used for specific purposes and don't need to include all EventType fields like bookingRequiresAuthentication. These methods don't feed into the general OutputEventTypesService_2024_06_14 flow.
Applied to files:
packages/features/bookings/lib/handleNewBooking.ts
📚 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/lib/getUserAvailability.ts
📚 Learning: 2025-09-03T09:52:51.182Z
Learnt from: hariombalhara
PR: calcom/cal.com#23541
File: packages/features/bookings/lib/di/modules/RegularBookingServiceModule.ts:22-28
Timestamp: 2025-09-03T09:52:51.182Z
Learning: The IBookingServiceDependencies interface in packages/features/bookings/lib/handleNewBooking.ts contains 6 properties: cacheService, checkBookingAndDurationLimitsService, prismaClient, bookingRepository, featuresRepository, and checkBookingLimitsService. This interface is used by RegularBookingService and matches the depsMap structure in RegularBookingServiceModule.
Applied to files:
packages/lib/getUserAvailability.ts
🧬 Code graph analysis (2)
packages/features/bookings/lib/handleNewBooking.ts (2)
packages/lib/getUserAvailability.ts (1)
EventType(62-62)packages/lib/di/containers/BookingLimits.ts (1)
getCheckBookingAndDurationLimitsService(25-29)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (1)
packages/lib/intervalLimits/limitManager.ts (1)
LimitManager(12-134)
⏰ 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 (10)
packages/features/bookings/lib/handleNewBooking.ts (2)
71-71: Week-start threading imports — LGTMType import and
weekStartNumusage are appropriate for passing a normalized week-start to limits.Also applies to: 84-84
739-740: Defaults and mappings align:weekStartNum("Monday")returns1, andcheckBookingLimits/checkBookingAndDurationLimitsboth default toweekStartDay = 1.handleNewBookingalways passes a defined value (fallback “Monday” → 1), so no inconsistencies—no changes needed.packages/lib/getUserAvailability.ts (6)
159-160: BookingLimits alias — LGTMType accurately captures a partial limits map or null.
321-322: Explicit Monday fallback — LGTMMatches the agreed repo-wide default and avoids undefined downstream.
405-412: Weekly-limit cache gating — LGTMRecomputation when either booking or duration limits include weekly units is correct.
Also applies to: 413-417
420-431: Threading weekStart into limits — LGTMPassing weekStart into
getBusyTimesFromLimitsensures aligned weekly windows.
443-457: Team-limits weekStart propagation — LGTMTeam limits now honor configured week boundaries.
620-623: Expose weekStart in period util wrapper — LGTMSignature aligns with underlying util and callers.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
18-34: End-to-end weekStart threading — LGTMAccepting
weekStartand constructingLimitManager(weekStart)aligns boundaries across both booking and duration paths.Also applies to: 47-49, 55-67
242-244: Team-limits weekStart propagation — LGTMTeam-limit manager and booking-limits call now respect weekStart.
Also applies to: 270-271, 283-284
SinghaAnirban005
force-pushed
the
bug/t
branch
2 times, most recently
from
e022c77 to
ae98ac7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/lib/intervalLimits/server/checkBookingLimits.ts (2)
24-26: Narrow weekStartDay to 0–6 and sanitize before use.Prevents bad inputs and keeps semantics consistent with Day.js’ 0–6. Also avoids silent coercion when callers pass unexpected values.
Apply:
@@ - includeManagedEvents?: boolean, - weekStartDay?: number + includeManagedEvents?: boolean, + weekStartDay?: WeekStartDay @@ +type WeekStartDay = 0 | 1 | 2 | 3 | 4 | 5 | 6; @@ - includeManagedEvents = false, - weekStartDay = 1, + includeManagedEvents = false, + weekStartDay = 1, @@ - weekStartDay?: number; + weekStartDay?: WeekStartDay; @@ - if (unit === "week") { - startDate = getCustomWeekStart(eventDateInOrganizerTz, weekStartDay).toDate(); - endDate = getCustomWeekEnd(eventDateInOrganizerTz, weekStartDay).toDate(); + if (unit === "week") { + const safeWeekStartDay = (((weekStartDay ?? 1) % 7) + 7) % 7 as WeekStartDay; + startDate = getCustomWeekStart(eventDateInOrganizerTz, safeWeekStartDay).toDate(); + endDate = getCustomWeekEnd(eventDateInOrganizerTz, safeWeekStartDay).toDate();Also applies to: 40-41, 63-75, 96-102
82-92: Consider deduplicating week-boundary logic.This custom-week logic now exists here and in getPeriodStartDatesBetween; extracting a shared helper (e.g., alignToWeekStart(date, weekStartIdx)) would reduce drift.
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
16-17: Hoist weekStart computation to avoid duplicate work.Minor DRY/readability win.
@@ - let currentDate: Dayjs; + let currentDate: Dayjs; + const weekName = weekStart || "Monday"; + const weekIndex = weekStartNum(weekName); @@ - const weekStartName = weekStart || "Monday"; - const weekStartIndex = weekStartNum(weekStartName); - - const currentDayIndex = currentDate.day(); - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; + const currentDayIndex = currentDate.day(); + const daysToSubtract = (currentDayIndex - weekIndex + 7) % 7; currentDate = currentDate.subtract(daysToSubtract, "day").startOf("day"); @@ - const weekStartName = weekStart || "Monday"; - const weekStartIndex = weekStartNum(weekStartName); - const currentDayIndex = tzDateTo.day(); - const daysToSubtract = (currentDayIndex - weekStartIndex + 7) % 7; + const currentDayIndex = tzDateTo.day(); + const daysToSubtract = (currentDayIndex - weekIndex + 7) % 7; return tzDateTo.subtract(daysToSubtract, "day").startOf("day");Also applies to: 26-28, 40-46
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (1)
147-147: DST-safe weekly period end.Using +6 days endOf("day") can drift around DST transitions. Prefer start + 1 week - 1 ms.
- const periodEnd = unit === "week" ? periodStart.add(6, "day").endOf("day") : periodStart.endOf(unit); + const periodEnd = + unit === "week" + ? periodStart.add(1, "week").subtract(1, "millisecond") + : periodStart.endOf(unit);Apply at both occurrences.
Also applies to: 211-211
📜 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.
📒 Files selected for processing (9)
packages/features/bookings/lib/handleNewBooking.ts(2 hunks)packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts(3 hunks)packages/lib/defaultEvents.ts(1 hunks)packages/lib/getUserAvailability.ts(6 hunks)packages/lib/intervalLimits/limitManager.ts(2 hunks)packages/lib/intervalLimits/server/checkBookingLimits.ts(4 hunks)packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts(11 hunks)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts(1 hunks)packages/prisma/selects/user.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/lib/defaultEvents.ts
- packages/features/bookings/lib/handleNewBooking/checkBookingAndDurationLimits.ts
- packages/features/bookings/lib/handleNewBooking.ts
- packages/lib/intervalLimits/limitManager.ts
- packages/prisma/selects/user.ts
- packages/lib/getUserAvailability.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/checkBookingLimits.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/checkBookingLimits.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.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/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/checkBookingLimits.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
📚 Learning: 2025-08-31T21:16:24.415Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts:36-47
Timestamp: 2025-08-31T21:16:24.415Z
Learning: In packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts, the user has decided to keep "Monday" as the fallback default for weekStart parameter to maintain consistency across the cal.com codebase.
Applied to files:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/checkBookingLimits.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 Learning: 2025-08-31T21:11:58.503Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23400
File: packages/lib/getUserAvailability.ts:321-322
Timestamp: 2025-08-31T21:11:58.503Z
Learning: In the getUserAvailability function in packages/lib/getUserAvailability.ts, weekStart should default to "Monday" when user?.weekStart is undefined for consistency across the codebase.
Applied to files:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.tspackages/lib/intervalLimits/server/checkBookingLimits.tspackages/lib/intervalLimits/server/getBusyTimesFromLimits.ts
📚 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:
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts
🧬 Code graph analysis (2)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
packages/lib/sentryWrapper.ts (1)
withReporting(107-139)
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (2)
packages/lib/intervalLimits/limitManager.ts (1)
LimitManager(12-134)packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (1)
getPeriodStartDatesBetween(61-64)
⏰ 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). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
packages/lib/intervalLimits/utils/getPeriodStartDatesBetween.ts (2)
24-47: Weekly alignment with Monday fallback — LGTM.Matches repo convention and fixes the off-by-week bug when weekStart ≠ locale start.
48-56: Confirm inclusivity of the last period.Loop is inclusive of lastStart; please ensure tests cover cases where dateTo lands mid-week and exactly on a boundary.
packages/lib/intervalLimits/server/getBusyTimesFromLimits.ts (3)
27-48: End-to-end weekStart threading — LGTM.weekStart now flows through the main, booking, and duration paths; LimitManager instantiated with the same context.
Also applies to: 66-67
112-118: Correct: period starts now use week-aware utility.
242-244: Team limits path weekStart — LGTM.Signature, LimitManager construction, and downstream call all respect weekStart.
Also applies to: 270-270, 283-284
|
@kart1ka I have resolve all type errors and coderabbit suggestions |
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <anirbansingha20@gmail.com>
Signed-off-by: Anirban Singha <143536290+SinghaAnirban005@users.noreply.github.com>
Signed-off-by: Anirban Singha <143536290+SinghaAnirban005@users.noreply.github.com>
SinghaAnirban005
force-pushed
the
bug/t
branch
from
ae98ac7 to
cf10b1c
Compare
|
This PR is being marked as stale due to inactivity. |
|
@cubic-dev-ai leave a review. |
@pallava-joshi I've started the AI code review. It'll take a few minutes to complete. |
pallava-joshi
left a comment
pallava-joshi
left a comment
There was a problem hiding this comment.
can you please resolve the merge conflicts. marking it draft until then.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
2 participants
What does this PR do?
This PR handles weekly booking limits correctly based on the "start of the week" parameter . Ensures the
weekStartparamter is respectedVisual Demo (For contributors especially)
Screencast.from.2025-08-31.22-29-22.webm
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?