perf: improve checkIsAvailable in getAvailable slots #18352

Udit-takkar · 2024-12-23T08:31:26Z

refactor: Update booking query to use userId and optimize availability

Changed the booking query to use userId instead of nested user object for filtering active bookings.
Refactored availability check logic to improve readability and performance by using valueOf() for time comparisons and consolidating multiple checks into a single return statement.

What does this PR do?

Fixes #XXXX (GitHub issue number)
Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)

Mandatory Tasks (DO NOT REMOVE)

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

How should this be tested?

…y check logic - Changed the booking query to use userId instead of nested user object for filtering active bookings. - Refactored availability check logic to improve readability and performance by using valueOf() for time comparisons and consolidating multiple checks into a single return statement.

graphite-app · 2024-12-23T08:35:11Z

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (12/23/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (12/23/24)

1 label was added to this PR based on Keith Williams's automation.

"Add foundation team as reviewer" took an action on this PR • (12/23/24)

1 reviewer was added to this PR based on Keith Williams's automation.

Udit-takkar · 2024-12-23T08:35:13Z

packages/trpc/server/routers/viewer/slots/util.ts

-  const currentBookingsAllUsers = await monitorCallbackAsync(
-    getExistingBookings,
-    startTimeDate,
-    endTimeDate,
-    eventType,
-    sharedQuery,
-    usersWithCredentials,
-    allUserIds
-  );
-
-  const outOfOfficeDaysAllUsers = await monitorCallbackAsync(
-    getOOODates,
-    startTimeDate,
-    endTimeDate,
-    allUserIds
-  );


both of them don't depend on each other so we can use Promise.all()

alishaz-polymath

Test Failing, but changes LGTM 🚀
Great improvement in readability! 👏

packages/lib/server/repository/booking.ts

vercel · 2024-12-23T09:11:51Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
cal	⬜️ Ignored (Inspect)	Visit Preview		Dec 27, 2024 2:17pm
calcom-web-canary	⬜️ Ignored (Inspect)	Visit Preview		Dec 27, 2024 2:17pm

emrysal · 2024-12-23T09:27:00Z

packages/trpc/server/routers/viewer/slots/util.ts


  return busy.every((busyTime) => {
-    const startTime = dayjs.utc(busyTime.start).utc();
-    const endTime = dayjs.utc(busyTime.end);
+    const busyStartValue = dayjs.utc(busyTime.start).valueOf();


Using toDate is preferred here, it's auto-cast to valueOf()

github-actions · 2024-12-23T09:34:07Z

E2E results are ready!

Workflow #42480.1 latest results

hariombalhara · 2024-12-27T07:25:36Z

packages/trpc/server/routers/viewer/slots/util.ts

+  const slotStartDate = time.utc().toDate();
+  const slotEndDate = time.add(eventLength, "minutes").utc().toDate();

  return busy.every((busyTime) => {
-    const startTime = dayjs.utc(busyTime.start).utc();
-    const endTime = dayjs.utc(busyTime.end);
+    const busyStartDate = dayjs.utc(busyTime.start).toDate();
+    const busyEndDate = dayjs.utc(busyTime.end).toDate();

-    if (endTime.isBefore(slotStartTime) || startTime.isAfter(slotEndTime)) {
+    // First check if there's any overlap at all
+    // If busy period ends before slot starts or starts after slot ends, there's no overlap
+    if (busyEndDate <= slotStartDate || busyStartDate >= slotEndDate) {
      return true;
    }

-    if (slotStartTime.isBetween(startTime, endTime, null, "[)")) {
-      return false;
-    } else if (slotEndTime.isBetween(startTime, endTime, null, "(]")) {
+    // Now check all possible overlap scenarios:
+
+    // 1. Slot start falls within busy period (inclusive start, exclusive end)
+    if (slotStartDate >= busyStartDate && slotStartDate < busyEndDate) {
      return false;
    }

-    // Check if start times are the same
-    if (time.utc().isBetween(startTime, endTime, null, "[)")) {
+    // 2. Slot end falls within busy period (exclusive start, inclusive end)
+    if (slotEndDate > busyStartDate && slotEndDate <= busyEndDate) {
      return false;
    }
-    // Check if slot end time is between start and end time
-    else if (slotEndTime.isBetween(startTime, endTime)) {
+
+    // 3. Busy period completely contained within slot
+    if (busyStartDate >= slotStartDate && busyEndDate <= slotEndDate) {


Do we have unit tests for this? We should add checkIfAvailable.timezone.test.ts that would ensure same code is tested in different timezones

If it isn't strictly dependent(which I think is the case), we can merge other changes and merge this one after unit tests

Udit-takkar · 2024-12-27T08:35:50Z

@hariombalhara opened this new PR #18382 without checkIsAvailable changes

* refactor: Update booking query to use userId and optimize availability check logic - Changed the booking query to use userId instead of nested user object for filtering active bookings. - Refactored availability check logic to improve readability and performance by using valueOf() for time comparisons and consolidating multiple checks into a single return statement. * fix: update tests * chore: use toDate() * tests: add checkavailable unit tests

graphite-app bot requested a review from a team December 23, 2024 08:31

dosubot bot added the 💻 refactor label Dec 23, 2024

Udit-takkar changed the title ~~refactor: Update booking query to use userId and optimize availabilit…~~ perf: improve getAvailable slots Dec 23, 2024

keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Dec 23, 2024

Udit-takkar commented Dec 23, 2024

View reviewed changes

Udit-takkar added the performance area: performance, page load, slow, slow endpoints, loading screen, unresponsive label Dec 23, 2024

vercel bot deployed to Preview – api December 23, 2024 08:39 View deployment

alishaz-polymath previously approved these changes Dec 23, 2024

View reviewed changes

packages/lib/server/repository/booking.ts Outdated Show resolved Hide resolved

graphite-app bot added the ready-for-e2e label Dec 23, 2024

Udit-takkar added the high-risk Requires approval by Foundation team label Dec 23, 2024

graphite-app bot requested a review from a team December 23, 2024 09:09

fix: update tests

c78443d

Udit-takkar dismissed alishaz-polymath’s stale review via c78443d December 23, 2024 09:11

vercel bot deployed to Preview – api December 23, 2024 09:21 View deployment

emrysal reviewed Dec 23, 2024

View reviewed changes

chore: use toDate()

3ce9700

vercel bot deployed to Preview – api December 23, 2024 10:17 View deployment

Udit-takkar requested a review from emrysal December 23, 2024 10:34

PeerRich added the High priority Created by Linear-GitHub Sync label Dec 26, 2024

hariombalhara reviewed Dec 27, 2024

View reviewed changes

Udit-takkar mentioned this pull request Dec 27, 2024

perf: getAvailableSlots improvements - 1 #18382

Merged

3 tasks

Merge branch 'main' into perf/get-slots-improv

e331430

vercel bot deployed to Preview – api December 27, 2024 14:14 View deployment

tests: add checkavailable unit tests

372893d

Udit-takkar added this to the v4.9 milestone Dec 27, 2024

Udit-takkar changed the title ~~perf: improve getAvailable slots~~ perf: improve checkIsAvailable in getAvailable slots Dec 27, 2024

vercel bot deployed to Preview – api December 27, 2024 14:24 View deployment

emrysal approved these changes Dec 27, 2024

View reviewed changes

emrysal merged commit e2f45bd into main Dec 27, 2024
40 checks passed

emrysal deleted the perf/get-slots-improv branch December 27, 2024 16:01

emrysal mentioned this pull request Jan 5, 2025

perf: Use improved checkIfAvailable and remove dupe code #18463

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf: improve checkIsAvailable in getAvailable slots #18352

perf: improve checkIsAvailable in getAvailable slots #18352

Udit-takkar commented Dec 23, 2024 •

edited

Loading

graphite-app bot commented Dec 23, 2024 •

edited

Loading

Udit-takkar Dec 23, 2024

alishaz-polymath left a comment

vercel bot commented Dec 23, 2024 •

edited

Loading

emrysal Dec 23, 2024

Udit-takkar Dec 23, 2024

github-actions bot commented Dec 23, 2024 •

edited

Loading

hariombalhara Dec 27, 2024

hariombalhara Dec 27, 2024

Udit-takkar commented Dec 27, 2024

perf: improve checkIsAvailable in getAvailable slots #18352

perf: improve checkIsAvailable in getAvailable slots #18352

Conversation

Udit-takkar commented Dec 23, 2024 • edited Loading

What does this PR do?

Mandatory Tasks (DO NOT REMOVE)

How should this be tested?

graphite-app bot commented Dec 23, 2024 • edited Loading

Graphite Automations

Udit-takkar Dec 23, 2024

Choose a reason for hiding this comment

alishaz-polymath left a comment

Choose a reason for hiding this comment

vercel bot commented Dec 23, 2024 • edited Loading

emrysal Dec 23, 2024

Choose a reason for hiding this comment

Udit-takkar Dec 23, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 23, 2024 • edited Loading

E2E results are ready!

hariombalhara Dec 27, 2024

Choose a reason for hiding this comment

hariombalhara Dec 27, 2024

Choose a reason for hiding this comment

Udit-takkar commented Dec 27, 2024

Udit-takkar commented Dec 23, 2024 •

edited

Loading

graphite-app bot commented Dec 23, 2024 •

edited

Loading

vercel bot commented Dec 23, 2024 •

edited

Loading

github-actions bot commented Dec 23, 2024 •

edited

Loading