Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: improve checkIsAvailable in getAvailable slots #18352

Merged
merged 5 commits into from
Dec 27, 2024
Merged

perf: improve checkIsAvailable in getAvailable slots #18352

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
903f074
refactor: Update booking query to use userId and optimize availabilit…
Udit-takkar Dec 23, 2024
c78443d
fix: update tests
Udit-takkar Dec 23, 2024
3ce9700
chore: use toDate()
Udit-takkar Dec 23, 2024
e331430
Merge branch 'main' into perf/get-slots-improv
Udit-takkar Dec 27, 2024
372893d
tests: add checkavailable unit tests
Udit-takkar Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions packages/lib/bookings/filterHostsByLeadThreshold.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,8 @@ describe("filterHostByLeadThreshold", () => {
where: {
OR: [
{
user: {
id: {
in: [2],
},
userId: {
in: [2],
},
OR: [
{
Expand Down
6 changes: 2 additions & 4 deletions packages/lib/server/repository/booking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,8 @@ const buildWhereClauseForActiveBookings = ({
}): Prisma.BookingWhereInput => ({
OR: [
{
user: {
id: {
in: users.map((user) => user.id),
},
userId: {
in: users.map((user) => user.id),
emrysal marked this conversation as resolved.
Show resolved Hide resolved
},
OR: [
{
Expand Down
61 changes: 31 additions & 30 deletions packages/trpc/server/routers/viewer/slots/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,33 +62,38 @@ export const checkIfIsAvailable = ({
return true;
}

const slotEndTime = time.add(eventLength, "minutes").utc();
const slotStartTime = time.utc();
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) {
Comment on lines +65 to +91
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

return false;
}
// Check if startTime is between slot
else if (startTime.isBetween(time, slotEndTime)) {

// 4. Slot completely contained within busy period
if (busyStartDate <= slotStartDate && busyEndDate >= slotEndDate) {
return false;
}

Expand Down Expand Up @@ -986,22 +991,18 @@ const calculateHostsAndAvailabilities = async ({
};

const allUserIds = usersWithCredentials.map((user) => user.id);
const currentBookingsAllUsers = await monitorCallbackAsync(
getExistingBookings,
startTimeDate,
endTimeDate,
eventType,
sharedQuery,
usersWithCredentials,
allUserIds
);

const outOfOfficeDaysAllUsers = await monitorCallbackAsync(
getOOODates,
startTimeDate,
endTimeDate,
allUserIds
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

const [currentBookingsAllUsers, outOfOfficeDaysAllUsers] = await Promise.all([
monitorCallbackAsync(
getExistingBookings,
startTimeDate,
endTimeDate,
eventType,
sharedQuery,
usersWithCredentials,
allUserIds
),
monitorCallbackAsync(getOOODates, startTimeDate, endTimeDate, allUserIds),
]);

const bookingLimits = parseBookingLimit(eventType?.bookingLimits);
const durationLimits = parseDurationLimit(eventType?.durationLimits);
Expand Down
Loading