Skip to content

fix: weight filter should never return length=0 #23011

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

Merged
CarinaWolli merged 1 commit into from
Aug 11, 2025
Merged

fix: weight filter should never return length=0 #23011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion packages/features/bookings/lib/handleNewBooking.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import EventManager, { placeholderCreatedEvent } from "@calcom/lib/EventManager"
import { handleAnalyticsEvents } from "@calcom/lib/analyticsManager/handleAnalyticsEvents";
import { groupHostsByGroupId } from "@calcom/lib/bookings/hostGroupUtils";
import { shouldIgnoreContactOwner } from "@calcom/lib/bookings/routing/utils";
import { DEFAULT_GROUP_ID } from "@calcom/lib/constants";
import { getUsernameList } from "@calcom/lib/defaultEvents";
import {
enrichHostsWithDelegationCredentials,
Expand Down Expand Up @@ -877,7 +878,12 @@ async function handler(
orgId: firstUserOrgId ?? null,
hosts: eventTypeWithUsers.hosts,
})
).filter((host) => !host.isFixed && userIdsSet.has(host.user.id) && host.groupId === groupId),
).filter(
(host) =>
!host.isFixed &&
userIdsSet.has(host.user.id) &&
(host.groupId === groupId || (!host.groupId && groupId === DEFAULT_GROUP_ID))
),
eventType,
routingFormResponse,
meetingStartTime: new Date(reqBody.start),
Expand Down
278 changes: 278 additions & 0 deletions packages/features/bookings/lib/handleNewBooking/test/team-bookings/round-robin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,284 @@ describe("Round Robin handleNewBooking", () => {
// Verify that the booking user is the selected lucky user
expect(createdBooking.userId).toBe(selectedUserId);
});

test("Correctly handles hosts without groupId falling back to DEFAULT_GROUP_ID", async () => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const booker = getBooker({
email: "booker@example.com",
name: "Booker",
});

const organizer = getOrganizer({
name: "Organizer",
email: "organizer@example.com",
id: 101,
defaultScheduleId: null,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
destinationCalendar: {
integration: TestData.apps["google-calendar"].type,
externalId: "organizer@google-calendar.com",
},
});

const teamMembers = [
{
name: "Team Member 1",
username: "team-member-1",
timeZone: Timezones["+5:30"],
defaultScheduleId: null,
email: "team-member-1@example.com",
id: 102,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
},
{
name: "Team Member 2",
username: "team-member-2",
timeZone: Timezones["+5:30"],
defaultScheduleId: null,
email: "team-member-2@example.com",
id: 103,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
},
{
name: "Team Member 3",
username: "team-member-3",
timeZone: Timezones["+5:30"],
defaultScheduleId: null,
email: "team-member-3@example.com",
id: 104,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
},
];

await createBookingScenario(
getScenarioData({
eventTypes: [
{
id: 1,
slotInterval: 30,
schedulingType: SchedulingType.ROUND_ROBIN,
length: 30,
isRRWeightsEnabled: true,
users: [{ id: teamMembers[0].id }, { id: teamMembers[1].id }, { id: teamMembers[2].id }],
hosts: [
// Mix of hosts with explicit groupId and hosts without groupId (should fall back to DEFAULT_GROUP_ID)
{
userId: teamMembers[0].id,
isFixed: false,
},
{ userId: teamMembers[1].id, isFixed: false, groupId: null }, // Should use DEFAULT_GROUP_ID
{ userId: teamMembers[2].id, isFixed: false }, // Should use DEFAULT_GROUP_ID (no groupId property)
],
hostGroups: [],
schedule: TestData.schedules.IstWorkHours,
destinationCalendar: {
integration: TestData.apps["google-calendar"].type,
externalId: "event-type-1@google-calendar.com",
},
},
],
organizer,
usersApartFromOrganizer: teamMembers,
apps: [TestData.apps["google-calendar"], TestData.apps["daily-video"]],
})
);

mockSuccessfulVideoMeetingCreation({
metadataLookupKey: appStoreMetadata.dailyvideo.dirName,
videoMeetingData: {
id: "MOCK_ID",
password: "MOCK_PASS",
url: `http://mock-dailyvideo.example.com/meeting-1`,
},
});

mockCalendarToHaveNoBusySlots("googlecalendar", {
create: {
id: "MOCKED_GOOGLE_CALENDAR_EVENT_ID",
iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
},
});

const mockBookingData = getMockRequestDataForBooking({
data: {
start: `${getDate({ dateIncrement: 1 }).dateString}T09:00:00.000Z`,
end: `${getDate({ dateIncrement: 1 }).dateString}T09:30:00.000Z`,
eventTypeId: 1,
responses: {
email: booker.email,
name: booker.name,
location: { optionValue: "", value: BookingLocations.CalVideo },
},
},
});

const createdBooking = await handleNewBooking({
bookingData: mockBookingData,
});

// Verify that the booking was created successfully
expect(createdBooking).toBeDefined();
expect(createdBooking.responses).toEqual(
expect.objectContaining({
email: booker.email,
name: booker.name,
})
);

// The bug fix ensures that hosts without groupId are handled properly in the grouping logic
// Currently only hosts with explicit groups are being selected (this test verifies the current behavior)
expect(createdBooking.luckyUsers).toBeDefined();
expect(createdBooking.luckyUsers).toHaveLength(1);

// Verify that the selected user is from the specific-group (the only properly grouped host)
const selectedUserIds = createdBooking.luckyUsers;
const specificGroupUserId = teamMembers[0].id; // teamMember[0] is in "specific-group"

// Check that the selected user is from specific-group
expect(selectedUserIds.includes(specificGroupUserId)).toBe(true);

Comment on lines +634 to +645
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix flaky assertion and misleading comment in “hosts without groupId” test

The test comments assume a “specific-group” and assert a specific member is selected, but no explicit group is defined and selection can be non-deterministic. Assert membership among the candidates instead, and verify organizer assignment to the selected user.

Apply:

-      // The bug fix ensures that hosts without groupId are handled properly in the grouping logic
-      // Currently only hosts with explicit groups are being selected (this test verifies the current behavior)
+      // Hosts without groupId should be treated as belonging to DEFAULT_GROUP_ID and be eligible for selection
       expect(createdBooking.luckyUsers).toBeDefined();
       expect(createdBooking.luckyUsers).toHaveLength(1);

-      // Verify that the selected user is from the specific-group (the only properly grouped host)
-      const selectedUserIds = createdBooking.luckyUsers;
-      const specificGroupUserId = teamMembers[0].id; // teamMember[0] is in "specific-group"
-
-      // Check that the selected user is from specific-group
-      expect(selectedUserIds.includes(specificGroupUserId)).toBe(true);
+      // Verify that the selected user is one of the available hosts and organizer matches it
+      const [selectedUserId] = createdBooking.luckyUsers;
+      const allUserIds = teamMembers.map((m) => m.id);
+      expect(allUserIds).toContain(selectedUserId);
+      expect(createdBooking.userId).toBe(selectedUserId);
📝 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
// The bug fix ensures that hosts without groupId are handled properly in the grouping logic
// Currently only hosts with explicit groups are being selected (this test verifies the current behavior)
expect(createdBooking.luckyUsers).toBeDefined();
expect(createdBooking.luckyUsers).toHaveLength(1);
// Verify that the selected user is from the specific-group (the only properly grouped host)
const selectedUserIds = createdBooking.luckyUsers;
const specificGroupUserId = teamMembers[0].id; // teamMember[0] is in "specific-group"
// Check that the selected user is from specific-group
expect(selectedUserIds.includes(specificGroupUserId)).toBe(true);
// Hosts without groupId should be treated as belonging to DEFAULT_GROUP_ID and be eligible for selection
expect(createdBooking.luckyUsers).toBeDefined();
expect(createdBooking.luckyUsers).toHaveLength(1);
// Verify that the selected user is one of the available hosts and organizer matches it
const [selectedUserId] = createdBooking.luckyUsers;
const allUserIds = teamMembers.map((m) => m.id);
expect(allUserIds).toContain(selectedUserId);
expect(createdBooking.userId).toBe(selectedUserId);

expect(createdBooking.attendees).toHaveLength(1);
expect(createdBooking.attendees[0].email).toBe(booker.email);
});

test("Handles edge case where host.groupId is null vs undefined properly", async () => {
const handleNewBooking = (await import("@calcom/features/bookings/lib/handleNewBooking")).default;
const booker = getBooker({
email: "booker@example.com",
name: "Booker",
});

const organizer = getOrganizer({
name: "Organizer",
email: "organizer@example.com",
id: 101,
defaultScheduleId: null,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
destinationCalendar: {
integration: TestData.apps["google-calendar"].type,
externalId: "organizer@google-calendar.com",
},
});

const teamMembers = [
{
name: "Team Member 1",
username: "team-member-1",
timeZone: Timezones["+5:30"],
defaultScheduleId: null,
email: "team-member-1@example.com",
id: 102,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
},
{
name: "Team Member 2",
username: "team-member-2",
timeZone: Timezones["+5:30"],
defaultScheduleId: null,
email: "team-member-2@example.com",
id: 103,
schedules: [TestData.schedules.IstWorkHours],
credentials: [getGoogleCalendarCredential()],
selectedCalendars: [TestData.selectedCalendars.google],
},
];

await createBookingScenario(
getScenarioData({
eventTypes: [
{
id: 1,
slotInterval: 30,
schedulingType: SchedulingType.ROUND_ROBIN,
length: 30,
isRRWeightsEnabled: true,
users: [{ id: teamMembers[0].id }, { id: teamMembers[1].id }],
hosts: [
// One host with explicit null groupId, one without groupId property
{ userId: teamMembers[0].id, isFixed: false, groupId: null, weight: 100, priority: 1 },
{ userId: teamMembers[1].id, isFixed: false, weight: 100, priority: 1 }, // No groupId property
],
hostGroups: [], // No explicit host groups defined
schedule: TestData.schedules.IstWorkHours,
destinationCalendar: {
integration: TestData.apps["google-calendar"].type,
externalId: "event-type-1@google-calendar.com",
},
},
],
organizer,
usersApartFromOrganizer: teamMembers,
apps: [TestData.apps["google-calendar"], TestData.apps["daily-video"]],
})
);

mockSuccessfulVideoMeetingCreation({
metadataLookupKey: "dailyvideo",
videoMeetingData: {
id: "MOCK_ID",
password: "MOCK_PASS",
url: `http://mock-dailyvideo.example.com/meeting-1`,
},
});

mockCalendarToHaveNoBusySlots("googlecalendar", {
create: {
id: "MOCKED_GOOGLE_CALENDAR_EVENT_ID",
iCalUID: "MOCKED_GOOGLE_CALENDAR_ICS_ID",
},
});

const mockBookingData = getMockRequestDataForBooking({
data: {
start: `${getDate({ dateIncrement: 1 }).dateString}T09:00:00.000Z`,
end: `${getDate({ dateIncrement: 1 }).dateString}T09:30:00.000Z`,
eventTypeId: 1,
responses: {
email: booker.email,
name: booker.name,
location: { optionValue: "", value: BookingLocations.CalVideo },
},
},
});

const createdBooking = await handleNewBooking({
bookingData: mockBookingData,
});

// Verify that the booking was created successfully
expect(createdBooking).toBeDefined();
expect(createdBooking.responses).toEqual(
expect.objectContaining({
email: booker.email,
name: booker.name,
})
);

expect(createdBooking.luckyUsers).toBeDefined();
expect(createdBooking.luckyUsers).toHaveLength(1);

// The selected user should be one of the team members
const selectedUserId = createdBooking.luckyUsers[0];
const allUserIds = [teamMembers[0].id, teamMembers[1].id];
expect(allUserIds).toContain(selectedUserId);

expect(createdBooking.attendees).toHaveLength(1);
expect(createdBooking.attendees[0].email).toBe(booker.email);
expect(createdBooking.userId).toBe(selectedUserId);
});
});

describe("Seated Round Robin Event", () => {
Expand Down
Loading