[PM-36062] refactor: Remove defer-price-migration-to-renewal feature flag#7883
[PM-36062] refactor: Remove defer-price-migration-to-renewal feature flag#7883amorask-bitwarden wants to merge 8 commits into
Conversation
…flag Removes the PM32645_DeferPriceMigrationToRenewal flag and its now-dead immediate-migration (OFF-branch) logic, making the deferred schedule-based price migration the unconditional behavior. Collapses the three PM32645-or-PM35215 gates to PM35215 only (still in flight) and deletes the orphaned legacy cancel/reinstate methods left behind.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the removal of the Code Review DetailsNo findings. Verification notes:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7883 +/- ##
==========================================
- Coverage 61.21% 61.13% -0.08%
==========================================
Files 2219 2219
Lines 98059 97864 -195
Branches 8848 8817 -31
==========================================
- Hits 60024 59829 -195
- Misses 35918 35921 +3
+ Partials 2117 2114 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Removing PM32645 had collapsed the PM32645-or-PM35215 schedule gates to PM35215-only, but PM35215 isn't enabled in production, which would drop the schedule release/recreate that PM32645-on currently provides for personal subscribers. Gate on schedule/metadata presence instead (matching PriceIncreaseScheduler.Release, which never checked a flag) and drop the now-unused IFeatureService from both types.
kdenney
left a comment
kdenney
left a comment
There was a problem hiding this comment.
Looks good. I have one non-blocking request but that's it.
|
|
||
| await organizationRepository.ReplaceAsync(organization); | ||
| await stripeAdapter.UpdateSubscriptionAsync(subscription.Id, options); | ||
| return true; |
There was a problem hiding this comment.
Would you mind adding a comment here (and the other locations where you are early-returning true) to explain why we want to just return true here if !scheduled? I know you only removed the flag in this PR but I assume you know why and I realized while reading through this that I don't lol so a comment would be helpful. 😄
There was a problem hiding this comment.
Thanks! Looks like you have broken integration tests too:
/home/runner/work/server/server/test/SeederApi.IntegrationTest/Pipeline/RecipeOrchestratorIntegrationTests.cs(123,24): error CS7036: There is no argument given that corresponds to the required parameter 'LicensingService' of 'SeederDependencies.SeederDependencies(DatabaseContext, IMapper, IPasswordHasher<User>, IManglerService, ILicensingService)' [/home/runner/work/server/server/test/SeederApi.IntegrationTest/SeederApi.IntegrationTest.csproj]
There was a problem hiding this comment.
Which is weird because you didn't edit those files lol. Maybe it just needs reran..I'll try that.
edit: nope, both unit and integration tests are failing. Might be a bad merge got into main.
|
3 participants
🎟️ Tracking
Resolves https://bitwarden.atlassian.net/browse/PM-36062
📔 Objective
Removes the
pm-32645-defer-price-migration-to-renewalfeature flag (FeatureFlagKeys.PM32645_DeferPriceMigrationToRenewal), which is fully rolled out, along with its legacy flag-OFF code:PriceIncreaseScheduler(Stripe subscription schedules). The inline immediate-migration branches inUpcomingInvoiceHandlerand the schedule-aware automatic-tax handling (UpcomingInvoiceHandler,UpdateBillingAddressCommand) are no longer flag-gated.SubscriberServicecancel paths,ReinstateSubscriptionCommand), making them unconditional. These were gatedPM32645 || PM35215; because PM32645 was at 100% the blocks already ran for any subscriber holding an active schedule, andPM35215_BusinessPlanPriceMigrationis not enabled in production. Collapsing the gate toPM35215-only would therefore have dropped schedule release/recreate for personal (Premium/Families) subscribers, so the schedule's presence — not a flag — is the gate now, matchingPriceIncreaseScheduler.Release. PM35215's own logic (business-plan schedule creation, etc.) is untouched, and the now-unusedIFeatureServiceis removed from both types.UserService.CancelPremiumAsync,IOrganizationService.ReinstateSubscriptionAsync(Guid), andIStripePaymentService.CancelSubscriptionAsync(ISubscriber, bool)/ReinstateSubscriptionAsync(ISubscriber).No production behavior change — PM32645 was at 100% rollout, so the now-unconditional paths match what production already runs. No API, DTO, or database changes. Tests updated to drop the removed flag-OFF coverage and to re-gate the schedule tests on schedule/metadata presence rather than the flag.