Skip to content

feat: add customReplyToEmail for EventTypeSettings atom#23686

Merged
Ryukemeister merged 24 commits intomainfrom
update-event-advanced-tab-atoms
Sep 9, 2025
Merged

feat: add customReplyToEmail for EventTypeSettings atom#23686
Ryukemeister merged 24 commits intomainfrom
update-event-advanced-tab-atoms

Conversation

@Ryukemeister
Copy link
Contributor

@Ryukemeister Ryukemeister commented Sep 8, 2025

What does this PR do?

  • Add customReplyToEmail feature for EventTypeSettings atom, previously this was only available for our web app.
  • Fixes CAL-6127 (Linear issue number - should be visible at the bottom of the GitHub issue description)
  • Fixes customReplyToEmail option not available #22631

Video Demo:

https://www.loom.com/share/40124762d60645888ad4f37f2e812487?sid=7fe715f6-66aa-486c-94db-e51e6263a721

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • (N/A) I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • (N/A) I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

This can be tested in the examples app we have under packages/platform/examples/base

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 8, 2025

Walkthrough

Adds verified-email management across backend and frontend. Backend: AtomsModule registers TeamsRepository; AtomsRepository gains methods to read/add secondary emails; TeamsRepository adds getTeamMemberEmails; VerificationAtomsService adds getVerifiedEmails and addVerifiedEmail and a client-id sanitiser; new controller endpoints GET/POST /atoms/emails/verified-emails with DTOs and output type. Frontend: hooks useGetVerifiedEmails and useAddVerifiedEmail, appendClientIdToEmail helper, AddVerifiedEmail component, EventAdvancedTab now renders customReplyToEmail unconditionally and receives verifiedEmails via a platform wrapper. Locale keys and a changeset added.

Possibly related PRs

Pre-merge checks (2 passed, 3 warnings)

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The current PR title indicates only the addition of the customReplyToEmail feature for the EventTypeSettings atom, but the changeset includes extensive additions for verified email workflows across modules and components that are not reflected in the title. This mismatch fails to capture the main scope of the pull request and may mislead reviewers about the breadth of changes. Revise the PR title to summarize the primary change across the pull request, such as “feat: support verified secondary emails and customReplyToEmail in EventTypeSettings atom,” or split unrelated verified email functionality into separate PRs with appropriate titles.
Out of Scope Changes Check ⚠️ Warning The pull request includes extensive new verified email features—repository methods, API controllers, services, DTOs, React components, hooks, and utilities—that are unrelated to exposing customReplyToEmail in the Atom package. These additions go beyond the objectives of linked issues #22631 and CAL-6127 and deviate from the intended scope. Split the verified email functionality into a separate pull request focused on the new API and UI components, keeping this PR focused solely on exposing the customReplyToEmail option in the EventTypeSettings atom; alternatively update the PR objectives and title to reflect the broader verified email feature set.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues Check ✅ Passed The code removes the conditional hiding for customReplyToEmail in EventAdvancedTab and makes the option available for platform subscriptions, directly satisfying the linked issues #22631 and CAL-6127. This exposes the customReplyToEmail event option in the Atom package and ensures feature parity with the web app.
Description Check ✅ Passed The pull request description clearly explains the addition of the customReplyToEmail feature and references the associated issues, which relates directly to part of the code changes. Though it does not detail the additional verified email API and UI modifications, this check only requires that the description be meaningfully related to the changeset.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-event-advanced-tab-atoms

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Ryukemeister Ryukemeister changed the title feat: add customReplyToEmail for EventTypeSettings atom feat: add customReplyToEmail for EventTypeSettings atom Sep 8, 2025
@keithwillcode keithwillcode added core area: core, team members only platform Anything related to our platform plan labels Sep 8, 2025
@linear
Copy link

linear bot commented Sep 8, 2025

@github-actions github-actions bot added Low priority Created by Linear-GitHub Sync ✨ feature New feature or request 🚨 needs approval This feature request has not been reviewed yet by the Product Team and needs approval beforehand labels Sep 8, 2025
@Ryukemeister Ryukemeister removed ✨ feature New feature or request Low priority Created by Linear-GitHub Sync 🚨 needs approval This feature request has not been reviewed yet by the Product Team and needs approval beforehand labels Sep 8, 2025
@github-actions github-actions bot added Low priority Created by Linear-GitHub Sync ✨ feature New feature or request 🚨 needs approval This feature request has not been reviewed yet by the Product Team and needs approval beforehand labels Sep 8, 2025
@vercel
Copy link

vercel bot commented Sep 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Sep 9, 2025 11:25am
cal-eu Ignored Ignored Sep 9, 2025 11:25am

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Nitpick comments (9)
apps/api/v2/src/modules/atoms/inputs/add-verified-email.input.ts (1)

4-8: Trim input and require non-empty string

Adds basic hardening. Optional but helps avoid UX issues with whitespace.

-import { ApiProperty } from "@nestjs/swagger";
-import { IsEmail } from "class-validator";
+import { ApiProperty } from "@nestjs/swagger";
+import { IsEmail, IsNotEmpty } from "class-validator";
+import { Transform } from "class-transformer";
 
 export class AddVerifiedEmailInput {
   @ApiProperty({ example: "user@example.com" })
-  @IsEmail()
+  @Transform(({ value }) => (typeof value === "string" ? value.trim() : value))
+  @IsNotEmpty()
+  @IsEmail()
   email!: string;
 }
apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts (1)

11-24: Use integer validation and transform for numeric IDs

Enforce integers and auto-transform query/body values to numbers.

-import { IsNumber, IsOptional, IsString, IsEmail } from "class-validator";
+import { IsInt, IsOptional, IsString, IsEmail } from "class-validator";
+import { Type } from "class-transformer";
@@
 export class GetVerifiedEmailsInput {
   @ApiProperty()
-  @IsNumber()
+  @Type(() => Number)
+  @IsInt()
   userId!: number;
@@
   @ApiPropertyOptional()
   @IsOptional()
-  @IsNumber()
+  @Type(() => Number)
+  @IsInt()
   teamId?: number;
 }

Confirm the controller parses GetVerifiedEmailsParams.teamId into a number (e.g., via ParseIntPipe or @Type(() => Number)), since it currently declares a string.

apps/api/v2/src/modules/atoms/atoms.repository.ts (1)

90-98: Consistency and correctness for email-only lookup

If email isn’t globally unique, this could return another user’s record. If it is unique, keep findUnique; otherwise use findFirst with userId.

-  async getExistingSecondaryEmailByEmail(email: string) {
-    const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({
-      where: {
-        email,
-      },
-    });
-
-    return existingSecondaryEmailRecord?.email;
-  }
+  async getExistingSecondaryEmailByEmail(email: string) {
+    const rec = await this.dbRead.prisma.secondaryEmail.findFirst({
+      where: { email, emailVerified: { not: null } },
+      select: { email: true },
+    });
+    return rec?.email ?? null;
+  }

Confirm whether SecondaryEmail.email is globally unique in the schema.

packages/platform/atoms/event-types/wrappers/EventAdvancedPlatformWrapper.tsx (2)

9-17: Pass a safe fallback for verifiedEmails

Avoid undefined downstream; pass an empty array when the query hasn’t resolved.

-  const { data: verifiedEmails } = useGetVerifiedEmails(props.team?.id);
+  const { data: verifiedEmails } = useGetVerifiedEmails(props.team?.id);
@@
-      verifiedEmails={verifiedEmails}
+      verifiedEmails={verifiedEmails ?? []}

21-21: Prefer named export over default for consistency

Our guidelines discourage default exports in TSX modules. Optional but helps tree-shaking and refactors.

-export default EventAdvancedPlatformWrapper;
+export { EventAdvancedPlatformWrapper };
packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (1)

65-65: Prefer named import if converting component to named export

If you adopt the named export suggestion for AddVerifiedEmail, update this import.

-import AddVerifiedEmail from "../../AddVerifiedEmail";
+import { AddVerifiedEmail } from "../../AddVerifiedEmail";
packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts (1)

14-25: Type the query result and simplify queryFn

Minor DX improvement: add a generic to useQuery so consumers get proper types.

-  return useQuery({
+  return useQuery<string[]>({
     queryKey: [QUERY_KEY, teamId],
-    queryFn: () => {
-      return http?.get<ApiResponse<string[]>>(pathname).then((res) => {
+    queryFn: async () => {
+      const res = await http.get<ApiResponse<string[]>>(pathname);
         if (res.data.status === SUCCESS_STATUS) {
           return (res.data as ApiSuccessResponse<string[]>).data;
         }
         throw new Error(res.data.error.message);
-      });
+      }
     },
     enabled: isInit && !!accessToken,
   });
packages/features/eventtypes/components/AddVerifiedEmail.tsx (2)

104-104: Prefer named export over default export

Follow repo guidance to avoid default exports in TSX modules.

-export default AddVerifiedEmail;
+export { AddVerifiedEmail };

If you apply this, update import sites (e.g., EventAdvancedTab.tsx line 65).


21-22: State duplication with useVerifyEmail modal (optional)

useVerifyEmail already maintains a visibility state; you can reuse it instead of a separate isEmailVerificationModalVisible to reduce divergence.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f5bb734 and 7fea00d.

📒 Files selected for processing (14)
  • apps/api/v2/src/modules/atoms/atoms.module.ts (2 hunks)
  • apps/api/v2/src/modules/atoms/atoms.repository.ts (1 hunks)
  • apps/api/v2/src/modules/atoms/controllers/atoms.verification.controller.ts (2 hunks)
  • apps/api/v2/src/modules/atoms/inputs/add-verified-email.input.ts (1 hunks)
  • apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts (1 hunks)
  • apps/api/v2/src/modules/atoms/outputs/get-verified-emails-output.ts (1 hunks)
  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (3 hunks)
  • apps/api/v2/src/modules/teams/teams/teams.repository.ts (1 hunks)
  • packages/features/eventtypes/components/AddVerifiedEmail.tsx (1 hunks)
  • packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (3 hunks)
  • packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts (1 hunks)
  • packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts (1 hunks)
  • packages/platform/atoms/event-types/wrappers/EventAdvancedPlatformWrapper.tsx (1 hunks)
  • packages/platform/atoms/lib/appendClientIdToEmail.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{service,repository}.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Avoid dot-suffixes like .service.ts or .repository.ts for new files; reserve .test.ts, .spec.ts, .types.ts for their specific purposes

Files:

  • apps/api/v2/src/modules/atoms/atoms.repository.ts
  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
  • apps/api/v2/src/modules/teams/teams/teams.repository.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v2/src/modules/atoms/atoms.repository.ts
  • packages/platform/atoms/lib/appendClientIdToEmail.ts
  • apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts
  • packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts
  • apps/api/v2/src/modules/atoms/inputs/add-verified-email.input.ts
  • packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts
  • apps/api/v2/src/modules/atoms/atoms.module.ts
  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
  • apps/api/v2/src/modules/atoms/outputs/get-verified-emails-output.ts
  • apps/api/v2/src/modules/atoms/controllers/atoms.verification.controller.ts
  • apps/api/v2/src/modules/teams/teams/teams.repository.ts
**/*.{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:

  • apps/api/v2/src/modules/atoms/atoms.repository.ts
  • packages/platform/atoms/lib/appendClientIdToEmail.ts
  • apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts
  • packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts
  • packages/features/eventtypes/components/AddVerifiedEmail.tsx
  • apps/api/v2/src/modules/atoms/inputs/add-verified-email.input.ts
  • packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts
  • apps/api/v2/src/modules/atoms/atoms.module.ts
  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
  • apps/api/v2/src/modules/atoms/outputs/get-verified-emails-output.ts
  • packages/platform/atoms/event-types/wrappers/EventAdvancedPlatformWrapper.tsx
  • apps/api/v2/src/modules/atoms/controllers/atoms.verification.controller.ts
  • apps/api/v2/src/modules/teams/teams/teams.repository.ts
  • packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.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:

  • apps/api/v2/src/modules/atoms/atoms.repository.ts
  • packages/platform/atoms/lib/appendClientIdToEmail.ts
  • apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts
  • packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts
  • packages/features/eventtypes/components/AddVerifiedEmail.tsx
  • apps/api/v2/src/modules/atoms/inputs/add-verified-email.input.ts
  • packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts
  • apps/api/v2/src/modules/atoms/atoms.module.ts
  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
  • apps/api/v2/src/modules/atoms/outputs/get-verified-emails-output.ts
  • packages/platform/atoms/event-types/wrappers/EventAdvancedPlatformWrapper.tsx
  • apps/api/v2/src/modules/atoms/controllers/atoms.verification.controller.ts
  • apps/api/v2/src/modules/teams/teams/teams.repository.ts
  • packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx
**/*.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/AddVerifiedEmail.tsx
  • packages/platform/atoms/event-types/wrappers/EventAdvancedPlatformWrapper.tsx
  • packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
🧬 Code graph analysis (7)
packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts (2)
packages/platform/types/api.ts (2)
  • ApiResponse (33-35)
  • ApiSuccessResponse (8-8)
packages/platform/constants/api.ts (1)
  • SUCCESS_STATUS (9-9)
packages/features/eventtypes/components/AddVerifiedEmail.tsx (5)
packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts (1)
  • useGetVerifiedEmails (10-26)
packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts (1)
  • useAddVerifiedEmail (15-64)
packages/platform/atoms/hooks/useVerifyEmail.ts (1)
  • useVerifyEmail (27-104)
packages/platform/atoms/hooks/useVerifyCode.ts (1)
  • useVerifyCode (24-115)
packages/features/bookings/components/VerifyCodeDialog.tsx (1)
  • VerifyCodeDialog (13-129)
packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts (3)
packages/platform/types/api.ts (2)
  • ApiResponse (33-35)
  • ApiErrorResponse (19-24)
packages/platform/atoms/lib/appendClientIdToEmail.ts (1)
  • appendClientIdToEmail (1-4)
packages/platform/constants/api.ts (1)
  • SUCCESS_STATUS (9-9)
apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (1)
apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts (1)
  • GetVerifiedEmailsInput (11-25)
apps/api/v2/src/modules/atoms/outputs/get-verified-emails-output.ts (1)
packages/platform/constants/api.ts (2)
  • SUCCESS_STATUS (9-9)
  • ERROR_STATUS (10-10)
packages/platform/atoms/event-types/wrappers/EventAdvancedPlatformWrapper.tsx (2)
packages/features/eventtypes/components/tabs/advanced/EventAdvancedTab.tsx (2)
  • EventAdvancedBaseProps (113-123)
  • EventAdvancedTab (403-1408)
packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts (1)
  • useGetVerifiedEmails (10-26)
apps/api/v2/src/modules/atoms/controllers/atoms.verification.controller.ts (7)
apps/api/v2/src/modules/auth/guards/api-auth/api-auth.guard.ts (1)
  • ApiAuthGuard (7-20)
apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts (1)
  • GetVerifiedEmailsParams (4-9)
apps/api/v2/src/modules/users/users.repository.ts (1)
  • UserWithProfile (10-13)
apps/api/v2/src/modules/atoms/outputs/get-verified-emails-output.ts (1)
  • GetVerifiedEmailsOutput (7-17)
packages/platform/constants/api.ts (1)
  • SUCCESS_STATUS (9-9)
apps/api/v2/src/modules/atoms/inputs/add-verified-email.input.ts (1)
  • AddVerifiedEmailInput (4-8)
packages/platform/types/api.ts (1)
  • ApiResponse (33-35)
🔇 Additional comments (4)
apps/api/v2/src/modules/teams/teams/teams.repository.ts (2)

124-148: Query shape looks good; minimal fields selected

select is used and restricted to id, email, and verified secondaryEmails.email. This aligns with our Prisma select-only guideline.


126-132: Relation teams exists on User model; no change needed.

apps/api/v2/src/modules/atoms/atoms.module.ts (1)

20-41: Provider registration LGTM

Registering TeamsRepository here unblocks DI for the verification service.

packages/features/eventtypes/components/AddVerifiedEmail.tsx (1)

48-51: No-op await on mutate; already addressed with mutateAsync

After switching to mutateAsync, this block is correct. If you keep mutate, remove await and handle completion via callbacks only.

Comment on lines +27 to +38
const verifiedEmailEntry = useMutation<
ApiResponse<{
status: string;
data: {
emailVerified: boolean;
};
}>,
unknown,
{
email: string;
}
>({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Correct mutation generics to match ApiResponse shape

ApiResponse already wraps { data, status }. Don’t nest status inside the generic. Also type the error as ApiErrorResponse.

-  const verifiedEmailEntry = useMutation<
-    ApiResponse<{
-      status: string;
-      data: {
-        emailVerified: boolean;
-      };
-    }>,
-    unknown,
+  const verifiedEmailEntry = useMutation<
+    ApiResponse<{ emailVerified: boolean }>,
+    ApiErrorResponse,
     {
       email: string;
     }
   >({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const verifiedEmailEntry = useMutation<
ApiResponse<{
status: string;
data: {
emailVerified: boolean;
};
}>,
unknown,
{
email: string;
}
>({
const verifiedEmailEntry = useMutation<
ApiResponse<{ emailVerified: boolean }>,
ApiErrorResponse,
{
email: string;
}
>({
// …options
});
🤖 Prompt for AI Agents
In packages/platform/atoms/event-types/hooks/useAddVerifiedEmail.ts around lines
27–38, the useMutation generics wrongly nest status inside the ApiResponse and
use an unknown error type; update the mutation signature to use ApiResponse<{
emailVerified: boolean }> as the success type and ApiErrorResponse as the error
type (i.e. useMutation<ApiResponse<{ emailVerified: boolean }>,
ApiErrorResponse, { email: string }>) and add/import ApiErrorResponse if it’s
missing.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2025

E2E results are ready!

Comment on lines 86 to 91
const memberEmailWithoutOauthClientId = member.email.replace(/\+([^+]+)@/, "@");

verifiedEmails.push(memberEmailWithoutOauthClientId);
member.secondaryEmails.forEach((secondaryEmail) => {
verifiedEmails.push(secondaryEmail.email.replace(/\+([^+]+)@/, "@"));
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't this remove all kind of aliases, not just the platform client id ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's true, I need to fix this one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regex is getting too confusing here will try a simpler approach

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (1)

73-105: Normalize and dedupe verified emails; current implementation returns duplicates and case variants

  • Return set-unique, lowercased, sanitized emails to stabilize UI options and APIs.
  • Avoid nested forEach pushes; iterate and add to a Set.

Apply:

-  async getVerifiedEmails(input: GetVerifiedEmailsInput): Promise<string[]> {
-    const { userId, userEmail, teamId } = input;
-    const userEmailWithoutOauthClientId = this.removeClientIdFromEmail(userEmail);
-
-    if (teamId) {
-      const verifiedEmails: string[] = [];
-      const teamMembers = await this.teamsRepository.getTeamMemberEmails(teamId);
-
-      if (teamMembers.length === 0) {
-        return verifiedEmails;
-      }
-
-      teamMembers.forEach((member) => {
-        const memberEmailWithoutOauthClientId = this.removeClientIdFromEmail(member.email);
-
-        verifiedEmails.push(memberEmailWithoutOauthClientId);
-        member.secondaryEmails.forEach((secondaryEmail) => {
-          verifiedEmails.push(this.removeClientIdFromEmail(secondaryEmail.email));
-        });
-      });
-
-      return verifiedEmails;
-    }
-
-    let verifiedEmails = [userEmailWithoutOauthClientId];
-
-    const secondaryEmails = await this.atomsRepository.getSecondaryEmails(userId);
-    verifiedEmails = verifiedEmails.concat(
-      secondaryEmails.map((secondaryEmail) => this.removeClientIdFromEmail(secondaryEmail.email))
-    );
-
-    return verifiedEmails;
-  }
+  async getVerifiedEmails(input: GetVerifiedEmailsInput): Promise<string[]> {
+    const { userId, userEmail, teamId } = input;
+    const normalize = (e: string) => this.removeClientIdFromEmail(e).toLowerCase();
+
+    if (teamId) {
+      const teamMembers = await this.teamsRepository.getTeamMemberEmails(teamId);
+      if (!teamMembers?.length) return [];
+      const out = new Set<string>();
+      for (const member of teamMembers) {
+        out.add(normalize(member.email));
+        for (const secondary of member.secondaryEmails ?? []) {
+          out.add(normalize(secondary.email));
+        }
+      }
+      return Array.from(out);
+    }
+
+    const out = new Set<string>([normalize(userEmail)]);
+    const secondaryEmails = await this.atomsRepository.getSecondaryEmails(userId);
+    for (const s of secondaryEmails ?? []) out.add(normalize(s.email));
+    return Array.from(out);
+  }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ffd1008 and 8934dd1.

📒 Files selected for processing (2)
  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (3 hunks)
  • packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/platform/atoms/event-types/hooks/useGetVerifiedEmails.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{service,repository}.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Avoid dot-suffixes like .service.ts or .repository.ts for new files; reserve .test.ts, .spec.ts, .types.ts for their specific purposes

Files:

  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
**/*.{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:

  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
**/*.{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:

  • apps/api/v2/src/modules/atoms/services/verification-atom.service.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: anglerfishlyy
PR: calcom/cal.com#0
File: :0-0
Timestamp: 2025-08-27T16:39:38.192Z
Learning: anglerfishlyy successfully implemented CAL-3076 email invitation feature for Cal.com team event-types in PR #23312. The feature allows inviting people via email directly from assignment flow, with automatic team invitation if email doesn't belong to existing team member. Implementation includes Host type modifications (userId?: number, email?: string, isPending?: boolean), CheckedTeamSelect component updates with CreatableSelect, TRPC schema validation with zod email validation, and integration with existing teamInvite system.
🧬 Code graph analysis (1)
apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (1)
apps/api/v2/src/modules/atoms/inputs/get-verified-emails-params.ts (1)
  • GetVerifiedEmailsInput (11-25)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Production builds / Build API v2
  • GitHub Check: Tests / Unit
  • GitHub Check: Production builds / Build API v1
  • GitHub Check: Production builds / Build Web App
  • GitHub Check: Production builds / Build Atoms
  • GitHub Check: Linters / lint
  • GitHub Check: Type check / check-types
🔇 Additional comments (1)
apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (1)

19-22: Constructor DI: LGTM

Injecting TeamsRepository alongside AtomsRepository is appropriate for the new methods.

@Ryukemeister Ryukemeister merged commit cfd1992 into main Sep 9, 2025
63 of 65 checks passed
@Ryukemeister Ryukemeister deleted the update-event-advanced-tab-atoms branch September 9, 2025 13:52
Copy link
Contributor

@supalarry supalarry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done ! Have some comments:

return userTeams;
}

async getSecondaryEmails(userId: number) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename it to getSecondaryEmailsVerified because this excludes non-verified ones.

return existingSecondaryEmailRecord?.email;
}

async addSecondaryEmail(userId: number, email: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rename to addSecondaryEmailVerified because again by default secondary emails are created as non-verified.

return await this.dbRead.prisma.secondaryEmail.findMany({
where: {
userId,
emailVerified: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to move the new functions to a new atoms-secondary-emails.repository.ts - if we keep adding everything in the same atoms repository it will be hard to understand and find things

});
}

async getTeamMemberEmails(teamId: number) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is within teams.repository but calls prisma.user table - this functions needs to be moved elsewhere - if a repository is called X then it only can access X table. Right now it returns users with emails included.

const { t } = useLocale();
const [verifiedEmail, setVerifiedEmail] = useState("");
const [isEmailVerificationModalVisible, setIsEmailVerificationModalVisible] = useState(false);
const isValidEmail = (email: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"packages/features" has email regex already - would be good to create a helper in the "packages/features" isValidEmail that calls this and re-uses it.

emrysal pushed a commit that referenced this pull request Sep 9, 2025
* init: endpoint for fetching verified emails

* fix: enable custom reply to email in frontend

* init endpoint to add verified emails

* add verified emails option for platform in frontend

* fixup: move useGetVerifiedEmails hook to correct folder

* update atoms module

* fixup: teamId should be string

* add methond to fetch team member emails

* update logic to fetch and add emails

* fixup: append client id with email

* fixup: pass teamId for fetching verified emails

* fixup: simplify check for existing emails

* fix: cleanup comments

* fix: implement code rabbit feedback

* fix: add translations

* fixup: update transaltions

* fix: update logic for addVerifiedEmail

* add changesets

* fix: implement PR feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only event-types area: event types, event-types platform Anything related to our platform plan ready-for-e2e size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

customReplyToEmail option not available

4 participants