diff --git a/modules/billing/services/billing.webhook.service.js b/modules/billing/services/billing.webhook.service.js index 8cd48101d..cdbe741b0 100644 --- a/modules/billing/services/billing.webhook.service.js +++ b/modules/billing/services/billing.webhook.service.js @@ -58,12 +58,15 @@ const validatePlan = (plan) => { const planRanks = Object.fromEntries((config.billing?.plans || []).map((p, i) => [p, i])); /** - * Reverse-map from Stripe price ID → plan name, built from config.stripe.prices at boot. - * config.stripe.prices = { growth: { monthly: 'price_xxx', annual: 'price_yyy' }, pro: { ... } } - * This avoids relying on price.metadata.planId (empty on Stripe webhook payloads — planId - * lives on the Product, not the Price) and avoids a Stripe API call per webhook. + * Build a reverse-map from Stripe price ID → plan name, sourced from `config.stripe.prices` + * at module load. Shape: `{ growth: { monthly: 'price_xxx', annual: 'price_yyy' }, pro: {...} }`. * - * Fix: resolvePlan no longer reads price.metadata (always empty in real Stripe webhook payloads). + * Why: `price.metadata.planId` is empty on real Stripe webhook payloads — `planId` lives on + * the Product, not the Price exposed by `customer.subscription.updated`. The reverse-map gives + * a robust priceId→plan lookup without an extra Stripe API call per webhook. `resolvePlan` + * keeps `price.metadata.planId` as a legacy fallback (test fixtures, manual Stripe edits). + * + * @returns {Record} priceId → planId map (built once at module init) */ const buildPriceIdToPlanMap = () => { const map = {}; @@ -95,7 +98,19 @@ const resolvePlan = (subscription) => { } // Legacy fallback: price metadata set explicitly (e.g. test fixtures or manual Stripe edits) const rawMeta = item?.price?.metadata?.planId || item?.plan?.metadata?.planId; - return validatePlan(rawMeta) || 'free'; + const fromMeta = validatePlan(rawMeta); + if (fromMeta) return fromMeta; + // Last-resort fallback — warn only when metadata is also absent so misconfigured + // config.stripe.prices is visible (otherwise this silently downgrades paid orgs to 'free', + // which is the exact bug #1250 fixed). When metadata is present but invalid, validatePlan() + // above already emitted an "unrecognized planId" warning — no double-warn needed. + if (priceId && !rawMeta) { + logger.warn('[billing.webhook] resolvePlan: priceId not in priceIdToPlan map and no metadata — falling back to free', { + priceId, + stripeSubscriptionId: subscription?.id, + }); + } + return 'free'; }; /** @@ -421,7 +436,7 @@ const handleSubscriptionUpdated = async (subscription, event) => { if (typeof subscription.cancel_at_period_end === 'boolean') { fields.cancelAtPeriodEnd = subscription.cancel_at_period_end; } - if (subscription.cancel_at) { + if (typeof subscription.cancel_at === 'number') { fields.cancelAt = new Date(subscription.cancel_at * 1000); } else if (subscription.cancel_at === null) { fields.cancelAt = null; @@ -464,10 +479,13 @@ const handleSubscriptionUpdated = async (subscription, event) => { let planChangeResetTriggered = false; if (previousItems) { const previousPriceId = previousItems[0]?.price?.id; - const previousPlan = (previousPriceId && priceIdToPlan[previousPriceId]) + const previousRaw = (previousPriceId && priceIdToPlan[previousPriceId]) || previousItems[0]?.price?.metadata?.planId || previousItems[0]?.plan?.metadata?.planId || null; + // Validate against the canonical plan enum — legacy metadata can carry stale or invalid + // values that would otherwise emit plan.changed + trigger forceRotateForPlanChange with junk. + const previousPlan = validatePlan(previousRaw); if (previousPlan && previousPlan !== newPlan) { const prevRank = planRanks[previousPlan]; const newRank = planRanks[newPlan]; diff --git a/modules/billing/tests/billing.webhook.priceid-map.unit.tests.js b/modules/billing/tests/billing.webhook.priceid-map.unit.tests.js index 2bcd05cf9..9a02b7c26 100644 --- a/modules/billing/tests/billing.webhook.priceid-map.unit.tests.js +++ b/modules/billing/tests/billing.webhook.priceid-map.unit.tests.js @@ -68,7 +68,9 @@ const makeBaseSetup = (configOverride = {}) => { model: () => ({}), }, })); - // Suppress retryWithBackoff delays in tests (replace setTimeout with immediate resolve) + // Mock the failed-backfill repository so the retry recorder is a no-op in unit tests + // (does not suppress the retryWithBackoff setTimeout delays themselves — tests that exercise + // retry pathways still pay the real delay, so keep retry attempt counts low in fixtures). jest.unstable_mockModule('../repositories/billing.failedBackfill.repository.js', () => ({ default: { record: jest.fn().mockResolvedValue({}) }, }));