fix: team metadata restriction for subscription #23327
Conversation
|
@anikdhabal is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughA new baseTeamMetadataSchema was introduced to centralize team metadata fields (including billingPeriod). The public teamMetadataSchema now derives from it via partial().nullable(), preserving optional-nullable fields. A new export, teamMetadataStrictSchema, extends the base and adds prefix validations for subscriptionId (sub_) and subscriptionItemId (si_), both nullable. Multiple modules replaced usage of teamMetadataSchema with teamMetadataStrictSchema for parsing, safeParse, and unwrap. One method type signature changed to use z.infer for existingMetadata in OrganizationRepository.updateStripeSubscriptionDetails. No other control-flow or exported signatures changed. Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (08/25/25)1 reviewer was added to this PR based on Keith Williams's automation. "Add ready-for-e2e label" took an action on this PR • (08/26/25)1 label was added to this PR based on Keith Williams's automation. |
anikdhabal
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/features/ee/teams/lib/payments.test.ts (5)
545-546: Fix misuse of.rejectswith a function — pass the Promise and await
rejectsexpects a Promise, not a function. This test can yield false positives/negatives.- expect(() => getTeamWithPaymentMetadata(team.id)).rejects.toThrow(); + await expect(getTeamWithPaymentMetadata(team.id)).rejects.toThrow();
559-560: Fix misuse of.rejectswith a function — pass the Promise and awaitRepeat of the issue above.
- expect(() => getTeamWithPaymentMetadata(team.id)).rejects.toThrow(); + await expect(getTeamWithPaymentMetadata(team.id)).rejects.toThrow();
573-574: Fix misuse of.rejectswith a function — pass the Promise and awaitRepeat of the issue above.
- expect(() => getTeamWithPaymentMetadata(team.id)).rejects.toThrow(); + await expect(getTeamWithPaymentMetadata(team.id)).rejects.toThrow();
688-709: Nested Promise.all prevents awaiting user creation; flatten the array
Promise.all([ array.map(async ...) ])passes a single element (an array of Promises) intoPromise.all, which resolves immediately and does not await the inner Promises. This can introduce test flakiness and race conditions.Apply:
- await Promise.all([ - Array(membersInTeam) - .fill(0) - .map(async (_, index) => { - return await prismock.membership.create({ - data: { - team: { - connect: { - id: organization.id, - }, - }, - user: { - create: { - name: "ABC", - email: `test-${index}@example.com`, - }, - }, - role: "MEMBER", - }, - }); - }), - ]); + await Promise.all( + Array.from({ length: membersInTeam }, (_, index) => + prismock.membership.create({ + data: { + team: { connect: { id: organization.id } }, + user: { create: { name: "ABC", email: `test-${index}@example.com` } }, + role: "MEMBER", + }, + }) + ) + );
39-43: Fix the mocked module path to match the production importThe test currently mocks
@calcom/lib/constant(singular), but production code imports from@calcom/lib/constants(plural), so the mock isn’t applied and tests may use the real value unexpectedly.• File:
packages/features/ee/teams/lib/payments.test.ts
• Lines: 39–43- vi.mock("@calcom/lib/constant", () => { - return { - MINIMUM_NUMBER_OF_ORG_SEATS: 30, - }; - }); + vi.mock("@calcom/lib/constants", () => ({ + MINIMUM_NUMBER_OF_ORG_SEATS: 30, + }));
🧹 Nitpick comments (2)
packages/prisma/zod-utils.ts (1)
380-391: Optional: Simplify Stripe ID prefix checks usingz.string().startsWithWe’ve confirmed that
packages/prismais on Zod v^3.22.4, which includes thestring().startsWithsugar method. You can replace the existingrefinecalls with a more concise API:• File:
packages/prisma/zod-utils.ts
Lines: ~380–391Proposed diff:
- subscriptionId: z - .string() - .refine((val) => val.startsWith("sub_"), { - message: "subscriptionId must start with 'sub_'", - }) - .nullable(), + subscriptionId: z + .string() + .startsWith("sub_", "subscriptionId must start with 'sub_'") + .nullable(), - subscriptionItemId: z - .string() - .refine((val) => val.startsWith("si_"), { - message: "subscriptionItemId must start with 'si_'", - }) - .nullable(), + subscriptionItemId: z + .string() + .startsWith("si_", "subscriptionItemId must start with 'si_'") + .nullable(),This keeps null‐safety intact and aligns with the pattern used in
packages/features/ee/teams/lib/payments.ts.packages/features/ee/teams/lib/payments.test.ts (1)
742-747: Nit: simplify mock implementations withmockResolvedValueThe explicit
new Promise((resolve) => resolve(data))wrappers are verbose.-return vi.mocked(stripe.checkout.sessions.create).mockImplementation( - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - //@ts-ignore - async () => new Promise((resolve) => resolve(data)) -); +return vi.mocked(stripe.checkout.sessions.create).mockResolvedValue( + // @ts-ignore + data +);Apply similarly to other mocks (
prices.create,prices.retrieve,subscriptions.retrieve,subscriptions.update) for brevity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/ee/teams/lib/payments.test.ts(11 hunks)packages/features/ee/teams/lib/payments.ts(1 hunks)packages/prisma/zod-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/prisma/zod-utils.tspackages/features/ee/teams/lib/payments.tspackages/features/ee/teams/lib/payments.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/prisma/zod-utils.tspackages/features/ee/teams/lib/payments.tspackages/features/ee/teams/lib/payments.test.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/prisma/zod-utils.tspackages/features/ee/teams/lib/payments.tspackages/features/ee/teams/lib/payments.test.ts
🧬 Code graph analysis (1)
packages/features/ee/teams/lib/payments.test.ts (1)
packages/features/ee/teams/lib/payments.ts (1)
getTeamWithPaymentMetadata(271-279)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
🔇 Additional comments (5)
packages/features/ee/teams/lib/payments.ts (1)
24-26: Good hardening: enforce Stripe ID prefixes at parse-timeValidating
subscriptionIdandsubscriptionItemIdwithstring().startsWithis aligned with Stripe’s ID semantics and avoids accidental misconfiguration. UsingorgSeatsdirectly fromteamMetadataSchema.unwrap().shape.orgSeatskeeps type parity with the shared schema. Looks good.Please ensure the repo uses a Zod version that supports
string().startsWith. If not, fall back to therefinestyle used inzod-utils.ts.packages/features/ee/teams/lib/payments.test.ts (4)
341-343: IDs updated to valid Stripe prefixes — goodSwitching test fixtures to
sub_andsi_prefixed IDs matches the new validation and real Stripe formats.
387-389: Consistent prefixed IDs — goodMaintains consistency across scenarios; mirrors production expectations.
437-439: Consistent prefixed IDs — goodMatches the schema contract and avoids false negatives.
483-485: Consistent prefixed IDs — goodContinues to align with the schema requirements.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/api/v2/src/modules/teams/teams/teams.repository.ts (1)
88-96: Fix null deref and error ordering in setDefaultConferencingApp.
- You parse metadata before checking team existence; parse(undefined) throws a ZodError instead of the intended NotFoundException.
- typeof null === "object", so spreading ...teamMetadata will throw when teamMetadata is null.
Also, the NotFoundException message says “user not found” in a team repository.
Apply this diff:
async setDefaultConferencingApp(teamId: number, appSlug?: string, appLink?: string) { - const team = await this.getById(teamId); - const teamMetadata = teamMetadataStrictSchema.parse(team?.metadata); - - if (!team) { - throw new NotFoundException("user not found"); - } + const team = await this.getById(teamId); + if (!team) { + throw new NotFoundException("team not found"); + } + const teamMetadata = teamMetadataStrictSchema.parse(team.metadata ?? null); @@ - data: { - metadata: - typeof teamMetadata === "object" - ? { - ...teamMetadata, - defaultConferencingApp: { - appSlug: appSlug, - appLink: appLink, - }, - } - : {}, - }, + data: { + metadata: { + ...(teamMetadata ?? {}), + defaultConferencingApp: { appSlug, appLink }, + }, + },packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts (1)
347-353: Enforce non-throwing metadata parsing in onboarding flowThe call to
teamMetadataStrictSchema.parse(…)on line 347 of
packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.tswill throw on any legacy or malformed metadata (e.g. missing “sub_” prefix), causing webhook-driven organization creation to hard-fail. This path must remain backward-compatible.• File: packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts
Line: 347Apply this refactor:
- const existingMetadata = teamMetadataStrictSchema.parse(organization.metadata); + const metadataParse = teamMetadataStrictSchema.safeParse(organization.metadata); + if (!metadataParse.success) { + log.warn( + "Legacy/invalid organization metadata during Stripe subscription update; proceeding without blocking.", + safeStringify({ orgId: organization.id, error: metadataParse.error.message }) + ); + } + const existingMetadata = metadataParse.success ? metadataParse.data : null;Additionally, your grep scan reveals numerous other direct
.parsecalls on team/organization metadata schemas. Audit any webhook-driven or background tasks that must tolerate legacy records, replacing blocking.parsewithsafeParse+ graceful fallback where appropriate.packages/trpc/server/routers/viewer/organizations/publish.handler.ts (1)
25-31: Prisma: replace include with select/_count to follow guidelines and reduce payload.Avoid include per repo guideline. You only need id, metadata, and members count.
- const prevTeam = await prisma.team.findUnique({ - where: { - id: orgId, - }, - include: { members: true }, - }); + const prevTeam = await prisma.team.findUnique({ + where: { id: orgId }, + select: { + id: true, + metadata: true, + _count: { select: { members: true } }, + }, + }); @@ - seatsUsed: prevTeam.members.length, + seatsUsed: prevTeam._count.members,Also applies to: 41-49
🧹 Nitpick comments (17)
apps/web/app/api/teams/[team]/upgrade/route.ts (4)
53-56: Validation now enforces Stripe id prefixes (nullable when absent).Using teamMetadataStrictSchema.safeParse(prevTeam.metadata) is appropriate. Two suggestions:
- Consider surfacing Zod error details in logs for faster triage.
- If legacy teams may carry non-prefixed subscription ids, decide whether you want a hard 400 here or a soft migration path.
Apply this diff to add debug logging without altering the response:
- metadata = teamMetadataStrictSchema.safeParse(prevTeam.metadata); - if (!metadata.success) { + metadata = teamMetadataStrictSchema.safeParse(prevTeam.metadata); + if (!metadata.success) { + // Optional: log issues to help debug bad payloads + console.debug("Invalid team metadata", metadata.error.flatten()); throw new HttpError({ statusCode: 400, message: "Invalid team metadata" }); }
65-67: Guard access to subscription.items.data[0].id.Stripe guarantees at least one item for a typical subscription, but defensive checks avoid runtime errors (and make 5xx root-causes clearer) if a plan misconfiguration ever slips through.
Apply this diff:
- subscriptionId: subscription.id || null, - subscriptionItemId: subscription.items.data[0].id || null, + subscriptionId: subscription?.id || null, + subscriptionItemId: subscription?.items?.data?.[0]?.id ?? null,Optionally hard-fail if the item is unexpectedly missing:
+ if (!subscription?.items?.data?.[0]?.id) { + throw new HttpError({ statusCode: 500, message: "Subscription item missing for checkout session" }); + }
44-47: Prisma: select only fields you need.Per repo guidelines, prefer select over broad fetches. Here you only use id, slug, isOrganization, and metadata.
- let team = await prisma.team.findFirst({ - where: { metadata: { path: ["paymentId"], equals: checkoutSession.id } }, - }); + let team = await prisma.team.findFirst({ + where: { metadata: { path: ["paymentId"], equals: checkoutSession.id } }, + select: { id: true, slug: true, isOrganization: true, metadata: true }, + }); @@ - const prevTeam = await prisma.team.findFirstOrThrow({ where: { id } }); + const prevTeam = await prisma.team.findFirstOrThrow({ + where: { id }, + select: { id: true, slug: true, metadata: true }, + }); @@ - const redirectUrl = team?.isOrganization + const redirectUrl = team?.isOrganization ? `${WEBAPP_URL}/settings/organizations/profile?upgraded=true` : `${WEBAPP_URL}/settings/teams/${team.id}/profile?upgraded=true`;Also applies to: 59-69, 95-98
82-87: Redundant re-parse of metadata.The second safeParse result isn’t used. If the purpose is to assert validity for existing teams, consider logging failures and continuing, or remove the block to simplify.
- if (!metadata) { - metadata = teamMetadataStrictSchema.safeParse(team.metadata); - if (!metadata.success) { - throw new HttpError({ statusCode: 400, message: "Invalid team metadata" }); - } - } + // Optional: re-validate existing metadata here if you need a hard assertion.packages/prisma/zod-utils.ts (2)
375-393: Solid consolidation via baseTeamMetadataSchema; confirm unknown-key policy.Centralizing fields (incl. billingPeriod) is a win. Note: z.object(...) without .passthrough() strips unknown keys. If previous teamMetadataSchema allowed unknowns, this will now silently drop them during parse/serialize flows.
If you want to preserve unknown keys:
-const baseTeamMetadataSchema = z.object({ +const baseTeamMetadataSchema = z + .object({ defaultConferencingApp: schemaDefaultConferencingApp.optional(), requestedSlug: z.string().or(z.null()), paymentId: z.string(), subscriptionId: z.string().nullable(), subscriptionItemId: z.string().nullable(), orgSeats: z.number().nullable(), orgPricePerSeat: z.number().nullable(), migratedToOrgFrom: z .object({ teamSlug: z.string().or(z.null()).optional(), lastMigrationTime: z.string().optional(), reverted: z.boolean().optional(), lastRevertTime: z.string().optional(), }) .optional(), billingPeriod: z.nativeEnum(BillingPeriod).optional(), -}); +}) + .passthrough();If stripping is intentional, please confirm to avoid data loss surprises in downstream merges.
394-413: Prefix constraints look correct; keep them nullable and optional.The refine checks enforce “sub_”/“si_” only when values are present; null is still allowed, which matches usage. Consider extracting the prefixes to named constants to avoid string drift.
+const STRIPE_SUB_PREFIX = "sub_"; +const STRIPE_SI_PREFIX = "si_"; export const teamMetadataStrictSchema = baseTeamMetadataSchema .extend({ subscriptionId: z .string() - .refine((val) => val.startsWith("sub_"), { + .refine((val) => val.startsWith(STRIPE_SUB_PREFIX), { message: "subscriptionId must start with 'sub_'", }) .nullable(), subscriptionItemId: z .string() - .refine((val) => val.startsWith("si_"), { + .refine((val) => val.startsWith(STRIPE_SI_PREFIX), { message: "subscriptionItemId must start with 'si_'", }) .nullable(), }) .partial() .nullable();apps/api/v2/src/modules/teams/teams/teams.repository.ts (1)
18-22: Optional: narrow Prisma selections.These reads return full Team rows but callers only need a subset. Trim with select to reduce payloads per guidelines.
Example:
- async getById(teamId: number) { - return this.dbRead.prisma.team.findUnique({ - where: { id: teamId }, - }); - } + async getById(teamId: number) { + return this.dbRead.prisma.team.findUnique({ + where: { id: teamId }, + select: { id: true, metadata: true }, // extend as needed per caller + }); + }Also applies to: 63-73
apps/web/playwright/lib/orgMigration.ts (3)
12-12: Importing both schemas is fine; keep usage consistent.If the goal is to only constrain Stripe fields at write/upgrade boundaries, prefer teamMetadataSchema in test helpers unless you explicitly want failures on legacy data.
591-601: Strict parse may reject legacy teams; consider safe fallback.dbRemoveTeamFromOrg only needs migratedToOrgFrom.teamSlug. Failing due to unrelated subscriptionId/ItemId prefixes could make org removal brittle.
Apply this diff to prefer strict, with graceful fallback to the relaxed schema:
- const teamMetadata = teamMetadataStrictSchema.parse(team?.metadata); + const parsed = teamMetadataStrictSchema.safeParse(team?.metadata); + const teamMetadata = parsed.success + ? parsed.data + : teamMetadataSchema.parse(team?.metadata);
257-265: Use select instead of include per guidelines.Both getTeamOrThrowError and the update that returns members use include. Switch to select to fetch only what you need.
const team = await prisma.team.findUnique({ where: { id: targetOrgId }, - include: { - organizationSettings: true, - }, + select: { + id: true, + slug: true, + metadata: true, + isOrganization: true, + organizationSettings: true, + }, }); @@ - include: { - members: true, - }, + select: { + id: true, + slug: true, + parentId: true, + members: true, // optionally narrow member fields if needed + },Also applies to: 608-611
packages/features/ee/billing/teams/internal-team-billing.ts (1)
50-56: Minor: log after validating subscriptionId.We currently log “Cancelling subscription null …” if subscriptionId is missing. Move the info log below the guard for cleaner logs.
- const { subscriptionId } = this.team.metadata; - log.info(`Cancelling subscription ${subscriptionId} for team ${this.team.id}`); - if (!subscriptionId) throw Error("missing subscriptionId"); + const { subscriptionId } = this.team.metadata; + if (!subscriptionId) throw Error("missing subscriptionId"); + log.info(`Cancelling subscription ${subscriptionId} for team ${this.team.id}`);packages/trpc/server/routers/viewer/teams/update.handler.ts (1)
98-105: Ensure requestedSlug is dropped even if strict parsing fails.Strict validation can fail on legacy metadata; in that case we currently keep requestedSlug around, which can cause stale metadata. Fall back to removing requestedSlug from raw JSON when strict parsing fails.
- const metadataParse = teamMetadataStrictSchema.safeParse(prevTeam.metadata); - if (metadataParse.success) { - const { requestedSlug: _, ...cleanMetadata } = metadataParse.data || {}; - data.metadata = { - ...cleanMetadata, - }; - } + const metadataParse = teamMetadataStrictSchema.safeParse(prevTeam.metadata); + if (metadataParse.success) { + const { requestedSlug: _, ...cleanMetadata } = metadataParse.data || {}; + data.metadata = { ...cleanMetadata }; + } else if (prevTeam.metadata && typeof prevTeam.metadata === "object" && !Array.isArray(prevTeam.metadata)) { + // Fallback for legacy metadata that fails strict validation: still drop requestedSlug. + const { requestedSlug: _rs, ...rest } = (prevTeam.metadata as Record<string, unknown>); + data.metadata = rest as unknown as Prisma.JsonValue; + }packages/trpc/server/routers/viewer/organizations/update.schema.ts (1)
31-31: Confirm unwrap/shape semantics and unknown key policy.teamMetadataStrictSchema is partial().nullable(); unwrap() will drop only the nullable, preserving partial keys. Verify whether the underlying base object is strict or passthrough. If you intend to retain unknown keys on writes, ensure the base schema uses .passthrough(); if you prefer dropping unknown keys, current setup is fine. Please confirm desired behavior before rollout.
Would you like me to prepare a brief doc blurb for /docs outlining the new validation rules (sub_/si_ prefixes) and how unknown keys are handled?
packages/trpc/server/routers/viewer/organizations/update.handler.ts (1)
168-171: Optional: fail early on invalid metadataDefense-in-depth: validate input.metadata against the strict schema (after the omit above) and return a typed TRPC error if invalid, rather than letting deeper layers surface errors.
Apply this diff (assuming the omit refactor above):
- const publicTeamMetadataSchema = teamMetadataStrictSchema + const publicTeamMetadataSchema = teamMetadataStrictSchema .unwrap() .omit({ subscriptionId: true, subscriptionItemId: true }); + const parsedPublicMetadata = publicTeamMetadataSchema.safeParse(input.metadata ?? {}); + if (!parsedPublicMetadata.success) { + throw new TRPCError({ code: "BAD_REQUEST", message: parsedPublicMetadata.error.message }); + }Then replace subsequent parses with parsedPublicMetadata.data.
packages/trpc/server/routers/viewer/organizations/adminUpdate.handler.ts (1)
37-37: Future-proof admin metadata mergesCurrently, only requestedSlug is merged. If admin updates ever include arbitrary metadata, apply the same “omit subscription fields” pattern used for viewer routes to avoid unintended writes to Stripe IDs.
Example:
-const { mergeMetadata } = getMetadataHelpers(teamMetadataStrictSchema.unwrap(), existingOrg.metadata || {}); +const strictUnwrapped = teamMetadataStrictSchema.unwrap(); +const { mergeMetadata } = getMetadataHelpers(strictUnwrapped, existingOrg.metadata || {}); +const publicTeamMetadataSchema = strictUnwrapped.omit({ subscriptionId: true, subscriptionItemId: true }); +// When/if merging admin-provided metadata: +// data.metadata = mergeMetadata(publicTeamMetadataSchema.parse(input.metadata ?? {}));packages/lib/server/repository/organization.ts (2)
9-11: Type-only import is fine, but you’ll need a runtime import to validateSince you changed the method signature to z.infer, keep this type-only import. If you adopt runtime validation in updateStripeSubscriptionDetails (recommended below), add a value import too.
452-463: No null‐spread risk detected; consider optional Stripe ID format checksI verified that the only caller of
updateStripeSubscriptionDetailsin
packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts
usesconst existingMetadata = teamMetadataStrictSchema.parse(organization.metadata); await OrganizationRepository.updateStripeSubscriptionDetails({ … });which guarantees a non‐null, fully validated metadata object. The spread of
existingMetadatainside the method cannot throw at runtime today.Recommendations:
- If you’d like the signature to reflect that guarantee, tighten the parameter to a non‐nullable schema type (e.g. use the strict Zod type directly).
- Optionally add a runtime check or Zod refinement to ensure:
•stripeSubscriptionIdstarts withsub_
•stripeSubscriptionItemIdstarts withsi_
before persisting, to guard against invalid values in future call sites.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/api/v2/src/modules/teams/teams/teams.repository.ts(2 hunks)apps/web/app/api/teams/[team]/upgrade/route.ts(3 hunks)apps/web/playwright/lib/orgMigration.ts(2 hunks)packages/features/ee/billing/teams/internal-team-billing.ts(1 hunks)packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts(2 hunks)packages/lib/server/repository/organization.ts(2 hunks)packages/prisma/zod-utils.ts(1 hunks)packages/trpc/server/routers/viewer/organizations/adminUpdate.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/publish.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/update.handler.ts(2 hunks)packages/trpc/server/routers/viewer/organizations/update.schema.ts(2 hunks)packages/trpc/server/routers/viewer/teams/update.handler.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/trpc/server/routers/viewer/teams/update.handler.tsapps/web/playwright/lib/orgMigration.tspackages/features/ee/billing/teams/internal-team-billing.tspackages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.tsapps/api/v2/src/modules/teams/teams/teams.repository.tspackages/trpc/server/routers/viewer/organizations/adminUpdate.handler.tspackages/trpc/server/routers/viewer/organizations/publish.handler.tsapps/web/app/api/teams/[team]/upgrade/route.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/lib/server/repository/organization.tspackages/trpc/server/routers/viewer/organizations/update.schema.tspackages/trpc/server/routers/viewer/organizations/update.handler.tspackages/prisma/zod-utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/trpc/server/routers/viewer/teams/update.handler.tsapps/web/playwright/lib/orgMigration.tspackages/features/ee/billing/teams/internal-team-billing.tspackages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.tsapps/api/v2/src/modules/teams/teams/teams.repository.tspackages/trpc/server/routers/viewer/organizations/adminUpdate.handler.tspackages/trpc/server/routers/viewer/organizations/publish.handler.tsapps/web/app/api/teams/[team]/upgrade/route.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/lib/server/repository/organization.tspackages/trpc/server/routers/viewer/organizations/update.schema.tspackages/trpc/server/routers/viewer/organizations/update.handler.tspackages/prisma/zod-utils.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/trpc/server/routers/viewer/teams/update.handler.tsapps/web/playwright/lib/orgMigration.tspackages/features/ee/billing/teams/internal-team-billing.tspackages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.tsapps/api/v2/src/modules/teams/teams/teams.repository.tspackages/trpc/server/routers/viewer/organizations/adminUpdate.handler.tspackages/trpc/server/routers/viewer/organizations/publish.handler.tsapps/web/app/api/teams/[team]/upgrade/route.tspackages/trpc/server/routers/viewer/organizations/createTeams.handler.tspackages/lib/server/repository/organization.tspackages/trpc/server/routers/viewer/organizations/update.schema.tspackages/trpc/server/routers/viewer/organizations/update.handler.tspackages/prisma/zod-utils.ts
**/*.{service,repository}.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Avoid dot-suffixes like
.service.tsor.repository.tsfor new files; reserve.test.ts,.spec.ts,.types.tsfor their specific purposes
Files:
apps/api/v2/src/modules/teams/teams/teams.repository.ts
🧬 Code graph analysis (13)
packages/trpc/server/routers/viewer/teams/update.handler.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
apps/web/playwright/lib/orgMigration.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/features/ee/billing/teams/internal-team-billing.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
apps/api/v2/src/modules/teams/teams/teams.repository.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/trpc/server/routers/viewer/organizations/adminUpdate.handler.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/trpc/server/routers/viewer/organizations/publish.handler.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
apps/web/app/api/teams/[team]/upgrade/route.ts (2)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)packages/features/ee/billing/teams/internal-team-billing.ts (2)
team(29-32)team(33-35)
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/lib/server/repository/organization.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/trpc/server/routers/viewer/organizations/update.schema.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/trpc/server/routers/viewer/organizations/update.handler.ts (1)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(396-412)
packages/prisma/zod-utils.ts (1)
packages/platform/libraries/index.ts (1)
teamMetadataSchema(63-63)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests / Unit
- GitHub Check: Type check / check-types
🔇 Additional comments (12)
apps/web/app/api/teams/[team]/upgrade/route.ts (1)
15-15: Good swap to strict schema import.Switching to teamMetadataStrictSchema is aligned with the intent to constrain Stripe identifiers and keeps the route’s validation consistent with the rest of the PR.
packages/features/ee/billing/teams/internal-team-billing.ts (3)
13-13: Good: strict schema import.Ensures this class only works with subscription ids that match Stripe’s prefixes.
20-21: unwrap usage is appropriate.teamMetadataStrictSchema.unwrap() yields a non-nullable, partial object, which fits getMetadataHelpers’ merge use.
143-158: Runtime guarantees rely on strict schema; keep explicit guards.updateQuantity correctly throws if subscriptionId or subscriptionItemId is missing. With the new schema optionality, these explicit checks remain essential. Looks good.
packages/features/ee/organizations/lib/server/createOrganizationFromOnboarding.ts (1)
33-33: Import switch to strict schema is correct and aligned with PR goals.Using teamMetadataStrictSchema here matches the intent to enforce Stripe ID prefixes in org/team metadata.
packages/trpc/server/routers/viewer/teams/update.handler.ts (1)
11-11: Schema import change looks good.Adopting teamMetadataStrictSchema here aligns with the Stripe ID prefix enforcement.
packages/trpc/server/routers/viewer/organizations/update.schema.ts (1)
4-4: Good move to the strict schema import.This ensures update inputs are validated with the new Stripe ID prefix constraints.
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (1)
298-307: LGTM: getSubscriptionId now enforces the “sub_” prefix.Using teamMetadataStrictSchema.safeParse ensures we only attempt cancellation with valid Stripe subscription IDs, reducing API errors.
packages/trpc/server/routers/viewer/organizations/publish.handler.ts (1)
6-6: Strict schema import is aligned with the Stripe metadata restriction.No concerns on the import itself.
packages/trpc/server/routers/viewer/organizations/update.handler.ts (2)
12-12: Importing strict schema is consistent with the PR goalSwitching to teamMetadataStrictSchema aligns this handler with the stricter Stripe metadata rules. No functional regressions here.
168-171: Sanity check: unwrap semantics verified
All viewer write routes now consistently use the strict schema’sunwrap()when merging metadata, preserving field validations without reintroducing the nullable wrapper. Confirmed usage in:
- packages/trpc/server/routers/viewer/organizations/update.handler.ts
- packages/trpc/server/routers/viewer/organizations/adminUpdate.handler.ts
No other
getMetadataHelpersormergeMetadatacalls remain using the non-strict schema for writes.packages/trpc/server/routers/viewer/organizations/adminUpdate.handler.ts (1)
7-7: LGTM: use strict schema in admin handlerImporting teamMetadataStrictSchema is consistent with the broader migration. No functional change here.
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
Show resolved
Hide resolved
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
Looks good to me, happy to merge 😎
E2E results are ready! |
3 participants
What does this PR do?
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):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist