feat: add cron job to migrate team/org billing data from bookings#24498
feat: add cron job to migrate team/org billing data from bookings#24498joeauyeung wants to merge 8 commits intomainfrom
Conversation
This cron job migrates team and organization subscription billing data from the Team.metadata field to the dedicated TeamBilling and OrganizationBilling tables. The job: - Queries bookings created within a configurable lookback period (default 24 hours) - Extracts unique team IDs from those bookings - Migrates billing data to the appropriate table (TeamBilling or OrganizationBilling) - Provides comprehensive logging and error tracking - Skips already-migrated teams and teams without subscription data This approach has zero impact on user experience as it runs asynchronously and processes teams incrementally as they become active through new bookings. Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
keithwillcode
added
core
WalkthroughIntroduces a new POST API endpoint at /api/cron/migrate-billing that migrates billing data for teams with recent bookings. It authenticates via CRON_API_KEY or Bearer CRON_SECRET, enforces lookbackHours (default 24, min 1, max 168), derives team IDs from recent bookings, and per team: loads data, skips if missing/already migrated, validates metadata, extracts subscriptionId/subscriptionItemId, builds billing payload (IDs, status, planName, optional dates), and writes to OrganizationBilling or TeamBilling. It aggregates migrated/skipped/errors, returns a JSON summary, logs details, handles 500s, and exports POST. Added README documents workflow, parameters, scheduling, mapping, and rollback. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
dosubot
bot
added
billing
![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 (1)
apps/web/app/api/cron/migrate-billing/README.md (1)
114-121: Add a language hint to the log code fence.Giving the log snippet a fence language (e.g., ```text) silences MD040 and keeps rendering consistent with the rest of the doc.
+
text +[cron:migrate-billing] Starting billing migration for bookings created since 2025-10-15T00:00:00.000Z +[cron:migrate-billing] Found 15 unique teams from recent bookings +[cron:migrate-billing] Team 123 already has billing record, skipping +[cron:migrate-billing] Migrated team 456 to TeamBilling table +[cron:migrate-billing] Migrated organization 789 to OrganizationBilling table +[cron:migrate-billing] Billing migration completed: {"ok":true,"teamsFound":15,"migrated":8,"skipped":7,"errors":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 (2)
apps/web/app/api/cron/migrate-billing/README.md(1 hunks)apps/web/app/api/cron/migrate-billing/route.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:
apps/web/app/api/cron/migrate-billing/route.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:
apps/web/app/api/cron/migrate-billing/route.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:
apps/web/app/api/cron/migrate-billing/route.ts
🧬 Code graph analysis (1)
apps/web/app/api/cron/migrate-billing/route.ts (2)
packages/prisma/zod-utils.ts (1)
teamMetadataStrictSchema(390-406)apps/web/app/api/defaultResponderForAppDir.ts (1)
defaultResponderForAppDir(14-72)
🪛 LanguageTool
apps/web/app/api/cron/migrate-billing/README.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...lookback period (default: last 24 hours) 2. Extracts unique team IDs from those book...
(QB_NEW_EN)
[grammar] ~12-~12: There might be a mistake here.
Context: ...acts unique team IDs from those bookings 3. For each team: - Checks if billing da...
(QB_NEW_EN)
[grammar] ~13-~13: There might be a mistake here.
Context: ...Ds from those bookings 3. For each team: - Checks if billing data already exists in...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...ng data already exists in the new tables - Skips if already migrated - Parses me...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...ew tables - Skips if already migrated - Parses metadata to extract subscription ...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...data to extract subscription information - Creates a record in either TeamBilling...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...s that make new bookings. This approach: - Has zero impact on user experience (runs...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...on user experience (runs asynchronously) - Processes teams incrementally as they be...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...eams incrementally as they become active - Avoids overwhelming the database with a ...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...e database with a single large migration - Provides clear logging and error trackin...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...umber of hours to look back for bookings - Default: 24 - Min: 1 - Max: 168 (1 w...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...o look back for bookings - Default: 24 - Min: 1 - Max: 168 (1 week) ### Exampl...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ... for bookings - Default: 24 - Min: 1 - Max: 168 (1 week) ### Example Requests ...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...orDetails": [] } ``` Response Fields: - ok: Whether the job completed successfully...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ...: Whether the job completed successfully - lookbackHours: Configured lookback period - `lookback...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...okbackHours: Configured lookback period - lookbackDate: Calculated cutoff date - teamsFound`:...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...- lookbackDate: Calculated cutoff date - teamsFound: Number of unique teams found in recent...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...of unique teams found in recent bookings - migrated: Number of teams successfully migrated ...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...: Number of teams successfully migrated - skipped`: Number of teams skipped (already migra...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...lready migrated or no subscription data) - errors: Number of teams that failed to migrate...
(QB_NEW_EN)
[grammar] ~81-~81: There might be a mistake here.
Context: ...: Number of teams that failed to migrate - errorDetails: Array of error details with teamId and...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... Start, completion, and migration events - warn: Missing teams - debug: Skip reasons ...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...migration events - warn: Missing teams - debug: Skip reasons - error: Migration fail...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...: Missing teams - debug: Skip reasons - error`: Migration failures ### Example Log Ou...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...| Metadata Field | Billing Table Field | |---------------|-------------------| | ...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ... | |---------------|-------------------| | subscriptionId | subscriptionId | ...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ... | subscriptionId | subscriptionId | | subscriptionItemId | `subscriptionIt...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...criptionItemId|subscriptionItemId| |paymentId|customerId| | - |st...
(QB_NEW_EN)
[grammar] ~131-~131: There might be a mistake here.
Context: ...ItemId| |paymentId|customerId| | - |status(default: "ACTIVE") | |...
(QB_NEW_EN)
[grammar] ~132-~132: There might be a mistake here.
Context: ...| | - |status(default: "ACTIVE") | |isOrganization|planName` (TEAM or...
(QB_NEW_EN)
[grammar] ~133-~133: There might be a mistake here.
Context: ...n|planName(TEAM or ORGANIZATION) | |subscriptionStartDate|subscriptio...
(QB_NEW_EN)
[grammar] ~134-~134: There might be a mistake here.
Context: ...iptionStartDate|subscriptionStart| |subscriptionTrialEndDate|subscrip...
(QB_NEW_EN)
[grammar] ~135-~135: There might be a mistake here.
Context: ...TrialEndDate|subscriptionTrialEnd| |subscriptionEndDate|subscriptionE...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ...# Skip Conditions Teams are skipped if: 1. Already migrated (billing record exists)...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...Already migrated (billing record exists) 2. No subscriptionId in metadata 3. No `s...
(QB_NEW_EN)
[grammar] ~142-~142: There might be a mistake here.
Context: ...ists) 2. No subscriptionId in metadata 3. No subscriptionItemId in metadata 4. T...
(QB_NEW_EN)
[grammar] ~143-~143: There might be a mistake here.
Context: ...a 3. No subscriptionItemId in metadata 4. Team not found in database ## Error Han...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
apps/web/app/api/cron/migrate-billing/README.md
114-114: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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
| if (![process.env.CRON_API_KEY, `Bearer ${process.env.CRON_SECRET}`].includes(`${apiKey}`)) { | ||
| return NextResponse.json({ message: "Not authenticated" }, { status: 401 }); |
There was a problem hiding this comment.
Harden cron auth when secrets are unset.
If CRON_API_KEY/CRON_SECRET aren’t configured, the allow list contains "Bearer undefined" (and undefined), so anyone sending that value is treated as authenticated. That’s a silent auth bypass. Guard by rejecting requests whenever no secret is configured and filter falsy entries before comparison.
- const apiKey = request.headers.get("authorization") || request.nextUrl.searchParams.get("apiKey");
-
- if (![process.env.CRON_API_KEY, `Bearer ${process.env.CRON_SECRET}`].includes(`${apiKey}`)) {
+ const headerToken = request.headers.get("authorization");
+ const queryToken = request.nextUrl.searchParams.get("apiKey");
+ const suppliedToken = headerToken ?? queryToken ?? undefined;
+ const allowedTokens = [
+ process.env.CRON_API_KEY,
+ process.env.CRON_SECRET ? `Bearer ${process.env.CRON_SECRET}` : undefined,
+ ].filter(Boolean) as string[];
+
+ if (!suppliedToken || allowedTokens.length === 0 || !allowedTokens.includes(suppliedToken)) {
return NextResponse.json({ message: "Not authenticated" }, { status: 401 });
}📝 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 (![process.env.CRON_API_KEY, `Bearer ${process.env.CRON_SECRET}`].includes(`${apiKey}`)) { | |
| return NextResponse.json({ message: "Not authenticated" }, { status: 401 }); | |
| const headerToken = request.headers.get("authorization"); | |
| const queryToken = request.nextUrl.searchParams.get("apiKey"); | |
| const suppliedToken = headerToken ?? queryToken ?? undefined; | |
| const allowedTokens = [ | |
| process.env.CRON_API_KEY, | |
| process.env.CRON_SECRET ? `Bearer ${process.env.CRON_SECRET}` : undefined, | |
| ].filter(Boolean) as string[]; | |
| if (!suppliedToken || allowedTokens.length === 0 || !allowedTokens.includes(suppliedToken)) { | |
| return NextResponse.json({ message: "Not authenticated" }, { status: 401 }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/app/api/cron/migrate-billing/route.ts around lines 20 to 21, the
current allow-list includes literal "undefined" entries when CRON_API_KEY or
CRON_SECRET are unset, allowing a silent auth bypass; change the logic to first
reject the request if neither CRON_API_KEY nor CRON_SECRET is configured, then
build the allowed tokens array by filtering out falsy values before comparing to
the incoming apiKey (e.g., only include process.env.CRON_API_KEY and `Bearer
${process.env.CRON_SECRET}` when they are truthy), and return 401 if no match.
| @@ -0,0 +1,168 @@ | |||
| import { defaultResponderForAppDir } from "app/api/defaultResponderForAppDir"; | |||
There was a problem hiding this comment.
This logic should not live directly in this route.ts. It goes against our patterns of having these entry points be thin. Let’s move logic to application services.
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
This PR has been closed due to inactivity. Please feel free to reopen it if you'd like to continue the work. |
Address Keith's review comment to keep route.ts thin by: - Creating BillingMigrationService to handle business logic - Refactoring route.ts to only handle HTTP concerns (auth, request parsing, response) - Also fixing auth bypass vulnerability when CRON secrets are unset Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
…igrationService Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
…level) Co-Authored-By: joe@cal.com <j.auyeung419@gmail.com>
2 participants
What does this PR do?
Adds a cron job to incrementally migrate team and organization billing data from the
Team.metadataJSON field to the dedicatedTeamBillingandOrganizationBillingtables.Migration Strategy:
lookbackHoursparameter)This approach has zero impact on the booking flow and processes teams incrementally as they become active through new bookings.
Link to Devin run: https://app.devin.ai/sessions/31059a89a9ed46e7a3b20fb9531b2166
Requested by: joe@cal.com (@joeauyeung)
Updates Since Last Revision
Refactored to follow Cal.com's architectural patterns per reviewer feedback:
BookingRepositoryandITeamBillingDataRepositoryinstead of calling Prisma directlygetTeamBillingDataRepository(),getBillingRepositoryFactory())BillingRepositoryFactoryinjected to create appropriate billing repository (team vs organization) based on entity typeBillingMigrationServicecovering migration scenarios, skip conditions, error handling, and edge casespackages/features/ee/billing/repository/teamBillingData/Key Implementation Details
Architecture:
route.ts- Thin controller handling auth and HTTP concerns onlyBillingMigrationService- Business logic for migration orchestration, receives dependencies via constructorBookingRepository.findDistinctTeamIdsByCreatedDateRange()- New method to find teams from recent bookingsPrismaTeamBillingDataRepository.findByIdIncludeBillingRecords()- New method to fetch team with billing statusBilling.tscontainer - AddedgetBillingRepositoryFactory()export for DIData Mapping:
subscriptionIdsubscriptionIdsubscriptionItemIdsubscriptionItemIdpaymentIdcustomerIdstatus(default: "ACTIVE")isOrganizationplanName(TEAM or ORGANIZATION)Authentication: Uses
CRON_API_KEYorCRON_SECRET(consistent with other cron jobs)Important Review Points
paymentIdin metadata is actually the StripecustomerIdfindDistinctTeamIdsByCreatedDateRangeandfindByIdIncludeBillingRecordsimplementations are correctbillingRepositoryFactoryModuleLoaderloads correctly with its dependenciesMandatory Tasks (DO NOT REMOVE)
How should this be tested?
Prerequisites:
CRON_API_KEYorCRON_SECRETin environment variablesTeam.metadataManual Testing:
Verify:
TeamBillingandOrganizationBillingtables to verify data was insertedUnit Tests:
yarn test packages/features/ee/billing/services/BillingMigrationService.test.tsChecklist