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

fix: Multiple rr-hosts combine to create erroneous availability #18772

Merged
merged 24 commits into from
Feb 11, 2025
+544 −169
Merged

fix: Multiple rr-hosts combine to create erroneous availability #18772

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
11baee3
wip: Open draft PR with passing test (should fail)
emrysal Jan 20, 2025
0e10874
Fix aggregatedAvailability by not merging rr host availability
emrysal Jan 21, 2025
abc4b70
Added test for validating fixed host behaviour
emrysal Jan 21, 2025
371c197
Add test for combined rr and fixed hosts
emrysal Jan 21, 2025
a72f030
Check date-ranges arent returned multiple times when offered by multi…
emrysal Jan 21, 2025
1d59916
Also sort date ranges
emrysal Jan 21, 2025
cf5cdd5
Adding failing test that should result in 5 slots, but returns 7 as i…
emrysal Jan 22, 2025
6407247
Slots returns duplicate slots for overlapping date ranges
emrysal Jan 22, 2025
7d50b48
As mergeOverlappingDateRanges enabled the unified start time, we need…
emrysal Jan 22, 2025
3c8bf92
Do not 'prettify' slots to the same degree if they fall on different …
emrysal Jan 22, 2025
00ab312
Added test to prove an end date before start also works
emrysal Jan 24, 2025
18ea8f8
Use a string instead of number as key
emrysal Jan 24, 2025
60d0043
Merge branch 'main' into bugfix/rr-busy-combinations-turn-some-slots-…
emrysal Jan 27, 2025
0667cec
chore: Deprecate organizerTimeZone, effectively unused
emrysal Jan 30, 2025
f6fe7d8
Adjust all dateRanges similarly
emrysal Jan 30, 2025
f826a34
Bring back eventTimeZone
emrysal Jan 30, 2025
143e132
Merge branch 'main' into bugfix/rr-busy-combinations-turn-some-slots-…
emrysal Feb 7, 2025
f2ded44
Remove adjustDateRanges in favour of slot generation changes
emrysal Feb 7, 2025
032ddc0
Convert to tz after all logic is finished.
emrysal Feb 7, 2025
4fc693f
clearer code flow
emrysal Feb 7, 2025
9f5fcc5
Remove console.log + add failing test due to inverse order
emrysal Feb 7, 2025
e62c2b5
Address issue with inversion when date ranges are given out of order
emrysal Feb 7, 2025
e3570aa
test name updated
emrysal Feb 11, 2025
38e63fd
updated test name
emrysal Feb 11, 2025
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
179 changes: 179 additions & 0 deletions packages/core/getAggregatedAvailability/getAggregatedAvailability.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import { describe, it, expect } from "vitest";

import type { Dayjs } from "@calcom/dayjs";
import dayjs from "@calcom/dayjs";

import { getAggregatedAvailability } from ".";

// Helper to check if a time range overlaps with availability
const isAvailable = (availability: { start: Dayjs; end: Dayjs }[], range: { start: Dayjs; end: Dayjs }) => {
return availability.some(({ start, end }) => {
return start <= range.start && end >= range.end;
});
};

describe("getAggregatedAvailability", () => {
// rr-host availability used to combine into erroneous slots, this confirms it no longer happens
it("should have no host available between 11:00 and 11:30 on January 23, 2025", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is failing on live code

const userAvailability = [
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:20:00.000Z") },
{ start: dayjs("2025-01-23T16:10:00.000Z"), end: dayjs("2025-01-23T16:30:00.000Z") },
],
user: { isFixed: false },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:15:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
{ start: dayjs("2025-01-23T13:20:00.000Z"), end: dayjs("2025-01-23T13:30:00.000Z") },
],
user: { isFixed: false },
},
];

const result = getAggregatedAvailability(userAvailability, "ROUND_ROBIN");
const timeRangeToCheckBusy = {
start: dayjs("2025-01-23T11:00:00.000Z"),
end: dayjs("2025-01-23T11:30:00.000Z"),
};

expect(isAvailable(result, timeRangeToCheckBusy)).toBe(false);

const timeRangeToCheckAvailable = {
start: dayjs("2025-01-23T11:00:00.000Z"),
end: dayjs("2025-01-23T11:20:00.000Z"),
};

expect(isAvailable(result, timeRangeToCheckAvailable)).toBe(true);
});
// validates fixed host behaviour, they all have to be available
it("should only have all fixed hosts available between 11:15 and 11:20 on January 23, 2025", () => {
const userAvailability = [
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:20:00.000Z") },
{ start: dayjs("2025-01-23T16:10:00.000Z"), end: dayjs("2025-01-23T16:30:00.000Z") },
],
user: { isFixed: true },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:15:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
{ start: dayjs("2025-01-23T13:20:00.000Z"), end: dayjs("2025-01-23T13:30:00.000Z") },
],
user: { isFixed: true },
},
];

const result = getAggregatedAvailability(userAvailability, "ROUND_ROBIN");
const timeRangeToCheckBusy = {
start: dayjs("2025-01-23T11:00:00.000Z"),
end: dayjs("2025-01-23T11:30:00.000Z"),
};

expect(isAvailable(result, timeRangeToCheckBusy)).toBe(false);

expect(result[0].start.format()).toEqual(dayjs("2025-01-23T11:15:00.000Z").format());
expect(result[0].end.format()).toEqual(dayjs("2025-01-23T11:20:00.000Z").format());
});

// Combines rr hosts and fixed hosts, both fixed and one of the rr hosts has to be available for the whole period
// All fixed user ranges are merged with each rr-host
it("Fixed hosts and at least one rr host available between 11:00-11:30 & 12:30-13:00 on January 23, 2025", () => {
// Both fixed user A and B are available 11:00-11:30 & 12:30-13:00 & 13:15-13:30
// Only user C (rr) is available 11:00-11:30 and only user D (rr) is available 12:30-13:00
// No rr users are available 13:15-13:30 and this date range should not be a result.
const userAvailability = [
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
{ start: dayjs("2025-01-23T12:30:00.000Z"), end: dayjs("2025-01-23T13:00:00.000Z") },
{ start: dayjs("2025-01-23T13:15:00.000Z"), end: dayjs("2025-01-23T13:30:00.000Z") },
{ start: dayjs("2025-01-23T16:10:00.000Z"), end: dayjs("2025-01-23T16:30:00.000Z") },
],
user: { isFixed: true },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
{ start: dayjs("2025-01-23T12:30:00.000Z"), end: dayjs("2025-01-23T13:00:00.000Z") },
{ start: dayjs("2025-01-23T13:15:00.000Z"), end: dayjs("2025-01-23T13:30:00.000Z") },
{ start: dayjs("2025-01-23T13:20:00.000Z"), end: dayjs("2025-01-23T13:30:00.000Z") },
],
user: { isFixed: true },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
],
user: { isFixed: false },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T12:30:00.000Z"), end: dayjs("2025-01-23T13:00:00.000Z") },
],
user: { isFixed: false },
},
];

const result = getAggregatedAvailability(userAvailability, "ROUND_ROBIN");
const timeRangeToCheckAvailable = {
start: dayjs("2025-01-23T11:00:00.000Z"),
end: dayjs("2025-01-23T11:30:00.000Z"),
};

expect(isAvailable(result, timeRangeToCheckAvailable)).toBe(true);

expect(result[0].start.format()).toEqual(dayjs("2025-01-23T11:00:00.000Z").format());
expect(result[0].end.format()).toEqual(dayjs("2025-01-23T11:30:00.000Z").format());
expect(result[1].start.format()).toEqual(dayjs("2025-01-23T12:30:00.000Z").format());
expect(result[1].end.format()).toEqual(dayjs("2025-01-23T13:00:00.000Z").format());
});

it("does not duplicate slots when multiple rr-hosts offer the same availability", () => {
const userAvailability = [
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
{ start: dayjs("2025-01-23T12:30:00.000Z"), end: dayjs("2025-01-23T13:00:00.000Z") },
{ start: dayjs("2025-01-23T13:15:00.000Z"), end: dayjs("2025-01-23T13:30:00.000Z") },
{ start: dayjs("2025-01-23T16:10:00.000Z"), end: dayjs("2025-01-23T16:30:00.000Z") },
],
user: { isFixed: true },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
],
user: { isFixed: false },
},
{
dateRanges: [],
oooExcludedDateRanges: [
{ start: dayjs("2025-01-23T11:00:00.000Z"), end: dayjs("2025-01-23T11:30:00.000Z") },
],
user: { isFixed: false },
},
];

const result = getAggregatedAvailability(userAvailability, "ROUND_ROBIN");
const timeRangeToCheckAvailable = {
start: dayjs("2025-01-23T11:00:00.000Z"),
end: dayjs("2025-01-23T11:30:00.000Z"),
};

expect(isAvailable(result, timeRangeToCheckAvailable)).toBe(true);
expect(result.length).toBe(1);
});
});
34 changes: 25 additions & 9 deletions packages/core/getAggregatedAvailability/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,22 @@ import { SchedulingType } from "@calcom/prisma/enums";

import { mergeOverlappingDateRanges } from "./date-range-utils/mergeOverlappingDateRanges";

function uniqueAndSortedDateRanges(ranges: DateRange[]): DateRange[] {
const seen = new Set<number>();

return ranges
.sort((a, b) => {
const startDiff = a.start.valueOf() - b.start.valueOf();
return startDiff !== 0 ? startDiff : a.end.valueOf() - b.end.valueOf();
})
.filter((range) => {
const key = range.start.valueOf() * 1e12 + range.end.valueOf();
Copy link
Contributor Author

@emrysal emrysal Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This line generates a unique key for each date range by combining the start and end timestamps. Multiplying the start timestamp by 1e12 ensures that the start and end values are distinct and non-overlapping when combined, allowing the function to accurately identify and filter out duplicate date ranges.

Copy link
Member

Choose a reason for hiding this comment

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

This is not working correctly for me.

I have the following data:

Date range 1:
start: 2025-01-27T14:00:00Z
end: 2025-01-27T04:30:00-05:00

Date range 2:
start: 2025-01-27T14:00:00Z
end: 2025-01-27T14:45:00Z

Even though the end time is different, Date Range 2 is considered a duplicate and not returned

if (seen.has(key)) return false;
seen.add(key);
return true;
});
}

export const getAggregatedAvailability = (
userAvailability: {
dateRanges: DateRange[];
Expand All @@ -16,22 +32,22 @@ export const getAggregatedAvailability = (
schedulingType === SchedulingType.COLLECTIVE ||
schedulingType === SchedulingType.ROUND_ROBIN ||
userAvailability.length > 1;

const fixedHosts = userAvailability.filter(
({ user }) => !schedulingType || schedulingType === SchedulingType.COLLECTIVE || user?.isFixed
);

const dateRangesToIntersect = fixedHosts.map((s) =>
!isTeamEvent ? s.dateRanges : s.oooExcludedDateRanges
const fixedDateRanges = mergeOverlappingDateRanges(
intersect(fixedHosts.map((s) => (!isTeamEvent ? s.dateRanges : s.oooExcludedDateRanges)))
);

const unfixedHosts = userAvailability.filter(({ user }) => user?.isFixed !== true);
if (unfixedHosts.length) {
const dateRangesToIntersect = !!fixedDateRanges.length ? [fixedDateRanges] : [];
const roundRobinHosts = userAvailability.filter(({ user }) => user?.isFixed !== true);
if (roundRobinHosts.length) {
dateRangesToIntersect.push(
unfixedHosts.flatMap((s) => (!isTeamEvent ? s.dateRanges : s.oooExcludedDateRanges))
roundRobinHosts.flatMap((s) => (!isTeamEvent ? s.dateRanges : s.oooExcludedDateRanges))
);
}

const availability = intersect(dateRangesToIntersect);

return mergeOverlappingDateRanges(availability);
// we no longer merge overlapping date ranges, rr-hosts need to be individually available here.
return uniqueAndSortedDateRanges(availability);
};
54 changes: 54 additions & 0 deletions packages/lib/slots.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,60 @@ describe("Tests the date-range slot logic", () => {
});
expect(result).toHaveLength(72);
});

it("can create multiple time slot groups when multiple date ranges are given", async () => {
const nextDay = dayjs.utc().add(1, "day").startOf("day");
const dateRanges = [
// 11:00-11:20,11:20-11:40,11:40-12:00
{
start: nextDay.hour(11),
end: nextDay.hour(12),
},
// 14:00-14:20,14:20-14:40,14:40-15:00
{
start: nextDay.hour(14),
end: nextDay.hour(15),
},
];
const result = getSlots({
inviteeDate: nextDay,
frequency: 20,
minimumBookingNotice: 0,
dateRanges: dateRanges,
eventLength: 20,
offsetStart: 0,
organizerTimeZone: "America/Toronto",
});

expect(result).toHaveLength(6);
});

it("can merge multiple time slot groups when multiple date ranges are given that overlap", async () => {
Copy link
Contributor Author

@emrysal emrysal Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Overlapping time slots weren't possible before as date ranges were merged, even rr hosts. Now they are possible because different rr hosts may have overlapping date ranges but we still want to handle individually.

const nextDay = dayjs.utc().add(1, "day").startOf("day");
const dateRanges = [
// 11:00-11:20,11:20-11:40,11:40-12:00
{
start: nextDay.hour(11),
end: nextDay.hour(12),
},
// 12:00-12:20,12:20-12:40
{
start: nextDay.hour(11).minute(20),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before this resulted in 7 slots, due to 11:20/11:40 overlap with dateRanges[0]. This is now corrected to the correct (5)

end: nextDay.hour(12).minute(40),
},
];
const result = getSlots({
inviteeDate: nextDay,
frequency: 20,
minimumBookingNotice: 0,
dateRanges: dateRanges,
eventLength: 20,
offsetStart: 0,
organizerTimeZone: "America/Toronto",
});

expect(result).toHaveLength(5);
});
});

describe("Tests the slot logic", () => {
Expand Down
26 changes: 15 additions & 11 deletions packages/lib/slots.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,19 @@ function buildSlotsWithDateRanges({
frequency = minimumOfOne(frequency);
eventLength = minimumOfOne(eventLength);
offsetStart = offsetStart ? minimumOfOne(offsetStart) : 0;
const slots: {
time: Dayjs;
userIds?: number[];
away?: boolean;
fromUser?: IFromUser;
toUser?: IToUser;
reason?: string;
emoji?: string;
}[] = [];
// there can only ever be one slot at a given start time, and based on duration also only a single length.
const slots = new Map<
string,
{
time: Dayjs;
userIds?: number[];
away?: boolean;
fromUser?: IFromUser;
toUser?: IToUser;
reason?: string;
emoji?: string;
}
>();

let interval = Number(process.env.NEXT_PUBLIC_AVAILABILITY_SCHEDULE_INTERVAL) || 1;
const intervalsWithDefinedStartTimes = [60, 30, 20, 15, 10, 5];
Expand Down Expand Up @@ -233,12 +237,12 @@ function buildSlotsWithDateRanges({
};
}

slots.push(slotData);
slots.set(slotData.time.toISOString(), slotData);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Key based on start time (same as rendering in Booker)

slotStartTime = slotStartTime.add(frequency + (offsetStart ?? 0), "minutes");
}
});

return slots;
return Array.from(slots.values());
}

function fromIndex<T>(cb: (val: T, i: number, a: T[]) => boolean, index: number) {
Expand Down
Loading