Skip to content

[PM-36613] Void open invoices for unpaid subscriptions#7589

Open
amorask-bitwarden wants to merge 2 commits intomainfrom
billing/PM-36613/void-canceled-subscription-invoices
Open

[PM-36613] Void open invoices for unpaid subscriptions#7589
amorask-bitwarden wants to merge 2 commits intomainfrom
billing/PM-36613/void-canceled-subscription-invoices

Conversation

@amorask-bitwarden
Copy link
Copy Markdown
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-36613

📔 Objective

Restores the void-open-invoices behavior on canceled Stripe subscriptions that was lost in #6918 (PM-31140). Since v2026.2.1 shipped on 2026-03-03, invoices on subscriptions whose payment fails and lapse to unpaid have been stuck in open status indefinitely — Stripe does not auto-void them when the subscription cancels.

The fix adds VoidOpenInvoicesAsync and TryVoidInvoiceAsync private helpers to SubscriptionUpdatedHandler and calls the helper from the existing SubscriptionWentUnpaid || SubscriptionWentIncompleteExpired branch, after DisableSubscriberAsync and SetSubscriptionToCancelAsync. The orphaned SubscriptionCancellationJob (dead code since PM-31140 — nothing scheduled it anymore), its test class, and its DI registration are deleted in the same commit.

Voiding scope is intentionally narrow. Only cancellations driven through the platform-managed unpaid lifecycle void their open invoices. Cancellations that arrive through other paths — voluntary user cancel, off-platform negotiated cancel, provider migration — leave open invoices intact. Per ops: those invoices are intentional artifacts that need to be preserved for manual reconciliation through other channels. Voiding indiscriminately on customer.subscription.deleted (the first iteration of this PR) would have erased that surface.

The cleanup is best-effort:

  • A failure inside ListInvoicesAsync logs at Error and returns. The disable + cancel_at calls that already ran are not rolled back.
  • A per-invoice VoidInvoiceAsync failure is logged and the loop continues to the next invoice. StripeException (the expected webhook re-delivery race against an already-voided invoice) logs at Warning; any other exception (transport-level failures like HttpRequestException or TaskCanceledException) logs at Error.

Pagination is handled by IStripeAdapter.ListInvoicesAsync with SelectAll = true (auto-paginating via Stripe SDK's ListAutoPagingAsync) — no manual cursor management on our side.

Out of scope: Backfill of invoices that went open between v2026.2.1 deploy and this fix's deploy. Those will not be retroactively voided. Ops decided against a polling background job for backfill; finance/ops can resolve historical leaked invoices manually if/when surfaced.

Test coverage

SubscriptionUpdatedHandlerTests adds:

  • Happy path: list returns multiple invoices → all get voided.
  • Per-invoice StripeException (webhook re-delivery against already-voided invoice) → loop continues.
  • Per-invoice non-Stripe transport exception (HttpRequestException) → loop continues.
  • ListInvoicesAsync failure → does not block subscriber-disable.
  • Non-unpaid transition (e.g., Active → Active quantity change) → no list, no void calls. Locks in the narrow-scoping decision.

One existing test (HandleAsync_UnpaidUserSubscription_DisablesPremiumAndSetsCancellation) had its DidNotReceive(ListInvoicesAsync) assertion flipped to Received(1) with the expected options — that assertion was locking in the post-PM-31140 absence we are now reversing.

A default _stripeAdapter.ListInvoicesAsync(...).Returns(empty list) mock was added to the test constructor so existing Unpaid-path tests don't NRE through the new void loop.

📸 Screenshots

N/A — no UI changes.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 6, 2026

@amorask-bitwarden amorask-bitwarden marked this pull request as ready for review May 6, 2026 14:13
@amorask-bitwarden amorask-bitwarden requested a review from a team as a code owner May 6, 2026 14:13
@amorask-bitwarden amorask-bitwarden requested a review from kdenney May 6, 2026 14:13
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.75%. Comparing base (869cd3a) to head (3618923).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7589   +/-   ##
=======================================
  Coverage   59.75%   59.75%           
=======================================
  Files        2102     2101    -1     
  Lines       92722    92716    -6     
  Branches     8261     8256    -5     
=======================================
- Hits        55409    55406    -3     
+ Misses      35355    35353    -2     
+ Partials     1958     1957    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@kdenney kdenney left a comment

Choose a reason for hiding this comment

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

This will conflict with my changes in #7480, but it should be simple to resolve and the logic looks good to me.

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