fix: adjust free monthly credits for orgs#23798
Conversation
Walkthrough
Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| "NEXT_PUBLIC_STRIPE_PREMIUM_PLAN_PRICE_MONTHLY", | ||
| "NEXT_PUBLIC_STRIPE_PRICING_TABLE_PUBLISHABLE_KEY", | ||
| "NEXT_PUBLIC_STRIPE_CREDITS_PRICE_ID", | ||
| "ORG_MONTHLY_CREDITS", |
There was a problem hiding this comment.
NIT: turbo json has env variables in alphabetical order
There was a problem hiding this comment.
doesn't really look sorted to me 🤔
![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
🧹 Nitpick comments (3)
.env.example (1)
206-206: Fix ordering to satisfy dotenv-linter and add a short doc.Move ORG_MONTHLY_CREDITS before STRIPE_TEAM_MONTHLY_PRICE_ID and document the unit/default.
-STRIPE_TEAM_MONTHLY_PRICE_ID= -NEXT_PUBLIC_STRIPE_CREDITS_PRICE_ID= -ORG_MONTHLY_CREDITS= +NEXT_PUBLIC_STRIPE_CREDITS_PRICE_ID= +# Free monthly credits per seat for organizations (integer). Defaults to 1000 if unset. +ORG_MONTHLY_CREDITS= +STRIPE_TEAM_MONTHLY_PRICE_ID= STRIPE_TEAM_PRODUCT_ID=turbo.json (1)
123-124: Remove duplicate entry in globalEnv.ORG_MONTHLY_CREDITS appears twice; keep the first and drop the second.
"STRIPE_ORG_MONTHLY_PRICE_ID", - "ORG_MONTHLY_CREDITS", "TANDEM_BASE_URL",Also applies to: 191-191
packages/features/ee/billing/credit-service.ts (1)
609-615: Harden Stripe price fetch with error handling; improve warn message.A bad/missing price ID will throw and bubble. Guard with try/catch and clarify which env var is missing.
- if (!priceId) { - log.warn("Monthly price ID not configured", { teamId }); - return 0; - } - - const monthlyPrice = await billingService.getPrice(priceId); + if (!priceId) { + log.warn("Monthly price ID not configured (missing STRIPE_TEAM_MONTHLY_PRICE_ID)", { teamId }); + return 0; + } + + let monthlyPrice; + try { + monthlyPrice = await billingService.getPrice(priceId); + } catch (err) { + log.error("Failed to fetch Stripe price", { teamId, priceId, err }); + return 0; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.env.example(1 hunks)packages/features/ee/billing/credit-service.ts(1 hunks)turbo.json(3 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/features/ee/billing/credit-service.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/features/ee/billing/credit-service.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/features/ee/billing/credit-service.ts
🧬 Code graph analysis (1)
packages/features/ee/billing/credit-service.ts (1)
packages/features/ee/billing/stripe-billling-service.ts (1)
StripeBillingService(5-199)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 206-206: [UnorderedKey] The ORG_MONTHLY_CREDITS key should go before the STRIPE_TEAM_MONTHLY_PRICE_ID key
(UnorderedKey)
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (2)
turbo.json (1)
336-336: Env exposure for build looks fine.Adding ORG_MONTHLY_CREDITS to @calcom/web#build env is appropriate.
packages/features/ee/billing/credit-service.ts (1)
606-608: Incorrect — non-org multiplier unchanged (teams still 50%); PR only changes org logicVerified in packages/features/ee/billing/credit-service.ts: org branch now uses ORG_MONTHLY_CREDITS (parseInt or default 1000) — lines 602–603; teams still compute creditsPerSeat = pricePerSeat * 0.5 — lines 616–618. The reported 20%→50% change for non-org teams is incorrect.
Likely an incorrect or invalid review comment.
| if (team.isOrganization) { | ||
| const orgMonthlyCredits = process.env.ORG_MONTHLY_CREDITS; | ||
| const creditsPerSeat = orgMonthlyCredits ? parseInt(orgMonthlyCredits) : 1000; | ||
| return activeMembers * creditsPerSeat; | ||
| } |
There was a problem hiding this comment.
Robust parsing of ORG_MONTHLY_CREDITS (0 treated as falsy today).
Current code ignores an explicit 0 and lacks NaN/range handling; also no radix. Parse safely, accept 0, clamp to >= 0.
- const orgMonthlyCredits = process.env.ORG_MONTHLY_CREDITS;
- const creditsPerSeat = orgMonthlyCredits ? parseInt(orgMonthlyCredits) : 1000;
+ const rawOrgMonthlyCredits = process.env.ORG_MONTHLY_CREDITS;
+ const parsedOrgMonthlyCredits =
+ rawOrgMonthlyCredits != null ? Number(rawOrgMonthlyCredits) : NaN;
+ const creditsPerSeat = Number.isFinite(parsedOrgMonthlyCredits)
+ ? Math.max(0, Math.floor(parsedOrgMonthlyCredits))
+ : 1000;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (team.isOrganization) { | |
| const orgMonthlyCredits = process.env.ORG_MONTHLY_CREDITS; | |
| const creditsPerSeat = orgMonthlyCredits ? parseInt(orgMonthlyCredits) : 1000; | |
| return activeMembers * creditsPerSeat; | |
| } | |
| if (team.isOrganization) { | |
| const rawOrgMonthlyCredits = process.env.ORG_MONTHLY_CREDITS; | |
| const parsedOrgMonthlyCredits = | |
| rawOrgMonthlyCredits != null ? Number(rawOrgMonthlyCredits) : NaN; | |
| const creditsPerSeat = Number.isFinite(parsedOrgMonthlyCredits) | |
| ? Math.max(0, Math.floor(parsedOrgMonthlyCredits)) | |
| : 1000; | |
| return activeMembers * creditsPerSeat; | |
| } |
🤖 Prompt for AI Agents
In packages/features/ee/billing/credit-service.ts around lines 600 to 604, the
environment value ORG_MONTHLY_CREDITS is currently treated as falsy (so "0" is
ignored) and lacks NaN/radix/range handling; change parsing to use
parseInt(orgMonthlyCredits, 10), then test the result for NaN and clamp it with
Math.max(0, parsed) so 0 is accepted but negative numbers become 0, and fall
back to the default (1000) only when parsing yields NaN or an invalid value; use
the clamped value as creditsPerSeat and return activeMembers * creditsPerSeat.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/features/ee/billing/credit-service.test.ts (1)
438-459: Assert no Stripe price lookup for org pathAdd a guard assertion so org credits never hit Stripe accidentally.
it("should calculate credits for organizations using ORG_MONTHLY_CREDITS", async () => { vi.stubEnv("ORG_MONTHLY_CREDITS", "1500"); @@ vi.mocked(TeamRepository).mockImplementation(() => mockTeamRepo as any); + + // Sanity check: org path must not hit Stripe pricing + const priceSpy = vi.spyOn(StripeBillingService.prototype, "getPrice"); @@ const result = await creditService.getMonthlyCredits(1); - expect(result).toBe(3000); // 2 members * 1500 credits per seat + expect(result).toBe(3000); // 2 members * 1500 credits per seat + expect(priceSpy).not.toHaveBeenCalled(); });Optional: add afterEach(() => vi.unstubAllEnvs()) to keep tests hermetic across env changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/ee/billing/credit-service.test.ts(2 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/features/ee/billing/credit-service.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/features/ee/billing/credit-service.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/features/ee/billing/credit-service.test.ts
🧬 Code graph analysis (1)
packages/features/ee/billing/credit-service.test.ts (1)
packages/lib/server/repository/team.ts (1)
TeamRepository(170-401)
⏰ 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). (3)
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
| it("should calculate credits for organizations with default 1000 credits per seat", async () => { | ||
| // Clear ORG_MONTHLY_CREDITS to test default behavior | ||
| vi.stubEnv("ORG_MONTHLY_CREDITS", undefined); | ||
| const mockTeamRepo = { | ||
| findTeamWithMembers: vi.fn().mockResolvedValue({ | ||
| id: 1, | ||
| isOrganization: true, | ||
| members: [{ accepted: true }, { accepted: true }, { accepted: true }], | ||
| }), | ||
| }; | ||
| vi.spyOn(StripeBillingService.prototype, "getPrice").mockImplementation( | ||
| mockStripeBillingService.getPrice | ||
| vi.mocked(TeamRepository).mockImplementation(() => mockTeamRepo as any); | ||
|
|
||
| const mockTeamBillingService = { | ||
| getSubscriptionStatus: vi.fn().mockResolvedValue("active"), | ||
| }; | ||
| vi.spyOn(InternalTeamBilling.prototype, "getSubscriptionStatus").mockImplementation( | ||
| mockTeamBillingService.getSubscriptionStatus | ||
| ); | ||
|
|
||
| const result = await creditService.getMonthlyCredits(1); | ||
| expect(result).toBe(1480); // (2 members * 3700 price) * 0.2 | ||
| expect(result).toBe(3000); // 3 members * 1000 credits per seat (default) | ||
| }); |
There was a problem hiding this comment.
Fix env cleanup: use vi.unstubEnv instead of passing undefined
vi.stubEnv expects a string; passing undefined can throw or leak prior value. Unset the var explicitly.
it("should calculate credits for organizations with default 1000 credits per seat", async () => {
// Clear ORG_MONTHLY_CREDITS to test default behavior
- vi.stubEnv("ORG_MONTHLY_CREDITS", undefined);
+ vi.unstubEnv("ORG_MONTHLY_CREDITS");Consider also asserting Stripe is not called here, mirroring the previous test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("should calculate credits for organizations with default 1000 credits per seat", async () => { | |
| // Clear ORG_MONTHLY_CREDITS to test default behavior | |
| vi.stubEnv("ORG_MONTHLY_CREDITS", undefined); | |
| const mockTeamRepo = { | |
| findTeamWithMembers: vi.fn().mockResolvedValue({ | |
| id: 1, | |
| isOrganization: true, | |
| members: [{ accepted: true }, { accepted: true }, { accepted: true }], | |
| }), | |
| }; | |
| vi.spyOn(StripeBillingService.prototype, "getPrice").mockImplementation( | |
| mockStripeBillingService.getPrice | |
| vi.mocked(TeamRepository).mockImplementation(() => mockTeamRepo as any); | |
| const mockTeamBillingService = { | |
| getSubscriptionStatus: vi.fn().mockResolvedValue("active"), | |
| }; | |
| vi.spyOn(InternalTeamBilling.prototype, "getSubscriptionStatus").mockImplementation( | |
| mockTeamBillingService.getSubscriptionStatus | |
| ); | |
| const result = await creditService.getMonthlyCredits(1); | |
| expect(result).toBe(1480); // (2 members * 3700 price) * 0.2 | |
| expect(result).toBe(3000); // 3 members * 1000 credits per seat (default) | |
| }); | |
| it("should calculate credits for organizations with default 1000 credits per seat", async () => { | |
| // Clear ORG_MONTHLY_CREDITS to test default behavior | |
| vi.unstubEnv("ORG_MONTHLY_CREDITS"); | |
| const mockTeamRepo = { | |
| findTeamWithMembers: vi.fn().mockResolvedValue({ | |
| id: 1, | |
| isOrganization: true, | |
| members: [{ accepted: true }, { accepted: true }, { accepted: true }], | |
| }), | |
| }; | |
| vi.mocked(TeamRepository).mockImplementation(() => mockTeamRepo as any); | |
| const mockTeamBillingService = { | |
| getSubscriptionStatus: vi.fn().mockResolvedValue("active"), | |
| }; | |
| vi.spyOn(InternalTeamBilling.prototype, "getSubscriptionStatus").mockImplementation( | |
| mockTeamBillingService.getSubscriptionStatus | |
| ); | |
| const result = await creditService.getMonthlyCredits(1); | |
| expect(result).toBe(3000); // 3 members * 1000 credits per seat (default) | |
| }); |
🤖 Prompt for AI Agents
In packages/features/ee/billing/credit-service.test.ts around lines 460 to 481,
replace the incorrect vi.stubEnv("ORG_MONTHLY_CREDITS", undefined) with
vi.unstubEnv("ORG_MONTHLY_CREDITS") to properly remove the env var (stubEnv
expects a string and passing undefined can leak prior values); additionally,
mirror the previous test by asserting the Stripe client (or the mocked Stripe
call used in other tests) was not invoked in this case to ensure no external
calls occur when using defaults.
E2E results are ready! |
4 participants
What does this PR do?
To calculate the free monthly credits an Org receives, we now use a new env variable instead of the previous 20% of monthly costs per member