feat: Ensure teams with conflicting slugs owned by the user are migrated(handled in backend, frontend already had this restriction) (#25291)

* feat: add backend validation for conflicting team slugs during org onboarding - Added findOwnedTeamsByUserId method to TeamRepository - Created buildTeamsAndInvites method in BaseOnboardingService that automatically: - Detects teams with same slug as organization - Marks conflicting teams for migration (isBeingMigrated: true) - Filters empty team names and invite emails - Updated BillingEnabledOrgOnboardingService to use new method - Updated SelfHostedOnboardingService to use new method - Added comprehensive tests for slug conflict scenarios This ensures backend validation even if frontend is bypassed, preventing slug conflicts during organization creation. All inheriting classes automatically get this validation without code changes. * refactor: use TeamRepository in listOwnedTeamsHandler Refactored listOwnedTeamsHandler to use TeamRepository.findOwnedTeamsByUserId instead of direct Prisma queries. This: - Reduces code duplication - Ensures consistency across the codebase - Follows repository pattern - Makes the handler more maintainable * fix: update tests to use renamed buildTeamsAndInvites method - Renamed testFilterTeamsAndInvites to testBuildTeamsAndInvites - Made test wrapper method async to match the async buildTeamsAndInvites - Added orgSlug parameter to all test calls - Updated all 9 test cases to use await with the new method signature - Fixed lint warnings by using proper types instead of 'any' - Imported OnboardingIntentResult and User types - Used Pick<User> for mockUser type - Removed all 'as any' type casts Fixes test failures where filterTeamsAndInvites was renamed to buildTeamsAndInvites in the base service. Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> * fix: mock TeamRepository in tests to prevent database calls Added vi.mock for TeamRepository to avoid database calls in unit tests. The buildTeamsAndInvites method now calls ensureConflictingSlugTeamIsMigrated which uses TeamRepository.findOwnedTeamsByUserId(). Mocking this prevents Prisma errors in CI while keeping the tests focused on filtering logic. The mock returns an empty array so no teams are found for migration, allowing the tests to verify the filtering behavior without database access. Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> * refactor: improve readability of ensureConflictingSlugTeamIsMigrated Refactored the conditional logic in ensureConflictingSlugTeamIsMigrated for better readability while preserving exact behavior. Changed from manual array manipulation to using .map() for updating team migration status. This is a cosmetic change with no functional differences. Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Udit Takkar <53316345+Udit-takkar@users.noreply.github.com>
2025-11-22 09:07:28 +00:00
parent 1578dee622
commit e29aa2097e
7 changed files with 311 additions and 66 deletions
@@ -9,6 +9,7 @@ import {
  setupDomain,
 } from "@calcom/features/ee/organizations/lib/server/orgCreationUtils";
 import { getOrganizationRepository } from "@calcom/features/ee/organizations/di/OrganizationRepository.container";
+import { TeamRepository } from "@calcom/features/ee/teams/repositories/TeamRepository";
 import { UserRepository } from "@calcom/features/users/repositories/UserRepository";
 import { DEFAULT_SCHEDULE, getAvailabilityFromSchedule } from "@calcom/lib/availability";
 import { WEBAPP_URL } from "@calcom/lib/constants";
@@ -143,8 +144,52 @@ export abstract class BaseOnboardingService implements IOrganizationOnboardingSe
    return organizationOnboarding;
  }

-  protected filterTeamsAndInvites(teams: TeamInput[] = [], invitedMembers: InvitedMemberInput[] = []) {
-    const teamsData = teams
+  private async ensureConflictingSlugTeamIsMigrated(
+    orgSlug: string,
+    teams: TeamInput[] = []
+  ): Promise<TeamInput[]> {
+    const teamRepository = new TeamRepository(prisma);
+    const ownedTeams = await teamRepository.findOwnedTeamsByUserId({ userId: this.user.id });
+
+    const conflictingTeam = ownedTeams.find((team) => team.slug === orgSlug);
+
+    if (!conflictingTeam) {
+      return teams;
+    }
+
+    const existingTeam = teams.find((t) => t.id === conflictingTeam.id);
+
+    if (existingTeam) {
+      if (existingTeam.isBeingMigrated) {
+        return teams;
+      }
+
+      return teams.map((team) =>
+        team.id === conflictingTeam.id
+          ? { ...team, isBeingMigrated: true }
+          : team
+      );
+    }
+
+    return [
+      ...teams,
+      {
+        id: conflictingTeam.id,
+        name: conflictingTeam.name,
+        isBeingMigrated: true,
+        slug: conflictingTeam.slug,
+      },
+    ];
+  }
+
+  protected async buildTeamsAndInvites(
+    orgSlug: string,
+    teams: TeamInput[] = [],
+    invitedMembers: InvitedMemberInput[] = []
+  ) {
+    const enrichedTeams = await this.ensureConflictingSlugTeamIsMigrated(orgSlug, teams);
+
+    const teamsData = enrichedTeams
      .filter((team) => team.name.trim().length > 0)
      .map((team) => ({
        id: team.id === -1 ? -1 : team.id,
@@ -43,7 +43,11 @@ export class BillingEnabledOrgOnboardingService extends BaseOnboardingService {
      })
    );

-    const { teamsData, invitedMembersData } = this.filterTeamsAndInvites(input.teams, input.invitedMembers);
+    const { teamsData, invitedMembersData } = await this.buildTeamsAndInvites(
+      input.slug,
+      input.teams,
+      input.invitedMembers
+    );

    log.debug(
      "BillingEnabledOrgOnboardingService - After filterTeamsAndInvites",
@@ -28,7 +28,7 @@ const teamsSchema = orgOnboardingTeamsSchema;
 /**
 * Handles organization onboarding when billing is disabled (self-hosted admin flow).
 *
- * Flow:
+* Flow:
 * 1. Create onboarding record
 * 2. Store teams/invites in database
 * 3. Immediately create organization, teams, and invite members
@@ -46,8 +46,12 @@ export class SelfHostedOrganizationOnboardingService extends BaseOnboardingServi
      })
    );

-    // Step 1: Filter and normalize teams/invites
-    const { teamsData, invitedMembersData } = this.filterTeamsAndInvites(input.teams, input.invitedMembers);
+    // Step 1: Build and validate teams/invites (includes conflict slug detection)
+    const { teamsData, invitedMembersData } = await this.buildTeamsAndInvites(
+      input.slug,
+      input.teams,
+      input.invitedMembers
+    );

    // Step 2: Create onboarding record with ALL data at once
    const organizationOnboarding = await this.createOnboardingRecord({
@@ -1,24 +1,35 @@
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";

+import type { User } from "@calcom/prisma/client";
 import { UserPermissionRole } from "@calcom/prisma/enums";

+vi.mock("@calcom/features/ee/teams/repositories/TeamRepository", () => ({
+  TeamRepository: class {
+    constructor() {}
+    findOwnedTeamsByUserId(_: { userId: number }) {
+      return Promise.resolve([]);
+    }
+  },
+}));
+
 import { BaseOnboardingService } from "../BaseOnboardingService";
-import type { CreateOnboardingIntentInput } from "../types";
+import type { CreateOnboardingIntentInput, OnboardingIntentResult } from "../types";

 class TestableBaseOnboardingService extends BaseOnboardingService {
-  async createOnboardingIntent(input: CreateOnboardingIntentInput): Promise<any> {
+  async createOnboardingIntent(_input: CreateOnboardingIntentInput): Promise<OnboardingIntentResult> {
    throw new Error("Not implemented");
  }

-  public testFilterTeamsAndInvites(
+  public async testBuildTeamsAndInvites(
+    orgSlug: string,
    teams: CreateOnboardingIntentInput["teams"],
    invitedMembers: CreateOnboardingIntentInput["invitedMembers"]
  ) {
-    return this.filterTeamsAndInvites(teams, invitedMembers);
+    return this.buildTeamsAndInvites(orgSlug, teams, invitedMembers);
  }
 }

-const mockUser = {
+const mockUser: Pick<User, "id" | "email" | "role" | "name"> = {
  id: 1,
  email: "user@example.com",
  role: UserPermissionRole.USER,
@@ -26,9 +37,9 @@ const mockUser = {
 };

 describe("BaseOnboardingService", () => {
-  describe("filterTeamsAndInvites", () => {
-    it("should filter out invites with empty emails", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+  describe("buildTeamsAndInvites", () => {
+    it("should filter out invites with empty emails", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const invites = [
        { email: "valid@example.com", teamName: "Marketing", role: "MEMBER" },
@@ -37,7 +48,7 @@ describe("BaseOnboardingService", () => {
        { email: "another@example.com", teamName: "Design", role: "MEMBER" },
      ];

-      const { invitedMembersData } = service.testFilterTeamsAndInvites([], invites);
+      const { invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", [], invites);

      expect(invitedMembersData).toHaveLength(2);
      expect(invitedMembersData).toEqual([
@@ -58,8 +69,8 @@ describe("BaseOnboardingService", () => {
      ]);
    });

-    it("should preserve all fields from invites including role", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should preserve all fields from invites including role", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const invites = [
        {
@@ -78,7 +89,7 @@ describe("BaseOnboardingService", () => {
        },
      ];

-      const { invitedMembersData } = service.testFilterTeamsAndInvites([], invites);
+      const { invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", [], invites);

      expect(invitedMembersData).toEqual([
        {
@@ -98,15 +109,15 @@ describe("BaseOnboardingService", () => {
      ]);
    });

-    it("should handle invites without optional fields", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should handle invites without optional fields", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const invites = [
        { email: "minimal@example.com" },
        { email: "withteam@example.com", teamName: "Sales" },
      ];

-      const { invitedMembersData } = service.testFilterTeamsAndInvites([], invites);
+      const { invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", [], invites);

      expect(invitedMembersData).toEqual([
        {
@@ -126,8 +137,8 @@ describe("BaseOnboardingService", () => {
      ]);
    });

-    it("should filter out teams with empty names", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should filter out teams with empty names", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const teams = [
        { id: 1, name: "Marketing", isBeingMigrated: false, slug: null },
@@ -136,7 +147,7 @@ describe("BaseOnboardingService", () => {
        { id: 4, name: "Engineering", isBeingMigrated: true, slug: "eng" },
      ];

-      const { teamsData } = service.testFilterTeamsAndInvites(teams, []);
+      const { teamsData } = await service.testBuildTeamsAndInvites("test-org", teams, []);

      expect(teamsData).toHaveLength(2);
      expect(teamsData).toEqual([
@@ -145,15 +156,15 @@ describe("BaseOnboardingService", () => {
      ]);
    });

-    it("should preserve team properties including migration status", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should preserve team properties including migration status", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const teams = [
        { id: -1, name: "New Team", isBeingMigrated: false, slug: null },
        { id: 42, name: "Existing Team", isBeingMigrated: true, slug: "existing-team" },
      ];

-      const { teamsData } = service.testFilterTeamsAndInvites(teams, []);
+      const { teamsData } = await service.testBuildTeamsAndInvites("test-org", teams, []);

      expect(teamsData).toEqual([
        { id: -1, name: "New Team", isBeingMigrated: false, slug: null },
@@ -161,26 +172,26 @@ describe("BaseOnboardingService", () => {
      ]);
    });

-    it("should handle empty teams and invites arrays", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should handle empty teams and invites arrays", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

-      const { teamsData, invitedMembersData } = service.testFilterTeamsAndInvites([], []);
+      const { teamsData, invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", [], []);

      expect(teamsData).toEqual([]);
      expect(invitedMembersData).toEqual([]);
    });

-    it("should handle undefined teams and invites", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should handle undefined teams and invites", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

-      const { teamsData, invitedMembersData } = service.testFilterTeamsAndInvites(undefined, undefined);
+      const { teamsData, invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", undefined, undefined);

      expect(teamsData).toEqual([]);
      expect(invitedMembersData).toEqual([]);
    });

-    it("should preserve invites with teamId=-1 for new teams", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should preserve invites with teamId=-1 for new teams", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const teams = [
        { id: -1, name: "Marketing", isBeingMigrated: false, slug: null },
@@ -192,7 +203,7 @@ describe("BaseOnboardingService", () => {
        { email: "user2@example.com", teamId: -1, teamName: "Sales", role: "ADMIN" },
      ];

-      const { teamsData, invitedMembersData } = service.testFilterTeamsAndInvites(teams, invites);
+      const { teamsData, invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", teams, invites);

      expect(teamsData).toHaveLength(2);
      expect(invitedMembersData).toHaveLength(2);
@@ -202,8 +213,8 @@ describe("BaseOnboardingService", () => {
      expect(invitedMembersData[1].role).toBe("ADMIN");
    });

-    it("should handle mixed scenarios with both org-level and team-specific invites", () => {
-      const service = new TestableBaseOnboardingService(mockUser as any);
+    it("should handle mixed scenarios with both org-level and team-specific invites", async () => {
+      const service = new TestableBaseOnboardingService(mockUser);

      const teams = [
        { id: -1, name: "Marketing", isBeingMigrated: false, slug: null },
@@ -216,7 +227,7 @@ describe("BaseOnboardingService", () => {
        { email: "eng@example.com", teamName: "Engineering", teamId: 42, role: "ADMIN" },
      ];

-      const { teamsData, invitedMembersData } = service.testFilterTeamsAndInvites(teams, invites);
+      const { teamsData, invitedMembersData } = await service.testBuildTeamsAndInvites("test-org", teams, invites);

      expect(teamsData).toHaveLength(2);
      expect(invitedMembersData).toHaveLength(3);
@@ -223,6 +223,181 @@ describe("BillingEnabledOrgOnboardingService", () => {
      });
    });

+    it("should automatically migrate team with conflicting slug", async () => {
+      // Create a team owned by the user with the same slug as the org
+      const conflictingTeam = await prismock.team.create({
+        data: {
+          id: 100,
+          name: "Test Org Team",
+          slug: "test-org", // Same as mockInput.slug
+        },
+      });
+
+      await prismock.membership.create({
+        data: {
+          userId: mockUser.id,
+          teamId: conflictingTeam.id,
+          role: MembershipRole.OWNER,
+          accepted: true,
+        },
+      });
+
+      const inputWithoutConflictingTeam = {
+        ...mockInput,
+        teams: [{ id: -1, name: "Engineering", isBeingMigrated: false, slug: null }],
+      };
+
+      await service.createOnboardingIntent(inputWithoutConflictingTeam);
+
+      // Verify the conflicting team was automatically added to migration
+      expect(mockPaymentService.createOrganizationOnboarding).toHaveBeenCalledWith(
+        expect.objectContaining({
+          teams: expect.arrayContaining([
+            { id: -1, name: "Engineering", isBeingMigrated: false, slug: null },
+            { id: 100, name: "Test Org Team", isBeingMigrated: true, slug: "test-org" },
+          ]),
+        })
+      );
+    });
+
+    it("should mark existing team for migration if slug conflicts", async () => {
+      // Create a team owned by the user with the same slug as the org
+      const conflictingTeam = await prismock.team.create({
+        data: {
+          id: 100,
+          name: "Test Org Team",
+          slug: "test-org", // Same as mockInput.slug
+        },
+      });
+
+      await prismock.membership.create({
+        data: {
+          userId: mockUser.id,
+          teamId: conflictingTeam.id,
+          role: MembershipRole.OWNER,
+          accepted: true,
+        },
+      });
+
+      const inputWithConflictingTeamNotMigrated = {
+        ...mockInput,
+        teams: [
+          { id: -1, name: "Engineering", isBeingMigrated: false, slug: null },
+          { id: 100, name: "Test Org Team", isBeingMigrated: false, slug: "test-org" },
+        ],
+      };
+
+      await service.createOnboardingIntent(inputWithConflictingTeamNotMigrated);
+
+      // Verify the conflicting team was marked for migration
+      expect(mockPaymentService.createOrganizationOnboarding).toHaveBeenCalledWith(
+        expect.objectContaining({
+          teams: expect.arrayContaining([
+            { id: -1, name: "Engineering", isBeingMigrated: false, slug: null },
+            { id: 100, name: "Test Org Team", isBeingMigrated: true, slug: "test-org" },
+          ]),
+        })
+      );
+    });
+
+    it("should not duplicate team if already marked for migration with conflicting slug", async () => {
+      // Create a team owned by the user with the same slug as the org
+      const conflictingTeam = await prismock.team.create({
+        data: {
+          id: 100,
+          name: "Test Org Team",
+          slug: "test-org", // Same as mockInput.slug
+        },
+      });
+
+      await prismock.membership.create({
+        data: {
+          userId: mockUser.id,
+          teamId: conflictingTeam.id,
+          role: MembershipRole.OWNER,
+          accepted: true,
+        },
+      });
+
+      const inputWithConflictingTeamAlreadyMigrated = {
+        ...mockInput,
+        teams: [
+          { id: -1, name: "Engineering", isBeingMigrated: false, slug: null },
+          { id: 100, name: "Test Org Team", isBeingMigrated: true, slug: "test-org" },
+        ],
+      };
+
+      await service.createOnboardingIntent(inputWithConflictingTeamAlreadyMigrated);
+
+      // Verify no duplication occurred
+      expect(mockPaymentService.createOrganizationOnboarding).toHaveBeenCalledWith(
+        expect.objectContaining({
+          teams: [
+            { id: -1, name: "Engineering", isBeingMigrated: false, slug: null },
+            { id: 100, name: "Test Org Team", isBeingMigrated: true, slug: "test-org" },
+          ],
+        })
+      );
+    });
+
+    it("should not migrate team with non-conflicting slug", async () => {
+      // Create a team owned by the user with a different slug
+      const nonConflictingTeam = await prismock.team.create({
+        data: {
+          id: 100,
+          name: "Different Team",
+          slug: "different-team",
+        },
+      });
+
+      await prismock.membership.create({
+        data: {
+          userId: mockUser.id,
+          teamId: nonConflictingTeam.id,
+          role: MembershipRole.OWNER,
+          accepted: true,
+        },
+      });
+
+      await service.createOnboardingIntent(mockInput);
+
+      // Verify the non-conflicting team was NOT added
+      expect(mockPaymentService.createOrganizationOnboarding).toHaveBeenCalledWith(
+        expect.objectContaining({
+          teams: mockInput.teams,
+        })
+      );
+    });
+
+    it("should only migrate teams where user is OWNER or ADMIN", async () => {
+      // Create a team with conflicting slug but user is only a MEMBER
+      const teamAsMember = await prismock.team.create({
+        data: {
+          id: 100,
+          name: "Team As Member",
+          slug: "test-org",
+        },
+      });
+
+      await prismock.membership.create({
+        data: {
+          userId: mockUser.id,
+          teamId: teamAsMember.id,
+          role: MembershipRole.MEMBER,
+          accepted: true,
+        },
+      });
+
+      await service.createOnboardingIntent(mockInput);
+
+      // Verify the team was NOT migrated (user isn't owner/admin)
+      expect(mockPaymentService.createOrganizationOnboarding).toHaveBeenCalledWith(
+        expect.objectContaining({
+          teams: mockInput.teams,
+        })
+      );
+    });
+
    it("should immediately create organization when admin creates org for self", async () => {
      vi.spyOn(constants, "IS_SELF_HOSTED", "get").mockReturnValue(false);

@@ -333,6 +333,35 @@ export class TeamRepository {
      }));
  }

+  /**
+   * Get teams where the user is an OWNER or ADMIN (excludes organizations)
+   */
+  async findOwnedTeamsByUserId({ userId }: { userId: number }) {
+    const memberships = await this.prismaClient.membership.findMany({
+      where: {
+        userId: userId,
+        accepted: true,
+        role: {
+          in: [MembershipRole.OWNER, MembershipRole.ADMIN],
+        },
+      },
+      include: {
+        team: {
+          select: {
+            id: true,
+            name: true,
+            slug: true,
+            isOrganization: true,
+          },
+        },
+      },
+    });
+
+    return memberships
+      .filter((mmship) => !mmship.team.isOrganization)
+      .map((mmship) => mmship.team);
+  }
+
  async findTeamWithOrganizationSettings(teamId: number) {
    return await this.prismaClient.team.findUnique({
      where: { id: teamId },
@@ -1,5 +1,5 @@
+import { TeamRepository } from "@calcom/features/ee/teams/repositories/TeamRepository";
 import { prisma } from "@calcom/prisma";
-import { MembershipRole } from "@calcom/prisma/enums";

 import type { TrpcSessionUser } from "../../../types";

@@ -10,29 +10,6 @@ type ListOptions = {
 };

 export const listOwnedTeamsHandler = async ({ ctx }: ListOptions) => {
-  const user = await prisma.user.findUnique({
-    where: {
-      id: ctx.user.id,
-    },
-    select: {
-      id: true,
-      teams: {
-        where: {
-          accepted: true,
-          role: {
-            in: [MembershipRole.OWNER, MembershipRole.ADMIN],
-          },
-        },
-        select: {
-          team: true,
-        },
-      },
-    },
-  });
-
-  return user?.teams
-    ?.filter((m) => {
-      return !m.team.isOrganization;
-    })
-    ?.map(({ team }) => team);
+  const teamRepository = new TeamRepository(prisma);
+  return await teamRepository.findOwnedTeamsByUserId({ userId: ctx.user.id });
 };