-
Notifications
You must be signed in to change notification settings - Fork 8k
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
feat: Round robin reassignment via round robin algorithm [CAL-3138] #14308
Conversation
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
This PR is being marked as stale due to inactivity. |
This PR is being marked as stale due to inactivity. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
where: { | ||
deleted: 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.
When changing organizers, we delete the previous organizer's 3rd party events. If we're rescheduling multiple times, we don't have to include already deleted references.
include: { | ||
workflowStep: { | ||
include: { | ||
workflow: 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.
would workflowSelect
from getAllWorkflows.ts
work here instead of using include
?
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.
The workflowSelect
nesting is workflow -> steps
. This query is step -> workflow
. I just selected the fields that would be relevant.
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.
Sorry missed that. Can we define all fields we need in a select instead of an include? Never use Includes - use select
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.
Changed this one to select
. The other include
statements are needed.
references: true, | ||
}; | ||
|
||
export const roundRobinReassignment = async ({ bookingId }: { bookingId: number }) => { |
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 function is really big and does a lot of things, I think we can improve that to make it better readable and easier to understand. This can be done later on though, I think we should focus on getting this merged now 🙏
packages/features/ee/round-robin/roundRobinReassignment.test.ts
Outdated
Show resolved
Hide resolved
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.
Nice work! I tested everything including calendar events and emails and looks good now 🙏
What does this PR do?
This PR allows teams to reassign round robin hosts via the round robin algorithm.
Fixes # (issue)
https://www.loom.com/share/3baf73e0a4754719871c6def104dabad
Requirement/Documentation
Type of change
How should this be tested?
Mandatory Tasks