Skip to content

Comments

fix: move team credits to org on upgarde#24386

Merged
CarinaWolli merged 3 commits intomainfrom
fix/move-team-credits-to-org
Oct 13, 2025
Merged

fix: move team credits to org on upgarde#24386
CarinaWolli merged 3 commits intomainfrom
fix/move-team-credits-to-org

Conversation

@CarinaWolli
Copy link
Member

What does this PR do?

When upgrading a team to an org we move all additional credits from the team to the org's credit balance

Loom: https://www.loom.com/share/2d57994df2ec4be8a6b008a8dad930ad

@graphite-app graphite-app bot requested a review from a team October 9, 2025 13:04
@keithwillcode keithwillcode added consumer core area: core, team members only labels Oct 9, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

  • Added CreditService.moveCreditsFromTeamToOrg({ teamId, orgId }) in packages/features/ee/billing/credit-service.ts to transfer a team’s additionalCredits to an organization within a single database transaction, creating the org balance if missing, zeroing the team’s additionalCredits, updating the org’s additionalCredits, logging outcomes, and returning transfer details.
  • Updated packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts to instantiate CreditService and invoke moveCreditsFromTeamToOrg after moving a team to an organization.
  • In tests (packages/trpc/server/routers/viewer/organizations/tests/createTeams.handler.test.ts), added a test validating credit transfer on team move and removed an ESLint-disable comment; the new test appears twice (duplicate).

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the primary change of transferring team credits to an organization on upgrade, aligning with the main intent of the PR, though it contains a minor typo.
Description Check ✅ Passed The description accurately summarizes the change by explaining that team credits are moved to the organization on upgrade and includes relevant context via a Loom link, making it clearly related to the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/move-team-credits-to-org

📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 445b307 and 6a2dedd.

📒 Files selected for processing (3)
  • packages/features/ee/billing/credit-service.ts (1 hunks)
  • packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts (1 hunks)
  • packages/trpc/server/routers/viewer/organizations/createTeams.handler.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 use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • packages/features/ee/billing/credit-service.ts
  • packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
  • packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.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.ts
  • packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
  • packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.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.ts
  • packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts
  • packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts
🧬 Code graph analysis (2)
packages/features/ee/billing/credit-service.ts (1)
packages/lib/server/repository/credits.ts (1)
  • CreditsRepository (6-242)
packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts (1)
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (1)
  • createTeamsHandler (31-132)
⏰ 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 (4)
packages/trpc/server/routers/viewer/organizations/createTeams.handler.ts (2)

2-2: LGTM: Import added correctly.

The CreditService import is properly added to support the credit transfer functionality.


235-236: LGTM: Credit transfer is properly integrated.

The credit transfer is correctly placed after the team update and within the try-catch block. The implementation properly awaits the async operation, and any failures will be caught, logged, and propagated appropriately.

Note that the team update (lines 225-233) and credit transfer are not in the same database transaction, which means if the credit transfer fails, the team will have been moved but credits won't transfer. However, this is acceptable since:

  1. The credit transfer itself is transactional (either fully succeeds or fully fails)
  2. The error will be logged and propagated, allowing for retry or manual intervention
  3. The team move is the primary operation, and credit transfer is supplementary
packages/trpc/server/routers/viewer/organizations/__tests__/createTeams.handler.test.ts (1)

426-475: LGTM: Comprehensive test for credit transfer functionality.

The test properly validates the credit transfer behavior:

  1. Sets up a team with 500 additional credits
  2. Moves the team to an organization
  3. Verifies the team's credits are reset to 0
  4. Verifies the organization's credits are incremented to 500

The test coverage is appropriate for the happy path scenario.

packages/features/ee/billing/credit-service.ts (1)

693-752: LGTM: Credit transfer logic is correct for current usage.

The moveCreditsFromTeamToOrg method is well-structured with proper transaction handling:

  1. Fetches the team's credit balance
  2. Early exits if no transferable credits
  3. Ensures the organization's credit balance exists
  4. Atomically resets team credits to 0 and increments org credits
  5. Logs the transfer details

The implementation is correct for the current usage pattern where teams are moved sequentially (as seen in createTeams.handler.ts line 96).

Note on potential race condition: If this method were ever called concurrently for the same team from multiple sources, there's a theoretical race condition where:

  1. Transaction A reads team balance: 500
  2. Transaction B reads team balance: 500
  3. Both set team balance to 0
  4. Both increment org balance by 500
  5. Result: Org receives 1000 instead of 500

However, this is not a concern for the current implementation since team moves are processed sequentially. If the usage pattern changes in the future to support concurrent moves, consider adding row-level locking (SELECT FOR UPDATE) or using optimistic locking with version checks.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dosubot dosubot bot added billing area: billing, stripe, payments, paypal, get paid organizations area: organizations, orgs labels Oct 9, 2025
@vercel
Copy link

vercel bot commented Oct 13, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Oct 13, 2025 7:52am
cal-eu Ignored Ignored Oct 13, 2025 7:52am

Copy link
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - tested and seems to work well!

@CarinaWolli CarinaWolli enabled auto-merge (squash) October 13, 2025 08:03
@github-actions
Copy link
Contributor

github-actions bot commented Oct 13, 2025

E2E results are ready!

@CarinaWolli CarinaWolli merged commit 3bcaf0e into main Oct 13, 2025
80 of 85 checks passed
@CarinaWolli CarinaWolli deleted the fix/move-team-credits-to-org branch October 13, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

billing area: billing, stripe, payments, paypal, get paid consumer core area: core, team members only organizations area: organizations, orgs ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants