From 2f1e54597612c1ff43652d794401b2ea21c4a97f Mon Sep 17 00:00:00 2001 From: Hariom Balhara Date: Sun, 7 Jan 2024 08:51:59 +0530 Subject: [PATCH] feat: Allow org creation with conflicting slug and renaming team slug during migration (#13065) --- apps/web/lib/orgMigration.test.ts | 25 ++++- apps/web/lib/orgMigration.ts | 100 ++++++++++++------ .../pages/api/orgMigration/moveTeamToOrg.ts | 7 +- .../admin/orgMigrations/moveTeamToOrg.tsx | 7 ++ packages/prisma/zod-utils.ts | 8 ++ .../viewer/organizations/create.handler.ts | 15 ++- 6 files changed, 121 insertions(+), 41 deletions(-) diff --git a/apps/web/lib/orgMigration.test.ts b/apps/web/lib/orgMigration.test.ts index cfe4c78b74..2bf8fb7093 100644 --- a/apps/web/lib/orgMigration.test.ts +++ b/apps/web/lib/orgMigration.test.ts @@ -317,11 +317,13 @@ describe("orgMigration", () => { await expectTeamToBeAPartOfOrg({ teamId: team1.id, orgId: dbOrg.id, + teamSlugInOrg: team1.slug, }); await expectTeamToBeAPartOfOrg({ teamId: team2.id, orgId: dbOrg.id, + teamSlugInOrg: team2.slug, }); await expectUserToBeNotAPartOfTheOrg({ @@ -873,6 +875,7 @@ describe("orgMigration", () => { id: 1, name: "Team 1", slug: "team1", + newSlug: "team1-new-slug", }, targetOrg: { id: 2, @@ -902,19 +905,23 @@ describe("orgMigration", () => { await moveTeamToOrg({ teamId: data.teamToMigrate.id, - targetOrgId: data.targetOrg.id, + targetOrg: { + id: data.targetOrg.id, + teamSlug: data.teamToMigrate.newSlug, + }, }); await expectTeamToBeAPartOfOrg({ teamId: data.teamToMigrate.id, orgId: data.targetOrg.id, + teamSlugInOrg: data.teamToMigrate.newSlug, }); expectTeamRedirectToBeEnabled({ from: { teamSlug: data.teamToMigrate.slug, }, - to: data.teamToMigrate.slug, + to: data.teamToMigrate.newSlug, orgSlug: data.targetOrg.slug, }); }); @@ -1198,7 +1205,15 @@ async function expectUserToBeNotAPartOfTheOrg({ expect(membership).toBeUndefined(); } -async function expectTeamToBeAPartOfOrg({ teamId, orgId }: { teamId: number; orgId: number }) { +async function expectTeamToBeAPartOfOrg({ + teamId, + orgId, + teamSlugInOrg, +}: { + teamId: number; + orgId: number; + teamSlugInOrg: string | null; +}) { const migratedTeam = await prismock.team.findUnique({ where: { id: teamId, @@ -1208,7 +1223,11 @@ async function expectTeamToBeAPartOfOrg({ teamId, orgId }: { teamId: number; org throw new Error(`Team with id ${teamId} does not exist`); } + if (!teamSlugInOrg) { + throw new Error(`teamSlugInOrg should be defined`); + } expect(migratedTeam.parentId).toBe(orgId); + expect(migratedTeam.slug).toBe(teamSlugInOrg); } async function expectTeamToBeNotPartOfAnyOrganization({ teamId }: { teamId: number }) { diff --git a/apps/web/lib/orgMigration.ts b/apps/web/lib/orgMigration.ts index 5a8b4d1fdf..5000a6d202 100644 --- a/apps/web/lib/orgMigration.ts +++ b/apps/web/lib/orgMigration.ts @@ -182,35 +182,39 @@ export async function removeUserFromOrg({ targetOrgId, userId }: { targetOrgId: * Make sure that the migration is idempotent */ export async function moveTeamToOrg({ - targetOrgId, + targetOrg, teamId, moveMembers, }: { - targetOrgId: number; + targetOrg: { id: number; teamSlug: string }; teamId: number; moveMembers?: boolean; }) { - const possibleOrg = await getTeamOrThrowError(targetOrgId); - const movedTeam = await dbMoveTeamToOrg({ teamId, targetOrgId }); + const possibleOrg = await getTeamOrThrowError(targetOrg.id); + const { oldTeamSlug, updatedTeam } = await dbMoveTeamToOrg({ teamId, targetOrg }); const teamMetadata = teamMetadataSchema.parse(possibleOrg?.metadata); if (!teamMetadata?.isOrganization) { - throw new Error(`${targetOrgId} is not an Org`); + throw new Error(`${targetOrg.id} is not an Org`); } const targetOrganization = possibleOrg; const orgMetadata = teamMetadata; - await addTeamRedirect(movedTeam.slug, targetOrganization.slug || orgMetadata.requestedSlug || null); - await setOrgSlugIfNotSet({ slug: targetOrganization.slug }, orgMetadata, targetOrgId); + await addTeamRedirect({ + oldTeamSlug, + teamSlug: updatedTeam.slug, + orgSlug: targetOrganization.slug || orgMetadata.requestedSlug || null, + }); + await setOrgSlugIfNotSet({ slug: targetOrganization.slug }, orgMetadata, targetOrg.id); if (moveMembers) { - for (const membership of movedTeam.members) { + for (const membership of updatedTeam.members) { await moveUserToOrg({ user: { id: membership.userId, }, targetOrg: { - id: targetOrgId, + id: targetOrg.id, membership: { role: membership.role, accepted: membership.accepted, @@ -220,21 +224,30 @@ export async function moveTeamToOrg({ }); } } - log.debug(`Successfully moved team ${teamId} to org ${targetOrgId}`); + log.debug(`Successfully moved team ${teamId} to org ${targetOrg.id}`); } /** * Make sure that the migration is idempotent */ export async function removeTeamFromOrg({ targetOrgId, teamId }: { targetOrgId: number; teamId: number }) { - const removedTeam = await dbRemoveTeamFromOrg({ teamId, targetOrgId }); + const removedTeam = await dbRemoveTeamFromOrg({ teamId }); await removeTeamRedirect(removedTeam.slug); log.debug(`Successfully removed team ${teamId} from org ${targetOrgId}`); } -async function dbMoveTeamToOrg({ teamId, targetOrgId }: { teamId: number; targetOrgId: number }) { +async function dbMoveTeamToOrg({ + teamId, + targetOrg, +}: { + teamId: number; + targetOrg: { + id: number; + teamSlug: string; + }; +}) { const team = await prisma.team.findUnique({ where: { id: teamId, @@ -251,21 +264,30 @@ async function dbMoveTeamToOrg({ teamId, targetOrgId }: { teamId: number; target }); } - if (team.parentId === targetOrgId) { - log.warn(`Team ${teamId} is already in org ${targetOrgId}`); - return team; - } + const teamMetadata = teamMetadataSchema.parse(team?.metadata); + const oldTeamSlug = teamMetadata?.migratedToOrgFrom?.teamSlug || team.slug; - await prisma.team.update({ + const updatedTeam = await prisma.team.update({ where: { id: teamId, }, data: { - parentId: targetOrgId, + slug: targetOrg.teamSlug, + parentId: targetOrg.id, + metadata: { + ...teamMetadata, + migratedToOrgFrom: { + teamSlug: team.slug, + lastMigrationTime: new Date().toISOString(), + }, + }, + }, + include: { + members: true, }, }); - return team; + return { oldTeamSlug, updatedTeam }; } async function getUniqueUserThatDoesntBelongToOrg( @@ -460,11 +482,25 @@ async function addRedirect({ } } -async function addTeamRedirect(teamSlug: string | null, orgSlug: string | null) { +async function addTeamRedirect({ + oldTeamSlug, + teamSlug, + orgSlug, +}: { + oldTeamSlug: string | null; + teamSlug: string | null; + orgSlug: string | null; +}) { + if (!oldTeamSlug) { + throw new HttpError({ + statusCode: 400, + message: "No oldSlug for team. Not adding the redirect", + }); + } if (!teamSlug) { throw new HttpError({ statusCode: 400, - message: "No slug for team. Not removing the redirect", + message: "No slug for team. Not adding the redirect", }); } if (!orgSlug) { @@ -477,13 +513,13 @@ async function addTeamRedirect(teamSlug: string | null, orgSlug: string | null) where: { from_type_fromOrgId: { type: RedirectType.Team, - from: teamSlug, + from: oldTeamSlug, fromOrgId: 0, }, }, create: { type: RedirectType.Team, - from: teamSlug, + from: oldTeamSlug, fromOrgId: 0, toUrl: `${orgUrlPrefix}/${teamSlug}`, }, @@ -678,7 +714,7 @@ async function removeUserAlongWithItsTeamsRedirects({ } } -async function dbRemoveTeamFromOrg({ teamId, targetOrgId }: { teamId: number; targetOrgId: number }) { +async function dbRemoveTeamFromOrg({ teamId }: { teamId: number }) { const team = await prisma.team.findUnique({ where: { id: teamId, @@ -692,13 +728,7 @@ async function dbRemoveTeamFromOrg({ teamId, targetOrgId }: { teamId: number; ta }); } - if (team.parentId !== targetOrgId) { - log.warn(`Team ${teamId} is not part of org ${targetOrgId}. Not updating`); - return { - slug: team.slug, - }; - } - + const teamMetadata = teamMetadataSchema.parse(team?.metadata); try { return await prisma.team.update({ where: { @@ -706,6 +736,14 @@ async function dbRemoveTeamFromOrg({ teamId, targetOrgId }: { teamId: number; ta }, data: { parentId: null, + slug: teamMetadata?.migratedToOrgFrom?.teamSlug || team.slug, + metadata: { + ...teamMetadata, + migratedToOrgFrom: { + reverted: true, + lastRevertTime: new Date().toISOString(), + }, + }, }, select: { slug: true, diff --git a/apps/web/pages/api/orgMigration/moveTeamToOrg.ts b/apps/web/pages/api/orgMigration/moveTeamToOrg.ts index bed2778fba..f6a3e2bec4 100644 --- a/apps/web/pages/api/orgMigration/moveTeamToOrg.ts +++ b/apps/web/pages/api/orgMigration/moveTeamToOrg.ts @@ -40,7 +40,7 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) return res.status(400).json({ message: JSON.stringify(parsedBody.error) }); } - const { teamId, targetOrgId, moveMembers } = parsedBody.data; + const { teamId, targetOrgId, moveMembers, teamSlugInOrganization } = parsedBody.data; const isAllowed = isAdmin; if (!isAllowed) { return res.status(403).json({ message: "Not Authorized" }); @@ -48,7 +48,10 @@ export default async function handler(req: NextApiRequest, res: NextApiResponse) try { await moveTeamToOrg({ - targetOrgId, + targetOrg: { + id: targetOrgId, + teamSlug: teamSlugInOrganization, + }, teamId, moveMembers, }); diff --git a/apps/web/pages/settings/admin/orgMigrations/moveTeamToOrg.tsx b/apps/web/pages/settings/admin/orgMigrations/moveTeamToOrg.tsx index 4ed978df88..c2e0d00551 100644 --- a/apps/web/pages/settings/admin/orgMigrations/moveTeamToOrg.tsx +++ b/apps/web/pages/settings/admin/orgMigrations/moveTeamToOrg.tsx @@ -21,6 +21,7 @@ export const getFormSchema = (t: TFunction) => { teamId: z.number().or(getStringAsNumberRequiredSchema(t)), targetOrgId: z.number().or(getStringAsNumberRequiredSchema(t)), moveMembers: z.boolean(), + teamSlugInOrganization: z.string(), }); }; @@ -103,6 +104,12 @@ export default function MoveTeamToOrg() { required placeholder="Enter teamId to move to org" /> + { }, }); - // An org doesn't have a parentId. A team that isn't part of an org also doesn't have a parentId. - // So, an org can't have the same slug as a non-org team. - // There is a unique index on [slug, parentId] in Team because we don't add the slug to the team always. We only add metadata.requestedSlug in some cases. So, DB won't prevent creation of such an organization. - const hasANonOrgTeamOrOrgWithSameSlug = await prisma.team.findFirst({ + const hasAnOrgWithSameSlug = await prisma.team.findFirst({ where: { slug: slug, parentId: null, + metadata: { + path: ["isOrganization"], + equals: true, + }, }, }); - if (hasANonOrgTeamOrOrgWithSameSlug || RESERVED_SUBDOMAINS.includes(slug)) + // Allow creating an organization with same requestedSlug as a non-org Team's slug + // It is needed so that later we can migrate the non-org Team(with the conflicting slug) to the newly created org + // Publishing the organization would fail if the team with the same slug is not migrated first + + if (hasAnOrgWithSameSlug || RESERVED_SUBDOMAINS.includes(slug)) throw new TRPCError({ code: "BAD_REQUEST", message: "organization_url_taken" }); if (userCollisions) throw new TRPCError({ code: "BAD_REQUEST", message: "admin_email_taken" });