Conversation
keithwillcode
added
core
WalkthroughDelegates prefetch decisions in the Booker feature to new utilities and updates the hook return signature. usePrefetch now calls isPrefetchNextMonthEnabled and getPrefetchMonthCount and returns both prefetchNextMonth and monthCount. New utilities added: areDifferentValidMonths, getPrefetchMonthCount, isMonthViewPrefetchEnabled, isPrefetchNextMonthEnabled, and isMonthChange. Comprehensive Vitest suites were added for these utilities, covering layout-specific behavior, month comparisons, date validity, and time-threshold scenarios. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
dosubot
bot
added
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts (1)
1-14: Avoid repeated parsing ofmonthParse
monthonce (monthD) as shown in the diff above to reduce repeated Day.js constructions on re-render. Minor, but improves readability and micro-perf in a hook-driven path.packages/features/bookings/Booker/utils/areDifferentValidMonths.ts (1)
1-14: Covers the NaN !== NaN pitfall; optional range checkGood guard against invalid month numbers; this replaces the prior
!isNaNpattern and preventsNaN !== NaNfrom misclassifying months. Consider also enforcing the 0–11 range to catch out-of-range values defensively.- if (!isFirstMonthValid || !isSecondMonthValid) { + const inRange = (m: number) => m >= 0 && m <= 11; + if (!isFirstMonthValid || !isSecondMonthValid || !inRange(firstMonth) || !inRange(secondMonth)) { return false; }Based on learnings.
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (1)
24-30: Cache parsed date and coerceprefetchNextMonthto boolean
- Instantiate
dayjs(date)once and reuse (const d = dayjs(date);) to avoid double parsing.- Wrap
isPrefetchNextMonthEnabled(...)inBoolean(...)so it always returnsfalseinstead ofundefined.- (Optional) Add an explicit return type:
export const usePrefetch = ( params: UsePrefetchParams ): { prefetchNextMonth: boolean; monthCount?: number } => { … }export const usePrefetch = ({ date, month, bookerLayout, bookerState }: UsePrefetchParams) => { - const monthAfterAdding1Month = dayjs(date).add(1, "month").month(); - const monthAfterAddingExtraDaysColumnView = dayjs(date) - .add(bookerLayout.columnViewExtraDays.current, "day") - .month(); + const d = dayjs(date); + const monthAfterAdding1Month = d.add(1, "month").month(); + const monthAfterAddingExtraDaysColumnView = d + .add(bookerLayout.columnViewExtraDays.current, "day") + .month(); - const prefetchNextMonth = isPrefetchNextMonthEnabled(bookerLayout.layout, date, month); + const prefetchNextMonth = Boolean( + isPrefetchNextMonthEnabled(bookerLayout.layout, date, month) + ); const monthCount = getPrefetchMonthCount( bookerLayout.layout, bookerState, monthAfterAdding1Month, monthAfterAddingExtraDaysColumnView );packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (2)
7-11: Tighten types and declare return type for stability and DX
bookerLayoutshould be the known union, notstring.- Declare the return type to avoid widening and accidental breaks.
One safe in-file approach (until you wire the shared type):
+type BookerLayout = "month_view" | "week_view" | "column_view"; export const getPrefetchMonthCount = ( - bookerLayout: string, + bookerLayout: BookerLayout, bookerState: BookerState, firstMonth: number, secondMonth: number -) => { +): number | undefined => {
18-22: Add focused tests for decision matrixPlease add unit tests covering:
- same vs different months
- layouts: "week_view" | "column_view" | "month_view"
- states: "selecting_time" vs others
Key expectations from current logic:
- returns
undefinedwhen months aren’t different- returns
2for column view when months differ- returns
2for non-week views during selecting_time when months differI can draft a Jest test matrix if helpful.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts(2 hunks)packages/features/bookings/Booker/utils/areDifferentValidMonths.ts(1 hunks)packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts(1 hunks)packages/features/bookings/Booker/utils/isLastWeekOfMonth.ts(1 hunks)packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts(1 hunks)packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.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/features/bookings/Booker/utils/areDifferentValidMonths.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.tspackages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/utils/isLastWeekOfMonth.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/Booker/utils/areDifferentValidMonths.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.tspackages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/utils/isLastWeekOfMonth.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/Booker/utils/areDifferentValidMonths.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.tspackages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/utils/isLastWeekOfMonth.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/areDifferentValidMonths.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.tspackages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
🧬 Code graph analysis (3)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (3)
packages/features/bookings/Booker/types.ts (1)
BookerState(155-155)packages/features/bookings/Booker/utils/areDifferentValidMonths.ts (1)
areDifferentValidMonths(1-14)packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (3)
packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)packages/features/bookings/Booker/utils/isLastWeekOfMonth.ts (1)
isLastWeekOfMonth(3-9)packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts (1)
isMonthViewPrefetchEnabled(3-14)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (2)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
isPrefetchNextMonthEnabled(6-16)packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
getPrefetchMonthCount(6-23)
⏰ 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 (3)
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts (1)
3-14: Guard invalid month input and make return type explicitApply this minimal, safer refactor (unchanged from your original diff):
-export const isMonthViewPrefetchEnabled = (date: string, month: string | null) => { - const isValidDate = dayjs(date).isValid(); - const twoWeeksAfter = dayjs(month).startOf("month").add(2, "week"); - const isSameMonth = dayjs().isSame(dayjs(month), "month"); - const isAfter2Weeks = dayjs().isAfter(twoWeeksAfter); +export const isMonthViewPrefetchEnabled = (date: string, month: string | null): boolean => { + const isValidDate = dayjs(date).isValid(); + const monthD = month ? dayjs(month) : null; + if (!monthD || !monthD.isValid()) return false; // explicit and predictable + const twoWeeksAfter = monthD.startOf("month").add(2, "week"); + const now = dayjs(); + const isSameMonth = now.isSame(monthD, "month"); + const isAfter2Weeks = now.isAfter(twoWeeksAfter); if (isAfter2Weeks && (!isValidDate || isSameMonth)) { return true; } return false; }
- If you need “two weeks after start of month” to be inclusive, note that the
isSameOrAfterplugin isn’t currently extended. To use it, importisSameOrAfterfromdayjs/plugin/isSameOrAfterand calldayjs.extend(isSameOrAfter)before invoking it.packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (2)
12-12: Good guard against NaN/invalid inputsUsing
areDifferentValidMonths(...)to gate comparisons prevents the classicNaN !== NaNpitfall and aligns with prior decisions inusePrefetch. Looks solid.Based on learnings
1-1: BookerLayouts enum usage is correct
The import from@calcom/prisma/zod-utilsprovides a TypeScriptenum BookerLayoutswhose members (MONTH_VIEW,WEEK_VIEW,COLUMN_VIEW) map to the expected string values, so comparisons likebookerLayout === BookerLayouts.WEEK_VIEWwork as intended. No changes needed.Likely an incorrect or invalid review comment.
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts
Outdated
Show resolved
Hide resolved
- Add tests for areDifferentValidMonths: validates month comparison logic with edge cases - Add tests for isLastWeekOfMonth: covers week boundaries and date edge cases - Add tests for isMonthViewPrefetchEnabled: tests time-based prefetch logic with mocked time - Add tests for getPrefetchMonthCount: validates conditional month count logic for different layouts - Add tests for isPrefetchNextMonthEnabled: integration tests for orchestration function All tests use vitest framework with 51 test cases covering key scenarios and edge cases. Tests follow existing patterns from Cal.com codebase and use integration approach. Co-Authored-By: rajiv@cal.com <sahalrajiv6900@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/features/bookings/Booker/utils/areDifferentValidMonths.test.ts (1)
42-52: Enforce valid month range or rename functionFunction
areDifferentValidMonthsonly checks finiteness and inequality, despite its name implying month values must be within 0–11. This mismatch can mislead and cover up invalid inputs.
- Add range validation (
0 ≤ month ≤ 11) before comparing.- Or rename to reflect actual behavior (e.g.
areDifferentFiniteNumbers).packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts (2)
45-66: Clarify the "invalid-date" test case intent.Lines 49 and 56 use
"invalid-date"as the date parameter. While this tests error handling, the expected behavior for invalid dates is unclear:
- Should
isPrefetchNextMonthEnabledvalidate and reject invalid dates?- Does it fall through to month-view logic that ignores the date parameter?
- Is the test verifying that invalid dates don't crash the function?
Consider adding a comment explaining why
"invalid-date"is used and what behavior it's validating, or use a more descriptive test name like"should handle invalid date gracefully when checking time threshold".
90-95: Weak assertion: specify expected boolean value.Line 94 only checks that the result is a boolean, not which boolean value is expected. This makes the test less specific and potentially allows wrong behavior to pass.
Apply this diff to make the assertion more specific:
it("should handle null month parameter for MONTH_VIEW", () => { vi.setSystemTime(new Date("2024-01-20T12:00:00Z")); const result = isPrefetchNextMonthEnabled(BookerLayouts.MONTH_VIEW, "invalid-date", null); - expect(typeof result).toBe("boolean"); + expect(result).toBe(true); // or false, depending on expected behavior with null month });packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts (1)
89-92: Consider documenting valid month range expectations.The test uses
-1as a month value, which is outside the valid JavaScript month range (0-11). While the function handles this gracefully by checking if months differ, it's unclear whether:
- Negative months are intentionally supported
- This is testing defensive behavior for invalid input
- The valid range should be documented or enforced
Consider adding a comment or test name clarification:
- it("should handle negative month numbers if they are different", () => { + it("should handle out-of-range month numbers if they are different", () => { const result = getPrefetchMonthCount(BookerLayouts.COLUMN_VIEW, "selecting_date", -1, 5); expect(result).toBe(2); });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/features/bookings/Booker/utils/areDifferentValidMonths.test.ts(1 hunks)packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts(1 hunks)packages/features/bookings/Booker/utils/isLastWeekOfMonth.test.ts(1 hunks)packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.ts(1 hunks)packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.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/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.tspackages/features/bookings/Booker/utils/isLastWeekOfMonth.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.tspackages/features/bookings/Booker/utils/areDifferentValidMonths.test.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.tspackages/features/bookings/Booker/utils/isLastWeekOfMonth.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.tspackages/features/bookings/Booker/utils/areDifferentValidMonths.test.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.tspackages/features/bookings/Booker/utils/isLastWeekOfMonth.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.tspackages/features/bookings/Booker/utils/areDifferentValidMonths.test.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.tspackages/features/bookings/Booker/utils/areDifferentValidMonths.test.tspackages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts
🧬 Code graph analysis (5)
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.ts (1)
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts (1)
isMonthViewPrefetchEnabled(3-14)
packages/features/bookings/Booker/utils/isLastWeekOfMonth.test.ts (1)
packages/features/bookings/Booker/utils/isLastWeekOfMonth.ts (1)
isLastWeekOfMonth(3-9)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts (2)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
isPrefetchNextMonthEnabled(6-16)packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)
packages/features/bookings/Booker/utils/areDifferentValidMonths.test.ts (1)
packages/features/bookings/Booker/utils/areDifferentValidMonths.ts (1)
areDifferentValidMonths(1-14)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts (2)
packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
getPrefetchMonthCount(6-23)packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)
⏰ 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 (18)
packages/features/bookings/Booker/utils/areDifferentValidMonths.test.ts (4)
1-3: LGTM! Clean imports.The imports follow best practices with named exports as per coding guidelines.
5-12: LGTM! Comprehensive coverage of valid different months.The test cases appropriately cover different scenarios including month boundaries (0, 11) and wrap-around cases.
14-20: LGTM! Proper coverage of identical month scenarios.The tests correctly validate that identical month numbers return false, covering edge months and mid-range values.
22-40: LGTM! Thorough validation of invalid inputs.The tests comprehensively cover NaN, Infinity, and -Infinity cases, validating that
Number.isFinitecorrectly rejects these values. This aligns with the intentional NaN handling mentioned in the retrieved learnings to prevent NaN comparison issues.Based on learnings.
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.ts (4)
1-12: LGTM! Proper test setup with fake timers.The test setup correctly uses Vitest fake timers with proper cleanup in afterEach, ensuring tests are isolated and time-dependent behavior can be controlled.
14-32: LGTM! Comprehensive coverage of post-2-week scenarios.The tests correctly validate the function behavior when the current time is after the 2-week threshold, covering invalid dates with same month, valid dates with same month, and valid dates with different months.
34-41: LGTM! Correctly validates early-month behavior.The tests properly verify that the function returns false before the 2-week threshold regardless of date validity, which matches the implementation logic.
58-63: LGTM! Good coverage across different months.The test correctly validates that the function works consistently throughout the year, verifying both invalid and valid date scenarios in June.
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts (4)
1-14: LGTM! Proper test setup with fake timers.The setup correctly uses fake timers for tests involving time-dependent logic, with proper cleanup in afterEach.
16-31: LGTM! Good coverage of WEEK_VIEW layout.Tests cover both true and false cases for last week detection, including multiple months.
33-43: LGTM! COLUMN_VIEW tests align with WEEK_VIEW behavior.Tests appropriately verify that COLUMN_VIEW follows the same last-week logic as WEEK_VIEW.
97-105: LGTM! Comprehensive leap year edge case coverage.Tests appropriately validate both leap year and non-leap year February edge cases.
packages/features/bookings/Booker/utils/getPrefetchMonthCount.test.ts (4)
1-23: LGTM! COLUMN_VIEW tests are comprehensive.Tests correctly verify that COLUMN_VIEW returns 2 for different months and undefined for same months, regardless of bookerState.
25-40: LGTM! WEEK_VIEW behavior is correct.Tests appropriately validate that WEEK_VIEW always returns undefined, regardless of state or month difference.
42-64: LGTM! State-dependent MONTH_VIEW logic is well tested.Tests correctly distinguish behavior between
selecting_timestate (returns 2 for different months) and other states (returns undefined).
66-81: LGTM! Invalid input handling aligns with learned behavior.Tests validate that NaN and Infinity inputs correctly return undefined, which aligns with the retrieved learning about intentional NaN handling to prevent incorrect month count calculations.
Based on learnings.
packages/features/bookings/Booker/utils/isLastWeekOfMonth.test.ts (2)
1-31: LGTM! Comprehensive coverage of last-week scenarios.Tests thoroughly validate true cases including:
- 31-day months (January)
- 30-day months (April)
- February leap year (2024)
- February non-leap year (2025)
33-45: LGTM! False case coverage is appropriate.Tests correctly validate that dates in the middle and beginning of months return false.
packages/features/bookings/Booker/utils/isLastWeekOfMonth.test.ts
Outdated
Show resolved
Hide resolved
packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.test.ts
Show resolved
Hide resolved
E2E results are ready! |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
6-14: Add explicit return type and consider using string literals.The function lacks an explicit return type annotation. Additionally, if
BookerLayoutsis only a type (not a runtime constant), consider using string literals ("week_view","column_view","month_view") instead of property access.export const isPrefetchNextMonthEnabled = ( bookerLayout: string, date: string, dateMonth: number, monthAfterAddingExtraDays: number, monthAfterAddingExtraDaysColumnView: number, month: string | null, extraDays?: number -) => { +): boolean => {
🧹 Nitpick comments (2)
packages/features/bookings/Booker/utils/isMonthChange.ts (1)
1-11: Add explicit return type for clarity.The function logic is correct and properly handles NaN values as intended. Consider adding an explicit
: booleanreturn type annotation to improve type safety and documentation.Based on learnings
-export const isMonthChange = (currentMonth: number, nextMonth: number) => { +export const isMonthChange = (currentMonth: number, nextMonth: number): boolean => { if (isNaN(currentMonth) || isNaN(nextMonth)) { return false; } if (currentMonth === nextMonth) { return false; } return true; };packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
23-26: Simplify: directly return the boolean value.The intermediate variable
shouldPrefetchNextMonthis unnecessary.if (bookerLayout === BookerLayouts.MONTH_VIEW || bookerLayout === "mobile") { - const shouldPrefetchNextMonth = isMonthViewPrefetchEnabled(date, month); - return shouldPrefetchNextMonth; + return isMonthViewPrefetchEnabled(date, month); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts(2 hunks)packages/features/bookings/Booker/utils/isMonthChange.ts(1 hunks)packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.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/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/components/hooks/usePrefetch.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts
🧬 Code graph analysis (2)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (2)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
isPrefetchNextMonthEnabled(6-29)packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
getPrefetchMonthCount(6-23)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (3)
packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)packages/features/bookings/Booker/utils/isMonthChange.ts (1)
isMonthChange(1-11)packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts (1)
isMonthViewPrefetchEnabled(3-14)
🔇 Additional comments (4)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (3)
4-5: LGTM: Clean refactoring with clear utility imports.The refactoring successfully extracts complex prefetch logic into focused utility functions, improving maintainability and testability.
26-40: LGTM: Proper delegation to utility functions.The hook correctly delegates prefetch decision logic to
isPrefetchNextMonthEnabledand month count calculation togetPrefetchMonthCount, passing all necessary parameters.
42-45: No changes needed: allusePrefetchconsumers already destructuremonthCount.packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
1-1: BookerLayouts is a runtime enum in @calcom/prisma/zod-utils; usingBookerLayouts.WEEK_VIEW(and others) is valid.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/bookings/Booker/utils/isMonthChange.ts (1)
1-11: Good NaN guard; consider unifying with areDifferentValidMonths and add explicit return typeThis logic mirrors areDifferentValidMonths. Consolidate to one util to avoid divergence. Also make the return type explicit for clarity.
Based on learnings
-export const isMonthChange = (currentMonth: number, nextMonth: number) => { +export const isMonthChange = (currentMonth: number, nextMonth: number): boolean => { if (isNaN(currentMonth) || isNaN(nextMonth)) { return false; } if (currentMonth === nextMonth) { return false; } return true; };packages/features/bookings/Booker/components/hooks/usePrefetch.ts (1)
26-40: LGTM: delegated logic and shape of returnHook cleanly delegates to utilities and returns { prefetchNextMonth, monthCount }. Consider narrowing bookerLayout.layout to a union (BookerLayouts | "mobile") in UsePrefetchParams when feasible.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts(2 hunks)packages/features/bookings/Booker/utils/isMonthChange.ts(1 hunks)packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.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/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/isMonthChange.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.tspackages/features/bookings/Booker/components/hooks/usePrefetch.ts
🧬 Code graph analysis (2)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (3)
packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)packages/features/bookings/Booker/utils/isMonthChange.ts (1)
isMonthChange(1-11)packages/features/bookings/Booker/utils/isMonthViewPrefetchEnabled.ts (1)
isMonthViewPrefetchEnabled(3-14)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (2)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
isPrefetchNextMonthEnabled(6-29)packages/features/bookings/Booker/utils/getPrefetchMonthCount.ts (1)
getPrefetchMonthCount(6-23)
⏰ 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
🔇 Additional comments (2)
packages/features/bookings/Booker/components/hooks/usePrefetch.ts (1)
4-5: LGTM: logic extraction improves readabilityClear separation of concerns with utility imports.
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
1-29: Correct runtime usage of BookerLayouts enum
BookerLayouts is an exported enum in @calcom/prisma/zod-utils and exists at runtime, so the current===comparisons are safe and all branches return a boolean. No changes required.
- Add tests for new isMonthChange helper function - Update isPrefetchNextMonthEnabled tests to match new signature - Tests now use pre-calculated month values instead of date strings All 39 tests passing locally Co-Authored-By: rajiv@cal.com <sahalrajiv6900@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts (1)
129-167: Clarify DATE validity and isolate invalid‐date behavior in MONTH_VIEW tests
- Replace
"invalid-date"with a valid date (e.g."2024-01-01") when testing the normal prefetch threshold, so the test name aligns with the scenario.- Add a distinct test (and name) that passes
"invalid-date"to explicitly verify the invalid‐date branch in isMonthViewPrefetchEnabled.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/features/bookings/Booker/utils/isMonthChange.test.ts(1 hunks)packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.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/features/bookings/Booker/utils/isMonthChange.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/bookings/Booker/utils/isMonthChange.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/bookings/Booker/utils/isMonthChange.test.tspackages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
📚 Learning: 2025-09-18T13:38:50.958Z
Learnt from: hariombalhara
PR: calcom/cal.com#23913
File: packages/features/bookings/Booker/components/hooks/usePrefetch.ts:43-50
Timestamp: 2025-09-18T13:38:50.958Z
Learning: In usePrefetch hook in packages/features/bookings/Booker/components/hooks/usePrefetch.ts, the !isNaN() checks before comparing monthAfterAdding1Month !== monthAfterAddingExtraDaysColumnView are intentional to prevent the NaN !== NaN issue where NaN values would incorrectly be considered different, leading to wrong monthCount calculations.
Applied to files:
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts
🧬 Code graph analysis (2)
packages/features/bookings/Booker/utils/isMonthChange.test.ts (1)
packages/features/bookings/Booker/utils/isMonthChange.ts (1)
isMonthChange(1-11)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts (2)
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.ts (1)
isPrefetchNextMonthEnabled(6-29)packages/embeds/embed-core/src/types.ts (1)
BookerLayouts(6-6)
⏰ 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: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (6)
packages/features/bookings/Booker/utils/isMonthChange.test.ts (1)
1-44: LGTM! Comprehensive test coverage.The test suite thoroughly covers all key scenarios for
isMonthChange:
- Month transitions (consecutive, non-consecutive, year wrap)
- Same-month cases
- Invalid input handling (NaN)
The test organization is clear and aligns well with the implementation logic.
packages/features/bookings/Booker/utils/isPrefetchNextMonthEnabled.test.ts (5)
9-15: LGTM! Proper timer management.The timer setup and teardown using
vi.useFakeTimers()andvi.useRealTimers()is correctly implemented for time-dependent tests.
17-91: LGTM! Thorough WEEK_VIEW test coverage.The test cases comprehensively cover:
- Month change with extraDays
- No month change with extraDays
- Undefined extraDays
- Zero extraDays
All scenarios properly validate the
!!extraDays && isMonthChange(...)logic.
93-127: LGTM! COLUMN_VIEW tests are correct.The tests properly validate the month-change logic specific to COLUMN_VIEW (using
monthAfterAddingExtraDaysColumnView).
169-188: Same verification needed for mobile layout test.Similar to the MONTH_VIEW tests, line 173 uses
"invalid-date". Please verify if this is intentional testing of edge-case behavior or if valid test data should be used instead.
190-207: LGTM! Unknown layout fallback is tested.The test correctly verifies that unrecognized layouts return
false, confirming the default fallback behavior.
| month: string | null, | ||
| extraDays?: number | ||
| ) => { | ||
| if (bookerLayout === BookerLayouts.WEEK_VIEW) { |
There was a problem hiding this comment.
no logic is changed for this function as well, isMonthChange does the same thing as before i.e. takes in current month and next month, checks if they are valid or not and then checks if current month and next month are same, if same it returns false which means we should not prefetch next, if they are different month then we return true for us to prefetch next month
4 participants
What does this PR do?
This PR refactors the
usePrefetchand abstracts the core logic into two main functionsgetPrefetchMonthCountandisPrefetchNextMonthEnabled