From 8b3f29a86f47d066c56b004ed667a6356142ae22 Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Mon, 26 Jan 2026 10:12:28 +0100 Subject: [PATCH] Resolve the pr comment on missed requirements --- .../SubscriptionUpdatedHandler.cs | 6 --- .../Commands/UpdatePaymentMethodCommand.cs | 21 ++++++++ ...tePremiumCloudHostedSubscriptionCommand.cs | 45 +++++++++-------- .../SubscriptionUpdatedHandlerTests.cs | 48 ++++--------------- .../UpdatePaymentMethodCommandTests.cs | 2 + 5 files changed, 59 insertions(+), 63 deletions(-) diff --git a/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs b/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs index 700baaa74a..f7178c8184 100644 --- a/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs +++ b/src/Billing/Services/Implementations/SubscriptionUpdatedHandler.cs @@ -109,12 +109,6 @@ public class SubscriptionUpdatedHandler : ISubscriptionUpdatedHandler break; } - if (await IsPremiumSubscriptionAsync(subscription)) - { - await CancelSubscription(subscription.Id); - await VoidOpenInvoices(subscription.Id); - } - await _userService.DisablePremiumAsync(userId.Value, currentPeriodEnd); break; diff --git a/src/Core/Billing/Payment/Commands/UpdatePaymentMethodCommand.cs b/src/Core/Billing/Payment/Commands/UpdatePaymentMethodCommand.cs index a5a9e3e9c9..53688cd743 100644 --- a/src/Core/Billing/Payment/Commands/UpdatePaymentMethodCommand.cs +++ b/src/Core/Billing/Payment/Commands/UpdatePaymentMethodCommand.cs @@ -3,7 +3,9 @@ using Bit.Core.Billing.Commands; using Bit.Core.Billing.Constants; using Bit.Core.Billing.Payment.Models; using Bit.Core.Billing.Services; +using Bit.Core.Billing.Subscriptions.Models; using Bit.Core.Entities; +using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Utilities; using Braintree; @@ -23,6 +25,7 @@ public interface IUpdatePaymentMethodCommand public class UpdatePaymentMethodCommand( IBraintreeGateway braintreeGateway, + IBraintreeService braintreeService, IGlobalSettings globalSettings, ILogger logger, ISetupIntentCache setupIntentCache, @@ -141,6 +144,24 @@ public class UpdatePaymentMethodCommand( await stripeAdapter.UpdateCustomerAsync(customer.Id, new CustomerUpdateOptions { Metadata = metadata }); } + // If the subscriber has an incomplete subscription, pay the invoice with the new PayPal payment method + if (!string.IsNullOrEmpty(subscriber.GatewaySubscriptionId)) + { + var subscription = await stripeAdapter.GetSubscriptionAsync(subscriber.GatewaySubscriptionId); + + if (subscription.Status == StripeConstants.SubscriptionStatus.Incomplete) + { + var invoice = await stripeAdapter.UpdateInvoiceAsync(subscription.LatestInvoiceId, + new InvoiceUpdateOptions + { + AutoAdvance = false, + Expand = ["customer"] + }); + + await braintreeService.PayInvoice(new UserId(subscriber.Id), invoice); + } + } + var payPalAccount = braintreeCustomer.DefaultPaymentMethod as PayPalAccount; return MaskedPaymentMethod.From(payPalAccount!); diff --git a/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs b/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs index e785bb22d3..7dc9067635 100644 --- a/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs +++ b/src/Core/Billing/Premium/Commands/CreatePremiumCloudHostedSubscriptionCommand.cs @@ -76,24 +76,7 @@ public class CreatePremiumCloudHostedSubscriptionCommand( // These are: 'canceled' (user canceled) and 'incomplete_expired' (payment failed and time expired). // We allow users with terminal subscriptions to create a new subscription even if user.Premium is still true, // enabling the resubscribe workflow without requiring Premium status to be cleared first. - var hasTerminalSubscription = false; - if (!string.IsNullOrEmpty(user.GatewaySubscriptionId)) - { - try - { - var existingSubscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId); - hasTerminalSubscription = existingSubscription.Status is - SubscriptionStatus.Canceled or - SubscriptionStatus.IncompleteExpired; - } - catch (Exception ex) - { - // Subscription doesn't exist in Stripe or can't be fetched (e.g., network issues, invalid ID) - // Log the issue but proceed with subscription creation to avoid blocking legitimate resubscribe attempts - _logger.LogWarning(ex, "Unable to fetch existing subscription {SubscriptionId} for user {UserId}. Proceeding with subscription creation", - user.GatewaySubscriptionId, user.Id); - } - } + var hasTerminalSubscription = await HasTerminalSubscriptionAsync(user); if (user.Premium && !hasTerminalSubscription) { @@ -158,7 +141,7 @@ public class CreatePremiumCloudHostedSubscriptionCommand( }, _ => { - if (subscription.Status is not (SubscriptionStatus.Active or SubscriptionStatus.Incomplete)) + if (subscription.Status != SubscriptionStatus.Active) { return; } @@ -395,4 +378,28 @@ public class CreatePremiumCloudHostedSubscriptionCommand( return subscription; } + + private async Task HasTerminalSubscriptionAsync(User user) + { + if (string.IsNullOrEmpty(user.GatewaySubscriptionId)) + { + return false; + } + + try + { + var existingSubscription = await stripeAdapter.GetSubscriptionAsync(user.GatewaySubscriptionId); + return existingSubscription.Status is + SubscriptionStatus.Canceled or + SubscriptionStatus.IncompleteExpired; + } + catch (Exception ex) + { + // Subscription doesn't exist in Stripe or can't be fetched (e.g., network issues, invalid ID) + // Log the issue but proceed with subscription creation to avoid blocking legitimate resubscribe attempts + _logger.LogWarning(ex, "Unable to fetch existing subscription {SubscriptionId} for user {UserId}. Proceeding with subscription creation", + user.GatewaySubscriptionId, user.Id); + return false; + } + } } diff --git a/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs b/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs index a9b9cd8a27..ac24317bad 100644 --- a/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs +++ b/test/Billing.Test/Services/SubscriptionUpdatedHandlerTests.cs @@ -377,7 +377,7 @@ public class SubscriptionUpdatedHandlerTests } [Fact] - public async Task HandleAsync_UnpaidUserSubscription_DisablesPremiumAndCancelsSubscription() + public async Task HandleAsync_UnpaidUserSubscription_DisablesPremium() { // Arrange var userId = Guid.NewGuid(); @@ -403,40 +403,26 @@ public class SubscriptionUpdatedHandlerTests var parsedEvent = new Event { Data = new EventData() }; - var premiumPlan = new PremiumPlan - { - Name = "Premium", - Available = true, - LegacyYear = null, - Seat = new PremiumPurchasable { Price = 10M, StripePriceId = IStripeEventUtilityService.PremiumPlanId }, - Storage = new PremiumPurchasable { Price = 4M, StripePriceId = "storage-plan-personal" } - }; - _pricingClient.ListPremiumPlans().Returns(new List { premiumPlan }); - _stripeEventService.GetSubscription(Arg.Any(), Arg.Any(), Arg.Any>()) .Returns(subscription); _stripeEventUtilityService.GetIdsFromMetadata(Arg.Any>()) .Returns(Tuple.Create(null, userId, null)); - _stripeFacade.ListInvoices(Arg.Any()) - .Returns(new StripeList { Data = new List() }); - // Act await _sut.HandleAsync(parsedEvent); // Assert await _userService.Received(1) .DisablePremiumAsync(userId, currentPeriodEnd); - await _stripeFacade.Received(1) - .CancelSubscription(subscriptionId, Arg.Any()); - await _stripeFacade.Received(1) - .ListInvoices(Arg.Is(o => - o.Status == StripeInvoiceStatus.Open && o.Subscription == subscriptionId)); + await _stripeFacade.DidNotReceive() + .CancelSubscription(Arg.Any(), Arg.Any()); + await _stripeFacade.DidNotReceive() + .ListInvoices(Arg.Any()); } [Fact] - public async Task HandleAsync_IncompleteExpiredUserSubscription_DisablesPremiumAndCancelsSubscription() + public async Task HandleAsync_IncompleteExpiredUserSubscription_DisablesPremium() { // Arrange var userId = Guid.NewGuid(); @@ -462,36 +448,22 @@ public class SubscriptionUpdatedHandlerTests var parsedEvent = new Event { Data = new EventData() }; - var premiumPlan = new PremiumPlan - { - Name = "Premium", - Available = true, - LegacyYear = null, - Seat = new PremiumPurchasable { Price = 10M, StripePriceId = IStripeEventUtilityService.PremiumPlanId }, - Storage = new PremiumPurchasable { Price = 4M, StripePriceId = "storage-plan-personal" } - }; - _pricingClient.ListPremiumPlans().Returns(new List { premiumPlan }); - _stripeEventService.GetSubscription(Arg.Any(), Arg.Any(), Arg.Any>()) .Returns(subscription); _stripeEventUtilityService.GetIdsFromMetadata(Arg.Any>()) .Returns(Tuple.Create(null, userId, null)); - _stripeFacade.ListInvoices(Arg.Any()) - .Returns(new StripeList { Data = new List() }); - // Act await _sut.HandleAsync(parsedEvent); // Assert await _userService.Received(1) .DisablePremiumAsync(userId, currentPeriodEnd); - await _stripeFacade.Received(1) - .CancelSubscription(subscriptionId, Arg.Any()); - await _stripeFacade.Received(1) - .ListInvoices(Arg.Is(o => - o.Status == StripeInvoiceStatus.Open && o.Subscription == subscriptionId)); + await _stripeFacade.DidNotReceive() + .CancelSubscription(Arg.Any(), Arg.Any()); + await _stripeFacade.DidNotReceive() + .ListInvoices(Arg.Any()); } [Fact] diff --git a/test/Core.Test/Billing/Payment/Commands/UpdatePaymentMethodCommandTests.cs b/test/Core.Test/Billing/Payment/Commands/UpdatePaymentMethodCommandTests.cs index da42127f33..a329eb53cb 100644 --- a/test/Core.Test/Billing/Payment/Commands/UpdatePaymentMethodCommandTests.cs +++ b/test/Core.Test/Billing/Payment/Commands/UpdatePaymentMethodCommandTests.cs @@ -4,6 +4,7 @@ using Bit.Core.Billing.Constants; using Bit.Core.Billing.Payment.Commands; using Bit.Core.Billing.Payment.Models; using Bit.Core.Billing.Services; +using Bit.Core.Services; using Bit.Core.Settings; using Bit.Core.Test.Billing.Extensions; using Braintree; @@ -32,6 +33,7 @@ public class UpdatePaymentMethodCommandTests { _command = new UpdatePaymentMethodCommand( _braintreeGateway, + Substitute.For(), _globalSettings, Substitute.For>(), _setupIntentCache,