-
Notifications
You must be signed in to change notification settings - Fork 245
dev #2283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dev #2283
Changes from 11 commits
90fd1d7
48a9b61
e9e387f
9807024
f16f009
a80de2d
68b31c7
39dca95
ef281ec
7267818
61dc59f
efdf5fd
bc5dbb3
70522cb
90273f6
d2d2043
f1a49c6
c29524e
fcc09de
d7d52a6
940bfad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { | ||
| type AutumnBillingPlan, | ||
| type CustomerLicenseUpdate, | ||
| CusProductStatus, | ||
| type Entitlement, | ||
| type FullCusProduct, | ||
|
|
@@ -8,6 +9,7 @@ import { | |
| type SyncBillingContext, | ||
| } from "@autumn/shared"; | ||
| import type { AutumnContext } from "@/honoUtils/HonoEnv"; | ||
| import { computeCustomerLicenseQuantityChanges } from "@/internal/billing/v2/compute/computeCustomerLicenseQuantityChanges"; | ||
| import { resolveSyncExistingUsagesConfig } from "@/internal/billing/v2/utils/handleCarryOvers/resolveSyncExistingUsagesConfig"; | ||
| import { initImmediateSyncCustomerProduct } from "./initImmediateSyncCustomerProduct"; | ||
|
|
||
|
|
@@ -21,6 +23,7 @@ export type ImmediatePhaseResult = { | |
| customPrices: Price[]; | ||
| customEntitlements: Entitlement[]; | ||
| insertPlanLicenses: InsertPlanLicenseSpec[]; | ||
| customerLicenseUpdates: CustomerLicenseUpdate[]; | ||
| }; | ||
|
|
||
| const expireCustomerProduct = ({ | ||
|
|
@@ -68,6 +71,7 @@ export const computeSyncImmediatePhase = ({ | |
| customPrices: [], | ||
| customEntitlements: [], | ||
| insertPlanLicenses: [], | ||
| customerLicenseUpdates: [], | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -76,14 +80,29 @@ export const computeSyncImmediatePhase = ({ | |
| const customPrices: Price[] = []; | ||
| const customEntitlements: Entitlement[] = []; | ||
| const insertPlanLicenses: InsertPlanLicenseSpec[] = []; | ||
| const customerLicenseUpdates: CustomerLicenseUpdate[] = []; | ||
|
|
||
| for (const productContext of immediatePhase.productContexts) { | ||
| const currentCustomerProduct = productContext.currentCustomerProduct; | ||
| if (currentCustomerProduct?.product_id === productContext.fullProduct.id) { | ||
| const licenseQuantityChanges = computeCustomerLicenseQuantityChanges({ | ||
| customerProduct: currentCustomerProduct, | ||
| customerLicenseQuantities: productContext.customerLicenseQuantities, | ||
| }); | ||
| if (licenseQuantityChanges.length > 0) { | ||
| customerLicenseUpdates.push( | ||
| ...licenseQuantityChanges.map(({ update }) => update), | ||
| ); | ||
| continue; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: A mixed sync can silently lose custom license/item changes when any existing license quantity also changes, because this Prompt for AI agents |
||
| } | ||
| } | ||
|
|
||
| const existingUsagesConfig = | ||
| carryOverUsage && productContext.currentCustomerProduct | ||
| carryOverUsage && currentCustomerProduct | ||
| ? resolveSyncExistingUsagesConfig({ | ||
| ctx, | ||
| carryOverUsages, | ||
| currentCustomerProduct: productContext.currentCustomerProduct, | ||
| currentCustomerProduct, | ||
| }) | ||
| : undefined; | ||
|
|
||
|
|
@@ -117,5 +136,6 @@ export const computeSyncImmediatePhase = ({ | |
| customPrices, | ||
| customEntitlements, | ||
| insertPlanLicenses, | ||
| customerLicenseUpdates, | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import type { SyncParamsV1 } from "@autumn/shared"; | ||
| import { CusProductStatus, type SyncParamsV1 } from "@autumn/shared"; | ||
| import type { AutumnContext } from "@/honoUtils/HonoEnv"; | ||
| import { persistCreateSchedule } from "@/internal/billing/v2/actions/createSchedule/utils/persistCreateSchedule"; | ||
| import { executeAutumnBillingPlan } from "@/internal/billing/v2/execute/executeAutumnBillingPlan"; | ||
| import { sendBillingUpdatedWebhook } from "@/internal/billing/v2/workflows/sendBillingUpdatedWebhook/sendBillingUpdatedWebhook"; | ||
| import { reconcileLicenseStateForCustomer } from "@/internal/licenses/actions/reconcile/reconcileLicenseState"; | ||
| import { computeSyncPlan } from "./compute/computeSyncPlan"; | ||
| import { handleSyncErrors } from "./errors/handleSyncErrors"; | ||
| import { logSyncContext } from "./logs/logSyncContext"; | ||
|
|
@@ -27,28 +28,15 @@ export type SyncV2Result = { | |
|
|
||
| /** | ||
| * Sync a Stripe subscription/schedule into Autumn state. | ||
| * | ||
| * Mirrors the v2 action convention used by createSchedule.ts: | ||
| * 1. setup — fetch sub, schedule, customer, products | ||
| * 2. errors — validate inputs against detection result | ||
| * 3. compute — apply caller overrides and produce an AutumnBillingPlan | ||
| * 4. execute — run the billing plan (cusProduct inserts/updates) | ||
| * 5. persist — write any scheduled phase rows (reuses createSchedule's | ||
| * `persistCreateSchedule` so the schedule + schedule_phases | ||
| * tables are written identically across actions) | ||
| * | ||
| * Webhook emission is opt-in via `webhook` so Stripe auto-sync callers | ||
| * (which already emit billing.updated from the originating action) don't | ||
| * double-send. | ||
| */ | ||
| export const syncV2 = async ({ | ||
| ctx, | ||
| params, | ||
| webhook, | ||
| tags, | ||
| }: { | ||
| ctx: AutumnContext; | ||
| params: SyncParamsV1; | ||
| webhook?: { tags?: string[] }; | ||
| tags?: string[]; | ||
| }): Promise<SyncV2Result> => { | ||
| // 1. Setup | ||
| const syncContext = await setupSyncContext({ ctx, params }); | ||
|
|
@@ -63,6 +51,13 @@ export const syncV2 = async ({ | |
|
|
||
| // 4. Execute | ||
| await executeAutumnBillingPlan({ ctx, autumnBillingPlan }); | ||
| if ((autumnBillingPlan.customerLicenseUpdates?.length ?? 0) > 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Licensed syncs that replace the current product do not reconcile license state: those plans have inserted/expired customer products but no Prompt for AI agents |
||
| await reconcileLicenseStateForCustomer({ | ||
| ctx, | ||
| idOrInternalId: syncContext.customer_id, | ||
| deleteCache: true, | ||
| }); | ||
| } | ||
|
|
||
| // 5. Persist scheduled phases (only when sync produced more than one phase) | ||
| let scheduleId: string | null = null; | ||
|
|
@@ -79,14 +74,20 @@ export const syncV2 = async ({ | |
| scheduledPhases = persisted.insertedPhases; | ||
| } | ||
|
|
||
| if (webhook) { | ||
| void sendBillingUpdatedWebhook({ | ||
| ctx, | ||
| autumnBillingPlan, | ||
| originalFullCustomer: syncContext.fullCustomer, | ||
| tags: webhook.tags, | ||
| }); | ||
| } | ||
| void sendBillingUpdatedWebhook({ | ||
| ctx, | ||
| autumnBillingPlan, | ||
| originalFullCustomer: syncContext.fullCustomer, | ||
| tags, | ||
| }); | ||
|
|
||
| const customerProductUpdates = ( | ||
|
Comment on lines
+77
to
+84
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: server/src/internal/billing/v2/actions/sync/syncV2.ts
Line: 77-84
Comment:
**Unconditional webhook now doubles events in `handleStripeSubscriptionUpdated`**
`syncV2` previously guarded `sendBillingUpdatedWebhook` behind an opt-in `webhook` param. The old code comment said explicitly: _"Webhook emission is opt-in via `webhook` so Stripe auto-sync callers (which already emit billing.updated from the originating action) don't double-send."_ That guard is now gone.
`handleStripeSubscriptionUpdated` calls `autoSyncUpdatedSubscription` (which runs `syncV2` → emits webhook #1 with `tags: ["sync:customer.subscription.updated"]`), then calls `emitBillingChangeWebhook` unconditionally at line 94. Whenever `expireRemovedCustomerProducts`, `syncCustomerProductStatus`, `handleStripeSubscriptionCanceled`, or `handleStripeSubscriptionRenewed` track any change into the event context, the second emission fires and customers receive two `billing.updated` events for one Stripe event. The same issue affects `autoSyncFromSubscription` (subscription.created path) which now also fires a webhook where it previously fired none.
How can I resolve this? If you propose a fix, please make it concise. |
||
| autumnBillingPlan.updateCustomerProducts ?? [] | ||
| ).concat( | ||
| autumnBillingPlan.updateCustomerProduct | ||
| ? [autumnBillingPlan.updateCustomerProduct] | ||
| : [], | ||
| ); | ||
|
|
||
| return { | ||
| customer_id: syncContext.customer_id, | ||
|
|
@@ -95,12 +96,8 @@ export const syncV2 = async ({ | |
| inserted_cus_product_ids: autumnBillingPlan.insertCustomerProducts.map( | ||
| (cp) => cp.id, | ||
| ), | ||
| expired_cus_product_ids: (autumnBillingPlan.updateCustomerProducts ?? []) | ||
| .concat( | ||
| autumnBillingPlan.updateCustomerProduct | ||
| ? [autumnBillingPlan.updateCustomerProduct] | ||
| : [], | ||
| ) | ||
| expired_cus_product_ids: customerProductUpdates | ||
| .filter(({ updates }) => updates.status === CusProductStatus.Expired) | ||
| .map((u) => u.customerProduct.id), | ||
| schedule_id: scheduleId, | ||
| scheduled_phases: scheduledPhases, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continueskips all product-level sync work when seat counts driftWhen
licenseQuantityChanges.length > 0for a product context, the loopcontinues before reaching theinitImmediateSyncCustomerProductpath.buildIncrementalSyncParamsguarantees these paths are mutually exclusive for DIFFERENT plans (product-id changed → early-return before seat check). However, both a seat drift AND a full-product re-sync can be needed for the same Autumn plan id in the version-upgrade case, since the gate inbuildIncrementalSyncParamscomparesproduct.id(public ID, stable across versions) notinternal_id. If a new product version with changed items is also accompanied by a Stripe seat-quantity change, the version upgrade would be silently skipped here.Prompt To Fix With AI