Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 46 additions & 30 deletions packages/features/bookings/lib/handleNewBooking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -670,9 +670,11 @@ async function handler(

const luckyUsers: typeof users = [];

// Get number of hosts to select from event metadata, default to 1
const numberOfHostsToSelect = eventType.metadata?.multipleRoundRobinHosts || 1;
// loop through all non-fixed hosts and get the lucky users
// This logic doesn't run when contactOwner is used because in that case, luckUsers.length === 1
while (luckyUserPool.length > 0 && luckyUsers.length < 1 /* TODO: Add variable */) {
while (luckyUserPool.length > 0 && luckyUsers.length < numberOfHostsToSelect) {
const freeUsers = luckyUserPool.filter(
(user) => !luckyUsers.concat(notAvailableLuckyUsers).find((existing) => existing.id === user.id)
Comment on lines +674 to 679
Copy link
Author

Choose a reason for hiding this comment

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

asdf

);
Expand All @@ -686,7 +688,10 @@ async function handler(
memberId: eventTypeWithUsers.users[0].id ?? null,
teamId: eventType.teamId,
});
const newLuckyUser = await getLuckyUser({
// Check if we need to select multiple hosts
const numberOfHostsToSelect = eventType.metadata?.multipleRoundRobinHosts || 1;
Copy link

Choose a reason for hiding this comment

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

Medium Logic

The numberOfHostsToSelect variable is declared twice - once at line 674 and again here. The second declaration shadows the first one, which could lead to confusion. Consider removing this redundant declaration.
Agent: 🔄 Zero

Copy link

Choose a reason for hiding this comment

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

Low Style

This declaration of numberOfHostsToSelect duplicates the one at line 674. Consider extracting it to avoid redundant computation and improve maintainability.
Agent: 🔄 Zero

Choose a reason for hiding this comment

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

Low

This duplicates the logic from line 674. The duplicate variable declaration creates maintenance risk and potential inconsistency. Use the already-declared numberOfHostsToSelect variable instead of re-reading the metadata.
Agent: 🔧 Backend


const newLuckyUsers = await getLuckyUser({
Comment on lines +691 to +694
Copy link
Author

Choose a reason for hiding this comment

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

bruh

// find a lucky user that is not already in the luckyUsers array
availableUsers: freeUsers,
allRRHosts: (
Expand All @@ -697,43 +702,54 @@ async function handler(
).filter((host) => !host.isFixed && userIdsSet.has(host.user.id)),
eventType,
routingFormResponse,
numberOfHostsToSelect,
});

// Handle single vs multiple users being returned
const newLuckyUser = Array.isArray(newLuckyUsers) ? newLuckyUsers[0] : newLuckyUsers;
if (!newLuckyUser) {
break; // prevent infinite loop
Comment on lines 703 to 711
Copy link
Author

Choose a reason for hiding this comment

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

bruh2

}

// Handle both cases - single user and multiple users
Copy link
Author

Choose a reason for hiding this comment

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

one two three

const usersToProcess = Array.isArray(newLuckyUsers) ? newLuckyUsers : [newLuckyUser];
Copy link
Author

Choose a reason for hiding this comment

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

asdf


Choose a reason for hiding this comment

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

yoot

if (req.body.isFirstRecurringSlot && eventType.schedulingType === SchedulingType.ROUND_ROBIN) {
// for recurring round robin events check if lucky user is available for next slots
try {
for (
let i = 0;
i < req.body.allRecurringDates.length && i < req.body.numSlotsToCheckForAvailability;
i++
) {
const start = req.body.allRecurringDates[i].start;
const end = req.body.allRecurringDates[i].end;

await ensureAvailableUsers(
{ ...eventTypeWithUsers, users: [newLuckyUser] },
{
dateFrom: dayjs(start).tz(reqBody.timeZone).format(),
dateTo: dayjs(end).tz(reqBody.timeZone).format(),
Copy link
Author

Choose a reason for hiding this comment

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

foo

timeZone: reqBody.timeZone,
originalRescheduledBooking,
},
loggerWithEventDetails,
shouldServeCache
// for recurring round robin events check if lucky users are available for next slots

Choose a reason for hiding this comment

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

log

for (const user of usersToProcess) {
try {
for (
Copy link
Author

Choose a reason for hiding this comment

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

asdf

let i = 0;
i < req.body.allRecurringDates.length && i < req.body.numSlotsToCheckForAvailability;
i++
) {
const start = req.body.allRecurringDates[i].start;
Copy link
Author

Choose a reason for hiding this comment

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

foobar

const end = req.body.allRecurringDates[i].end;

await ensureAvailableUsers(
{ ...eventTypeWithUsers, users: [user] },
{
dateFrom: dayjs(start).tz(reqBody.timeZone).format(),

Choose a reason for hiding this comment

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

test

dateTo: dayjs(end).tz(reqBody.timeZone).format(),
timeZone: reqBody.timeZone,
originalRescheduledBooking,
},
loggerWithEventDetails,
shouldServeCache
);
}
// if no error, then lucky user is available for the next slots
luckyUsers.push(user);
Comment on lines +738 to +742
Copy link
Author

Choose a reason for hiding this comment

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

foo

} catch {
notAvailableLuckyUsers.push(user);
loggerWithEventDetails.info(
`Round robin host ${user.name} not available for first two slots. Trying to find another host.`
);
}
// if no error, then lucky user is available for the next slots
luckyUsers.push(newLuckyUser);
} catch {
Copy link
Author

Choose a reason for hiding this comment

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

foobar

notAvailableLuckyUsers.push(newLuckyUser);
loggerWithEventDetails.info(
`Round robin host ${newLuckyUser.name} not available for first two slots. Trying to find another host.`
);
}
} else {
luckyUsers.push(newLuckyUser);
// Add all lucky users
luckyUsers.push(...usersToProcess);

Choose a reason for hiding this comment

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

Medium

Using luckyUsers.push(...usersToProcess) bypasses the loop condition luckyUsers.length < numberOfHostsToSelect and could add more users than requested. This could lead to over-allocation of hosts for a booking.
Agent: 🔧 Backend

}
}
// ALL fixed users must be available
Expand Down
Loading