Fix/persist slot reservation - #29792
Open
Ashid332 wants to merge 4 commits into
Open
Conversation
- Replaces flawed containment logic with strict interval intersection math - Centralizes Prisma where-clause generation into prismaIntervalOverlap.ts - Migrates getBusyTimes, BookingRepository, and HolidayRepository to new utility - Adds comprehensive behavioral and unit test coverage for overlapping scenarios
fix: resolve busy time overlap logic and centralize interval queries
Contributor
|
Welcome to Cal.diy, @Ashid332! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
Contributor
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a reusable Prisma interval-overlap clause helper with strict and inclusive modes plus validation tests. Booking, busy-time, and holiday queries now use the helper for date-range filtering, with booking-limit overlap tests. The slot reservation hook now persists identifiers in 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/features/bookings/Booker/useSlotReservationId.test.ts (1)
8-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove comments that simply restate code.
As per coding guidelines, do not add comments that simply restate what the code does. The intent here is already obvious from
set(null...).♻️ Proposed refactor
- // Clear the module level variable by setting it to null - const [, set] = useSlotReservationId(); - set(null as unknown as string); + const [, set] = useSlotReservationId(); + set(null as unknown as string);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/bookings/Booker/useSlotReservationId.test.ts` around lines 8 - 10, Remove the redundant comment above the setter call in the useSlotReservationId test; keep the existing set(null as unknown as string) behavior unchanged.Source: Coding guidelines
packages/features/bookings/Booker/useSlotReservationId.ts (2)
10-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSafely check
sessionStorageavailability.While the
try/catchblock successfully swallowsReferenceErrorexceptions during Server-Side Rendering (SSR), it's a best practice to explicitly check ifsessionStorageis defined before attempting to access it.♻️ Proposed refactor
- try { - sessionStorage.setItem("slotReservationId", uid); - } catch (e) { - // Ignore sessionStorage errors (e.g. in incognito or iframe with partitioned storage) - } + if (typeof sessionStorage !== "undefined") { + try { + sessionStorage.setItem("slotReservationId", uid); + } catch (e) { + // Ignore sessionStorage errors (e.g. in incognito or iframe with partitioned storage) + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/bookings/Booker/useSlotReservationId.ts` around lines 10 - 15, Update the sessionStorage write in the reservation ID setter to first verify that sessionStorage is defined before calling setItem, while retaining the existing try/catch for storage access failures in environments such as SSR, incognito mode, or partitioned iframes.
16-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse early returns and safely access
sessionStorage.As per coding guidelines, use early returns to reduce nesting. Additionally, check if
sessionStorageis defined before accessing it to prevent throwing and catchingReferenceErrorduring Server-Side Rendering (SSR).♻️ Proposed refactor
- function get() { - if (!slotReservationId) { - try { - slotReservationId = sessionStorage.getItem("slotReservationId"); - } catch (e) { - // Ignore sessionStorage errors - } - } - return slotReservationId; - } + function get() { + if (slotReservationId) return slotReservationId; + + if (typeof sessionStorage !== "undefined") { + try { + slotReservationId = sessionStorage.getItem("slotReservationId"); + } catch (e) { + // Ignore sessionStorage errors + } + } + + return slotReservationId; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/features/bookings/Booker/useSlotReservationId.ts` around lines 16 - 25, Update get to return immediately when slotReservationId is already set, then verify sessionStorage is defined before calling getItem so SSR does not rely on catching a ReferenceError; preserve the existing error-swallowing behavior for storage access failures.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/features/bookings/Booker/useSlotReservationId.test.ts`:
- Around line 18-54: Update the tests using useSlotReservationId so the first
destructured value is asserted directly as the returned string or null, not
invoked as a function. After set operations, re-invoke useSlotReservationId() to
obtain and assert the updated value, while retaining the sessionStorage
assertions. Remove redundant comments such as “Should not throw” without
changing the existing error-handling coverage.
In `@packages/features/busyTimes/services/getBusyTimes.test.ts`:
- Around line 462-474: Update the mock booking setup and limit-check request in
the test around createMockBookingResult and getBusyTimesForLimitChecks to use
native Date values instead of dayjs: construct the start/end dates with Date and
serialize request dates with toISOString(). Preserve the existing date values
and test behavior.
- Around line 402-422: Update setupPrismaMock to eliminate the `as any` cast by
using the inferred Prisma `findMany` argument type or a properly typed where
value, and replace all `dayjs` parsing/comparisons in this mock with native
`Date` construction and timestamp comparisons. Preserve the existing lt/gt
overlap filtering behavior and requested date values.
---
Nitpick comments:
In `@packages/features/bookings/Booker/useSlotReservationId.test.ts`:
- Around line 8-10: Remove the redundant comment above the setter call in the
useSlotReservationId test; keep the existing set(null as unknown as string)
behavior unchanged.
In `@packages/features/bookings/Booker/useSlotReservationId.ts`:
- Around line 10-15: Update the sessionStorage write in the reservation ID
setter to first verify that sessionStorage is defined before calling setItem,
while retaining the existing try/catch for storage access failures in
environments such as SSR, incognito mode, or partitioned iframes.
- Around line 16-25: Update get to return immediately when slotReservationId is
already set, then verify sessionStorage is defined before calling getItem so SSR
does not rely on catching a ReferenceError; preserve the existing
error-swallowing behavior for storage access failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0e53de3-8289-42dc-ad97-0e726b351146
📒 Files selected for processing (8)
packages/features/bookings/Booker/useSlotReservationId.test.tspackages/features/bookings/Booker/useSlotReservationId.tspackages/features/bookings/repositories/BookingRepository.tspackages/features/busyTimes/services/getBusyTimes.test.tspackages/features/busyTimes/services/getBusyTimes.tspackages/features/holidays/repositories/HolidayRepository.tspackages/lib/prismaIntervalOverlap.test.tspackages/lib/prismaIntervalOverlap.ts
Comment on lines
+18
to
+54
| it("should return null initially if sessionStorage is empty", () => { | ||
| const [get] = useSlotReservationId(); | ||
| expect(get()).toBeNull(); | ||
| }); | ||
|
|
||
| it("should persist slot reservation id to sessionStorage", () => { | ||
| const [get, set] = useSlotReservationId(); | ||
| set("test-uid-123"); | ||
|
|
||
| expect(get()).toBe("test-uid-123"); | ||
| expect(sessionStorage.getItem("slotReservationId")).toBe("test-uid-123"); | ||
| }); | ||
|
|
||
| it("should retrieve slot reservation id from sessionStorage on initialization", () => { | ||
| sessionStorage.setItem("slotReservationId", "test-uid-456"); | ||
|
|
||
| const [get] = useSlotReservationId(); | ||
| expect(get()).toBe("test-uid-456"); | ||
| }); | ||
|
|
||
| it("should not crash if sessionStorage throws an error (e.g. in incognito iframe)", () => { | ||
| // Mock sessionStorage to throw errors to simulate partitioned storage security errors | ||
| const setItemMock = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => { | ||
| throw new Error("SecurityError"); | ||
| }); | ||
| const getItemMock = vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => { | ||
| throw new Error("SecurityError"); | ||
| }); | ||
|
|
||
| const [get, set] = useSlotReservationId(); | ||
|
|
||
| // Should not throw | ||
| expect(() => set("test-uid-789")).not.toThrow(); | ||
|
|
||
| // Memory fallback should still work within the same session | ||
| expect(get()).toBe("test-uid-789"); | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix TypeError: get is not a function in tests and remove redundant comments.
The useSlotReservationId() hook returns [get(), set] as const, meaning the first element is the evaluated string | null value, not a function. Destructuring it as [get] and invoking get() will cause the tests to crash with a TypeError.
Additionally, as per coding guidelines, remove comments like // Should not throw that simply restate the code.
Update the tests to correctly evaluate the returned primitive value and re-invoke the hook to fetch the updated value after set operations.
🐛 Proposed fixes
- it("should return null initially if sessionStorage is empty", () => {
- const [get] = useSlotReservationId();
- expect(get()).toBeNull();
- });
+ it("should return null initially if sessionStorage is empty", () => {
+ const [uid] = useSlotReservationId();
+ expect(uid).toBeNull();
+ });
- it("should persist slot reservation id to sessionStorage", () => {
- const [get, set] = useSlotReservationId();
- set("test-uid-123");
-
- expect(get()).toBe("test-uid-123");
- expect(sessionStorage.getItem("slotReservationId")).toBe("test-uid-123");
- });
+ it("should persist slot reservation id to sessionStorage", () => {
+ const [, set] = useSlotReservationId();
+ set("test-uid-123");
+
+ const [uid] = useSlotReservationId();
+ expect(uid).toBe("test-uid-123");
+ expect(sessionStorage.getItem("slotReservationId")).toBe("test-uid-123");
+ });
- it("should retrieve slot reservation id from sessionStorage on initialization", () => {
- sessionStorage.setItem("slotReservationId", "test-uid-456");
-
- const [get] = useSlotReservationId();
- expect(get()).toBe("test-uid-456");
- });
+ it("should retrieve slot reservation id from sessionStorage on initialization", () => {
+ sessionStorage.setItem("slotReservationId", "test-uid-456");
+
+ const [uid] = useSlotReservationId();
+ expect(uid).toBe("test-uid-456");
+ });
- it("should not crash if sessionStorage throws an error (e.g. in incognito iframe)", () => {
- // Mock sessionStorage to throw errors to simulate partitioned storage security errors
- const setItemMock = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => {
- throw new Error("SecurityError");
- });
- const getItemMock = vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => {
- throw new Error("SecurityError");
- });
-
- const [get, set] = useSlotReservationId();
-
- // Should not throw
- expect(() => set("test-uid-789")).not.toThrow();
-
- // Memory fallback should still work within the same session
- expect(get()).toBe("test-uid-789");
+ it("should not crash if sessionStorage throws an error (e.g. in incognito iframe)", () => {
+ // Mock sessionStorage to throw errors to simulate partitioned storage security errors
+ const setItemMock = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => {
+ throw new Error("SecurityError");
+ });
+ const getItemMock = vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => {
+ throw new Error("SecurityError");
+ });
+
+ const [, set] = useSlotReservationId();
+
+ expect(() => set("test-uid-789")).not.toThrow();
+
+ // Memory fallback should still work within the same session
+ const [uid] = useSlotReservationId();
+ expect(uid).toBe("test-uid-789");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("should return null initially if sessionStorage is empty", () => { | |
| const [get] = useSlotReservationId(); | |
| expect(get()).toBeNull(); | |
| }); | |
| it("should persist slot reservation id to sessionStorage", () => { | |
| const [get, set] = useSlotReservationId(); | |
| set("test-uid-123"); | |
| expect(get()).toBe("test-uid-123"); | |
| expect(sessionStorage.getItem("slotReservationId")).toBe("test-uid-123"); | |
| }); | |
| it("should retrieve slot reservation id from sessionStorage on initialization", () => { | |
| sessionStorage.setItem("slotReservationId", "test-uid-456"); | |
| const [get] = useSlotReservationId(); | |
| expect(get()).toBe("test-uid-456"); | |
| }); | |
| it("should not crash if sessionStorage throws an error (e.g. in incognito iframe)", () => { | |
| // Mock sessionStorage to throw errors to simulate partitioned storage security errors | |
| const setItemMock = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => { | |
| throw new Error("SecurityError"); | |
| }); | |
| const getItemMock = vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => { | |
| throw new Error("SecurityError"); | |
| }); | |
| const [get, set] = useSlotReservationId(); | |
| // Should not throw | |
| expect(() => set("test-uid-789")).not.toThrow(); | |
| // Memory fallback should still work within the same session | |
| expect(get()).toBe("test-uid-789"); | |
| it("should return null initially if sessionStorage is empty", () => { | |
| const [uid] = useSlotReservationId(); | |
| expect(uid).toBeNull(); | |
| }); | |
| it("should persist slot reservation id to sessionStorage", () => { | |
| const [, set] = useSlotReservationId(); | |
| set("test-uid-123"); | |
| const [uid] = useSlotReservationId(); | |
| expect(uid).toBe("test-uid-123"); | |
| expect(sessionStorage.getItem("slotReservationId")).toBe("test-uid-123"); | |
| }); | |
| it("should retrieve slot reservation id from sessionStorage on initialization", () => { | |
| sessionStorage.setItem("slotReservationId", "test-uid-456"); | |
| const [uid] = useSlotReservationId(); | |
| expect(uid).toBe("test-uid-456"); | |
| }); | |
| it("should not crash if sessionStorage throws an error (e.g. in incognito iframe)", () => { | |
| // Mock sessionStorage to throw errors to simulate partitioned storage security errors | |
| const setItemMock = vi.spyOn(Storage.prototype, "setItem").mockImplementation(() => { | |
| throw new Error("SecurityError"); | |
| }); | |
| const getItemMock = vi.spyOn(Storage.prototype, "getItem").mockImplementation(() => { | |
| throw new Error("SecurityError"); | |
| }); | |
| const [, set] = useSlotReservationId(); | |
| expect(() => set("test-uid-789")).not.toThrow(); | |
| // Memory fallback should still work within the same session | |
| const [uid] = useSlotReservationId(); | |
| expect(uid).toBe("test-uid-789"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/features/bookings/Booker/useSlotReservationId.test.ts` around lines
18 - 54, Update the tests using useSlotReservationId so the first destructured
value is asserted directly as the returned string or null, not invoked as a
function. After set operations, re-invoke useSlotReservationId() to obtain and
assert the updated value, while retaining the sessionStorage assertions. Remove
redundant comments such as “Should not throw” without changing the existing
error-handling coverage.
Source: Coding guidelines
Comment on lines
+402
to
+422
| const requestedStart = dayjs("2026-07-15T10:00:00Z"); | ||
| const requestedEnd = dayjs("2026-07-15T11:00:00Z"); | ||
|
|
||
| const setupPrismaMock = (existingBookings: any[]) => { | ||
| vi.mocked(prisma.booking.findMany).mockImplementation(async (args) => { | ||
| const where = args?.where as any; | ||
| const startTimeCondition = where?.startTime; | ||
| const endTimeCondition = where?.endTime; | ||
|
|
||
| return existingBookings.filter((booking) => { | ||
| // Emulate Prisma's lt/gt logic | ||
| const startsBeforeEnd = startTimeCondition?.lt | ||
| ? dayjs(booking.startTime).isBefore(dayjs(startTimeCondition.lt)) | ||
| : true; | ||
| const endsAfterStart = endTimeCondition?.gt | ||
| ? dayjs(booking.endTime).isAfter(dayjs(endTimeCondition.gt)) | ||
| : true; | ||
| return startsBeforeEnd && endsAfterStart; | ||
| }); | ||
| }); | ||
| }; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid using any and replace dayjs with native Date.
As per coding guidelines:
- "Never use
as any- use proper type-safe solutions instead" - "Use
date-fnsor nativeDateinstead of Day.js when timezone awareness isn't needed"
The use of as any bypasses type safety, and dayjs is unnecessarily used here to parse standard UTC timestamps and compare dates.
♻️ Proposed fixes
- const requestedStart = dayjs("2026-07-15T10:00:00Z");
- const requestedEnd = dayjs("2026-07-15T11:00:00Z");
+ const requestedStart = new Date("2026-07-15T10:00:00Z");
+ const requestedEnd = new Date("2026-07-15T11:00:00Z");
- const setupPrismaMock = (existingBookings: any[]) => {
+ const setupPrismaMock = (existingBookings: ReturnType<typeof createMockBookingResult>[]) => {
vi.mocked(prisma.booking.findMany).mockImplementation(async (args) => {
- const where = args?.where as any;
+ const where = args?.where as {
+ startTime?: { lt?: Date };
+ endTime?: { gt?: Date };
+ };
const startTimeCondition = where?.startTime;
const endTimeCondition = where?.endTime;
return existingBookings.filter((booking) => {
// Emulate Prisma's lt/gt logic
const startsBeforeEnd = startTimeCondition?.lt
- ? dayjs(booking.startTime).isBefore(dayjs(startTimeCondition.lt))
+ ? booking.startTime.getTime() < new Date(startTimeCondition.lt).getTime()
: true;
const endsAfterStart = endTimeCondition?.gt
- ? dayjs(booking.endTime).isAfter(dayjs(endTimeCondition.gt))
+ ? booking.endTime.getTime() > new Date(endTimeCondition.gt).getTime()
: true;
return startsBeforeEnd && endsAfterStart;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const requestedStart = dayjs("2026-07-15T10:00:00Z"); | |
| const requestedEnd = dayjs("2026-07-15T11:00:00Z"); | |
| const setupPrismaMock = (existingBookings: any[]) => { | |
| vi.mocked(prisma.booking.findMany).mockImplementation(async (args) => { | |
| const where = args?.where as any; | |
| const startTimeCondition = where?.startTime; | |
| const endTimeCondition = where?.endTime; | |
| return existingBookings.filter((booking) => { | |
| // Emulate Prisma's lt/gt logic | |
| const startsBeforeEnd = startTimeCondition?.lt | |
| ? dayjs(booking.startTime).isBefore(dayjs(startTimeCondition.lt)) | |
| : true; | |
| const endsAfterStart = endTimeCondition?.gt | |
| ? dayjs(booking.endTime).isAfter(dayjs(endTimeCondition.gt)) | |
| : true; | |
| return startsBeforeEnd && endsAfterStart; | |
| }); | |
| }); | |
| }; | |
| const requestedStart = new Date("2026-07-15T10:00:00Z"); | |
| const requestedEnd = new Date("2026-07-15T11:00:00Z"); | |
| const setupPrismaMock = (existingBookings: ReturnType<typeof createMockBookingResult>[]) => { | |
| vi.mocked(prisma.booking.findMany).mockImplementation(async (args) => { | |
| const where = args?.where as { | |
| startTime?: { lt?: Date }; | |
| endTime?: { gt?: Date }; | |
| }; | |
| const startTimeCondition = where?.startTime; | |
| const endTimeCondition = where?.endTime; | |
| return existingBookings.filter((booking) => { | |
| // Emulate Prisma's lt/gt logic | |
| const startsBeforeEnd = startTimeCondition?.lt | |
| ? booking.startTime.getTime() < new Date(startTimeCondition.lt).getTime() | |
| : true; | |
| const endsAfterStart = endTimeCondition?.gt | |
| ? booking.endTime.getTime() > new Date(endTimeCondition.gt).getTime() | |
| : true; | |
| return startsBeforeEnd && endsAfterStart; | |
| }); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/features/busyTimes/services/getBusyTimes.test.ts` around lines 402 -
422, Update setupPrismaMock to eliminate the `as any` cast by using the inferred
Prisma `findMany` argument type or a properly typed where value, and replace all
`dayjs` parsing/comparisons in this mock with native `Date` construction and
timestamp comparisons. Preserve the existing lt/gt overlap filtering behavior
and requested date values.
Source: Coding guidelines
Comment on lines
+462
to
+474
| const mockBooking = createMockBookingResult({ | ||
| startTime: dayjs(existingStart).toDate(), | ||
| endTime: dayjs(existingEnd).toDate(), | ||
| }); | ||
| setupPrismaMock([mockBooking]); | ||
|
|
||
| const busyTimesService = getBusyTimesService(); | ||
| const busyTimes = await busyTimesService.getBusyTimesForLimitChecks({ | ||
| userIds: [1], | ||
| eventTypeId: 1, | ||
| startDate: requestedStart.format(), | ||
| endDate: requestedEnd.format(), | ||
| // PER_YEAR is explicitly excluded from date range expansion in getStartEndDateforLimitCheck |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace dayjs with native Date.
Consistent with the guideline to avoid dayjs when timezone awareness isn't needed, you can use native Date constructors and toISOString() here.
♻️ Proposed fix
- const mockBooking = createMockBookingResult({
- startTime: dayjs(existingStart).toDate(),
- endTime: dayjs(existingEnd).toDate(),
- });
+ const mockBooking = createMockBookingResult({
+ startTime: new Date(existingStart),
+ endTime: new Date(existingEnd),
+ });
setupPrismaMock([mockBooking]);
const busyTimesService = getBusyTimesService();
const busyTimes = await busyTimesService.getBusyTimesForLimitChecks({
userIds: [1],
eventTypeId: 1,
- startDate: requestedStart.format(),
- endDate: requestedEnd.format(),
+ startDate: requestedStart.toISOString(),
+ endDate: requestedEnd.toISOString(),
// PER_YEAR is explicitly excluded from date range expansion in getStartEndDateforLimitCheck📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const mockBooking = createMockBookingResult({ | |
| startTime: dayjs(existingStart).toDate(), | |
| endTime: dayjs(existingEnd).toDate(), | |
| }); | |
| setupPrismaMock([mockBooking]); | |
| const busyTimesService = getBusyTimesService(); | |
| const busyTimes = await busyTimesService.getBusyTimesForLimitChecks({ | |
| userIds: [1], | |
| eventTypeId: 1, | |
| startDate: requestedStart.format(), | |
| endDate: requestedEnd.format(), | |
| // PER_YEAR is explicitly excluded from date range expansion in getStartEndDateforLimitCheck | |
| const mockBooking = createMockBookingResult({ | |
| startTime: new Date(existingStart), | |
| endTime: new Date(existingEnd), | |
| }); | |
| setupPrismaMock([mockBooking]); | |
| const busyTimesService = getBusyTimesService(); | |
| const busyTimes = await busyTimesService.getBusyTimesForLimitChecks({ | |
| userIds: [1], | |
| eventTypeId: 1, | |
| startDate: requestedStart.toISOString(), | |
| endDate: requestedEnd.toISOString(), | |
| // PER_YEAR is explicitly excluded from date range expansion in getStartEndDateforLimitCheck |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/features/busyTimes/services/getBusyTimes.test.ts` around lines 462 -
474, Update the mock booking setup and limit-check request in the test around
createMockBookingResult and getBusyTimesForLimitChecks to use native Date values
instead of dayjs: construct the start/end dates with Date and serialize request
dates with toISOString(). Preserve the existing date values and test behavior.
Source: Coding guidelines
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist