fix: round-robin booked out of availability (#12376)

## What does this PR do?

Fixes that it can happen that Round Robin host is booked outside of availability. 

I found and fixed the following two scenarios where this can happen: 
- when host has a date override
- when host is available for only a part the event time (for example, booking time 9:00-11:00 and user is only available between 10:00-11:00)

Fixes #10315
Fixes #11690

It also fixes that it can happen that round robin doesn't correctly pick the luck user (least recently booked). This happened when a user was an attendee of a booking before, then we always compared this booking and never the actual last booking of this user. 

## Type of change

- [x] Bug fix (non-breaking change which fixes an issue)

## How should this be tested?

#### Booked outside of availability: 
1. 
- Create Round Robin event and assign user1 and user2 as round robin hosts
   - event duration: 120 minutes 
- user 1 availability: 
  - Monday to Friday 9:00-17:00
- user2 availability: 
  - Monday to Friday 10:00-17:00
- Book event at a 9:00 slot -> check if i user1 is booked 
- Book event again at a 9:00 slot -> check if user1 is booked again (user2 is not available at that time)

2.
- Change availability of user2
   - Mark Monday as unavailable 
   -   Add date override on any day this month 
- Book any Monday this month -> see that user 1 is booked 
- Again Book any Monday this month -> see that user 1 is booked again 

#### Wrong lucky user 
- Book event and add user1's email as the attendee email address
- Book several slots where both users should be available, and see that it alternates between user1 and user2 (before it ended up always booking user1)

## Mandatory Tasks

- [ ] Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.
This commit is contained in:
Carina Wollendorfer 2023-11-15 20:49:03 +01:00 committed by GitHub
parent 48e7b616b8
commit 270d4f6e82
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 126 additions and 5 deletions

View File

@ -1,6 +1,8 @@
import { expect } from "@playwright/test";
import { randomString } from "@calcom/lib/random";
import { SchedulingType } from "@calcom/prisma/client";
import type { Schedule, TimeRange } from "@calcom/types/schedule";
import { test } from "./lib/fixtures";
import {
@ -342,3 +344,92 @@ test.describe("Booking on different layouts", () => {
await expect(page.locator("[data-testid=success-page]")).toBeVisible();
});
});
test.describe("Booking round robin event", () => {
test.beforeEach(async ({ page, users }) => {
const teamMatesObj = [{ name: "teammate-1" }];
const dateRanges: TimeRange = {
start: new Date(new Date().setUTCHours(10, 0, 0, 0)), //one hour after default schedule (teammate-1's schedule)
end: new Date(new Date().setUTCHours(17, 0, 0, 0)),
};
const schedule: Schedule = [[], [dateRanges], [dateRanges], [dateRanges], [dateRanges], [dateRanges], []];
const testUser = await users.create(
{ username: "test-user", name: "Test User", email: "testuser@example.com", schedule },
{
hasTeam: true,
schedulingType: SchedulingType.ROUND_ROBIN,
teamEventLength: 120,
teammates: teamMatesObj,
}
);
const team = await testUser.getFirstTeam();
await page.goto(`/team/${team.team.slug}`);
});
test("Does not book round robin host outside availability with date override", async ({ page, users }) => {
const [testUser] = users.get();
testUser.apiLogin();
const team = await testUser.getFirstTeam();
// Click first event type (round robin)
await page.click('[data-testid="event-type-link"]');
await page.click('[data-testid="incrementMonth"]');
// books 9AM slots for 120 minutes (test-user is not available at this time, availability starts at 10)
await page.locator('[data-testid="time"]').nth(0).click();
await page.waitForLoadState("networkidle");
await page.locator('[name="name"]').fill("Test name");
await page.locator('[name="email"]').fill(`${randomString(4)}@example.com`);
await page.click('[data-testid="confirm-book-button"]');
await page.waitForURL((url) => {
return url.pathname.startsWith("/booking");
});
await expect(page.locator("[data-testid=success-page]")).toBeVisible();
await expect(page.locator("[data-testid=success-page]")).toBeVisible();
const host = await page.locator('[data-testid="booking-host-name"]');
const hostName = await host.innerText();
//expect teammate-1 to be booked, test-user is not available at this time
expect(hostName).toBe("teammate-1");
// make another booking to see if also for the second booking teammate-1 is booked
await page.goto(`/team/${team.team.slug}`);
await page.click('[data-testid="event-type-link"]');
await page.click('[data-testid="incrementMonth"]');
await page.click('[data-testid="incrementMonth"]');
// Again book a 9AM slot for 120 minutes where test-user is not available
await page.locator('[data-testid="time"]').nth(0).click();
await page.waitForLoadState("networkidle");
await page.locator('[name="name"]').fill("Test name");
await page.locator('[name="email"]').fill(`${randomString(4)}@example.com`);
await page.click('[data-testid="confirm-book-button"]');
await page.waitForURL((url) => {
return url.pathname.startsWith("/booking");
});
await expect(page.locator("[data-testid=success-page]")).toBeVisible();
const hostSecondBooking = await page.locator('[data-testid="booking-host-name"]');
const hostNameSecondBooking = await hostSecondBooking.innerText();
expect(hostNameSecondBooking).toBe("teammate-1"); // teammate-1 should be booked again
});
});

View File

@ -10,6 +10,7 @@ import { WEBAPP_URL } from "@calcom/lib/constants";
import { prisma } from "@calcom/prisma";
import { MembershipRole, SchedulingType } from "@calcom/prisma/enums";
import { teamMetadataSchema } from "@calcom/prisma/zod-utils";
import type { Schedule } from "@calcom/types/schedule";
import { selectFirstAvailableTimeSlotNextMonth, teamEventSlug, teamEventTitle } from "../lib/testUtils";
import { TimeZoneEnum } from "./types";
@ -46,6 +47,7 @@ const createTeamEventType = async (
schedulingType?: SchedulingType;
teamEventTitle?: string;
teamEventSlug?: string;
teamEventLength?: number;
}
) => {
return await prisma.eventType.create({
@ -65,10 +67,16 @@ const createTeamEventType = async (
id: user.id,
},
},
hosts: {
create: {
userId: user.id,
isFixed: scenario?.schedulingType === SchedulingType.COLLECTIVE ? true : false,
},
},
schedulingType: scenario?.schedulingType ?? SchedulingType.COLLECTIVE,
title: scenario?.teamEventTitle ?? `${teamEventTitle}-team-id-${team.id}`,
slug: scenario?.teamEventSlug ?? `${teamEventSlug}-team-id-${team.id}`,
length: 30,
length: scenario?.teamEventLength ?? 30,
},
});
};
@ -135,6 +143,7 @@ export const createUsersFixture = (page: Page, emails: API | undefined, workerIn
schedulingType?: SchedulingType;
teamEventTitle?: string;
teamEventSlug?: string;
teamEventLength?: number;
isOrg?: boolean;
hasSubteam?: true;
isUnpublished?: true;
@ -489,6 +498,7 @@ type CustomUserOpts = Partial<Pick<Prisma.User, CustomUserOptsKeys>> & {
// ignores adding the worker-index after username
useExactUsername?: boolean;
roleInOrganization?: MembershipRole;
schedule?: Schedule;
};
// creates the actual user in the db.
@ -520,7 +530,7 @@ const createUser = (
timeZone: opts?.timeZone ?? TimeZoneEnum.UK,
availability: {
createMany: {
data: getAvailabilityFromSchedule(DEFAULT_SCHEDULE),
data: getAvailabilityFromSchedule(opts?.schedule ?? DEFAULT_SCHEDULE),
},
},
},

View File

@ -134,7 +134,7 @@ test.describe("Teams - NonOrg", () => {
// Anyone of the teammates could be the Host of the booking.
const chosenUser = await page.getByTestId("booking-host-name").textContent();
expect(chosenUser).not.toBeNull();
expect(teamMatesObj.some(({ name }) => name === chosenUser)).toBe(true);
expect(teamMatesObj.concat([{ name: ownerObj.name }]).some(({ name }) => name === chosenUser)).toBe(true);
// TODO: Assert whether the user received an email
});
@ -370,7 +370,7 @@ test.describe("Teams - Org", () => {
await expect(page.locator(`[data-testid="attendee-name-${testName}"]`)).toHaveText(testName);
// All the teammates should be in the booking
for (const teammate of teamMatesObj) {
for (const teammate of teamMatesObj.concat([{ name: owner.name || "" }])) {
await expect(page.getByText(teammate.name, { exact: true })).toBeVisible();
}
}
@ -412,7 +412,7 @@ test.describe("Teams - Org", () => {
// Anyone of the teammates could be the Host of the booking.
const chosenUser = await page.getByTestId("booking-host-name").textContent();
expect(chosenUser).not.toBeNull();
expect(teamMatesObj.some(({ name }) => name === chosenUser)).toBe(true);
expect(teamMatesObj.concat([{ name: ownerObj.name }]).some(({ name }) => name === chosenUser)).toBe(true);
// TODO: Assert whether the user received an email
});
});

View File

@ -401,6 +401,25 @@ async function ensureAvailableUsers(
}
let foundConflict = false;
let dateRangeForBooking = false;
//check if event time is within the date range
for (const dateRange of dateRanges) {
if (
(dayjs.utc(input.dateFrom).isAfter(dateRange.start) ||
dayjs.utc(input.dateFrom).isSame(dateRange.start)) &&
(dayjs.utc(input.dateTo).isBefore(dateRange.end) || dayjs.utc(input.dateTo).isSame(dateRange.end))
) {
dateRangeForBooking = true;
break;
}
}
if (!dateRangeForBooking) {
continue;
}
try {
foundConflict = checkForConflicts(bufferedBusyTimes, input.dateFrom, duration);
} catch {

View File

@ -77,6 +77,7 @@ async function leastRecentlyBookedUser<T extends Pick<User, "id" | "email">>({
if (aggregate[user.id]) return; // Bookings are ordered DESC, so if the reducer aggregate
// contains the user id, it's already got the most recent booking marked.
if (!booking.attendees.map((attendee) => attendee.email).includes(user.email)) return;
if (organizerIdAndAtCreatedPair[user.id] > booking.createdAt) return; // only consider bookings if they were created after organizer bookings
aggregate[user.id] = booking.createdAt;
});
return aggregate;