feat: ✨reassign managed event types#22461
Conversation
|
@shaun-ak is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
bot
added
❗️ migrations
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (07/14/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (07/14/25)1 label was added to this PR based on Keith Williams's automation. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic found 4 issues across 17 files. Review them in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
packages/features/ee/managed-event-types/lib/handleChildrenEventTypes.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/round-robin/roundRobinManualReassignment.ts
Outdated
Show resolved
Hide resolved
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds allowManagedEventReassignment across DB, Prisma schema, Zod, platform I/O types, and selection utilities. Surfaces the flag in UI (team event type form checkbox, booking list item, reassignment dialog) and updates action visibility. ReassignDialog gains managed-specific copy and option visibility. Backend logic for reassignment and host resolution uses parent event type for MANAGED scheduling; booking access checks resolve team from parent when applicable. Adds English locale strings, tests, and test builders. Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/features/ee/managed-event-types/lib/handleChildrenEventTypes.ts (1)
204-223: Prisma transaction limit concern remains valid.The existing concern about Prisma's 10-operation limit for array-form transactions still applies here. If many users are added, the upsert operations could exceed this limit.
packages/lib/server/repository/booking.ts (1)
155-161: Database optimization opportunity remains.This still adds an extra database round-trip for managed event types. The parent teamId could be fetched in the initial query using nested select to improve performance.
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
96-96: Boolean logic issue remains unfixed.Using
&&returns the numericparentIdinstead of a boolean. This can cause confusion whenisManagedEventTypeis treated as a boolean flag.Apply this fix:
- const isManagedEventType = eventType.schedulingType === SchedulingType.MANAGED && eventType.parentId; + const isManagedEventType = eventType.schedulingType === SchedulingType.MANAGED && !!eventType.parentId;apps/web/components/dialog/ReassignDialog.tsx (1)
70-72: Type eventTypeSchedulingType as SchedulingType enum for better type safety.The eventTypeSchedulingType parameter should be constrained to the SchedulingType enum rather than string to prevent invalid values and provide compile-time type checking.
-}: ReassignDialog & { eventTypeSchedulingType: string; allowManagedEventReassignment?: boolean }) => { +}: ReassignDialog & { eventTypeSchedulingType: SchedulingType; allowManagedEventReassignment?: boolean }) => {
🧹 Nitpick comments (2)
packages/prisma/zod-utils.ts (1)
683-685: Minor ordering nit – keep props alphabetised for grep-ability.New keys are appended at the end which breaks the (mostly) alphabetical block.
Not a blocker, but consider resorting the object to minimise future merge conflicts.apps/web/public/static/locales/en/common.json (1)
3369-3373: Minor naming / consistency nits
team_member_managed_event_reassign_descriptionis > 60 chars; consider a
shorter key such asmanaged_event_reassign_team_descto keep key length
uniform.- For consistency with the neighbouring strings (
…_description) you may want
to finish the sentence with a period (.).These are purely cosmetic, feel free to ignore if your internal i18n
guidelines already approve the current style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/web/components/booking/BookingListItem.tsx(3 hunks)apps/web/components/dialog/ReassignDialog.tsx(6 hunks)apps/web/public/static/locales/en/common.json(1 hunks)packages/features/ee/managed-event-types/lib/handleChildrenEventTypes.ts(3 hunks)packages/features/ee/round-robin/roundRobinManualReassignment.ts(2 hunks)packages/features/ee/teams/components/TeamEventTypeForm.tsx(2 hunks)packages/features/schedules/components/Schedule.tsx(0 hunks)packages/lib/server/repository/booking.ts(3 hunks)packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts(1 hunks)packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts(1 hunks)packages/prisma/migrations/20250706054307_add_allow_managed_event_reassignment/migration.sql(1 hunks)packages/prisma/migrations/20250710130920_remove_teamid_slug_unique_constraint/migration.sql(1 hunks)packages/prisma/migrations/20250712152559_/migration.sql(1 hunks)packages/prisma/schema.prisma(1 hunks)packages/prisma/zod-utils.ts(1 hunks)packages/prisma/zod/custom/eventtype.ts(1 hunks)packages/trpc/server/routers/viewer/bookings/get.handler.ts(1 hunks)packages/trpc/server/routers/viewer/teams/roundRobin/getRoundRobinHostsToReasign.handler.ts(3 hunks)
💤 Files with no reviewable changes (1)
- packages/features/schedules/components/Schedule.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/web/public/static/locales/en/common.json (1)
undefined
<retrieved_learning>
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
</retrieved_learning>
🧬 Code Graph Analysis (4)
packages/features/ee/managed-event-types/lib/handleChildrenEventTypes.ts (1)
packages/platform/libraries/index.ts (1)
SchedulingType(30-30)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
packages/platform/libraries/index.ts (1)
SchedulingType(30-30)
packages/trpc/server/routers/viewer/teams/roundRobin/getRoundRobinHostsToReasign.handler.ts (3)
packages/platform/libraries/index.ts (1)
SchedulingType(30-30)packages/features/bookings/lib/handleNewBooking/ensureAvailableUsers.ts (1)
ensureAvailableUsers(258-258)packages/features/bookings/lib/handleNewBooking/types.ts (1)
IsFixedAwareUser(35-43)
apps/web/components/dialog/ReassignDialog.tsx (1)
packages/platform/libraries/index.ts (1)
SchedulingType(30-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Security Check
🔇 Additional comments (24)
packages/prisma/migrations/20250706054307_add_allow_managed_event_reassignment/migration.sql (1)
1-2: LGTM! Well-structured migration.The migration correctly adds the new boolean column with a sensible default value of
false, ensuring existing records are handled properly.packages/prisma/migrations/20250710130920_remove_teamid_slug_unique_constraint/migration.sql (1)
1-2: Clarify the necessity of dropping and re-adding the unique constraint.This migration drops the unique constraint on
teamIdandslug, which is later re-added in migration20250712152559_. This pattern is unusual and potentially risky as it temporarily allows duplicate data.Please explain why this constraint needed to be dropped and re-added rather than handling this in a single migration.
packages/trpc/server/routers/viewer/bookings/get.handler.ts (1)
491-491: LGTM! Correctly adds the new field to booking queries.The
allowManagedEventReassignmentfield is properly added to the EventType selection in the booking data query, ensuring it's available to frontend components.packages/prisma/zod/custom/eventtype.ts (1)
33-33: LGTM! Properly adds validation for the new field.The
allowManagedEventReassignmentfield is correctly added as an optional boolean to the input validation schema, maintaining consistency with the database schema.packages/prisma/migrations/20250712152559_/migration.sql (1)
1-8: Confirm no duplicate EventType teamId/slug combinations existBefore applying this migration, ensure the table doesn’t contain any duplicate
(teamId, slug)pairs that would cause the unique index creation to fail:• Run this SQL in your target database:
SELECT "teamId", "slug", COUNT(*) AS cnt FROM "EventType" GROUP BY "teamId", "slug" HAVING COUNT(*) > 1;• If duplicates are found, clean them up (merge or remove) prior to running the migration.
• Consider bundling a pre-migration cleanup script or data-fix step to guarantee data integrity.packages/prisma/schema.prisma (1)
199-203: Field looks good – double-check downstream plumbing.
allowManagedEventReassignmentis declared with the correct type and default. ✔
Please runprisma generate && prisma migrate deploy(or equivalent CI step) to ensure:
- the new column is created in all environments,
- generated TS types are refreshed so that
EventTypenow exposesallowManagedEventReassignment.No action in code required; this is just a reminder to keep dev / CI in sync.
packages/features/ee/teams/components/TeamEventTypeForm.tsx (2)
13-13: Import addition looks good.The CheckboxField import is correctly added and will be used for the new managed event reassignment setting.
158-166: Well-implemented conditional checkbox for managed event reassignment.The implementation correctly:
- Conditionally renders only for managed event types
- Uses proper form registration with the
registerfunction- Follows established localization patterns with
t()function- Maintains consistent styling and structure with other form fields
The feature integration is clean and follows React Hook Form best practices used throughout the component.
packages/platform/types/event-types/event-types_2024_06_14/inputs/create-event-type.input.ts (1)
532-537: Property addition is well-implemented and follows established patterns.The
allowManagedEventReassignmentproperty is correctly implemented with:
- Appropriate optional boolean typing
- Proper validation decorators (
@IsBoolean,@IsOptional)- Clear and descriptive API documentation
- Logical placement within the
CreateTeamEventTypeInput_2024_06_14classThe implementation is consistent with other optional boolean properties in the class and properly scoped to team event types where managed events are relevant.
apps/web/public/static/locales/en/common.json (1)
3364-3368: Check for redundant key names vs. existing round-robin stringsThe new keys use the pattern
allow_managed_event_reassignment,reassign_managed_event_host, etc.
Earlier in this file we already have very similar keys for round-robin
(allowManagedEventReassignmentfeature’s sibling:reassign_round_robin_host,
round_robin_reassign_description, …).Please confirm that:
- The new keys are really needed and are not accidentally duplicating
existing semantics.- The consuming code paths reference these exact new keys – otherwise the UI
will silently fall back to the old strings.If duplication is unavoidable, consider re-using the generic strings instead
of introducing almost-identical copies to keep the dictionary slim.packages/platform/types/event-types/event-types_2024_06_14/outputs/event-type.output.ts (1)
503-508: LGTM! Well-implemented property addition.The new
allowManagedEventReassignmentproperty is properly defined with appropriate validation decorators and clear API documentation.packages/features/ee/managed-event-types/lib/handleChildrenEventTypes.ts (2)
196-197: Properties correctly added to managed event type creation.The
allowManagedEventReassignmentandschedulingTypeproperties are properly included with appropriate default fallbacks.
296-297: Properties correctly added to managed event type updates.Consistent with the creation logic, these properties are properly included in the update payload for existing child event types.
packages/lib/server/repository/booking.ts (1)
138-173: Access control logic correctly handles managed event types.The implementation properly resolves the correct teamId for access checks, ensuring that managed event types use their parent's team context.
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
109-116: Host selection logic correctly handles managed event types.The implementation properly uses parent event type hosts for managed event types, with appropriate fallback to users when no hosts are defined.
packages/trpc/server/routers/viewer/teams/roundRobin/getRoundRobinHostsToReasign.handler.ts (3)
135-136: Managed event type detection implemented correctly.Unlike other files, this properly creates a boolean flag by using the full boolean expression.
139-139: Host retrieval correctly uses parent event type for managed events.The conditional logic properly selects the parent event type ID for fetching hosts when dealing with managed event types.
154-157: Availability checks correctly use parent event type data.The logic appropriately fetches and uses the parent event type for availability calculations when dealing with managed event types.
apps/web/components/booking/BookingListItem.tsx (3)
60-62: LGTM: Clean type extension for managed event reassignment.The optional property addition is well-structured and maintains backward compatibility.
303-307: LGTM: Correctly extends reassign functionality to managed events.The conditional logic properly checks both the scheduling type and the permission flag, ensuring reassignment is only available when explicitly enabled.
515-516: LGTM: Properly passes context to ReassignDialog.The new props correctly provide the dialog with the scheduling type and permission information needed for managed event reassignment.
apps/web/components/dialog/ReassignDialog.tsx (3)
13-13: LGTM: Correct import for SchedulingType enum.The import is properly sourced from the prisma enums package and necessary for the type checking logic.
177-178: LGTM: Correct logic for identifying managed events with reassignment enabled.The condition properly checks both the scheduling type and permission flag to determine when managed event UI should be displayed.
188-189: LGTM: Appropriate UI customization for managed event types.The conditional rendering and text changes correctly tailor the dialog experience for managed events by hiding irrelevant options and providing contextually appropriate labels and descriptions.
Also applies to: 199-199, 213-222
|
|
||
| for (const chunk of chunkArray(upsertOps, 10)) { | ||
| await prisma.$transaction(chunk); | ||
| } |
There was a problem hiding this comment.
On the parent managed event type, we need to add host records, so that this is used to get available hosts while reassignment initiation.
There was a problem hiding this comment.
This should be easily convertible to a bulk query where eventTypeId is fixed which is of parent and we are just having different userIds.
| } else { | ||
| eventTypeHosts = eventType.hosts.length ? eventType.hosts : buildFallbackHosts(eventType.users); | ||
| } | ||
|
|
There was a problem hiding this comment.
If it is a managed event type booking, then we check hosts list of the parent managed event type.
When we add assignment to the parent event type, the code adds the user of the children event type created as host to the parent managed event type.
There was a problem hiding this comment.
Instead of having it as an if condition, we might want to consider Strategy pattern and introduce ManagedEventManualReassignmentService extending ManualReassignmentService(to be created) maybe.
-- Edit
Though it is a small change, we might be fine keeping it for now, but long term we might want to have different classes.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
packages/features/ee/teams/components/TeamEventTypeForm.tsx (1)
11-13: Consolidate form component imports from the same moduleMinor cleanup: import
Form,TextField, andCheckboxFieldin a single statement to reduce duplication.-import { Form } from "@calcom/ui/components/form"; -import { TextField } from "@calcom/ui/components/form"; -import { CheckboxField } from "@calcom/ui/components/form"; +import { Form, TextField, CheckboxField } from "@calcom/ui/components/form";packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)
101-105: Avoid sentinel -1 for parentId; tighten the managed ET guard.You already coerce to boolean with !! (good). Given that guard, using a sentinel -1 is unnecessary and can trigger a throw in getEventTypesFromDB variants that error on not found. Prefer a non-null assertion after the guard for clarity.
- const parentEventType = isManagedEventType ? await getEventTypesFromDB(eventType.parentId ?? -1) : null; + const parentEventType = isManagedEventType ? await getEventTypesFromDB(eventType.parentId!) : null;Also confirm the behavior of getEventTypesFromDB here: if it throws when not found, the subsequent truthy check of parentEventType is more about “managed vs non-managed” than existence; that’s fine but a short code comment would help future readers.
106-115: Fallback host shape: verify required fields and defaults.The synthetic hosts omit any potential identifier fields (e.g., id) and use hardcoded defaults (priority=2, weight=100, schedule=null, createdAt=epoch). If downstream utilities (e.g., getTeamMembers) rely on missing properties, this can break managed flows when parent hosts are empty.
- Verify the minimal host shape expected across the code path.
- Consider centralizing these defaults or deriving from existing constants to avoid magic numbers.
I can help extract a Host type alias and a typed factory to ensure consistency if needed.
apps/web/components/booking/BookingListItem.tsx (1)
72-74: Remove unused top-level allowManagedEventReassignment or wire it up.You extended BookingItem with allowManagedEventReassignment, but you’re not using booking.allowManagedEventReassignment anywhere; instead you read booking.eventType.allowManagedEventReassignment. This extra field is inconsistent and can confuse consumers.
If not needed, drop it:
-type BookingItem = RouterOutputs["viewer"]["bookings"]["get"]["bookings"][number] & { - allowManagedEventReassignment?: boolean; -}; +type BookingItem = RouterOutputs["viewer"]["bookings"]["get"]["bookings"][number];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/web/components/booking/BookingListItem.tsx(3 hunks)apps/web/components/booking/bookingActions.ts(1 hunks)apps/web/public/static/locales/en/common.json(1 hunks)apps/web/test/lib/handleChildrenEventTypes.test.ts(8 hunks)packages/features/ee/round-robin/roundRobinManualReassignment.ts(2 hunks)packages/features/ee/teams/components/TeamEventTypeForm.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/public/static/locales/en/common.json
- apps/web/test/lib/handleChildrenEventTypes.test.ts
- apps/web/components/booking/bookingActions.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/ee/teams/components/TeamEventTypeForm.tsxapps/web/components/booking/BookingListItem.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/features/ee/teams/components/TeamEventTypeForm.tsxapps/web/components/booking/BookingListItem.tsxpackages/features/ee/round-robin/roundRobinManualReassignment.ts
**/*.ts
📄 CodeRabbit Inference Engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/features/ee/round-robin/roundRobinManualReassignment.ts
🧠 Learnings (1)
📚 Learning: 2025-07-22T11:42:47.623Z
Learnt from: CarinaWolli
PR: calcom/cal.com#22296
File: packages/lib/bookings/filterHostsBySameRoundRobinHost.ts:41-42
Timestamp: 2025-07-22T11:42:47.623Z
Learning: The filterHostsBySameRoundRobinHost function in packages/lib/bookings/filterHostsBySameRoundRobinHost.ts has a known limitation where it doesn't work correctly with fixed hosts or round robin groups. This is pre-existing technical debt that was already broken before the round robin groups feature. CarinaWolli has documented this in Linear issue CAL-6134 for future fix.
Applied to files:
packages/features/ee/round-robin/roundRobinManualReassignment.ts
🧬 Code Graph Analysis (2)
apps/web/components/booking/BookingListItem.tsx (1)
packages/platform/libraries/index.ts (1)
SchedulingType(30-30)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (2)
packages/platform/libraries/index.ts (1)
SchedulingType(30-30)apps/web/lib/booking.ts (1)
getEventTypesFromDB(9-116)
🔇 Additional comments (2)
packages/features/ee/round-robin/roundRobinManualReassignment.ts (1)
34-34: Importing SchedulingType looks correct.Import aligns with how enums are sourced elsewhere in this file.
apps/web/components/booking/BookingListItem.tsx (1)
21-21: Importing SchedulingType is appropriate.Used to provide a default scheduling type. See follow-up on using nullish coalescing below.
packages/features/ee/round-robin/roundRobinManualReassignment.ts
Outdated
Show resolved
Hide resolved
|
Hey @PeerRich , can anyone from the core team review this please? Thanks! |
|
This PR is being marked as stale due to inactivity. |
| // If the booking doesn't belong to the user and there's no team then return early | ||
| if (!booking.eventType || !booking.eventType.teamId) return false; | ||
| if (!booking.eventType) return false; | ||
|
|
||
| let teamId = booking.eventType.teamId; | ||
|
|
||
| // For managed event types, get teamId from parent event type | ||
| if (booking.eventType.schedulingType === SchedulingType.MANAGED && booking.eventType.parentId) { | ||
| teamId = booking.eventType.parent?.teamId || teamId; | ||
| } | ||
|
|
||
| if (!teamId) return false; | ||
|
|
There was a problem hiding this comment.
This is application logic and belongs to service. I see there is existing logic in there and that too would move to service method.
# Conflicts: # apps/web/components/booking/BookingListItem.tsx # apps/web/test/lib/handleChildrenEventTypes.test.ts # packages/features/bookings/repositories/BookingRepository.ts # packages/features/ee/round-robin/roundRobinManualReassignment.ts # packages/lib/test/builder.ts # packages/prisma/zod-utils.ts # packages/prisma/zod/custom/eventtype.ts
hariombalhara
left a comment
hariombalhara
left a comment
There was a problem hiding this comment.
Requesting changes as per the comments I did earlier.
Also @shaun-ak I am curious how the actual reassignment would happen because it would be the first time where during reassignment we are working with two different event-types' bookings.
User1EventType - Managed by ParentEventType1
User2EventType- Managed by ParentEventType1
So, if a booking that was created with User1EventType is reassigned to User2. Then we are involved with two different event-types' bookings. I think we need some testing around
- What kind of emails are sent
- What kind of webhook is triggered
|
Hey @hariombalhara , thank you for the PR review. I have a question -- When we reassign then currently the code updates the user of the booking (i.e for example- member 0 -> member 1), should we also update the eventypeId of the booking to point to the reassigned host's eventtypeId? Also, I have manually tested the kind of emails being sent and it seems to work correctly. |
|
Hey @shaun-ak |
|
Thank you @alishaz-polymath for the detailed feedback. Will follow up with the PR and see how the final implementation is done. |
3 participants
What does this PR do?
Adds the functionality to reassign managed event types.
Video Demo (if applicable):
https://cap.link/1sqc7ydxg9c7r8a
Mandatory Tasks (DO NOT REMOVE)
Summary by cubic
Added support for reassigning managed event type bookings to other team members, allowing admins to manually select a new host for these bookings.