Skip to content

[PM-37084] Business Aware Schedule Recovery and Cancellation#7686

Open
sbrown-livefront wants to merge 35 commits into
mainfrom
billing/pm-37084/business-aware-schedule-recovery
Open

[PM-37084] Business Aware Schedule Recovery and Cancellation#7686
sbrown-livefront wants to merge 35 commits into
mainfrom
billing/pm-37084/business-aware-schedule-recovery

Conversation

@sbrown-livefront
Copy link
Copy Markdown
Collaborator

@sbrown-livefront sbrown-livefront commented May 20, 2026

🎟️ Tracking

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

📔 Objective

Introduces refactorings to the billing and subscription scheduling logic, particularly around price increase scheduling and business plan migrations. The changes add a more robust and unified scheduling pathway, improve feature flag handling, and enhance testability and maintainability.

Key changes include:

Unified Price Increase Scheduling

  • Introduced a new ScheduleForSubscription method in IPriceIncreaseScheduler and its implementation, which intelligently dispatches scheduling logic based on subscriber type and organization eligibility. This centralizes and simplifies price increase scheduling for both personal and business plans.

  • Added a new OrganizationPriceIncreaseOptions record to allow fine-grained control over scheduling guards, such as skipping if already scheduled.

  • Refactored business plan price migration scheduling to use the new unified scheduling pathway, reducing code duplication and improving maintainability.

Improved Metadata and Feature Flag Handling

  • Added a new metadata key CancellingUserId to MetadataKeys and updated cancellation logic to consistently use this key, improving traceability of who initiated cancellations.
  • Enhanced feature flag checks to ensure new business plan migration logic is only enabled when the appropriate feature flag is active, and to unify cancellation metadata handling. [

Stripe Integration and Testability

  • Improved retrieval of Stripe subscription objects by expanding additional fields (customer, customer.discount) to ensure all necessary data is available for downstream logic.
  • Added a helper method for waiting on Stripe Test Clock advancement, improving reliability and testability of time-dependent business logic.

Code Cleanup and Refactoring

  • Removed outdated or redundant scheduling calls and replaced them with calls to the new unified scheduler, streamlining the codebase.

📸 Screenshots

Screen.Recording.2026-05-21.at.2.42.23.PM.mov

@sbrown-livefront sbrown-livefront changed the title Billing/pm 37084/business aware schedule recovery [PM-37084] Business Aware Schedule Recovery May 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 65.87302% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.48%. Comparing base (8bd4af7) to head (3babcfe).

Files with missing lines Patch % Lines
src/Core/Billing/Pricing/PriceIncreaseScheduler.cs 63.01% 23 Missing and 4 partials ⚠️
...Services/Implementations/UpcomingInvoiceHandler.cs 17.64% 11 Missing and 3 partials ⚠️
...illing/Pricing/OrganizationPriceIncreaseOptions.cs 50.00% 1 Missing ⚠️
...ling/Services/Implementations/SubscriberService.cs 95.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7686      +/-   ##
==========================================
- Coverage   64.90%   60.48%   -4.42%     
==========================================
  Files        2141     2142       +1     
  Lines       94654    94742      +88     
  Branches     8456     8469      +13     
==========================================
- Hits        61435    57307    -4128     
- Misses      31118    35424    +4306     
+ Partials     2101     2011      -90     

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

@sbrown-livefront sbrown-livefront self-assigned this May 20, 2026
@sbrown-livefront sbrown-livefront added the ai-review Request a Claude code review label May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR unifies price-increase scheduling under a new IPriceIncreaseScheduler.ScheduleForSubscription dispatch that routes by subscriber type (user / organization / provider) and Track A eligibility. Cohort validation moves out of UpcomingInvoiceHandler into the scheduler, callers are updated to use the unified entry point, Stripe expansions (customer.discount) are added where downstream business-plan logic depends on customer-level discounts, and cancellation metadata handling is consolidated under MetadataKeys.CancellingUserId with a feature-flag gate (PM35215_BusinessPlanPriceMigration). Test coverage is comprehensive across the new scheduler, subscriber service, and reinstate command paths.

Code Review Details

No new findings. All previously surfaced concerns (Stripe expansion gaps on the unified dispatch in ReinstateSubscriptionCommand, SubscriptionUpdatedHandler.RemovePendingCancellationAsync, and SubscriberService.ResumeFromUnpaidCancellationAsync; the stale HandleAsync_ActiveSubscription_SchedulesBeforeRemovingCancellation test name) have been addressed in subsequent commits.

Comment thread src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs
Comment thread test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs
Comment thread src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs
@sbrown-livefront sbrown-livefront marked this pull request as ready for review May 21, 2026 18:53
@sbrown-livefront sbrown-livefront requested a review from a team as a code owner May 21, 2026 18:53
@sbrown-livefront sbrown-livefront changed the title [PM-37084] Business Aware Schedule Recovery [PM-37084] Business Aware Schedule Recovery and Cancellation May 21, 2026
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 - just left a question on a refactoring opp we have here. Let me know if you want to discuss further.

var subscription = await stripeAdapter.GetSubscriptionAsync(
subscriber.GatewaySubscriptionId,
new SubscriptionGetOptions { Expand = ["discounts"] });
new SubscriptionGetOptions { Expand = ["discounts", "customer", "customer.discount"] });
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.

⛏️ Expanding customer.discount necessarily expands customer.

public async Task HandleAsync(Event parsedEvent)
{
var subscription = await _stripeEventService.GetSubscription(parsedEvent, true, ["customer", "discounts", "latest_invoice", "test_clock"]);
var subscription = await _stripeEventService.GetSubscription(parsedEvent, true, ["customer", "customer.discount", "discounts", "latest_invoice", "test_clock"]);
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.

⛏ Same thing here.

/// <param name="subscription"> The subscription to schedule a price increase for. </param>
/// <param name="organizationId"> The ID of the organization associated with the subscription. </param>
/// <returns> True if the schedule was dispatched successfully, false otherwise. </returns>
private async Task<bool> DispatchOrganizationScheduleAsync(Subscription subscription, Guid organizationId)
Copy link
Copy Markdown
Contributor

@amorask-bitwarden amorask-bitwarden May 21, 2026

Choose a reason for hiding this comment

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

I think we have a refactoring opportunity here that I overlooked in the original implementation. This is almost the same exact logic that's in the UpcomingInvoiceHandler.ScheduleBusinessPlanPriceMigration. It seems like we're going to need the same logic in multiple areas, but I'm not positive they're supposed to behave exactly the same way between the original scheduling and a reinstatement. For example, some of the validation checks such as assignment.ScheduledAt is not null might not translate. Wanna give this a whirl or discuss further?

Comment thread src/Core/Billing/Services/Implementations/SubscriberService.cs
@sonarqubecloud
Copy link
Copy Markdown

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