fix: Collective event types with seats - hosts count towards the seat count #14930
Conversation
|
@Amit91848 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
github-actions
bot
added
seats
Graphite Automations"Add community label" took an action on this PR • (05/07/24)1 label was added to this PR based on Keith Williams's automation. "Add consumer team as reviewer" took an action on this PR • (05/07/24)1 reviewer was added to this PR based on Keith Williams's automation. |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
packages/core/getUserAvailability.ts
Outdated
| dateFrom: Dayjs, | ||
| dateTo: Dayjs, | ||
| isTeamEvent?: boolean, | ||
| hosts?: string[] |
There was a problem hiding this comment.
_getCurrentSeats now takes in 5 parameters, which is a lot for a function. I propose combining eventTypeId, isTeamEvent and hosts into 1 eventType parameter:
eventType: Pick<EventType, "id" | "schedulingType" | "hosts">,
dateFrom: Dayjs,
dateTo: Dayjs
and then within _getCurrentSeats we access event type properties to check:
const isTeamEvent = eventType?.schedulingType !== null;
const hostEmails = eventType?.hosts.map((host) => host.user.email);
And to invoke function:
currentSeats = await getCurrentSeats(eventType, dateFrom, dateTo);
There was a problem hiding this comment.
eventType: Pick<EventType, "id" | "schedulingType" | "hosts">, dateFrom: Dayjs, dateTo: Dayjs
For some reason Pick doesn't work on EventType or User so had to hardcode the eventType
packages/core/getUserAvailability.ts
Outdated
| @@ -215,11 +258,13 @@ const _getUserAvailability = async function getUsersWorkingHoursLifeTheUniverseA | |||
| let eventType: EventType | null = initialData?.eventType || null; | |||
| if (!eventType && eventTypeId) eventType = await getEventType(eventTypeId); | |||
|
|
|||
| const isTeamEvent = eventType?.schedulingType !== null; | |||
There was a problem hiding this comment.
In comment about "_getCurrentSeats now takes in 5 parameters" I suggest "const isTeamEvent" logic should be moved inside "_getCurrentSeats", but then I am also thinking:
I think it's better to be explicit about when something is a team event:
const isTeamEvent = eventType?.schedulingType === SchedulingType.COLLECTIVE || eventType?.schedulingType === SchedulingType.ROUND_ROBIN || eventType?.schedulingType === SchedulingType.COLLECTIVE
My logic for this is that currently SchedulingType indeed shows only scheduling types of teams, but what if someone adds a SchedulingType that is non-team? Then this logic would potentially break.
There was a problem hiding this comment.
We can also directly filter on hosts as only team events have hosts but that might change in the future so using hosts and schedulingType
What does this PR do?
Fixes #14926
Before :
Screen.Recording.2024-05-08.at.2.36.14.PM.mov
After:
Screen.Recording.2024-05-08.at.2.37.05.PM.mov
Type of change
Mandatory Tasks