Skip to content

Comments

fix: atoms e2e test selected managed user#23919

Closed
ThyMinimalDev wants to merge 6 commits intomainfrom
fix-atoms-e2e-tests
Closed

fix: atoms e2e test selected managed user#23919
ThyMinimalDev wants to merge 6 commits intomainfrom
fix-atoms-e2e-tests

Conversation

@ThyMinimalDev
Copy link
Contributor

@ThyMinimalDev ThyMinimalDev commented Sep 18, 2025

What does this PR do?

ensures admin user is selected by default in platform examples app

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.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

atoms e2e

@ThyMinimalDev ThyMinimalDev requested a review from a team September 18, 2025 13:54
@vercel
Copy link

vercel bot commented Sep 18, 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 19, 2025 3:38pm
cal-eu Ignored Ignored Sep 19, 2025 3:38pm

@ThyMinimalDev ThyMinimalDev requested a review from a team as a code owner September 18, 2025 13:54
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 18, 2025

Walkthrough

Replaces the Select initial value to the first option whose email contains "lauris". The managed-user API response now uses fields from managedUserResponseFive; membership creation calls pass an explicit role and the helper createOrgTeamMembershipMember signature was changed to accept role: "MEMBER" | "ADMIN" = "MEMBER". An E2E test was un-skipped so it now runs. The event-type create handler no longer invokes the PBAC org-level permission call in the org branch and instead sets hasOrgEventTypeCreatePermission = isOrgAdmin (the prior PBAC call/block is removed/commented).

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title clearly indicates a fix to the atoms e2e test related to selecting the managed user by default, which aligns with the pull request’s main change of preselecting the admin user to ensure tests pass.
Description Check ✅ Passed The description explains that the pull request ensures the admin user is selected by default and specifies how to test via atoms e2e, which directly relates to the changes in default selection and e2e test activation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-atoms-e2e-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.

❤️ Share

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

@keithwillcode keithwillcode added core area: core, team members only foundation platform Anything related to our platform plan labels Sep 18, 2025
Comment on lines +136 to +139
id: managedUserResponseFive?.user?.id,
email: (managedUserResponseFive.user.email as string) ?? "",
username: (managedUserResponseFive.user.username as string) ?? "",
accessToken: (managedUserResponseFive.accessToken as string) ?? "",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

managed user 5 is the admin with permission to create team event types

Comment on lines +87 to +89
defaultValue={options.find((opt: TUser | null) => opt?.email.includes("lauris"))}
onChange={(opt: TUser | null) => setSelectedUser(opt)}
options={options}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default value was not selected properly

@dosubot dosubot bot added the automated-tests area: unit tests, e2e tests, playwright label Sep 18, 2025
keithwillcode
keithwillcode previously approved these changes Sep 18, 2025
@keithwillcode keithwillcode enabled auto-merge (squash) September 18, 2025 13:58
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: 1

🧹 Nitpick comments (1)
packages/platform/examples/base/src/pages/_app.tsx (1)

86-90: Harden the defaultValue predicate and types for react-select.

Avoid optional-chaining that can mask type issues and drop the unnecessary | null in the callback param. Also add a null fallback to defaultValue to keep the prop type consistent.

-          defaultValue={options.find((opt: TUser | null) => opt?.email.includes("lauris"))}
+          defaultValue={
+            options.find((opt: TUser) => opt.email.toLowerCase().includes("lauris")) ?? null
+          }

Optional: define an explicit Option type (extends TUser with label/value) and use <Select<Option, false>> to get proper TS safety.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 a44a3e5 and d598072.

📒 Files selected for processing (2)
  • packages/platform/examples/base/src/pages/_app.tsx (1 hunks)
  • packages/platform/examples/base/src/pages/api/managed-user.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.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/platform/examples/base/src/pages/_app.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/platform/examples/base/src/pages/_app.tsx
  • packages/platform/examples/base/src/pages/api/managed-user.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:

  • packages/platform/examples/base/src/pages/_app.tsx
  • packages/platform/examples/base/src/pages/api/managed-user.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:

  • packages/platform/examples/base/src/pages/api/managed-user.ts

Comment on lines +136 to +139
id: managedUserResponseFive?.user?.id,
email: (managedUserResponseFive.user.email as string) ?? "",
username: (managedUserResponseFive.user.username as string) ?? "",
accessToken: (managedUserResponseFive.accessToken as string) ?? "",
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Guard against undefined managed user response to avoid runtime crash; then return non-null fields.

managedUserResponseFive can be undefined if the upstream call fails; accessing .user.email or .accessToken will throw. Add a guard and use non-optional access in the response.

-    id: managedUserResponseFive?.user?.id,
-    email: (managedUserResponseFive.user.email as string) ?? "",
-    username: (managedUserResponseFive.user.username as string) ?? "",
-    accessToken: (managedUserResponseFive.accessToken as string) ?? "",
+    id: managedUserResponseFive.user.id,
+    email: managedUserResponseFive.user.email as string,
+    username: managedUserResponseFive.user.username as string,
+    accessToken: managedUserResponseFive.accessToken as string,

Add this guard just above the return (outside the hunk):

if (
  !managedUserResponseFive ||
  !managedUserResponseFive.user?.id ||
  !managedUserResponseFive.user?.email ||
  !managedUserResponseFive.user?.username ||
  !managedUserResponseFive.accessToken
) {
  throw new Error("Failed to create/select Lauris managed user: missing required fields.");
}
🤖 Prompt for AI Agents
In packages/platform/examples/base/src/pages/api/managed-user.ts around lines
136 to 139, the code dereferences managedUserResponseFive and its nested fields
without checking for undefined, risking runtime crashes; add a guard just above
the return that verifies managedUserResponseFive is defined and that
managedUserResponseFive.user.id, managedUserResponseFive.user.email,
managedUserResponseFive.user.username, and managedUserResponseFive.accessToken
are all present, and throw a clear Error if any are missing so the subsequent
return can safely use non-optional field access.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 18, 2025

E2E results are ready!

emrysal
emrysal previously approved these changes Sep 19, 2025
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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts (1)

24-24: Use click() instead of check() on a button selector.

Locator.check() only supports input[type=checkbox|radio]. Using it on a button will fail.

Apply this diff:

-  await page.locator('[data-testid="create-team-event-type-atom"] button[value="COLLECTIVE"]').check();
+  await page.locator('[data-testid="create-team-event-type-atom"] button[value="COLLECTIVE"]').click();
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)

90-93: Bug: users connect condition is wrong; schedulingType makes it always skip connecting creator.

schedulingType is always truthy, so the current condition never connects the creator for personal event types.

-    // Only connecting the current user for non-managed event types and non team event types
-    users: isManagedEventType || schedulingType ? undefined : { connect: { id: userId } },
+    // Connect current user only for non-managed, non-team event types
+    users: isManagedEventType || !!teamId ? undefined : { connect: { id: userId } },
🧹 Nitpick comments (4)
packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts (3)

5-5: Remove stale ESLint disable.

Since the test is no longer skipped, this suppression is redundant.

-// eslint-disable-next-line playwright/no-skipped-test

7-11: Avoid double navigation unless needed.

If “/” is not required to initialize context, jump directly to “/event-types” to reduce test time.

-  await page.goto("/");
-
   await page.goto("/event-types");
   await expect(page).toHaveURL("/event-types");

70-77: Assert deletion outcome to reduce flakiness.

Verify the card disappears after confirming deletion instead of only checking body visibility.

   await page.locator('[data-testid="dialog-confirmation"]').click();

-  await expect(page.locator("body")).toBeVisible();
+  await expect(
+    page.locator('[data-testid="team-event-type-card"] h1:has-text("Platform team e2e event")')
+  ).toHaveCount(0);
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)

110-117: Prisma: select only required fields from membership.

Follow repo guidelines; you only need id and role here.

-    const hasMembership = await ctx.prisma.membership.findFirst({
+    const hasMembership = await ctx.prisma.membership.findFirst({
       where: {
         userId,
         teamId: teamId,
         accepted: true,
       },
+      select: { id: true, role: true },
     });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 ce7b865 and 2fa5715.

📒 Files selected for processing (3)
  • packages/platform/examples/base/src/pages/api/managed-user.ts (4 hunks)
  • packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts (1 hunks)
  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/platform/examples/base/src/pages/api/managed-user.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:

  • packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts
  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.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:

  • packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts
  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.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:

  • packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts
  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
🧠 Learnings (4)
📓 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.
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.

Applied to files:

  • packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts
📚 Learning: 2025-08-26T20:23:28.396Z
Learnt from: Udit-takkar
PR: calcom/cal.com#22995
File: packages/features/calAIPhone/providers/retellAI/services/AgentService.ts:83-88
Timestamp: 2025-08-26T20:23:28.396Z
Learning: In calcom/cal.com PR #22995, the workflow update handler in packages/trpc/server/routers/viewer/workflows/update.handler.ts includes workflow-level authorization via isAuthorized(userWorkflow, ctx.user.id, "workflow.update") which validates the user can update the workflow before calling updateToolsFromAgentId (per maintainer Udit-takkar).

Applied to files:

  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.

Applied to files:

  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
🧬 Code graph analysis (1)
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)
packages/lib/server/repository/membership.ts (1)
  • hasMembership (87-99)
🔇 Additional comments (2)
packages/platform/examples/base/tests/create-team-event-type-atom/create-team-event-type.e2e.ts (1)

6-6: Un-skipped test: good to run again; verify precondition (default admin selected).

Enabling the test is fine. To prevent flaky failures tied to the new default selection behavior, add an assertion that the admin user is preselected before proceeding.

packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)

148-150: Improved unauthorized log context—looks good.

Helpful signal without leaking sensitive data.

Comment on lines 123 to 144
// Only check for team-level permissions if the user is actually a member of the team.
if (hasMembership) {
try {
const permissions = await getResourcePermissions({
userId,
teamId,
resource: Resource.EventType,
userRole: hasMembership.role,
fallbackRoles: {
create: {
roles: [MembershipRole.ADMIN, MembershipRole.OWNER],
},
},
},
});
hasCreatePermission = permissions.canCreate;
} catch (error) {
// If PBAC check fails, fall back to role-based check
console.warn(
`PBAC check failed for user ${userId} on team ${teamId}, falling back to role check:`,
error
);
hasCreatePermission = ["ADMIN", "OWNER"].includes(hasMembership.role);
});
hasCreatePermission = permissions.canCreate;
} catch (error) {
console.warn(
`PBAC check failed for user ${userId} on team ${teamId}, falling back to role check:`,
error
);
hasCreatePermission = ["ADMIN", "OWNER"].includes(hasMembership.role);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Non‑member org‑admin path allows cross‑org team targeting; add org ownership guard.

When user isn’t a team member but has org-level create permission, nothing ensures the teamId belongs to the user’s org. An org admin could create event types in another org’s team by ID. Gate on team.organizationId match (allow system ADMINs to bypass if intended).

     let hasCreatePermission = false;

     // Only check for team-level permissions if the user is actually a member of the team.
     if (hasMembership) {
       try {
         const permissions = await getResourcePermissions({
           userId,
           teamId,
           resource: Resource.EventType,
           userRole: hasMembership.role,
           fallbackRoles: {
             create: {
               roles: [MembershipRole.ADMIN, MembershipRole.OWNER],
             },
           },
         });
         hasCreatePermission = permissions.canCreate;
       } catch (error) {
         console.warn(
           `PBAC check failed for user ${userId} on team ${teamId}, falling back to role check:`,
           error
         );
         hasCreatePermission = ["ADMIN", "OWNER"].includes(hasMembership.role);
       }
     }
+
+    // If not a member but relying on org-level permission, ensure team belongs to user's org.
+    if (!hasMembership && hasOrgEventTypeCreatePermission && !isSystemAdmin && ctx.user.organizationId) {
+      const team = await ctx.prisma.team.findUnique({
+        where: { id: teamId },
+        select: { organizationId: true },
+      });
+      if (!team || team.organizationId !== ctx.user.organizationId) {
+        console.warn(
+          `Cross-org create blocked: user ${userId} org ${ctx.user.organizationId} -> team ${teamId} org ${team?.organizationId}`
+        );
+        throw new TRPCError({ code: "UNAUTHORIZED" });
+      }
+    }
📝 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
// Only check for team-level permissions if the user is actually a member of the team.
if (hasMembership) {
try {
const permissions = await getResourcePermissions({
userId,
teamId,
resource: Resource.EventType,
userRole: hasMembership.role,
fallbackRoles: {
create: {
roles: [MembershipRole.ADMIN, MembershipRole.OWNER],
},
},
},
});
hasCreatePermission = permissions.canCreate;
} catch (error) {
// If PBAC check fails, fall back to role-based check
console.warn(
`PBAC check failed for user ${userId} on team ${teamId}, falling back to role check:`,
error
);
hasCreatePermission = ["ADMIN", "OWNER"].includes(hasMembership.role);
});
hasCreatePermission = permissions.canCreate;
} catch (error) {
console.warn(
`PBAC check failed for user ${userId} on team ${teamId}, falling back to role check:`,
error
);
hasCreatePermission = ["ADMIN", "OWNER"].includes(hasMembership.role);
}
// Only check for team-level permissions if the user is actually a member of the team.
if (hasMembership) {
try {
const permissions = await getResourcePermissions({
userId,
teamId,
resource: Resource.EventType,
userRole: hasMembership.role,
fallbackRoles: {
create: {
roles: [MembershipRole.ADMIN, MembershipRole.OWNER],
},
},
});
hasCreatePermission = permissions.canCreate;
} catch (error) {
console.warn(
`PBAC check failed for user ${userId} on team ${teamId}, falling back to role check:`,
error
);
hasCreatePermission = ["ADMIN", "OWNER"].includes(hasMembership.role);
}
}
// If not a member but relying on org-level permission, ensure team belongs to user's org.
if (!hasMembership && hasOrgEventTypeCreatePermission && !isSystemAdmin && ctx.user.organizationId) {
const team = await ctx.prisma.team.findUnique({
where: { id: teamId },
select: { organizationId: true },
});
if (!team || team.organizationId !== ctx.user.organizationId) {
console.warn(
`Cross-org create blocked: user ${userId} org ${ctx.user.organizationId} -> team ${teamId} org ${team?.organizationId}`
);
throw new TRPCError({ code: "UNAUTHORIZED" });
}
}
🤖 Prompt for AI Agents
In packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts around
lines 123–144, add a guard for the non‑member org‑admin path so an org‑level
create permission cannot target a team in a different organization: when user is
not a team member but getResourcePermissions grants create, fetch the target
team (or reuse an already-fetched team) and verify team.organizationId ===
userOrgId before allowing create; permit bypass only for system-level ADMINs if
that behavior is intended. If the organization IDs don’t match, deny/fallback to
role-based check or throw an authorization error.

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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (3)

91-95: Bug: users connect is effectively disabled (condition always truthy).

schedulingType is always truthy, so the user is never connected for personal, non‑managed event types.

-    // Only connecting the current user for non-managed event types and non team event types
-    users: isManagedEventType || schedulingType ? undefined : { connect: { id: userId } },
+    // Connect current user only for non-managed, non-team event types
+    users: !isManagedEventType && !teamId ? { connect: { id: userId } } : undefined,

61-79: Restore org‑level PBAC; do not silently bypass with isOrgAdmin

Re‑enable the getResourcePermissions org check and set hasOrgEventTypeCreatePermission = orgPermissions.canCreate; on failure log the error and only then fall back to isOrgAdmin; remove the commented/dead PBAC block.
File: packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (lines ~61–79).


94-95: Validate schedule ownership before connect.

Current code directly does:

    schedule: scheduleId ? { connect: { id: scheduleId } } : undefined,

Before connecting, verify ownership: if teamId is provided assert schedule exists with where: { id: scheduleId, teamId } else assert where: { id: scheduleId, userId }. Use ctx.prisma.schedule.findFirst({ where: , select: { id: true } }) and throw TRPCError({ code: "UNAUTHORIZED" }) (or BAD_REQUEST) when not found. Apply this check in create handler (packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts — lines ~94-95) and the repository usage at packages/lib/server/repository/eventTypeRepository.ts (line ~114).

♻️ Duplicate comments (1)
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)

111-124: LGTM: Team creation now requires membership.

This addresses the prior cross‑org team targeting risk flagged earlier; non‑members are blocked before permission checks.

🧹 Nitpick comments (3)
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (3)

111-111: Avoid gating the team path on schedulingType.

if (teamId && schedulingType) is redundant/fragile; the team path should hinge on teamId alone (schema should ensure schedulingType validity).

-  if (teamId && schedulingType) {
+  if (teamId) {

112-118: Prisma: select only needed fields in membership query.

Follow repo guidelines; fetch only role.

-    const hasMembership = await ctx.prisma.membership.findFirst({
-      where: {
-        userId,
-        teamId: teamId,
-        accepted: true,
-      },
-    });
+    const hasMembership = await ctx.prisma.membership.findFirst({
+      where: { userId, teamId, accepted: true },
+      select: { role: true },
+    });

187-194: Guard against null profile.id before create.

profile.id is typed as possibly null; fail fast with a clear error instead of relying on Prisma errors.

-    const eventTypeRepo = new EventTypeRepository(ctx.prisma);
+    const eventTypeRepo = new EventTypeRepository(ctx.prisma);
+    if (!profile?.id) {
+      throw new TRPCError({ code: "BAD_REQUEST", message: "Missing profile for current user." });
+    }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • 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 2fa5715 and e603771.

📒 Files selected for processing (1)
  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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:

  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.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:

  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.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:

  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
🧠 Learnings (2)
📓 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.
📚 Learning: 2025-08-26T08:08:23.395Z
Learnt from: SinghaAnirban005
PR: calcom/cal.com#23343
File: packages/features/insights/server/trpc-router.ts:1080-1101
Timestamp: 2025-08-26T08:08:23.395Z
Learning: In packages/features/insights/server/trpc-router.ts, when filtering personal event types (userId provided, no teamId, not isAll), the query correctly uses user.id (authenticated user) instead of the input userId parameter for security reasons. This prevents users from accessing other users' personal event types by passing arbitrary user IDs.

Applied to files:

  • packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts
🔇 Additional comments (1)
packages/trpc/server/routers/viewer/eventTypes/heavy/create.handler.ts (1)

166-185: Confirm org‑lock bypass semantics after PBAC change.

With PBAC removed, only org admins bypass lockEventTypeCreationForUsers. If non‑admin roles previously had PBAC‑granted create rights, they’ll now be blocked. Confirm this product decision.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2025

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added Stale and removed Stale labels Oct 4, 2025
@github-actions
Copy link
Contributor

This PR is being marked as stale due to inactivity.

@github-actions github-actions bot added the Stale label Oct 19, 2025
auto-merge was automatically disabled December 18, 2025 14:44

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated-tests area: unit tests, e2e tests, playwright core area: core, team members only foundation platform Anything related to our platform plan ready-for-e2e size/M Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants