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
2025-07-31 01:53:41 +01:00
parent 704bbf2757
commit 5360cb511d
6 changed files with 314 additions and 18 deletions
@@ -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}
                        />
                      )}
                    />
@@ -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: {
@@ -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}`
+      );
    }
  }

@@ -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);
+  });
 });
@@ -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",
@@ -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({