Skip to content

fix(subscriptions): read previous price from both webhook API shapes - #21122

Open
david1alvarez wants to merge 1 commit into
mainfrom
PAY-3900
Open

fix(subscriptions): read previous price from both webhook API shapes#21122
david1alvarez wants to merge 1 commit into
mainfrom
PAY-3900

Conversation

@david1alvarez

@david1alvarez david1alvarez commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Because

  • Our Stripe webhook version is out of sync with the SDK, so inbound event
    payloads don't match the shapes our code reads. We need to flip it and be able
    to roll back.
  • Basil removed Subscription.plan, which is the only trigger for upgrade and
    downgrade emails — after the flip those emails would silently stop.
  • Basil also reports the subscription item's billing-period change on every
    renewal, so treating any item change as a price change would email every
    subscriber an upgrade notice each cycle.

This pull request

  • Reads the previous price from previous_attributes.plan (acacia) or the
    subscription item's plan/price (basil), normalizing a price into plan shape once.
  • Treats an item diff as an upgrade only when the price id differs from the current one.
  • Replaces the as any on previous_attributes with type annotations.
  • Tags both Stripe webhook receivers with stripe_api_version.
  • Adds coverage for both payload shapes, the cadence fallback, and the renewal guard.

Issue that this pull request solves

Addresses: PAY-3900

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: the derivation of planOld in
    extractSubscriptionUpdateEventDetailsForEmail.
  • Suggested review order: the stripe.ts block, then the four new cases in stripe.spec.ts.
  • Risky or complex parts: a non-null planOld is the only thing that sends an
    upgrade/downgrade email, and the id comparison is what keeps renewals out of that path.

Screenshots (Optional)

Please attach the screenshots of the changes made in case of change in user interface.

Other information (Optional)

  • This is step 1 of the four on PAY-3900. Flipping the endpoint version, running the
    Firestore sync migration, and removing the dual-format code are still open — hence
    Addresses: rather than Closes:.
  • The ticket's step 1 assumes the webhook format reaches Firestore. It doesn't: the
    mirror is written only from stripe.*.retrieve, so its shape follows the SDK pin and
    the flip can't pollute it. Steps 3 and 4 may be smaller than scoped.

Because:

* The Stripe webhook endpoint's own api_version sets inbound payload
  shape, so the version flip has to be revertible without breaking parsing.
* Basil removed Subscription.plan, which is what drives our upgrade and
  downgrade emails.

This commit:

* Reads the previous price from previous_attributes.plan, items.data[].plan
  or items.data[].price, and only when the price id moved.
* Tags both Stripe webhook receivers with stripe_api_version.

Addresses: PAY-3900
@david1alvarez
david1alvarez marked this pull request as ready for review August 31, 2026 18:54
@david1alvarez
david1alvarez requested a review from a team as a code owner August 31, 2026 18:54
Copilot AI balanced review requested due to automatic review settings August 31, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds backward-compatible Stripe webhook handling during the API-version transition.

Changes:

  • Normalizes previous plan/price webhook shapes.
  • Prevents renewal events from triggering upgrade emails.
  • Adds Stripe API-version telemetry and subscription tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
stripe-webhook.ts Tags legacy webhook telemetry with API version.
stripe.ts Derives previous pricing across Stripe payload shapes.
stripe.spec.ts Tests payload normalization and renewal filtering.
stripe-webhooks.service.ts Tags NestJS webhook telemetry with API version.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +2448 to +2449
interval: planOldDiff.interval ?? planNew.interval,
interval_count: planOldDiff.interval_count ?? planNew.interval_count,
request.payload,
request.headers['stripe-signature']
);
Sentry.setTag('stripe_api_version', event.api_version);
Comment on lines +33 to +36
Sentry.setTag(
'stripe_api_version',
webhookEventResponse.event.api_version
);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants