-
Notifications
You must be signed in to change notification settings - Fork 9.3k
feat: Add option to include no shows in RR calculations #21063
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.
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
base: main
Are you sure you want to change the base?
Conversation
…when getting bookings for RR
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
</RadioArea.Group> | ||
)} | ||
/> | ||
{schedulingType === "ROUND_ROBIN" && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only show the RR distribution box for RR events
...(!includeNoShowInRRCalculation | ||
? { | ||
OR: [{ noShowHost: false }, { noShowHost: null }], | ||
} | ||
: {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if includeNoShow...
is false then we add the noShowHost
conditional
@@ -66,7 +65,7 @@ const buildWhereClauseForActiveBookings = ({ | |||
}, | |||
}, | |||
], | |||
attendees: { some: { noShow: false } }, | |||
...(!includeNoShowInRRCalculation ? { attendees: { some: { noShow: false } } } : {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here for the attendee no show prop
@@ -164,6 +164,7 @@ model EventType { | |||
isRRWeightsEnabled Boolean @default(false) | |||
fieldTranslations EventTypeTranslation[] | |||
maxLeadThreshold Int? | |||
includeNoShowInRRCalculation Boolean @default(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only change in the schema. Others are just auto formatting
@@ -814,6 +815,7 @@ export class EventTypeRepository { | |||
rrSegmentQueryValue: true, | |||
isRRWeightsEnabled: true, | |||
maxLeadThreshold: true, | |||
includeNoShowInRRCalculation: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up @CarinaWolli the same logic that we use for determining the luckyUser
is also used in getAvailableSlots
. I added this as it was throwing a type error.
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (05/02/25)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mrge found 6 issues across 16 files. View them in mrge.io
@@ -98,6 +98,7 @@ export const filterHostsByLeadThreshold = async <T extends BaseHost<BaseUser>>({ | |||
parentId?: number | null; | |||
rrResetInterval: RRResetInterval | null; | |||
} | null; | |||
includeNoShowInRRCalculation: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing documentation for the new property that explains its purpose and impact on round robin calculations
@@ -67,6 +67,7 @@ interface GetLuckyUserParams<T extends PartialUser> { | |||
id: number; | |||
isRRWeightsEnabled: boolean; | |||
team: { parentId?: number | null; rrResetInterval: RRResetInterval | null } | null; | |||
includeNoShowInRRCalculation: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing default value for includeNoShowInRRCalculation parameter in getBookingsOfInterval function
@@ -29,6 +29,7 @@ const buildWhereClauseForActiveBookings = ({ | |||
endDate, | |||
users, | |||
virtualQueuesData, | |||
includeNoShowInRRCalculation = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default value in function parameter doesn't match type definition
@@ -41,20 +42,18 @@ | |||
selectedOptionIds: string | number | string[]; | |||
}; | |||
} | null; | |||
includeNoShowInRRCalculation: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Required parameter follows optional parameters in function signature
@@ -291,6 +267,7 @@ | |||
startDate, | |||
endDate, | |||
virtualQueuesData, | |||
includeNoShowInRRCalculation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing documentation for new parameter in getAllBookingsForRoundRobin
@@ -127,6 +127,7 @@ export const useEventTypeForm = ({ | |||
}, | |||
isRRWeightsEnabled: eventType.isRRWeightsEnabled, | |||
maxLeadThreshold: eventType.maxLeadThreshold, | |||
includeNoShowInRRCalculation: eventType.includeNoShowInRRCalculation, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rule violated: Enforce Singular Naming for Single-Item Functions
Property name uses singular 'noShow' when referring to multiple no-show bookings
What does this PR do?
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Summary by mrge
Added an option to include no-show bookings in round robin (RR) calculations for event types.