Skip to content

[PM-31781] skip unpaid automations for exempt orgs#7480

Open
kdenney wants to merge 9 commits intomainfrom
billing/PM-31781/exempt-org-unpaid-automations
Open

[PM-31781] skip unpaid automations for exempt orgs#7480
kdenney wants to merge 9 commits intomainfrom
billing/PM-31781/exempt-org-unpaid-automations

Conversation

@kdenney
Copy link
Copy Markdown
Contributor

@kdenney kdenney commented Apr 15, 2026

🎟️ Tracking

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

📔 Objective

Added exemption logic to SubscriptionUpdatedHandler that skips unpaid billing automations (disable + cancellation) for organizations with ExemptFromBillingAutomation = true. The flag is cleared after use so the exemption is single-use.

Changes

SubscriptionUpdatedHandler.cs

  • Separated SubscriptionWentUnpaid and SubscriptionWentIncompleteExpired into distinct branches. The exemption only applies to unpaid, not incomplete_expired.
  • Added SkipUnpaidBillingAutomationsForExemptOrganizationAsync: checks Enabled && ExemptFromBillingAutomation, clears the flag, and logs. Returns false for user/provider subscribers.
  • Note: The acceptance criteria mentions subscription_create but it is impossible for a subscription to enter unpaid status with that billing reason, so I removed that check altogether from SubscriptionWentUnpaid.
  • Note: The acceptance criteria also mentions subscription_update and automatic_pending_invoice_item_invoice billing reasons, but no code was needed for these because they have always been excluded from the SubscriptionWentUnpaid check, so they never get processed as unpaid, and thus the acceptance criteria is already satisfied. (do not delete and do not remove the exempt flag)

StripeConstants.cs

  • Added AutomaticPendingInvoiceItemInvoice and SubscriptionUpdate billing reason constants.

InvoiceCreatedHandler.cs, PaymentFailedHandler.cs, StripeEventUtilityService.cs

  • Replaced hardcoded billing reason strings with StripeConstants.BillingReasons constants. No behavioral change.

SubscriptionUpdatedHandlerTests.cs

  • Exempt org + subscription_cycle → skip disable, clear flag
  • Exempt org + subscription_update → no action, flag preserved
  • Exempt org + automatic_pending_invoice_item_invoice → no action, flag preserved

@kdenney kdenney added the ai-review Request a Claude code review label Apr 15, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 15, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds exemption logic to SubscriptionUpdatedHandler so organizations with ExemptFromBillingAutomation = true are skipped from disable + cancellation when their subscription goes unpaid, then clears the flag for single-use semantics. The previously flagged critical issue — clearing the exemption before the rest of HandleAsync finished, which could let a Stripe webhook retry consume the exemption and cancel the protected org — has been resolved by deferring ClearBillingAutomationExemptionAsync to the end of the organization branch, and a regression test (HandleAsync_UnpaidOrganizationSubscription_WithExemptOrganization_WhenSubsequentWorkFails_DoesNotClearExemption) pins this behavior. The latest commit also adds an early-exit guard when the subscriber cannot be loaded from the DB and refactors the Match(...) lambdas into typed pattern-matched dispatch over ISubscriber, removing the per-branch null checks that were sprinkled through the previous code.

Code Review Details

No findings.

Behavior change worth noting (intentional and covered by tests): when the subscriber referenced by the Stripe event cannot be found in the local DB, the handler now exits early and SetSubscriptionToCancelAsync does not run. Previously, for a not-found provider the cancellation was still scheduled in Stripe (HandleAsync_UnpaidProviderSubscription_WhenProviderNotFound_StillSetsCancellation); the renamed HandleAsync_WhenProviderNotFound_SkipsHandler reflects the new contract. Worth confirming with the billing/AppSec stakeholders that orphaned-Stripe-subscription scenarios are an acceptable trade-off for the cleaner guard.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 15, 2026

Logo
Checkmarx One – Scan Summary & Details149a4b6f-69e9-4c00-962c-4343fa703ee0

Great job! No new security vulnerabilities introduced in this pull request

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 91.17647% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.61%. Comparing base (e8c109a) to head (5273d6b).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...ices/Implementations/SubscriptionUpdatedHandler.cs 94.89% 3 Missing and 2 partials ⚠️
.../Services/Implementations/InvoiceCreatedHandler.cs 0.00% 2 Missing ⚠️
...g/Services/Implementations/PaymentFailedHandler.cs 0.00% 1 Missing ⚠️
...vices/Implementations/StripeEventUtilityService.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7480      +/-   ##
==========================================
+ Coverage   59.50%   59.61%   +0.10%     
==========================================
  Files        2089     2096       +7     
  Lines       92418    92584     +166     
  Branches     8214     8250      +36     
==========================================
+ Hits        54994    55191     +197     
+ Misses      35481    35439      -42     
- Partials     1943     1954      +11     

☔ 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.

@kdenney kdenney marked this pull request as ready for review April 16, 2026 15:54
@kdenney kdenney requested a review from a team as a code owner April 16, 2026 15:54
Copy link
Copy Markdown
Contributor

@amorask-bitwarden amorask-bitwarden left a comment

Choose a reason for hiding this comment

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

Looks great - nice work and thanks for updating those magic strings.

@bw-ghapp bw-ghapp Bot temporarily deployed to EU-QA Cloud April 23, 2026 12:50 Inactive
@bw-ghapp bw-ghapp Bot deployed to EU-QA Cloud April 23, 2026 13:20 Active
@kdenney kdenney marked this pull request as draft April 24, 2026 20:32
@kdenney kdenney marked this pull request as ready for review April 24, 2026 20:33
@kdenney kdenney closed this Apr 24, 2026
@kdenney kdenney reopened this Apr 24, 2026
Comment thread src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

@kdenney kdenney requested a review from amorask-bitwarden May 1, 2026 22:55
@kdenney
Copy link
Copy Markdown
Contributor Author

kdenney commented May 1, 2026

@amorask-bitwarden Added the db existence guard we discussed and also refactored to not have duplicate db lookups in the latest push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants