-
Notifications
You must be signed in to change notification settings - Fork 9.3k
feat: Multiple Round Robin Host Selection #20796
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
@romitg2 is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (04/21/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (04/21/25)1 label 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 8 issues across 17 files. View them in mrge.io
|
||
*/ | ||
-- AlterTable | ||
ALTER TABLE "EventType" ALTER COLUMN "roundRobinHostsCount" SET NOT 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.
Migration will fail if any NULL values exist in the roundRobinHostsCount column despite the initial default value.
@@ -148,6 +148,7 @@ export type FormValues = { | |||
forwardParamsSuccessRedirect: boolean | null; | |||
secondaryEmailId?: number; | |||
isRRWeightsEnabled: boolean; | |||
roundRobinHostsCount: 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.
The type should be 'number & { min: 1 }' or similar to indicate that this field only accepts positive integers. Currently, it accepts any number which could lead to bugs.
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.
doesn't really require, as we're having that check in other part of code.
@@ -0,0 +1,2 @@ | |||
-- AlterTable | |||
ALTER TABLE "EventType" ADD COLUMN "roundRobinHostsCount" INTEGER DEFAULT 1; |
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.
Column should be defined as NOT NULL since it has a default value and validation in code ensures it's always >= 1.
@@ -0,0 +1,2 @@ | |||
-- AlterTable | |||
ALTER TABLE "EventType" ADD COLUMN "roundRobinHostsCount" INTEGER DEFAULT 1; |
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.
Consider adding a CHECK constraint to enforce roundRobinHostsCount >= 1 at the database level.
await users.deleteAll(); | ||
}); | ||
|
||
test("success booking with multiple hosts", async ({ page, users }) => { |
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: E2E Tests Best Practices
Missing URL assertions after navigation actions
await expect(page2.getByTestId("success-page")).toBeVisible(); | ||
}); | ||
|
||
test("show alert if multiple hosts count is more than available hosts", async ({ page, users }) => { |
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: E2E Tests Best Practices
Missing URL assertions after navigation actions
@TusharBhatt1 @retrogtx review 🙏 |
@CarinaWolli @keithwillcode how would we like to have manual reassignment for multiple hosts?
|
What does this PR do?
This PR Implements a feature where we can have more than 1 hosts selected in round robin manner for an booking.
eg. when someone books an event and we want 2 team members to get assigned to this booking in round robin manner then we can have do that now.
In current version we have only 1 round robin host assignment.
Fixes Multiple Round Robin Hosts #11953 (GitHub issue number)
Fixes CAL-XXXX (Linear issue number - should be visible at the bottom of the GitHub issue description)
/claim Multiple Round Robin Hosts #11953
Feature Walkthrough:
https://www.loom.com/share/67200414fd6c4a50a09cd66bc57f1a1b?sid=fb9ad177-a984-431a-8d40-3a3e34f45899
Code Changes:
https://www.loom.com/share/6bda787c70ba41b9a4036c0e25a6ed06?sid=362a9814-5834-4f80-b131-10611aa29f73
(other than that i've self-tested and it's working fine in edge cases.)
how to test this?
Mandatory Tasks (DO NOT REMOVE)
Summary by mrge
Added support for assigning multiple hosts in round robin events, allowing more than one team member to be selected per booking.