feat: add PlatformOrganizationBillingTasker with sync and trigger.dev versions #26803
Conversation
… versions - Create PlatformOrganizationBillingTasker extending Tasker base class - Add sync tasker (PlatformOrganizationBillingSyncTasker) for synchronous execution - Add trigger.dev tasker (PlatformOrganizationBillingTriggerTasker) for async execution - Create trigger.dev task for incrementing subscription usage - Add PlatformOrganizationBillingTaskService with business logic - Add PlatformOrganizationBillingRepository for data access - Add createSubscriptionUsageRecord method to StripeBillingService - Follow BookingEmailAndSmsTasker pattern for consistency Co-Authored-By: [email protected] <[email protected]>
🤖 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:
|
packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingRepository.ts
Outdated
Show resolved
Hide resolved
packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingRepository.ts
Outdated
Show resolved
Hide resolved
- Move findPlatformOrgByUserId to OrganizationRepository - Create new PlatformBillingRepository for billing queries - Remove PlatformOrganizationBillingRepository (consolidated) - Update task service and trigger.dev task to use new repositories Co-Authored-By: [email protected] <[email protected]>
- Create di/tasker/ directory following BookingEmailAndSmsTasker pattern - Add tokens.ts with DI tokens for all billing tasker classes - Add module files for PlatformBillingRepository, TaskService, SyncTasker, TriggerTasker, Tasker - Add container files with getter functions - Update trigger.dev task to use DI container instead of manual instantiation Co-Authored-By: [email protected] <[email protected]>
| return this.prismaClient.platformBilling.findUnique({ | ||
| where: { | ||
| id: teamId, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
DevinAI add select statement to only get the needed fields
There was a problem hiding this comment.
Done! Added a select statement to only fetch subscriptionId since that's the only field used by the task service.
Only select subscriptionId field since that's the only field used by the task service Co-Authored-By: [email protected] <[email protected]>
- Add platform billing tasker exports to platform-libraries/organizations.ts - Export IBillingProviderService type from platform-libraries - Create StripeBillingProviderService wrapper implementing IBillingProviderService - Create PrismaPlatformBillingRepository extending PlatformBillingRepository - Create NestJS service wrappers for all platform billing tasker classes - Create PlatformBillingTaskerModule with all providers and exports - Update PlatformOrganizationBillingTaskService to use Pick<IBillingProviderService> Co-Authored-By: [email protected] <[email protected]>
E2E results are ready! |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 38 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingTaskService.ts">
<violation number="1" location="packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingTaskService.ts:26">
P1: Dead code: This `if (!team.id)` check will never execute. The `findPlatformOrgByUserId` method uses `findFirstOrThrow`, which throws an exception if no team is found (not returning null). If a team is found, `team.id` is always a valid number.
To properly handle the "user not part of platform org" case, use `findFirst` instead and check for null, or wrap the call in try-catch.</violation>
</file>
<file name="apps/api/v2/src/lib/services/stripe-billing-provider.service.ts">
<violation number="1" location="apps/api/v2/src/lib/services/stripe-billing-provider.service.ts:31">
P2: Silent failure in billing-critical operation. The method logs errors but returns void without propagating failures to the caller. Consider throwing an exception (e.g., `NotFoundException` or `BadRequestException`) so callers can handle failures appropriately and implement retry logic if needed.</violation>
</file>
<file name="apps/api/v2/src/modules/billing/billing.module.ts">
<violation number="1" location="apps/api/v2/src/modules/billing/billing.module.ts:48">
P0: Removing `IsUserInBillingOrg` from providers will break the billing controller. This guard is still used on 4 endpoints (`GET /:teamId/check`, `POST /:teamId/subscribe`, `POST /:teamId/upgrade`, `DELETE /:teamId/unsubscribe`). NestJS needs the guard in providers to resolve its `OrganizationsRepository` dependency.</violation>
</file>
<file name="packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingSyncTasker.ts">
<violation number="1" location="packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingSyncTasker.ts:20">
P3: Grammatical typo: "Delayed task are" should be "Delayed tasks are" (this error appears in all three methods).</violation>
</file>
<file name="packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingTasker.ts">
<violation number="1" location="packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingTasker.ts:34">
P2: Error is silently discarded in catch block. Consider capturing and logging at least the error message for debugging purposes (avoiding full error object to prevent sensitive data exposure). This applies to all three catch blocks in this file.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingTaskService.ts
Outdated
Show resolved
Hide resolved
apps/api/v2/src/lib/services/stripe-billing-provider.service.ts
Outdated
Show resolved
Hide resolved
| this.logger.info(`PlatformOrganizationBillingTasker incrementUsage success:`, taskResponse, { | ||
| userId: payload.userId, | ||
| }); | ||
| } catch { |
There was a problem hiding this comment.
P2: Error is silently discarded in catch block. Consider capturing and logging at least the error message for debugging purposes (avoiding full error object to prevent sensitive data exposure). This applies to all three catch blocks in this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingTasker.ts, line 34:
<comment>Error is silently discarded in catch block. Consider capturing and logging at least the error message for debugging purposes (avoiding full error object to prevent sensitive data exposure). This applies to all three catch blocks in this file.</comment>
<file context>
@@ -0,0 +1,89 @@
+ this.logger.info(`PlatformOrganizationBillingTasker incrementUsage success:`, taskResponse, {
+ userId: payload.userId,
+ });
+ } catch {
+ taskResponse = { runId: "task-failed" };
+ this.logger.error(`PlatformOrganizationBillingTasker incrementUsage failed`, taskResponse, {
</file context>
packages/features/ee/organizations/lib/billing/tasker/PlatformOrganizationBillingSyncTasker.ts
Outdated
Show resolved
Hide resolved
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…768316385 Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/ee/organizations/lib/billing/tasker/trigger/increment-usage.ts">
<violation number="1" location="packages/features/ee/organizations/lib/billing/tasker/trigger/increment-usage.ts:24">
P1: Swallowing the error by returning instead of re-throwing bypasses trigger.dev's retry mechanism. The task config specifies `retry: { maxAttempts: 3 }`, but this retry logic will never execute because the task completes "successfully" after logging the error. For a billing task, this could result in lost usage records without any retry attempts.
If the intent is to prevent retries for certain error types, consider checking the error type and only returning for non-retryable errors (e.g., validation errors), while re-throwing for transient failures (network issues, rate limits).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } catch (error) { | ||
| if (error instanceof Error) logger.error(error.message); | ||
| else logger.error("Unknown error in incrementUsage", { error }); | ||
| return; |
There was a problem hiding this comment.
P1: Swallowing the error by returning instead of re-throwing bypasses trigger.dev's retry mechanism. The task config specifies retry: { maxAttempts: 3 }, but this retry logic will never execute because the task completes "successfully" after logging the error. For a billing task, this could result in lost usage records without any retry attempts.
If the intent is to prevent retries for certain error types, consider checking the error type and only returning for non-retryable errors (e.g., validation errors), while re-throwing for transient failures (network issues, rate limits).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/ee/organizations/lib/billing/tasker/trigger/increment-usage.ts, line 24:
<comment>Swallowing the error by returning instead of re-throwing bypasses trigger.dev's retry mechanism. The task config specifies `retry: { maxAttempts: 3 }`, but this retry logic will never execute because the task completes "successfully" after logging the error. For a billing task, this could result in lost usage records without any retry attempts.
If the intent is to prevent retries for certain error types, consider checking the error type and only returning for non-retryable errors (e.g., validation errors), while re-throwing for transient failures (network issues, rate limits).</comment>
<file context>
@@ -21,7 +21,7 @@ export const incrementUsage: TaskWithSchema<typeof INCREMENT_USAGE_JOB_ID, typeo
if (error instanceof Error) logger.error(error.message);
else logger.error("Unknown error in incrementUsage", { error });
- throw error;
+ return;
}
},
</file context>
Fix confidence (alpha): 9/10
| return; | |
| throw error; |
✅ Addressed in 465e137
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
…ries Co-Authored-By: unknown <>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/ee/organizations/lib/billing/tasker/trigger/config.ts">
<violation number="1" location="packages/features/ee/organizations/lib/billing/tasker/trigger/config.ts:15">
P1: Setting `maxAttempts: 1` disables all retries, contradicting the PR description which states the error handling fix enables 'trigger.dev's retry mechanism (`maxAttempts: 3`)'. For billing operations interacting with Stripe, having no retries is risky - transient failures could cause lost billing records. Consider keeping `maxAttempts: 3` as originally intended.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| queue: platformBillingQueue, | ||
| machine: "small-2x", | ||
| retry: { | ||
| maxAttempts: 1, |
There was a problem hiding this comment.
P1: Setting maxAttempts: 1 disables all retries, contradicting the PR description which states the error handling fix enables 'trigger.dev's retry mechanism (maxAttempts: 3)'. For billing operations interacting with Stripe, having no retries is risky - transient failures could cause lost billing records. Consider keeping maxAttempts: 3 as originally intended.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/ee/organizations/lib/billing/tasker/trigger/config.ts, line 15:
<comment>Setting `maxAttempts: 1` disables all retries, contradicting the PR description which states the error handling fix enables 'trigger.dev's retry mechanism (`maxAttempts: 3`)'. For billing operations interacting with Stripe, having no retries is risky - transient failures could cause lost billing records. Consider keeping `maxAttempts: 3` as originally intended.</comment>
<file context>
@@ -5,14 +5,14 @@ type PlatformBillingTask = Pick<Parameters<typeof schemaTask>[0], "machine" | "r
machine: "small-2x",
retry: {
- maxAttempts: 3,
+ maxAttempts: 1,
factor: 1.8,
minTimeoutInMs: 500,
</file context>
Fix confidence (alpha): 10/10
| maxAttempts: 1, | |
| maxAttempts: 3, |
✅ Addressed in e07fab9
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. |
Co-Authored-By: unknown <>
|
|
||
| async getBillingData(teamId: number) { | ||
| async getBillingData(teamId: number): Promise<{ | ||
| team: Awaited<ReturnType<(typeof OrganizationsRepository.prototype)["findByIdIncludeBilling"]>>; |
There was a problem hiding this comment.
Not a fan of this
@devin can you replace this a inteface type?
There was a problem hiding this comment.
DevinAI implement this feedback
…pe in getBillingData Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/api/v2/src/modules/billing/services/billing.service.ts">
<violation number="1" location="apps/api/v2/src/modules/billing/services/billing.service.ts:43">
P2: The `BillingData` type uses `plan: string` instead of the original `plan: PlatformPlan | "none"`. This type widening loses type safety - consumers can no longer rely on TypeScript to validate that only valid plan values are used. Consider updating the `BillingData` type definition in `billing-service.interface.ts` to use `plan: PlatformPlan | "none"` to preserve type safety.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
|
|
||
| async getBillingData(teamId: number) { | ||
| async getBillingData(teamId: number): Promise<BillingData> { |
There was a problem hiding this comment.
P2: The BillingData type uses plan: string instead of the original plan: PlatformPlan | "none". This type widening loses type safety - consumers can no longer rely on TypeScript to validate that only valid plan values are used. Consider updating the BillingData type definition in billing-service.interface.ts to use plan: PlatformPlan | "none" to preserve type safety.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/v2/src/modules/billing/services/billing.service.ts, line 43:
<comment>The `BillingData` type uses `plan: string` instead of the original `plan: PlatformPlan | "none"`. This type widening loses type safety - consumers can no longer rely on TypeScript to validate that only valid plan values are used. Consider updating the `BillingData` type definition in `billing-service.interface.ts` to use `plan: PlatformPlan | "none"` to preserve type safety.</comment>
<file context>
@@ -40,11 +40,7 @@ export class BillingService implements IBillingService, OnModuleDestroy {
- status: "valid" | "no_subscription" | "no_billing";
- plan: PlatformPlan | "none";
- }> {
+ async getBillingData(teamId: number): Promise<BillingData> {
const teamWithBilling = await this.teamsRepository.findByIdIncludeBilling(teamId);
</file context>
There was a problem hiding this comment.
DevinAI check if this feedback is valid and implement if needed
There was a problem hiding this comment.
The Cubic AI feedback is valid. The BillingData type was using plan: string which loses type safety compared to the original plan: PlatformPlan | "none".
I've updated the BillingData type in billing-service.interface.ts to use plan: PlatformPlan | "none" to preserve type safety.
…for type safety Co-Authored-By: [email protected] <[email protected]>
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…768316385 Co-Authored-By: unknown <>
Devin AI is resolving merge conflictsThis PR has merge conflicts with the Devin will:
If you prefer to resolve conflicts manually, you can close the Devin session and handle it yourself. |
…768316385 Co-Authored-By: unknown <>
sean-brydon
left a comment
sean-brydon
left a comment
There was a problem hiding this comment.
Tested trigger versions and sync versions seemed to work very well! Nice job!
What does this PR do?
Implements a
PlatformOrganizationBillingTaskerfollowing the pattern established byBookingEmailAndSmsTasker. This tasker will eventually replace the billing processor logic inapps/api/v2/src/modules/billing/billing.processor.ts.Components created:
PlatformOrganizationBillingTasker- Main orchestrator extending the Tasker base classPlatformOrganizationBillingSyncTasker- Synchronous execution versionPlatformOrganizationBillingTriggerTasker- Async version using trigger.devPlatformOrganizationBillingTaskService- Business logic for incrementing subscription usagePlatformBillingRepository- New repository inpackages/features/ee/organizations/repositories/for billing queriesChanges to existing code:
createSubscriptionUsageRecordmethod toIBillingProviderServiceinterfaceStripeBillingServiceto handle metered subscription usagefindPlatformOrgByUserIdmethod toOrganizationRepositoryUpdates since last revision
DI Implementation using @evyweb/ioctopus (features package):
packages/features/ee/organizations/di/tasker/directory following theBookingEmailAndSmsTaskerDI patterngetPlatformOrganizationBillingTaskService()) instead of manual instantiationNestJS DI Implementation (API v2):
PlatformBillingTaskerModuleinapps/api/v2/src/lib/modules/@calcom/platform-libraries/organizations:PlatformBillingTaskerPlatformBillingSyncTaskerServicePlatformBillingTriggerTaskerServicePlatformBillingTaskServicePrismaPlatformBillingRepositoryextendingPlatformBillingRepositoryStripeBillingProviderServiceimplementingPick<IBillingProviderService, "createSubscriptionUsageRecord">IBillingProviderServicetype from@calcom/platform-libraries/organizationsError handling fixes (Cubic AI review feedback):
increment-usage.tsto re-throw errors after logging, enabling trigger.dev's retry mechanismmaxAttempts: 3in config.ts to ensure billing task retries are enabled for transient failuresMandatory Tasks (DO NOT REMOVE)
How should this be tested?
createSubscriptionUsageRecordmethod, you would need a Stripe subscription with a metered price itemuserIdthat belongs to a platform organizationPlatformBillingTaskerModuleis created but not yet imported into any consuming moduleHuman Review Checklist
findPlatformOrgByUserIdquery correctly finds platform organizations viaorgProfilesrelationcreateSubscriptionUsageRecordcorrectly identifies metered subscription itemsmaxAttempts: 3,concurrencyLimit: 5)OrganizationsModule)StripeBillingProviderServicepartial implementation (Pick<IBillingProviderService, ...>) is sufficientPlatformBillingTaskerModuleis created but not yet wired up to any consuming moduleChecklist
Link to Devin run: https://app.devin.ai/sessions/220b2aacd19c4dceb080c9a72ea3540f
Link to Devin run (Cubic AI fixes): https://app.devin.ai/sessions/18f96b6f74c64c9294fd29a65dc1fbf3
Requested by: @ThyMinimalDev