feat: add database transaction to createManagedOrganization flow to ensure atomicity#27979
feat: add database transaction to createManagedOrganization flow to ensure atomicity#27979bandhan-majumder wants to merge 3 commits intocalcom:mainfrom
Conversation
bandhan-majumder
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/server/routers/viewer/apiKeys/_auth-middleware.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/apiKeys/_auth-middleware.ts:15">
P2: Inefficient database query fetching full record for existence check</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (!teamId) return; | ||
| const team = await prisma.team.findFirst({ | ||
| const client = prismaTransactionClient ?? prisma | ||
| const team = await client.team.findFirst({ |
There was a problem hiding this comment.
P2: Inefficient database query fetching full record for existence check
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/trpc/server/routers/viewer/apiKeys/_auth-middleware.ts, line 15:
<comment>Inefficient database query fetching full record for existence check</comment>
<file context>
@@ -7,10 +7,12 @@ export async function checkPermissions(args: {
if (!teamId) return;
- const team = await prisma.team.findFirst({
+ const client = prismaTransactionClient ?? prisma
+ const team = await client.team.findFirst({
where: {
id: teamId,
</file context>
| isOrganization: true, | ||
| isPlatform: true, | ||
| metadata: organizationData.metadata, | ||
| if (existingManagedOrganization) { |
There was a problem hiding this comment.
This does not send API response like previous, it just throws error. Open to change if reviewer thinks it can be better. Same goes to the over all catch block's error throw.
| throw new Error(`Managed organization creation failed. ${error instanceof Error ? error.message : String(error)}`) | ||
| } | ||
| }, { | ||
| // @see: https://www.prisma.io/docs/orm/prisma-client/queries/transactions |
There was a problem hiding this comment.
this is not required, but is mentioned in the prisma docs. Therefore, added it with docs link
| ...outputOrganization, | ||
| apiKey, | ||
| }; | ||
| } catch (error) { |
There was a problem hiding this comment.
as I mentioned in the above comment https://github.com/calcom/cal.com/pull/27979/changes#r2813240667
| managedOrgId: number, | ||
| prismaTransactionClient?: Prisma.TransactionClient | ||
| ) { | ||
| const writeClient = prismaTransactionClient ?? this.dbWrite.prisma; |
There was a problem hiding this comment.
This function require both the read and write db service. So added both of them falling back to the default.Open to suggestions here. A single client can also do the work but not appropriate usage I think as we use read,write services where they are appropriate.
1 participant
What does this PR do?
adds database transaction to createManagedOrganization flow to ensure atomicity. The previous db calls were sequential but not atomic. I have passed the client where it was needed to make sure same client is used. Client is a optional parameter which falls back to dbWrite or dbRead if not provided to make sure it does not totally break if transaction client is not provided.
createManagedOrganizationflow to ensure atomicity #24384 (GitHub issue number)Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
N/A
Image Demo (if applicable):
N/A
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
https://cal.com/docs/platform/guides/managed-orgs
Checklist
Summary by cubic
Make createManagedOrganization atomic by wrapping org creation, membership, profile, billing, and API key creation in a single Prisma transaction with Serializable isolation. Prevents partial organizations and aligns with CAL-6555 and #24384.
Written for commit 2566dcb. Summary will update on new commits.