feat: allow emails and invite people to a team event-type#23314
feat: allow emails and invite people to a team event-type#23314anglerfishlyy wants to merge 2 commits intocalcom:mainfrom
Conversation
|
@anglerfishlyy is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe change introduces utilities to parse and normalize comma/space-separated email hosts, deduplicate entries by userId/email, and lowercase emails. Host handling now supports both userId-based and email-based entries via a unified mapToHost function, preserving existing priority/weight and scheduleId (fallback to defaultScheduleId). onChange paths normalize via email expansion; onActive paths map selected team members into hosts for both Fixed and RoundRobin modes. The UI is simplified to a single AddMembersWithSwitch flow for both modes. formState usage is reduced where unnecessary. No exported/public signatures are modified. ✨ 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
Status, Documentation and Community
|
dosubot
bot
added
event-types
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/24/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add community label" took an action on this PR • (08/24/25)1 label was added to this PR based on Keith Williams's automation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx (3)
311-311: Undefined handler: handleFixedHostsToggle will cause a compile-time errorThe handler referenced in onCheckedChange is not defined/imported anywhere in this file.
Apply this minimal fix (adjust behavior if more complex logic is required):
- onCheckedChange={handleFixedHostsToggle} + onCheckedChange={(checked) => setIsDisabled(checked)}
740-755: updatedHosts only reconciles by userId; extend to email-based hosts to preserve scheduleId/groupIdWhen adding email hosts, scheduleId/groupId won’t be preserved because reconciliation only checks userId. Match on email (case-insensitive) as well.
Apply:
- return changedHosts.map((newValue) => { - const existingHost = existingHosts.find((host: Host) => host.userId === newValue.userId); + return changedHosts.map((newValue) => { + const existingHost = existingHosts.find((host: Host & { email?: string }) => { + if (newValue.userId && host.userId) return host.userId === newValue.userId; + if ((newValue as any).email && host.email) + return host.email.toLowerCase() === (newValue as any).email.toLowerCase(); + return false; + });
463-484: RoundRobin handleMembersActivation also overwrites fixed hosts and ignores email membersEven though this block wasn’t newly added, it now conflicts with the new email-based flow.
- Preserve fixed hosts when activating members.
- Use the same mapToHost helper to support email entries and consistent defaults.
Proposed direction:
- setValue( - "hosts", - teamMembers.map((teamMember) => { - const host = currentHosts.find((host) => host.userId === parseInt(teamMember.value, 10)); - return { - isFixed: false, - userId: parseInt(teamMember.value, 10), - priority: host?.priority ?? 2, - weight: host?.weight ?? 100, - scheduleId: host?.scheduleId || teamMember.defaultScheduleId, - groupId: host?.groupId || groupId, - }; - }), - { shouldDirty: true } - ); + const fixedHosts = currentHosts.filter((h) => h.isFixed); + const rrHosts = teamMembers.map((tm) => ({ ...mapToHost(tm, false, currentHosts), groupId })); + setValue("hosts", [...fixedHosts, ...rrHosts], { shouldDirty: true });Also replace || with ?? for scheduleId if you keep the inline mapping.
🧹 Nitpick comments (4)
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx (4)
79-100: expandCommaSeparatedEmailHosts: add guardrails and clarity
- Consider a hard cap on expansions to avoid pathological inputs (e.g., 50 addresses per entry).
- Avoid setting userId: undefined; once Host type is updated, simply omit userId for email entries.
- Clarify deduping scope: current dedupe ignores groupId; confirm whether the same email is allowed across different RR groups.
Example cap:
+const MAX_EMAILS_PER_ENTRY = 50; ... - for (const e of emails) { + for (const e of emails.slice(0, MAX_EMAILS_PER_ENTRY)) { expanded.push({ ...h, email: e, isPending: true, - userId: undefined, scheduleId: null, }); }If duplicates should be allowed across different groups, include groupId in the dedupe key.
39-41: EMAIL_REGEX is OK; consider minor robustness tweaks
- You already lowercase later; no need for /i, but consider unicode domain names in the future (IDNA). For now, this ASCII-focused regex is fine.
- Keep resetting lastIndex as you do in parseEmails.
42-53: parseEmails: efficient and deduplicates; add a soft limit and early exit on large inputsTo prevent accidental large pastes from ballooning allocations, add a max token limit and bail once reached.
Proposed tweak:
-const parseEmails = (input: string): string[] => { +const parseEmails = (input: string, max = 200): string[] => { if (!input) return []; const found = new Set<string>(); let m: RegExpExecArray | null; EMAIL_REGEX.lastIndex = 0; while ((m = EMAIL_REGEX.exec(input)) !== null) { const email = m[1].trim().toLowerCase(); if (email) found.add(email); + if (found.size >= max) break; } return Array.from(found); };
189-196: Localize link textThe “Learn more” label should use t() per guidelines. You’re already using ServerTrans; either move the label inside the translation string or wrap it with t().
Example:
- Learn more + {t("learn_more")}Or include the text in the i18n string and keep the component as an empty anchor.
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx(8 hunks)
🧰 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/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.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/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
🧬 Code graph analysis (1)
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx (2)
packages/features/eventtypes/lib/types.ts (3)
Host(27-34)TeamMember(35-41)FormValues(78-174)packages/features/eventtypes/components/AddMembersWithSwitch.tsx (1)
AddMembersWithSwitch(245-349)
🔇 Additional comments (1)
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx (1)
283-283: Applying expandCommaSeparatedEmailHosts in onChange is the right placeGood normalization point; this keeps the rest of the code working with fully expanded, deduped hosts.
Also applies to: 322-323, 499-500, 611-617
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
Show resolved
Hide resolved
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
Show resolved
Hide resolved
packages/features/eventtypes/components/tabs/assignment/EventTeamAssignmentTab.tsx
Show resolved
Hide resolved
|
@coderabbitai , can you explain this pull request and the changes that i have to make to it for fixing #13532 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll help you understand your PR and what changes might be needed to fix the issue. Let me first gather information about the issue you're trying to resolve. @anglerfishlyy, based on my analysis of issue #13532 and your PR, here's what you're working on and what might need attention: The Issue You're SolvingIssue #13532 requests the ability to "Allow users to assign event-types to emails directly instead of having to invite them to the team first." This would streamline the workflow by letting users assign event types to people via email without a separate invitation step. Your PR Changes ExplainedYour PR makes significant changes to support email-based host assignments: Key Changes:
What Your Code Does:
Potential Areas to Review/FixBased on the code analysis, here are areas that might need attention:
Would you like me to examine any specific part of the code more closely or help you implement fixes for any of these areas? 🏁 Script executed: Length of output: 2151 🏁 Script executed: Length of output: 3024 🏁 Script executed: Length of output: 37207 @anglerfishlyy, now I have a complete picture of your PR and the issue you're solving. Here's a comprehensive breakdown: Understanding Issue #13532The issue requests three specific features:
Your PR Implementation Analysis✅ What's Working WellEmail Parsing Infrastructure: const EMAIL_REGEX = /(?:^|[\s,;])([a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[A-Za-z]{2,})(?=$|[\s,;])/g;
const parseEmails = (input: string): string[] => {
// Handles multiple comma/semicolon/space separated emails
}
const expandCommaSeparatedEmailHosts = (incoming: Host[]): Host[] => {
// Expands comma-separated emails into individual host entries
}Host Format Support:
🚨 Critical Issues That Need Fixing
// Line 218 - This function is called but not defined
onCheckedChange={handleFixedHostsToggle}You need to add this function: const handleFixedHostsToggle = useCallback((checked: boolean) => {
setIsDisabled(checked);
if (!checked) {
// Clear fixed hosts when disabling
const allHosts = getValues("hosts");
const nonFixedHosts = allHosts.filter((host) => !host.isFixed);
setValue("hosts", nonFixedHosts, { shouldDirty: true });
}
}, [getValues, setValue, setIsDisabled]);
const validateEmail = (email: string): boolean => {
return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
};🔧 Specific Changes Needed
🎯 To Complete the Issue RequirementsYou need to verify/implement: Backend Changes (check if these exist): API Integration:
Would you like me to help you identify exactly what backend changes are needed, or would you prefer guidance on fixing the specific frontend issues first? 🏁 Script executed: Length of output: 44 🏁 Script executed: Length of output: 88 🏁 Script executed: Length of output: 88 🏁 Script executed: Length of output: 78 🏁 Script executed: Length of output: 57 🏁 Script executed: Length of output: 54 🏁 Script executed: Length of output: 2604 🏁 Script executed: Length of output: 1210 🏁 Script executed: Length of output: 513 🏁 Script executed: Length of output: 546 🏁 Script executed: Length of output: 222 🏁 Script executed: Length of output: 206 🏁 Script executed: Length of output: 7696 🏁 Script executed: Length of output: 68 🏁 Script executed: Length of output: 109 🏁 Script executed: Length of output: 71 🏁 Script executed: Length of output: 383 Based on the analysis so far, I can now provide you with a comprehensive understanding of what needs to be fixed in your PR. 🚨 Critical Issue DiscoveredLooking at the export type Host = {
isFixed: boolean;
userId: number; // ❌ This is required, not optional
priority: number;
weight: number;
scheduleId?: number | null;
groupId: string | null;
};Your PR is trying to create hosts like 🔧 Required Fixes1. Update the Host Type DefinitionFirst, you need to modify the // In packages/features/eventtypes/lib/types.ts
export type Host = {
isFixed: boolean;
userId?: number; // Make optional
email?: string; // Add email support
isPending?: boolean; // Add pending status
priority: number;
weight: number;
scheduleId?: number | null;
groupId: string | null;
};2. Fix the Missing FunctionAdd the missing const handleFixedHostsToggle = useCallback((checked: boolean) => {
setIsDisabled(checked);
if (!checked) {
// Clear fixed hosts when disabling
const allHosts = getValues("hosts");
const nonFixedHosts = allHosts.filter((host) => !host.isFixed);
setValue("hosts", nonFixedHosts, { shouldDirty: true });
}
}, [getValues, setValue, setIsDisabled]);Let me check the backend structure to understand the complete picture: 3. Backend Integration RequirementsBased on the issue requirements, your backend needs to:
4. Testing Your ChangesTo test if your changes work:
5. Next StepsYou should:
Would you like me to help you identify the specific backend files that need updating, or would you prefer to start with the frontend fixes first? 🏁 Script executed: Length of output: 6048 🏁 Script executed: Length of output: 231 🏁 Script executed: Length of output: 557 🏁 Script executed: Length of output: 496 🏁 Script executed: Length of output: 156 🏁 Script executed: Length of output: 496 🏁 Script executed: Length of output: 79 🏁 Script executed: Length of output: 22696 🏁 Script executed: Length of output: 1886 🏁 Script executed: Length of output: 874 🏁 Script executed: Length of output: 166 Now I have a complete picture! Let me provide you with the definitive analysis and fixes needed for your PR. 🎯 Complete Analysis: What Your PR NeedsThe ProblemYour PR has the right approach but there are critical type and backend integration issues:
// Line 464-470: Backend validation
if (!hosts.every((host) => teamMemberIds.includes(host.userId)) && !eventType.team?.parentId) {
throw new TRPCError({
code: "FORBIDDEN",
});
}🚨 Critical Fixes RequiredFix #1: Update the Host Type Definition// In packages/features/eventtypes/lib/types.ts
export type Host = {
isFixed: boolean;
userId?: number; // ✅ Make optional
email?: string; // ✅ Add email support
isPending?: boolean; // ✅ Add pending status
priority: number;
weight: number;
scheduleId?: number | null;
groupId: string | null;
};Fix #2: Add Missing Function// In your EventTeamAssignmentTab.tsx, add this missing function:
const handleFixedHostsToggle = useCallback((checked: boolean) => {
setIsDisabled(checked);
if (!checked) {
const allHosts = getValues("hosts");
const nonFixedHosts = allHosts.filter((host: Host) => !host.isFixed);
setValue("hosts", nonFixedHosts, { shouldDirty: true });
}
}, [getValues, setValue, setIsDisabled]);Fix #3: Backend Schema UpdatesYou need to update the backend schema to accept email-based hosts. The backend update handler needs to:
Based on the team invitation system I found (there's a complete Backend Changes Required:// In the update handler, modify the host processing logic
if (teamId && hosts) {
// Separate userId-based and email-based hosts
const userIdHosts = hosts.filter(host => host.userId);
const emailHosts = hosts.filter(host => host.email && host.isPending);
// Existing validation for userId hosts
const teamMemberIds = await membershipRepo.listAcceptedTeamMemberIds({ teamId });
if (!userIdHosts.every((host) => teamMemberIds.includes(host.userId)) && !eventType.team?.parentId) {
throw new TRPCError({
code: "FORBIDDEN",
});
}
// Process email hosts - invite them to team
for (const emailHost of emailHosts) {
// Call the existing team invitation logic
// This should invite the user and create a pending membership
}
// Update the host creation logic to handle both types
}Fix #4: Integration with Team InvitationsYou need to integrate with the existing 🎯 Your Action PlanImmediate Steps:
Backend Integration Steps:
Testing:
Would you like me to help you implement any of these specific fixes, or do you want to tackle them in a particular order? 🏁 Script executed: Length of output: 4260 🏁 Script executed: Length of output: 9654 🏁 Script executed: Length of output: 1752 Perfect! Now I have the complete picture. Your PR is very close to working, but there are some critical backend schema and integration issues. Let me give you the definitive solution. 🎯 The Complete Fix GuideProblem Analysis
🔧 Fix #1: Update Backend Schema// In packages/trpc/server/routers/viewer/eventTypes/types.ts
// Replace the existing hostSchema:
const hostSchema = z.object({
userId: z.number().optional(), // ✅ Make optional
email: z.string().email().optional(), // ✅ Add email support
isPending: z.boolean().optional(), // ✅ Add pending status
profileId: z.number().or(z.null()).optional(),
isFixed: z.boolean().optional(),
priority: z.number().min(0).max(4).optional().nullable(),
weight: z.number().min(0).optional().nullable(),
scheduleId: z.number().optional().nullable(),
groupId: z.string().optional().nullable(),
}).refine((data) => {
// ✅ Ensure either userId OR email is provided
return data.userId !== undefined || data.email !== undefined;
}, {
message: "Either userId or email must be provided",
});🔧 Fix #2: Update Frontend Host Type// In packages/features/eventtypes/lib/types.ts
export type Host = {
isFixed: boolean;
userId?: number; // ✅ Make optional
email?: string; // ✅ Add email support
isPending?: boolean; // ✅ Add pending status
priority: number;
weight: number;
scheduleId?: number | null;
groupId: string | null;
};🔧 Fix #3: Add Missing Function (Frontend)// In your EventTeamAssignmentTab.tsx, add this function:
const handleFixedHostsToggle = useCallback((checked: boolean) => {
setIsDisabled(checked);
if (!checked) {
const allHosts = getValues("hosts");
const nonFixedHosts = allHosts.filter((host: Host) => !host.isFixed);
setValue("hosts", nonFixedHosts, { shouldDirty: true });
}
}, [getValues, setValue, setIsDisabled]);🔧 Fix #4: Update Backend Handler (Most Important)In if (teamId && hosts) {
// Separate userId-based and email-based hosts
const userIdHosts = hosts.filter(host => host.userId);
const emailHosts = hosts.filter(host => host.email && host.isPending);
// Existing validation for userId hosts only
const teamMemberIds = await membershipRepo.listAcceptedTeamMemberIds({ teamId });
if (!userIdHosts.every((host) => teamMemberIds.includes(host.userId)) && !eventType.team?.parentId) {
throw new TRPCError({
code: "FORBIDDEN",
});
}
// ✅ Process email-based hosts - invite them to team as Members
if (emailHosts.length > 0) {
// Import the invitation function
const { inviteMembersWithNoInviterPermissionCheck } = await import("../teams/inviteMember/inviteMember.handler");
try {
await inviteMembersWithNoInviterPermissionCheck({
inviterName: ctx.user.name,
teamId: teamId,
language: ctx.user.locale || "en",
creationSource: "INVITATION" as any,
orgSlug: eventType.team?.slug || null,
invitations: emailHosts.map(host => ({
usernameOrEmail: host.email!,
role: MembershipRole.MEMBER
})),
isDirectUserAction: false, // Since this is automated
});
} catch (error) {
// Log the error but don't fail the entire operation
console.warn("Failed to invite some email hosts:", error);
}
}
// Update host processing to handle both types
const allHosts = [...userIdHosts, ...emailHosts];
const oldHostsSet = new Set(eventType.hosts.map((oldHost) => oldHost.userId));
const newHostsSet = new Set(allHosts.filter(h => h.userId).map((host) => host.userId));
const existingHosts = allHosts.filter((newHost) => newHost.userId && oldHostsSet.has(newHost.userId));
const newHosts = allHosts.filter((newHost) => !newHost.userId || !oldHostsSet.has(newHost.userId));
const removedHosts = eventType.hosts.filter((oldHost) => !newHostsSet.has(oldHost.userId));
data.hosts = {
deleteMany: {
OR: removedHosts.map((host) => ({
userId: host.userId,
eventTypeId: id,
})),
},
create: newHosts.filter(host => host.userId).map((host) => {
return {
userId: host.userId!,
isFixed: data.schedulingType === SchedulingType.COLLECTIVE || host.isFixed,
priority: host.priority ?? 2,
weight: host.weight ?? 100,
groupId: host.groupId,
};
}),
update: existingHosts.map((host) => ({
where: {
userId_eventTypeId: {
userId: host.userId!,
eventTypeId: id,
},
},
data: {
isFixed: data.schedulingType === SchedulingType.COLLECTIVE || host.isFixed,
priority: host.priority ?? 2,
weight: host.weight ?? 100,
scheduleId: host.scheduleId ?? null,
groupId: host.groupId,
},
})),
};
}🔧 Fix #5: Add Required ImportAt the top of import { MembershipRole } from "@calcom/prisma/enums";🧪 Testing Your Changes
🎯 Your Implementation Order
💡 Why This Works
The key insight is that email-based hosts trigger team invitations but don't get stored as hosts until the invitation is accepted. This maintains data integrity while enabling the requested functionality. Start with the frontend fixes first, then move to backend integration. Let me know if you need help with any specific part! |
|
@anglerfishlyy I see you have opened two PRs (the other one being #23314) for the same issue. Can you please close the one which is not relevant? |
2 participants
Summary of changes
Updated the assignment flow to support inviting people by email.
If an email does not belong to an existing team member, the user is automatically invited to the team as a Member.
Added support for multiple emails, using comma-separated input (user1@example.com, user2@example.com).
Updated frontend and backend to handle new host format { email, isPending } in addition to { userId }.
Why this is useful
Makes it easier to quickly assign event-types to new collaborators without requiring them to be manually added to the team first.
Supports inviting multiple participants in a single step, improving workflow efficiency.
Notes
Bounty
/claim #13532