From 5360cb511d819d49d90b905b6eae82564ece8ce7 Mon Sep 17 00:00:00 2001
From: Hariom Balhara <hariombalhara@gmail.com>
Date: Thu, 31 Jul 2025 06:23:41 +0530
Subject: [PATCH] fix: Errors in org onboarding in some edge cases (#22711)

* Automatically enable migration of a team that conflicts with Orgs slug

* Allow changing the orgs slug and name if user goes back and changes it

* avoid crashing on some scenarios

* Revert "Allow changing the orgs slug and name if user goes back and changes it"

This reverts commit f8872b0116868d59757ef911c4940df79f93c6e0.

* fix: handle unpublished teams gracefully in org migration

- Change 'No oldSlug for team' from error to warning for unpublished teams
- Keep 'No slug for team' as error since org onboarding ensures teams have names
- Add test for migrating unpublished teams without oldSlug
- Add clarifying comments about when each condition can occur
---
 .../new/_components/AddNewTeamsForm.tsx       |  13 +-
 .../createOrganizationFromOnboarding.test.ts  |  28 ++++
 .../createOrganizationFromOnboarding.ts       |  14 +-
 .../__tests__/createTeams.handler.test.ts     | 106 +++++++++++++
 .../organizations/createTeams.handler.ts      |  22 ++-
 .../inviteMember/inviteMember.handler.test.ts | 149 ++++++++++++++++++
 6 files changed, 314 insertions(+), 18 deletions(-)

diff --git a/apps/web/modules/settings/organizations/new/_components/AddNewTeamsForm.tsx b/apps/web/modules/settings/organizations/new/_components/AddNewTeamsForm.tsx
index 8adb8ce9a8..529363abab 100644
--- a/apps/web/modules/settings/organizations/new/_components/AddNewTeamsForm.tsx
+++ b/apps/web/modules/settings/organizations/new/_components/AddNewTeamsForm.tsx
@@ -78,9 +78,11 @@ const AddNewTeamsFormChild = ({ teams }: { teams: { id: number; name: string; sl
       teams: teamsToCreateFromStore.length ? teamsToCreateFromStore : [{ name: "" }],
       moveTeams: teams.map((team) => {
         const teamToMigrateInStore = teamsToMigrateFromStore.find((t) => t.id === team.id);
+        const slugConflictsWithOrg = team.slug === orgSlug;
         return {
           id: team.id,
-          shouldMove: !!teamToMigrateInStore,
+          // The team with conflicting slug must be moved
+          shouldMove: slugConflictsWithOrg || !!teamToMigrateInStore,
           newSlug: teamToMigrateInStore?.slug || getSuggestedSlug({ teamSlug: team.slug, orgSlug }),
           name: team.name,
         };
@@ -160,6 +162,9 @@ const AddNewTeamsFormChild = ({ teams }: { teams: { id: number; name: string; sl
             </label>
             <ul className="mb-8 space-y-4">
               {moveTeams.map((team, index) => {
+                const currentTeam = teams.find((t) => t.id === team.id);
+                // If the team slug conflicts with the org slug, this team must be moved
+                const slugConflictsWithOrg = currentTeam?.slug === orgSlug;
                 return (
                   <li key={team.id}>
                     <Controller
@@ -167,9 +172,11 @@ const AddNewTeamsFormChild = ({ teams }: { teams: { id: number; name: string; sl
                       render={({ field: { value, onChange } }) => (
                         <CheckboxField
                           defaultValue={value}
-                          checked={value}
+                          checked={value || slugConflictsWithOrg}
                           onChange={onChange}
-                          description={teams.find((t) => t.id === team.id)?.name ?? ""}
+                          description={currentTeam?.name ?? ""}
+                          // Must not allow toggling off if the slug conflicts with the org slug
+                          disabled={slugConflictsWithOrg}
                         />
                       )}
                     />
diff --git a/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.ts b/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.ts
index f32cdfed2f..f13fd58ea0 100644
--- a/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.ts
+++ b/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.test.ts
@@ -348,6 +348,34 @@ describe("createOrganizationFromOnboarding", () => {
       );
     });
 
+    it("should invite members with isDirectUserAction set to false", async () => {
+      const { organizationOnboarding } = await createOnboardingEligibleUserAndOnboarding({
+        user: {
+          username: "org-owner",
+          email: "owner@example.com",
+        },
+      });
+
+      // Call createOrganizationFromOnboarding
+      const result = await createOrganizationFromOnboarding({
+        organizationOnboarding,
+        paymentSubscriptionId: "sub_123",
+        paymentSubscriptionItemId: "si_123",
+      });
+
+      // Verify inviteMembersWithNoInviterPermissionCheck was called with isDirectUserAction: false
+      expect(inviteMembersWithNoInviterPermissionCheck).toHaveBeenCalledWith(
+        expect.objectContaining({
+          teamId: result.organization.id,
+          invitations: [
+            { usernameOrEmail: "member1@example.com", role: MembershipRole.MEMBER },
+            { usernameOrEmail: "member2@example.com", role: MembershipRole.MEMBER },
+          ],
+          isDirectUserAction: false,
+        })
+      );
+    });
+
     it("should update stripe customer ID for existing user", async () => {
       const { user: existingUser, organizationOnboarding } = await createOnboardingEligibleUserAndOnboarding({
         user: {
diff --git a/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts b/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts
index 5708d581df..9aec6570e5 100644
--- a/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts
+++ b/packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts
@@ -305,6 +305,7 @@ async function inviteMembers(invitedMembers: InvitedMember[], organization: Team
       usernameOrEmail: member.email,
       role: MembershipRole.MEMBER,
     })),
+    isDirectUserAction: false,
   });
 }
 
@@ -548,8 +549,17 @@ export const createOrganizationFromOnboarding = async ({
     } catch (error) {
       // Almost always the reason would be that the organization's slug conflicts with a team's slug
       // The owner might not have chosen the conflicting team for migration - Can be confirmed by checking `teams` column in the database.
-      log.error("RecoverableError: Error while setting slug for organization", safeStringify(error));
-      throw new Error("Unable to set slug for organization");
+      log.error(
+        "RecoverableError: Error while setting slug for organization",
+        safeStringify(error),
+        safeStringify({
+          attemptedSlug: organizationOnboarding.slug,
+          organizationId: organization.id,
+        })
+      );
+      throw new Error(
+        `Unable to set slug '${organizationOnboarding.slug}' for organization ${organization.id}`
+      );
     }
   }
 
diff --git a/packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts b/packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts
index 3cc7257308..0f96c23825 100644
--- a/packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts
+++ b/packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts
@@ -311,10 +311,116 @@ describe("createTeams handler", () => {
     // Verify platform team was not moved
     const platformTeam = await prismock.team.findFirst({
       where: { name: "Platform Team" },
+      // eslint-disable-next-line @calcom/eslint/no-prisma-include-true
       include: { parent: true },
     });
 
     expect(platformTeam?.parent?.isPlatform).toBe(true);
     expect(platformTeam?.slug).toBe("platform-team"); // Original slug unchanged
   });
+
+  it("should handle moving non-existent teams gracefully", async () => {
+    const { owner, organization } = await createScenario({
+      teams: [{ name: "Existing Team", slug: "existing-team", addToParentId: "createdOrganization" }],
+    });
+
+    const nonExistentTeamId = 99999; // ID that doesn't exist
+
+    // Should not throw error when trying to move non-existent team
+    const result = await createTeamsHandler({
+      ctx: {
+        user: {
+          id: owner.id,
+          organizationId: organization.id,
+        },
+      },
+      input: {
+        teamNames: ["New Team"],
+        orgId: organization.id,
+        moveTeams: [
+          {
+            id: nonExistentTeamId,
+            shouldMove: true,
+            newSlug: "moved-non-existent",
+          },
+        ],
+        creationSource: CreationSource.WEBAPP,
+      },
+    });
+
+    expect(result).toEqual({ duplicatedSlugs: [] });
+
+    // Verify new team was created successfully
+    const createdTeams = await prismock.team.findMany({
+      where: { parentId: organization.id },
+    });
+
+    expect(createdTeams).toHaveLength(2); // Existing team + New Team
+    expect(createdTeams.map((t) => t.name).sort()).toEqual(["Existing Team", "New Team"]);
+
+    // Verify non-existent team was not created/moved
+    const nonExistentTeam = await prismock.team.findFirst({
+      where: { slug: "moved-non-existent" },
+    });
+
+    expect(nonExistentTeam).toBeNull();
+  });
+
+  it("should handle moving unpublished teams (without slug) gracefully", async () => {
+    const { owner, organization } = await createScenario();
+
+    // Create an unpublished team (team without a slug)
+    const unpublishedTeam = await createTestTeam({
+      name: "Unpublished Team",
+      slug: null,
+      parentId: null,
+    });
+
+    // Add membership for the owner to the team
+    await createTestMembership({
+      userId: owner.id,
+      teamId: unpublishedTeam.id,
+      role: MembershipRole.OWNER,
+    });
+
+    // Should not throw error when trying to move unpublished team
+    const result = await createTeamsHandler({
+      ctx: {
+        user: {
+          id: owner.id,
+          organizationId: organization.id,
+        },
+      },
+      input: {
+        teamNames: [],
+        orgId: organization.id,
+        moveTeams: [
+          {
+            id: unpublishedTeam.id,
+            shouldMove: true,
+            newSlug: "published-team-slug",
+          },
+        ],
+        creationSource: CreationSource.WEBAPP,
+      },
+    });
+
+    expect(result).toEqual({ duplicatedSlugs: [] });
+
+    // Verify team was moved successfully
+    const movedTeam = await prismock.team.findFirst({
+      where: { id: unpublishedTeam.id },
+    });
+
+    expect(movedTeam).toBeTruthy();
+    expect(movedTeam?.parentId).toBe(organization.id);
+    expect(movedTeam?.slug).toBe("published-team-slug");
+
+    // Verify no redirect was created (since there was no oldSlug for unpublished team)
+    const redirects = await prismock.tempOrgRedirect.findMany({});
+
+    // Filter for redirects with null 'from' value
+    const redirectsWithNullFrom = redirects.filter((redirect) => redirect.from === null);
+    expect(redirectsWithNullFrom).toHaveLength(0);
+  });
 });
diff --git a/packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts b/packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
index 56d6df4311..dbc27b1870 100644
--- a/packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
+++ b/packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
@@ -137,12 +137,6 @@ export const createTeamsHandler = async ({ ctx, input }: CreateTeamsOptions) =>
   return { duplicatedSlugs };
 };
 
-class NoUserError extends TRPCError {
-  constructor() {
-    super({ code: "BAD_REQUEST", message: "no_user" });
-  }
-}
-
 class NotAuthorizedError extends TRPCError {
   constructor() {
     super({ code: "FORBIDDEN", message: "not_authorized" });
@@ -214,10 +208,12 @@ async function moveTeam({
   });
 
   if (!team) {
-    throw new TRPCError({
-      code: "BAD_REQUEST",
-      message: `Team with id: ${teamId} not found`,
+    log.warn(`Team with id: ${teamId} not found. Skipping migration.`, {
+      teamId,
+      orgId: org.id,
+      orgSlug: org.slug,
     });
+    return;
   }
 
   if (team.parent?.isPlatform) {
@@ -325,12 +321,12 @@ async function addTeamRedirect({
 }) {
   logger.info(`Adding redirect for team: ${oldTeamSlug} -> ${teamSlug}`);
   if (!oldTeamSlug) {
-    throw new TRPCError({
-      code: "BAD_REQUEST",
-      message: "No oldSlug for team. Not adding the redirect",
-    });
+    // This can happen for unpublished teams that don't have a slug yet
+    logger.warn(`No oldSlug for team. Not adding the redirect`);
+    return;
   }
   if (!teamSlug) {
+    // This should not happen as org onboarding ensures teams have slugs
     throw new TRPCError({
       code: "BAD_REQUEST",
       message: "No slug for team. Not adding the redirect",
diff --git a/packages/trpc/server/routers/viewer/teams/inviteMember/inviteMember.handler.test.ts b/packages/trpc/server/routers/viewer/teams/inviteMember/inviteMember.handler.test.ts
index e54bc48d95..37c07bde5a 100644
--- a/packages/trpc/server/routers/viewer/teams/inviteMember/inviteMember.handler.test.ts
+++ b/packages/trpc/server/routers/viewer/teams/inviteMember/inviteMember.handler.test.ts
@@ -367,6 +367,155 @@ describe("inviteMemberHandler", () => {
         expect(e.message).toBe(INVITE_STATUS.USER_ALREADY_INVITED_OR_MEMBER);
       }
     });
+
+    it("With one email that is already a member and isDirectUserAction=false, it should NOT throw error", async () => {
+      const userToBeInvited = buildExistingUser({
+        id: 1,
+        email: "user1@example.com",
+        username: "user1",
+      });
+
+      const loggedInUser = getLoggedInUser();
+
+      const team = {
+        id: 1,
+        name: "Team 1",
+        parent: null,
+        isOrganization: false,
+      };
+
+      inviteMemberUtilsScenarios.getTeamOrThrow.fakeReturnTeam(team, {
+        teamId: team.id,
+      });
+
+      inviteMemberUtilsScenarios.findUsersWithInviteStatus.useAdvancedMock(
+        [
+          {
+            ...userToBeInvited,
+            canBeInvited: INVITE_STATUS.USER_ALREADY_INVITED_OR_MEMBER,
+            newRole: MembershipRole.MEMBER,
+          },
+        ],
+        {
+          invitations: [
+            {
+              role: MembershipRole.MEMBER,
+              usernameOrEmail: userToBeInvited.email,
+            },
+          ],
+          team,
+        }
+      );
+
+      // Call inviteMembersWithNoInviterPermissionCheck directly with isDirectUserAction=false
+      const { inviteMembersWithNoInviterPermissionCheck } = await import("./inviteMember.handler");
+      
+      const result = await inviteMembersWithNoInviterPermissionCheck({
+        inviterName: loggedInUser.name,
+        teamId: team.id,
+        language: "en",
+        creationSource: "WEBAPP" as const,
+        orgSlug: null,
+        invitations: [
+          {
+            usernameOrEmail: userToBeInvited.email,
+            role: MembershipRole.MEMBER,
+          },
+        ],
+        isDirectUserAction: false,
+      });
+
+      // Should not throw error, should return successfully with 0 users invited
+      expect(result).toEqual({
+        usernameOrEmail: userToBeInvited.email,
+        numUsersInvited: 0,
+      });
+
+      // Verify that handleNewUsersInvites and handleExistingUsersInvites were not called
+      // since the user is already a member
+      expect(inviteMemberUtilsMock.handleNewUsersInvites).not.toHaveBeenCalled();
+      expect(inviteMemberUtilsMock.handleExistingUsersInvites).not.toHaveBeenCalled();
+    });
+
+    it("With multiple emails where some are already members and isDirectUserAction=false, it should NOT throw error and invite only eligible users", async () => {
+      const existingMember = buildExistingUser({
+        id: 1,
+        email: "existing@example.com",
+        username: "existing",
+      });
+
+      const newUser = {
+        id: null,
+        email: "newuser@example.com",
+      };
+
+      const team = {
+        id: 1,
+        name: "Team 1",
+        parent: null,
+        isOrganization: false,
+      };
+
+      inviteMemberUtilsScenarios.getTeamOrThrow.fakeReturnTeam(team, {
+        teamId: team.id,
+      });
+
+      const allInvitations = [
+        {
+          role: MembershipRole.MEMBER,
+          usernameOrEmail: existingMember.email,
+        },
+        {
+          role: MembershipRole.MEMBER,
+          usernameOrEmail: newUser.email,
+        },
+      ];
+
+      inviteMemberUtilsScenarios.findUsersWithInviteStatus.useAdvancedMock(
+        [
+          {
+            ...existingMember,
+            canBeInvited: INVITE_STATUS.USER_ALREADY_INVITED_OR_MEMBER,
+            newRole: MembershipRole.MEMBER,
+          },
+        ],
+        {
+          invitations: allInvitations,
+          team,
+        }
+      );
+
+      // Call inviteMembersWithNoInviterPermissionCheck directly with isDirectUserAction=false
+      const { inviteMembersWithNoInviterPermissionCheck } = await import("./inviteMember.handler");
+      
+      const result = await inviteMembersWithNoInviterPermissionCheck({
+        inviterName: "Test Inviter",
+        teamId: team.id,
+        language: "en",
+        creationSource: "WEBAPP" as const,
+        orgSlug: null,
+        invitations: allInvitations,
+        isDirectUserAction: false,
+      });
+
+      // Should not throw error, should return successfully with 1 user invited (the new user)
+      expect(result).toEqual({
+        usernameOrEmail: [existingMember.email, newUser.email],
+        numUsersInvited: 1,
+      });
+
+      // Verify that only new user was invited
+      expect(inviteMemberUtilsMock.handleNewUsersInvites).toHaveBeenCalledWith(
+        expect.objectContaining({
+          invitationsForNewUsers: [
+            {
+              role: MembershipRole.MEMBER,
+              usernameOrEmail: newUser.email,
+            },
+          ],
+        })
+      );
+    });
   });
   it("When rate limit exceeded, it should throw error", async () => {
     const userToBeInvited = buildExistingUser({