Skip to content

Commit ee973c6

Browse files
romitg2devin-ai-integration[bot]bot_apkemrysal
authored
fix: harden seed script org settings upsert and P2002 error handling (calcom#28527)
* fix: harden seed script org settings upsert and P2002 error handling Co-Authored-By: romitgabani1 <romitgabani1.work@gmail.com> * fix: remove PII from P2002 log and recover existing user instead of returning null - Replace username interpolation in log message with generic text (Cubic violation #2, confidence 9/10) - On P2002, fetch the existing user from DB and return it with membership data instead of returning null, which was dropping users from org setup on retries (Cubic violation #3, confidence 9/10) Co-Authored-By: bot_apk <apk@cognition.ai> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: bot_apk <apk@cognition.ai> Co-authored-by: Alex van Andel <me@alexvanandel.com>
1 parent e9e9667 commit ee973c6

File tree

3 files changed

+52
-111
lines changed

3 files changed

+52
-111
lines changed

apps/api/v1/test/lib/utils/isAdmin.integration-test.ts

Lines changed: 2 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,64 +1,15 @@
1+
import prisma from "@calcom/prisma";
12
import type { Request, Response } from "express";
23
import type { NextApiRequest, NextApiResponse } from "next";
34
import { createMocks } from "node-mocks-http";
4-
import { describe, it, expect, beforeAll } from "vitest";
5-
6-
import prisma from "@calcom/prisma";
7-
5+
import { describe, expect, it } from "vitest";
86
import { isAdminGuard } from "../../../lib/utils/isAdmin";
97
import { ScopeOfAdmin } from "../../../lib/utils/scopeOfAdmin";
108

119
type CustomNextApiRequest = NextApiRequest & Request;
1210
type CustomNextApiResponse = NextApiResponse & Response;
1311

1412
describe("isAdmin guard", () => {
15-
beforeAll(async () => {
16-
const acmeOrg = await prisma.team.findFirst({
17-
where: {
18-
slug: "acme",
19-
isOrganization: true,
20-
},
21-
});
22-
23-
if (acmeOrg) {
24-
await prisma.organizationSettings.upsert({
25-
where: {
26-
organizationId: acmeOrg.id,
27-
},
28-
update: {
29-
isAdminAPIEnabled: true,
30-
},
31-
create: {
32-
organizationId: acmeOrg.id,
33-
orgAutoAcceptEmail: "acme.com",
34-
isAdminAPIEnabled: true,
35-
},
36-
});
37-
}
38-
39-
const dunderOrg = await prisma.team.findFirst({
40-
where: {
41-
slug: "dunder-mifflin",
42-
isOrganization: true,
43-
},
44-
});
45-
46-
if (dunderOrg) {
47-
await prisma.organizationSettings.upsert({
48-
where: {
49-
organizationId: dunderOrg.id,
50-
},
51-
update: {
52-
isAdminAPIEnabled: false,
53-
},
54-
create: {
55-
organizationId: dunderOrg.id,
56-
orgAutoAcceptEmail: "dunder-mifflin.com",
57-
isAdminAPIEnabled: false,
58-
},
59-
});
60-
}
61-
});
6213
it("Returns false when user does not exist in the system", async () => {
6314
const { req } = createMocks<CustomNextApiRequest, CustomNextApiResponse>({
6415
method: "POST",

apps/api/v1/test/lib/utils/retrieveScopedAccessibleUsers.integration-test.ts

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,11 @@
1-
import { describe, it, expect, beforeAll } from "vitest";
2-
31
import prisma from "@calcom/prisma";
4-
2+
import { describe, expect, it } from "vitest";
53
import {
64
getAccessibleUsers,
75
retrieveOrgScopedAccessibleUsers,
86
} from "../../../lib/utils/retrieveScopedAccessibleUsers";
97

108
describe("retrieveScopedAccessibleUsers tests", () => {
11-
beforeAll(async () => {
12-
const acmeOrg = await prisma.team.findFirst({
13-
where: {
14-
slug: "acme",
15-
isOrganization: true,
16-
},
17-
});
18-
19-
if (acmeOrg) {
20-
await prisma.organizationSettings.upsert({
21-
where: {
22-
organizationId: acmeOrg.id,
23-
},
24-
update: {
25-
isAdminAPIEnabled: true,
26-
},
27-
create: {
28-
organizationId: acmeOrg.id,
29-
orgAutoAcceptEmail: "acme.com",
30-
isAdminAPIEnabled: true,
31-
},
32-
});
33-
}
34-
});
359
describe("getAccessibleUsers", () => {
3610
it("Does not return members when only admin user ID is supplied", async () => {
3711
const adminUser = await prisma.user.findFirstOrThrow({ where: { email: "owner1-acme@example.com" } });

scripts/seed.ts

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,12 @@ async function createOrganizationAndAddMembersAndTeams({
329329
});
330330

331331
if (existingTeam) {
332-
console.log(`Organization with slug '${orgData.slug}' already exists, skipping.`);
332+
console.log(`Organization with slug '${orgData.slug}' already exists, ensuring settings are up to date.`);
333+
await prisma.organizationSettings.upsert({
334+
where: { organizationId: existingTeam.id },
335+
update: { ...orgData.organizationSettings },
336+
create: { organizationId: existingTeam.id, ...orgData.organizationSettings },
337+
});
333338
return;
334339
}
335340

@@ -341,14 +346,13 @@ async function createOrganizationAndAddMembersAndTeams({
341346
};
342347
})[] = [];
343348

344-
try {
345-
const batchSize = 50;
346-
// Process members in batches of in parallel
347-
for (let i = 0; i < orgMembers.length; i += batchSize) {
348-
const batch = orgMembers.slice(i, i + batchSize);
349+
const batchSize = 50;
350+
for (let i = 0; i < orgMembers.length; i += batchSize) {
351+
const batch = orgMembers.slice(i, i + batchSize);
349352

350-
const batchResults = await Promise.all(
351-
batch.map(async (member) => {
353+
const batchResults = await Promise.all(
354+
batch.map(async (member) => {
355+
try {
352356
const newUser = await createUserAndEventType({
353357
user: {
354358
...member.memberData,
@@ -375,13 +379,6 @@ async function createOrganizationAndAddMembersAndTeams({
375379
],
376380
});
377381

378-
const orgMemberInDb = {
379-
...newUser,
380-
inTeams: member.inTeams,
381-
orgMembership: member.orgMembership,
382-
orgProfile: member.orgProfile,
383-
};
384-
385382
// Create temp org redirect with upsert to handle duplicates
386383
await prisma.tempOrgRedirect.upsert({
387384
where: {
@@ -402,26 +399,45 @@ async function createOrganizationAndAddMembersAndTeams({
402399
},
403400
});
404401

405-
return orgMemberInDb;
406-
})
407-
);
402+
return {
403+
...newUser,
404+
inTeams: member.inTeams,
405+
orgMembership: member.orgMembership,
406+
orgProfile: member.orgProfile,
407+
};
408+
} catch (e) {
409+
if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2002") {
410+
console.log("Organization member already seeded, skipping");
411+
const existingUser = await prisma.user.findUnique({
412+
where: {
413+
email_username: {
414+
email: member.memberData.email,
415+
username: member.memberData.username,
416+
},
417+
},
418+
});
419+
if (!existingUser) throw e;
420+
return {
421+
...existingUser,
422+
inTeams: member.inTeams,
423+
orgMembership: member.orgMembership,
424+
orgProfile: member.orgProfile,
425+
};
426+
}
427+
throw e;
428+
}
429+
})
430+
);
408431

409-
orgMembersInDb.push(...batchResults);
410-
}
411-
} catch (e) {
412-
if (e instanceof Prisma.PrismaClientKnownRequestError) {
413-
if (e.code === "P2002") {
414-
console.log(`One of the organization members already exists, skipping the entire seeding`);
415-
return;
416-
}
417-
}
418-
console.error(e);
432+
orgMembersInDb.push(...batchResults.filter((r): r is NonNullable<typeof r> => r !== null));
419433
}
420434

421-
await Promise.all([
435+
await Promise.all(
422436
usersOutsideOrg.map(async (user) => {
423-
return await prisma.user.create({
424-
data: {
437+
await prisma.user.upsert({
438+
where: { email_username: { email: user.email, username: user.username } },
439+
update: {},
440+
create: {
425441
username: user.username,
426442
name: user.name,
427443
email: user.email,
@@ -433,8 +449,8 @@ async function createOrganizationAndAddMembersAndTeams({
433449
},
434450
},
435451
});
436-
}),
437-
]);
452+
})
453+
);
438454

439455
const { organizationSettings, ...restOrgData } = orgData;
440456

0 commit comments

Comments
 (0)