Conversation
WalkthroughThe PR introduces AtomsSecondaryEmailsRepository for managing secondary emails (read/write via Prisma) and registers it in AtomsModule. Secondary email methods are removed from AtomsRepository. VerificationAtomsService updates its constructor to use AtomsSecondaryEmailsRepository and UsersRepository, replaces TeamsRepository.getTeamMemberEmails with UsersRepository.getUserEmailsVerifiedForTeam, and switches all secondary-email operations to the new repository. TeamsRepository removes getTeamMemberEmails, while UsersRepository adds getUserEmailsVerifiedForTeam. On the frontend, AddVerifiedEmail switches from a local validator to a shared isValidEmail utility, which is added in packages/features/isValidEmail.ts. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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. Comment |
keithwillcode
added
core
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/features/eventtypes/components/AddVerifiedEmail.tsx (1)
77-84: Normalize input before validation; prevent silent no-ops.
- Trim before validating/sending to avoid trailing-space failures.
- Optionally disable the button when invalid instead of doing nothing.
Apply:
- onClick={() => { - if (!!verifiedEmail && isValidEmail(verifiedEmail)) { + onClick={() => { + const normalized = verifiedEmail.trim(); + if (normalized && isValidEmail(normalized)) { setIsEmailVerificationModalVisible(true); - handleVerifyEmail(); + handleVerifyEmail(); } }}Optional (disable when invalid):
- <Button + <Button type="button" color="minimal" size="sm" StartIcon="arrow-right" + disabled={!verifiedEmail.trim() || !isValidEmail(verifiedEmail.trim())}apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (3)
73-96: Ensure only verified primaries are returned for teams and de-duplicate resultsCurrently, primary emails are pushed without a verified check and duplicates are possible. Either rely on the repository to filter primaries (preferred) or guard here; also dedupe.
- const verifiedEmails: string[] = []; - const teamMembers = await this.usersRepository.getUserEmailsVerifiedForTeam(teamId); + const seen = new Set<string>(); + const teamMembers = await this.usersRepository.getUserEmailsVerifiedForTeam(teamId); if (teamMembers.length === 0) { - return verifiedEmails; + return []; } - teamMembers.forEach((member) => { - const memberEmailWithoutOauthClientId = this.removeClientIdFromEmail(member.email); - - verifiedEmails.push(memberEmailWithoutOauthClientId); - member.secondaryEmails.forEach((secondaryEmail) => { - verifiedEmails.push(this.removeClientIdFromEmail(secondaryEmail.email)); - }); - }); - - return verifiedEmails; + for (const member of teamMembers) { + // If repository filters primaries by emailVerified!=null, this check is redundant but harmless. + const primary = this.removeClientIdFromEmail(member.email); + seen.add(primary); + for (const secondaryEmail of member.secondaryEmails) { + seen.add(this.removeClientIdFromEmail(secondaryEmail.email)); + } + } + return Array.from(seen);
117-131: Block secondary emails that are another user’s primaryThis path allows adding a secondary that matches someone else’s primary. Check user table before insert.
}): Promise<boolean> { const existingSecondaryEmail = await this.atomsSecondaryEmailsRepository.getExistingSecondaryEmailByUserAndEmail(userId, email); const alreadyExistingEmail = await this.atomsSecondaryEmailsRepository.getExistingSecondaryEmail(email); + const primaryOwner = await this.usersRepository.findByEmail(email); if (alreadyExistingEmail) { throw new BadRequestException("Email already exists"); } + if (primaryOwner && primaryOwner.id !== userId) { + throw new BadRequestException("Email is already in use by another user"); + } if (existingPrimaryEmail === email || existingSecondaryEmail === email) { return true; }
134-144: Guard plus-segmentation to avoid mangling emails without a “+clientId” suffixWhen no “+” exists, this returns “@Domain”. Add a simple guard.
removeClientIdFromEmail(email: string): string { const [localPart, domain] = email.split("@"); const localPartSegments = localPart.split("+"); - localPartSegments.pop(); + if (localPartSegments.length <= 1) { + return email; + } + localPartSegments.pop(); const baseEmail = localPartSegments.join("+"); const normalizedEmail = `${baseEmail}@${domain}`; return normalizedEmail; }
🧹 Nitpick comments (7)
packages/features/isValidEmail.ts (1)
1-1: Prefer shared schema validation; add coverage and trimming.
- Keep regex if you want lightweight, but we already rely on zod elsewhere. Consider delegating to zod for consistency with backend rules and fewer false positives.
- Add unit tests (valid:
a+b@ex.co,üñîçøðé@mañana.esif supported; invalid: missing TLD, spaces).Apply if you want zod-based validation:
-export const isValidEmail = (email: string) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); +import { z } from "zod"; +const emailSchema = z.string().email(); +export const isValidEmail = (email: string) => emailSchema.safeParse(email.trim()).success;packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts (3)
84-89: Consider defensive check using >= to future-proof count logic.If data anomalies ever allow more than one reservation per host, strict equality could under-report unavailability.
- if (existingSlotReservations === hosts.length) { + if (existingSlotReservations >= hosts.length) {
7-21: Explicit return types and minor tidy.Add
Promise<void>return types for clarity; optionally compute partitions in one pass.-export async function validateRoundRobinSlotAvailability( +export async function validateRoundRobinSlotAvailability( eventTypeId: number, startDate: DateTime, endDate: DateTime, hosts: Host[] -) { +) : Promise<void> { - const fixedHosts = hosts.filter((host) => host.isFixed === true); - const nonFixedHosts = hosts.filter((host) => host.isFixed === false); + const { fixedHosts, nonFixedHosts } = hosts.reduce( + (acc, h) => { + (h.isFixed ? acc.fixedHosts : acc.nonFixedHosts).push(h); + return acc; + }, + { fixedHosts: [] as Host[], nonFixedHosts: [] as Host[] } + );
1-91: Avoid race conditions: validate + reserve should be transactional with unique index.This validator can suffer TOCTOU if a reservation is inserted after the check. Ensure the write path uses a DB transaction and uniqueness (e.g., unique on
[eventTypeId, slotUtcStartDate, slotUtcEndDate, hostId?]or an equivalent constraint) to enforce correctness under concurrency.packages/platform/libraries/index.ts (1)
7-7: Prefer direct re-export to reduce churn and keep style consistentImport + export can be collapsed into a single re-export.
- import { validateRoundRobinSlotAvailability } from "@calcom/features/ee/round-robin/utils/validateRoundRobinSlotAvailability"; ... -export { validateRoundRobinSlotAvailability }; +export { validateRoundRobinSlotAvailability } from "@calcom/features/ee/round-robin/utils/validateRoundRobinSlotAvailability";Also applies to: 142-142
apps/api/v2/src/modules/atoms/atoms.module.ts (1)
2-2: Provider wiring looks good; consider filename per guidelinesThe new repository is correctly registered. Per guidelines, avoid new dot-suffix filenames; consider renaming
atoms-secondary-emails.repository.tstoatomsSecondaryEmailsRepository.ts(and update imports) if feasible.Also applies to: 37-37
apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts (1)
5-7: Naming/style nudge (optional): avoid new dot-suffix filenamesGuidelines discourage creating new
.repository.tsfiles. ConsideratomsSecondaryEmailsRepository.tsfor new modules, unless you’re aligning with prevailing project conventions.
📜 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.
📒 Files selected for processing (10)
apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts(1 hunks)apps/api/v2/src/modules/atoms/atoms.module.ts(2 hunks)apps/api/v2/src/modules/atoms/atoms.repository.ts(0 hunks)apps/api/v2/src/modules/atoms/services/verification-atom.service.ts(6 hunks)apps/api/v2/src/modules/teams/teams/teams.repository.ts(0 hunks)apps/api/v2/src/modules/users/users.repository.ts(1 hunks)packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts(1 hunks)packages/features/eventtypes/components/AddVerifiedEmail.tsx(1 hunks)packages/features/isValidEmail.ts(1 hunks)packages/platform/libraries/index.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/api/v2/src/modules/teams/teams/teams.repository.ts
- apps/api/v2/src/modules/atoms/atoms.repository.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/modules/users/users.repository.tsapps/api/v2/src/modules/atoms/services/verification-atom.service.tsapps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.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:
apps/api/v2/src/modules/users/users.repository.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.tsapps/api/v2/src/modules/atoms/atoms.module.tspackages/features/isValidEmail.tsapps/api/v2/src/modules/atoms/services/verification-atom.service.tsapps/api/v2/src/modules/atoms/atoms-secondary-emails.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/users/users.repository.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.tspackages/features/eventtypes/components/AddVerifiedEmail.tsxapps/api/v2/src/modules/atoms/atoms.module.tspackages/features/isValidEmail.tsapps/api/v2/src/modules/atoms/services/verification-atom.service.tsapps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.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/users/users.repository.tspackages/platform/libraries/index.tspackages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.tspackages/features/eventtypes/components/AddVerifiedEmail.tsxapps/api/v2/src/modules/atoms/atoms.module.tspackages/features/isValidEmail.tsapps/api/v2/src/modules/atoms/services/verification-atom.service.tsapps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts
**/*.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
🧠 Learnings (3)
📚 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/utils/validateRoundRobinSlotAvailability.ts
📚 Learning: 2025-08-27T16:39:38.192Z
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.
Applied to files:
packages/features/eventtypes/components/AddVerifiedEmail.tsx
📚 Learning: 2025-08-08T09:29:11.681Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22919
File: packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts:118-143
Timestamp: 2025-08-08T09:29:11.681Z
Learning: In calcom/cal.com PR #22919, packages/features/calAIPhone/interfaces/AIPhoneService.interface.ts (TypeScript), the AIPhoneServiceAgentListItem is required to include user.email in listAgents responses (per maintainer Udit-takkar). Future reviews should not flag this as unnecessary PII unless requirements change.
Applied to files:
packages/features/eventtypes/components/AddVerifiedEmail.tsx
🧬 Code graph analysis (2)
packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts (1)
packages/platform/libraries/index.ts (2)
validateRoundRobinSlotAvailability(142-142)HttpError(60-60)
apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts (2)
apps/api/v2/src/modules/atoms/services/verification-atom.service.ts (1)
Injectable(17-145)apps/api/v2/src/modules/users/users.repository.ts (1)
Injectable(15-437)
🔇 Additional comments (2)
packages/features/eventtypes/components/AddVerifiedEmail.tsx (1)
4-4: Good dedupe: use shared validator.packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts (1)
37-45: No change needed for DateTime precision. Prisma’sDateTimefields map to millisecond-precision types by default (e.g.timestamp(3)on PostgreSQL), sostartDate.toISO()already matches the stored format.
| async getSecondaryEmailsVerified(userId: number) { | ||
| return await this.dbRead.prisma.secondaryEmail.findMany({ | ||
| where: { | ||
| userId, | ||
| emailVerified: { | ||
| not: null, | ||
| }, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Select only needed fields per Prisma guidelines
Only email is consumed by callers; avoid fetching entire records.
async getSecondaryEmailsVerified(userId: number) {
- return await this.dbRead.prisma.secondaryEmail.findMany({
+ return await this.dbRead.prisma.secondaryEmail.findMany({
where: {
userId,
emailVerified: {
not: null,
},
},
+ select: { email: true },
});
}📝 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.
| async getSecondaryEmailsVerified(userId: number) { | |
| return await this.dbRead.prisma.secondaryEmail.findMany({ | |
| where: { | |
| userId, | |
| emailVerified: { | |
| not: null, | |
| }, | |
| }, | |
| }); | |
| } | |
| async getSecondaryEmailsVerified(userId: number) { | |
| return await this.dbRead.prisma.secondaryEmail.findMany({ | |
| where: { | |
| userId, | |
| emailVerified: { | |
| not: null, | |
| }, | |
| }, | |
| select: { | |
| email: true, | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts around
lines 9 to 18, callers only use the email field but the query returns full
records; update the Prisma query to select only the email (e.g. add a select: {
email: true } option to findMany) so only needed data is fetched and adjust the
returned shape if necessary for callers.
| async getExistingSecondaryEmailByUserAndEmail(userId: number, email: string) { | ||
| const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({ | ||
| where: { | ||
| userId_email: { userId, email }, | ||
| }, | ||
| }); | ||
|
|
||
| return existingSecondaryEmailRecord?.email; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use select to reduce payload
Return only the email.
async getExistingSecondaryEmailByUserAndEmail(userId: number, email: string) {
- const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({
+ const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({
where: {
userId_email: { userId, email },
},
+ select: { email: true },
});📝 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.
| async getExistingSecondaryEmailByUserAndEmail(userId: number, email: string) { | |
| const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({ | |
| where: { | |
| userId_email: { userId, email }, | |
| }, | |
| }); | |
| return existingSecondaryEmailRecord?.email; | |
| } | |
| async getExistingSecondaryEmailByUserAndEmail(userId: number, email: string) { | |
| const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({ | |
| where: { | |
| userId_email: { userId, email }, | |
| }, | |
| select: { email: true }, | |
| }); | |
| return existingSecondaryEmailRecord?.email; | |
| } |
🤖 Prompt for AI Agents
In apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts around
lines 20 to 28, the current prisma.findUnique call fetches the whole record but
we only need the email; update the query to include a select clause (select: {
email: true }) so Prisma returns only the email field, keeping the rest of the
function logic unchanged (return existingSecondaryEmailRecord?.email).
| async getExistingSecondaryEmail(email: string) { | ||
| const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({ | ||
| where: { | ||
| email, | ||
| }, | ||
| }); | ||
|
|
||
| return existingSecondaryEmailRecord?.email; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use select for the global email lookup
Same rationale as above.
async getExistingSecondaryEmail(email: string) {
- const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({
+ const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({
where: {
email,
},
+ select: { email: true },
});📝 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.
| async getExistingSecondaryEmail(email: string) { | |
| const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({ | |
| where: { | |
| email, | |
| }, | |
| }); | |
| return existingSecondaryEmailRecord?.email; | |
| } | |
| async getExistingSecondaryEmail(email: string) { | |
| const existingSecondaryEmailRecord = await this.dbRead.prisma.secondaryEmail.findUnique({ | |
| where: { | |
| email, | |
| }, | |
| select: { | |
| email: true, | |
| }, | |
| }); | |
| return existingSecondaryEmailRecord?.email; | |
| } |
🤖 Prompt for AI Agents
In apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts around
lines 30 to 38, the Prisma findUnique call retrieves the full secondaryEmail
record but the method only needs the email string; change the query to use
select: { email: true } so only the email column is fetched (keeping the same
return of existingSecondaryEmailRecord?.email), and ensure null handling and
typings remain correct.
| async addSecondaryEmailVerified(userId: number, email: string) { | ||
| const existingSecondaryEmailRecord = await this.dbWrite.prisma.secondaryEmail.create({ | ||
| data: { | ||
| userId, | ||
| email, | ||
| emailVerified: new Date(), | ||
| }, | ||
| }); | ||
|
|
||
| return existingSecondaryEmailRecord?.email; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid race conditions on insert; upsert and return minimal fields
Concurrent requests can violate unique constraints. Prefer upsert on the composite key and return only the email.
async addSecondaryEmailVerified(userId: number, email: string) {
- const existingSecondaryEmailRecord = await this.dbWrite.prisma.secondaryEmail.create({
- data: {
- userId,
- email,
- emailVerified: new Date(),
- },
- });
-
- return existingSecondaryEmailRecord?.email;
+ const record = await this.dbWrite.prisma.secondaryEmail.upsert({
+ where: { userId_email: { userId, email } },
+ create: { userId, email, emailVerified: new Date() },
+ update: { emailVerified: new Date() },
+ select: { email: true },
+ });
+ return record.email;
}📝 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.
| async addSecondaryEmailVerified(userId: number, email: string) { | |
| const existingSecondaryEmailRecord = await this.dbWrite.prisma.secondaryEmail.create({ | |
| data: { | |
| userId, | |
| email, | |
| emailVerified: new Date(), | |
| }, | |
| }); | |
| return existingSecondaryEmailRecord?.email; | |
| } | |
| async addSecondaryEmailVerified(userId: number, email: string) { | |
| const record = await this.dbWrite.prisma.secondaryEmail.upsert({ | |
| where: { userId_email: { userId, email } }, | |
| create: { userId, email, emailVerified: new Date() }, | |
| update: { emailVerified: new Date() }, | |
| select: { email: true }, | |
| }); | |
| return record.email; | |
| } |
🤖 Prompt for AI Agents
In apps/api/v2/src/modules/atoms/atoms-secondary-emails.repository.ts around
lines 40 to 50, the current create can race on the unique composite
(userId,email); replace it with a Prisma upsert using the composite unique key
in the where clause, provide identical create and update payloads (userId,
email, emailVerified: new Date()), and select only the email field in the return
(e.g. select: { email: true }) so the operation is safe under concurrent
requests and returns the minimal required value.
| const existingBooking = await prisma.booking.findFirst({ | ||
| where: { | ||
| eventTypeId, | ||
| startTime: startDate.toJSDate(), | ||
| endTime: endDate.toJSDate(), | ||
| }, | ||
| select: { attendees: true, userId: true, status: true }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Narrow Prisma select to only needed fields to avoid over-fetching/PII.
You're not using status and you only need attendee ids. Use select narrowly per guidelines.
- const existingBooking = await prisma.booking.findFirst({
+ const existingBooking = await prisma.booking.findFirst({
where: {
eventTypeId,
startTime: startDate.toJSDate(),
endTime: endDate.toJSDate(),
},
- select: { attendees: true, userId: true, status: true },
+ select: {
+ userId: true,
+ attendees: { select: { id: true } },
+ },
});📝 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.
| const existingBooking = await prisma.booking.findFirst({ | |
| where: { | |
| eventTypeId, | |
| startTime: startDate.toJSDate(), | |
| endTime: endDate.toJSDate(), | |
| }, | |
| select: { attendees: true, userId: true, status: true }, | |
| }); | |
| const existingBooking = await prisma.booking.findFirst({ | |
| where: { | |
| eventTypeId, | |
| startTime: startDate.toJSDate(), | |
| endTime: endDate.toJSDate(), | |
| }, | |
| select: { | |
| userId: true, | |
| attendees: { select: { id: true } }, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts
around lines 29 to 36, the Prisma query currently selects attendees, userId and
status but status isn’t used and attendees are fetched entirely (potential PII);
change the select to only fetch userId and attendees' ids (e.g. attendees: {
select: { id: true } }) and remove status from the select so only the minimal
required fields are returned.
| const existingSlotReservation = await prisma.selectedSlots.count({ | ||
| where: { | ||
| eventTypeId, | ||
| slotUtcStartDate: startDate.toISO() ?? "", | ||
| slotUtcEndDate: endDate.toISO() ?? "", | ||
| // Only consider non-expired reservations | ||
| releaseAt: { gt: DateTime.utc().toJSDate() }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure UTC and reject invalid ISO; avoid empty-string fallbacks.
- Columns are
slotUtcStartDate/slotUtcEndDate; usetoUTC().toISO()to match. ?? ""silently degrades to impossible matches; validate inputs and throw 400 instead.
- const existingSlotReservation = await prisma.selectedSlots.count({
+ if (!startDate.isValid || !endDate.isValid) {
+ throw new HttpError({ statusCode: 400, message: "Invalid start or end date" });
+ }
+ const existingSlotReservation = await prisma.selectedSlots.count({
where: {
eventTypeId,
- slotUtcStartDate: startDate.toISO() ?? "",
- slotUtcEndDate: endDate.toISO() ?? "",
+ slotUtcStartDate: startDate.toUTC().toISO(),
+ slotUtcEndDate: endDate.toUTC().toISO(),
// Only consider non-expired reservations
releaseAt: { gt: DateTime.utc().toJSDate() },
},
});📝 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.
| const existingSlotReservation = await prisma.selectedSlots.count({ | |
| where: { | |
| eventTypeId, | |
| slotUtcStartDate: startDate.toISO() ?? "", | |
| slotUtcEndDate: endDate.toISO() ?? "", | |
| // Only consider non-expired reservations | |
| releaseAt: { gt: DateTime.utc().toJSDate() }, | |
| }, | |
| }); | |
| // Reject invalid ISO dates up front | |
| if (!startDate.isValid || !endDate.isValid) { | |
| throw new HttpError({ statusCode: 400, message: "Invalid start or end date" }); | |
| } | |
| const existingSlotReservation = await prisma.selectedSlots.count({ | |
| where: { | |
| eventTypeId, | |
| slotUtcStartDate: startDate.toUTC().toISO(), | |
| slotUtcEndDate: endDate.toUTC().toISO(), | |
| // Only consider non-expired reservations | |
| releaseAt: { gt: DateTime.utc().toJSDate() }, | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts
around lines 37 to 45, the query currently uses startDate.toISO() and
endDate.toISO() with a "?? ''" fallback which can silently produce invalid
queries; change to use startDate.toUTC().toISO() and endDate.toUTC().toISO() so
the ISO strings are in UTC to match slotUtcStartDate/slotUtcEndDate, remove the
empty-string fallbacks, and add input validation before querying: if either
toUTC().toISO() returns null/undefined, throw a 400 Bad Request describing the
invalid slot date(s) rather than proceeding with an empty string match. Ensure
the rest of the where clause (releaseAt > now) remains unchanged.
| const hasHostAsAttendee = hosts.some( | ||
| (host) => | ||
| existingBooking?.attendees.some((attendee) => attendee.id === host.userId) || | ||
| existingBooking?.userId === host.userId | ||
| ); |
There was a problem hiding this comment.
Runtime crash risk: optional-chaining bug on attendees.
existingBooking?.attendees.some(...) will throw when existingBooking is null because .some is accessed on undefined. Use ?. on attendees too.
- const hasHostAsAttendee = hosts.some(
- (host) =>
- existingBooking?.attendees.some((attendee) => attendee.id === host.userId) ||
- existingBooking?.userId === host.userId
- );
+ const hasHostAsAttendee = hosts.some((host) => {
+ const isAttendee = existingBooking?.attendees?.some((attendee) => attendee.id === host.userId) ?? false;
+ const isOwner = existingBooking?.userId === host.userId;
+ return isAttendee || isOwner;
+ });📝 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.
| const hasHostAsAttendee = hosts.some( | |
| (host) => | |
| existingBooking?.attendees.some((attendee) => attendee.id === host.userId) || | |
| existingBooking?.userId === host.userId | |
| ); | |
| const hasHostAsAttendee = hosts.some((host) => { | |
| const isAttendee = existingBooking?.attendees?.some((attendee) => attendee.id === host.userId) ?? false; | |
| const isOwner = existingBooking?.userId === host.userId; | |
| return isAttendee || isOwner; | |
| }); |
🤖 Prompt for AI Agents
In packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts
around lines 47 to 51, the expression existingBooking?.attendees.some(...) can
throw because attendees may be undefined when existingBooking is null; change
the check to use optional chaining on attendees (e.g.,
existingBooking?.attendees?.some(...)) or safely coalesce to false so the call
to .some is never invoked on undefined, and keep the existingBooking?.userId
comparison as-is.
| async function validateNonFixedHostsAvailability( | ||
| eventTypeId: number, | ||
| startDate: DateTime, | ||
| endDate: DateTime, | ||
| hosts: Host[] | ||
| ) { | ||
| const existingSlotReservations = await prisma.selectedSlots.count({ | ||
| where: { | ||
| eventTypeId, | ||
| slotUtcStartDate: startDate.toISO() ?? "", | ||
| slotUtcEndDate: endDate.toISO() ?? "", | ||
| // Only consider non-expired reservations | ||
| releaseAt: { gt: DateTime.utc().toJSDate() }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same UTC/validation fix to non-fixed path.
Mirror the UTC conversion and input validation; also drop empty-string fallbacks.
- const existingSlotReservations = await prisma.selectedSlots.count({
+ if (!startDate.isValid || !endDate.isValid) {
+ throw new HttpError({ statusCode: 400, message: "Invalid start or end date" });
+ }
+ const existingSlotReservations = await prisma.selectedSlots.count({
where: {
eventTypeId,
- slotUtcStartDate: startDate.toISO() ?? "",
- slotUtcEndDate: endDate.toISO() ?? "",
+ slotUtcStartDate: startDate.toUTC().toISO(),
+ slotUtcEndDate: endDate.toUTC().toISO(),
// Only consider non-expired reservations
releaseAt: { gt: DateTime.utc().toJSDate() },
},
});📝 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.
| async function validateNonFixedHostsAvailability( | |
| eventTypeId: number, | |
| startDate: DateTime, | |
| endDate: DateTime, | |
| hosts: Host[] | |
| ) { | |
| const existingSlotReservations = await prisma.selectedSlots.count({ | |
| where: { | |
| eventTypeId, | |
| slotUtcStartDate: startDate.toISO() ?? "", | |
| slotUtcEndDate: endDate.toISO() ?? "", | |
| // Only consider non-expired reservations | |
| releaseAt: { gt: DateTime.utc().toJSDate() }, | |
| }, | |
| }); | |
| async function validateNonFixedHostsAvailability( | |
| eventTypeId: number, | |
| startDate: DateTime, | |
| endDate: DateTime, | |
| hosts: Host[] | |
| ) { | |
| // Validate that the incoming dates are well-formed | |
| if (!startDate.isValid || !endDate.isValid) { | |
| throw new HttpError({ statusCode: 400, message: "Invalid start or end date" }); | |
| } | |
| const existingSlotReservations = await prisma.selectedSlots.count({ | |
| where: { | |
| eventTypeId, | |
| // Normalize to UTC and drop the empty-string fallback | |
| slotUtcStartDate: startDate.toUTC().toISO(), | |
| slotUtcEndDate: endDate.toUTC().toISO(), | |
| // Only consider non-expired reservations | |
| releaseAt: { gt: DateTime.utc().toJSDate() }, | |
| }, | |
| }); | |
| // ...rest of function... | |
| } |
🤖 Prompt for AI Agents
In packages/features/ee/round-robin/utils/validateRoundRobinSlotAvailability.ts
around lines 68 to 82, the non-fixed-hosts query currently uses
startDate.toISO() and endDate.toISO() with empty-string fallbacks and doesn't
ensure UTC; update to validate the DateTime inputs and use their UTC ISO strings
(e.g., startDate.toUTC().toISO() and endDate.toUTC().toISO()) without fallback
empty strings, and throw or return early if either conversion yields
null/undefined so the prisma.count where clause always receives valid UTC ISO
datetime strings.
E2E results are ready! |
4 participants
What does this PR do?
customReplyToEmailfor EventTypeSettings atom #23686 where I implement the PR feedback