[PM-41064] fix: Preserve discounts across subscription schedule rebuilds

src/Billing/Services/Implementations/UpcomingInvoiceHandler.cs

@@ -535,26 +535,20 @@ private async Task EnableAutomaticTaxAsync(Subscription subscription)
         if (activeSchedule != null)
         {
             var now = subscription.TestClock?.FrozenTime ?? DateTime.UtcNow;
+
+            DiscountExtensions.RequireScheduleDiscountExpansions(subscription, logger);
+
             var phases = new List<SubscriptionSchedulePhaseOptions>();
 
-            for (var i = 0; i < activeSchedule.Phases.Count; i++)
+            foreach (var phase in activeSchedule.Phases)
             {
-                var phase = activeSchedule.Phases[i];
-
                 // Skip phases that have already completed
                 if (phase.EndDate <= now)
                 {
                     continue;
                 }
 
-                // When a phase's predecessor has ended, the phase is already active and
-                // its one-time migration discount has been applied and consumed.
-                // Re-including it would cause Stripe to re-apply it.
-                var discountConsumed = i > 0 && activeSchedule.Phases[i - 1].EndDate <= now;
-
-                // Gate on StartDate > now, not !discountConsumed (false for the active phase 0),
-                // so we never re-stack the customer coupon onto the already-billing current period.
-                var customerDiscount = phase.StartDate > now ? subscription.Customer?.Discount : null;
+                var isFuture = phase.StartDate > now;
 
                 phases.Add(new SubscriptionSchedulePhaseOptions
                 {
@@ -563,12 +557,14 @@ private async Task EnableAutomaticTaxAsync(Subscription subscription)
                     Items = phase.Items.Select(item => new SubscriptionSchedulePhaseItemOptions
                     {
                         Price = item.PriceId,
-                        Quantity = item.Quantity
+                        Quantity = item.Quantity,
+                        Discounts = DiscountExtensions.BuildPhaseItemLevelDiscounts(
+                            item.Discounts?.Select(d => d.CouponId) ?? [])
                     }).ToList(),
-                    Discounts = discountConsumed
-                        ? []
-                        : customerDiscount.MergeDiscountCouponIds(
-                            phase.Discounts?.Select(d => d.CouponId)).ToPhaseDiscountOptions(),
+                    Discounts = isFuture
+                        ? DiscountExtensions.BuildPhaseLevelDiscounts(
+                            subscription, [], preservedCouponIds: phase.Discounts?.Select(d => d.CouponId))
+                        : DiscountExtensions.BuildCurrentPhaseDiscounts(subscription),
                     Metadata = phase.Metadata,
                     ProrationBehavior = phase.ProrationBehavior,
                     AutomaticTax = new SubscriptionSchedulePhaseAutomaticTaxOptions

src/Core/Billing/Extensions/DiscountExtensions.cs

@@ -1,4 +1,5 @@
-using Stripe;
+using Microsoft.Extensions.Logging;
+using Stripe;
 using static Bit.Core.Billing.Constants.StripeConstants;
 
 namespace Bit.Core.Billing.Extensions;
@@ -16,52 +17,173 @@ coupon is not null &&
         string.Equals(coupon.Duration, CouponDurations.Forever, StringComparison.OrdinalIgnoreCase);
 
     /// <summary>
-    /// Merges customer-level, existing subscription/phase, and newly applied coupon IDs into one
-    /// ordered, de-duplicated list so the new coupon STACKS with pre-existing discounts. Stripe's
-    /// subscription/phase-level discounts override the customer-level one, so the customer coupon
-    /// must be copied into the array explicitly to stack. Order: customer first, then existing, then new.
+    /// Builds phase-level discounts, de-duplicated by coupon: the customer's coupon (by coupon id),
+    /// the subscription's live discounts (by discount id, so a one-time coupon isn't re-granted),
+    /// coupons preserved from a future phase (by coupon id — they live on the phase, not the
+    /// subscription), then new coupons (by coupon id). Returns null when empty; an empty array would
+    /// delete the phase's discounts.
     /// </summary>
-    /// <param name="customerDiscount">Customer-level discount to carry over (any present coupon, regardless of validity), or null. Pass from an expanded customer object.</param>
-    /// <param name="existingDiscountCouponIds">Coupon IDs already on the subscription/phase, in order (materialized — <c>d.Source.Coupon.Id</c> NPEs on unexpanded discounts).</param>
-    /// <param name="newCouponIds">Coupon ID(s) being applied (churn / proactive / milestone).</param>
-    /// <returns>Ordered, de-duplicated coupon IDs.</returns>
-    public static IReadOnlyList<string> MergeDiscountCouponIds(
-        this Discount? customerDiscount,
-        IEnumerable<string?>? existingDiscountCouponIds,
-        params string?[] newCouponIds)
+    /// <param name="subscription">The live subscription whose customer and subscription discounts are carried forward.</param>
+    /// <param name="newCouponIds">Coupon IDs being newly applied to the phase.</param>
+    /// <param name="preservedCouponIds">Coupon IDs preserved from a future phase that has no equivalent on the live subscription.</param>
+    public static List<SubscriptionSchedulePhaseDiscountOptions>? BuildPhaseLevelDiscounts(
+        Subscription subscription,
+        IReadOnlyList<string> newCouponIds,
+        IEnumerable<string?>? preservedCouponIds = null)
     {
-        var ordered = new List<string>();
+        var discounts = new List<SubscriptionSchedulePhaseDiscountOptions>();
         var seen = new HashSet<string>(StringComparer.Ordinal);
 
-        void Add(string? couponId)
+        var customerCouponId = subscription.Customer?.Discount?.Source?.CouponId;
+        if (!string.IsNullOrEmpty(customerCouponId) && seen.Add(customerCouponId))
+        {
+            discounts.Add(new SubscriptionSchedulePhaseDiscountOptions { Coupon = customerCouponId });
+        }
+
+        foreach (var discount in subscription.Discounts ?? [])
+        {
+            var couponId = discount.Source?.CouponId;
+            if (couponId is not null && !seen.Add(couponId))
+            {
+                continue;
+            }
+            discounts.Add(new SubscriptionSchedulePhaseDiscountOptions { Discount = discount.Id });
+        }
+
+        foreach (var couponId in preservedCouponIds ?? [])
+        {
+            if (!string.IsNullOrEmpty(couponId) && seen.Add(couponId))
+            {
+                discounts.Add(new SubscriptionSchedulePhaseDiscountOptions { Coupon = couponId });
+            }
+        }
+
+        foreach (var couponId in newCouponIds)
         {
             if (!string.IsNullOrEmpty(couponId) && seen.Add(couponId))
             {
-                ordered.Add(couponId);
+                discounts.Add(new SubscriptionSchedulePhaseDiscountOptions { Coupon = couponId });
             }
         }
 
-        // Customer coupon first; carried whenever present, regardless of validity.
-        Add(customerDiscount?.Source?.Coupon?.Id);
+        return discounts.Count == 0 ? null : discounts;
+    }
+
+    /// <summary>
+    /// Discounts for a phase that is already active (its start date is in the past): carries only the
+    /// discounts still live on the subscription, by discount id. Unlike <see cref="BuildPhaseLevelDiscounts"/>,
+    /// the customer's coupon is deliberately omitted. When a phase carries explicit discounts Stripe
+    /// suppresses the customer-level coupon, so listing it would newly stack it onto the current period
+    /// (an over-charge in the customer's favor); when the phase carries none, the customer coupon still
+    /// cascades on its own. Either way the active phase's effective discount is unchanged. Returns null
+    /// when empty; an empty array would delete the phase's discounts.
+    /// </summary>
+    /// <param name="subscription">The live subscription whose active-phase discounts are carried forward.</param>
+    public static List<SubscriptionSchedulePhaseDiscountOptions>? BuildCurrentPhaseDiscounts(Subscription subscription) =>
+        subscription.Discounts is { Count: > 0 }
+            ? subscription.Discounts
+                .Select(discount => new SubscriptionSchedulePhaseDiscountOptions { Discount = discount.Id })
+                .ToList()
+            : null;
+
+    /// <summary>
+    /// Subscription-scope equivalent of <see cref="BuildPhaseLevelDiscounts"/>: live discounts by
+    /// discount id, customer and new coupons by coupon id. Returns null when empty.
+    /// </summary>
+    /// <param name="subscription">The live subscription whose customer and subscription discounts are carried forward.</param>
+    /// <param name="newCouponIds">Coupon IDs being newly applied to the subscription.</param>
+    public static List<SubscriptionDiscountOptions>? BuildSubscriptionLevelDiscounts(
+        Subscription subscription,
+        IReadOnlyList<string> newCouponIds)
+    {
+        var discounts = new List<SubscriptionDiscountOptions>();
+        var seen = new HashSet<string>(StringComparer.Ordinal);
 
-        foreach (var id in existingDiscountCouponIds ?? [])
+        var customerCouponId = subscription.Customer?.Discount?.Source?.CouponId;
+        if (!string.IsNullOrEmpty(customerCouponId) && seen.Add(customerCouponId))
         {
-            Add(id);
+            discounts.Add(new SubscriptionDiscountOptions { Coupon = customerCouponId });
         }
 
-        foreach (var id in newCouponIds)
+        foreach (var discount in subscription.Discounts ?? [])
         {
-            Add(id);
+            var couponId = discount.Source?.CouponId;
+            if (couponId is not null && !seen.Add(couponId))
+            {
+                continue;
+            }
+            discounts.Add(new SubscriptionDiscountOptions { Discount = discount.Id });
         }
 
-        return ordered;
+        foreach (var couponId in newCouponIds)
+        {
+            if (!string.IsNullOrEmpty(couponId) && seen.Add(couponId))
+            {
+                discounts.Add(new SubscriptionDiscountOptions { Coupon = couponId });
+            }
+        }
+
+        return discounts.Count == 0 ? null : discounts;
     }
 
-    public static List<SubscriptionDiscountOptions> ToSubscriptionDiscountOptions(
-        this IReadOnlyList<string> couponIds) =>
-        [.. couponIds.Select(id => new SubscriptionDiscountOptions { Coupon = id })];
+    /// <summary>
+    /// Builds item-level discounts from coupon ids only — Stripe rejects a discount id on a phase item.
+    /// De-duplicates, skips empty ids, returns null when empty.
+    /// </summary>
+    /// <param name="couponIds">Coupon IDs to apply to the phase item.</param>
+    public static List<SubscriptionSchedulePhaseItemDiscountOptions>? BuildPhaseItemLevelDiscounts(
+        IEnumerable<string?> couponIds)
+    {
+        var discounts = new List<SubscriptionSchedulePhaseItemDiscountOptions>();
+        var seen = new HashSet<string>(StringComparer.Ordinal);
+
+        foreach (var couponId in couponIds)
+        {
+            if (!string.IsNullOrEmpty(couponId) && seen.Add(couponId))
+            {
+                discounts.Add(new SubscriptionSchedulePhaseItemDiscountOptions { Coupon = couponId });
+            }
+        }
 
-    public static List<SubscriptionSchedulePhaseDiscountOptions> ToPhaseDiscountOptions(
-        this IReadOnlyList<string> couponIds) =>
-        [.. couponIds.Select(id => new SubscriptionSchedulePhaseDiscountOptions { Coupon = id })];
+        return discounts.Count == 0 ? null : discounts;
+    }
+
+    /// <summary>
+    /// Throws when <paramref name="subscription"/> is missing an expansion the discount builders rely on:
+    /// <c>discounts</c> present only as unexpanded id stubs, a missing <c>customer</c> (either would silently
+    /// drop discounts), or a <c>test_clock</c> left unexpanded when the subscription is on one (its absence
+    /// resolves the current phase against the wrong time, which flips the current-vs-future decision that
+    /// drives discount carry-over). Logs before throwing.
+    /// </summary>
+    /// <param name="subscription">The subscription to check for missing expansions.</param>
+    /// <param name="logger">Logger used to record the failure before throwing.</param>
+    public static void RequireScheduleDiscountExpansions(Subscription subscription, ILogger logger)
+    {
+        if (subscription.Discounts is { Count: > 0 } && subscription.Discounts.Any(discount => discount is null))
+        {
+            logger.LogError(
+                "Subscription {SubscriptionId} was loaded without expanding \"discounts\"; existing discounts would be silently dropped",
+                subscription.Id);
+            throw new InvalidOperationException(
+                $"Subscription {subscription.Id} was loaded without expanding \"discounts\". Expand \"discounts.source.coupon\" first.");
+        }
+
+        if (subscription.Customer is null)
+        {
+            logger.LogError(
+                "Subscription {SubscriptionId} was loaded without expanding \"customer\"; a customer-level coupon would be silently dropped",
+                subscription.Id);
+            throw new InvalidOperationException(
+                $"Subscription {subscription.Id} was loaded without expanding \"customer\". Expand \"customer.discount.source.coupon\" first.");
+        }
+
+        if (subscription.TestClockId is not null && subscription.TestClock is null)
+        {
+            logger.LogError(
+                "Subscription {SubscriptionId} is on test clock {TestClockId}, which was not expanded",
+                subscription.Id, subscription.TestClockId);
+            throw new InvalidOperationException(
+                $"Subscription {subscription.Id} is on a test clock that was not expanded. Expand \"test_clock\" first.");
+        }
+    }
 }

src/Core/Billing/Organizations/AnnualUpgradeOffer/AnnualUpgradeLineMapper.cs

@@ -97,7 +97,11 @@ internal static class AnnualUpgradeLineMapper
     private static bool IsUnusable(Discount? discount) =>
         discount is null || string.IsNullOrEmpty(discount.Source?.CouponId);
 
-    // A discount with no coupon would silently drop a subscription-level coupon, so redemption refuses instead.
+    // A discount with no coupon id would only come from a promotion-code source; Bitwarden applies discounts
+    // exclusively via bare coupons (no promotion codes), so every real discount has one and this never rejects
+    // a valid subscriber. It exists to refuse rather than silently drop a discount that couldn't be carried.
+    // If promotion-code discounts are ever introduced, revisit this: a promo code's coupon lives on
+    // discount.Coupon, not necessarily Source.CouponId.
     private static bool HasUnusableDiscounts(Subscription subscription) =>
         (subscription.Discounts ?? []).Any(IsUnusable) ||
         subscription.Items.Data.Any(item => (item.Discounts ?? []).Any(IsUnusable));

src/Core/Billing/Organizations/AnnualUpgradeOffer/Commands/RedeemAnnualUpgradeOfferCommand.cs

@@ -1,6 +1,7 @@
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.Billing.Commands;
 using Bit.Core.Billing.Constants;
+using Bit.Core.Billing.Extensions;
 using Bit.Core.Billing.Organizations.Helpers;
 using Bit.Core.Billing.Organizations.PlanMigration.Queries;
 using Bit.Core.Billing.Organizations.Schedules;
@@ -75,7 +76,8 @@ public Task<BillingCommandResult<None>> Run(Organization organization) => Handle
             {
                 Price = line.TargetPriceId,
                 Quantity = line.Item.Quantity,
-                Discounts = ItemDiscounts(line.Item)
+                Discounts = DiscountExtensions.BuildPhaseItemLevelDiscounts(
+                    line.Item.Discounts?.Select(d => d?.Source?.CouponId) ?? [])
             })
             .ToList();
 
@@ -128,20 +130,12 @@ OrganizationSubscriptionScheduleOwnership.AnnualUpgrade or
             {
                 StartDate = phase1.StartDate,
                 EndDate = phase1.EndDate,
-                Items = [.. phase1.Items.Select(i =>
+                Items = [.. phase1.Items.Select(i => new SubscriptionSchedulePhaseItemOptions
                 {
-                    var itemDiscounts = i.Discounts is { Count: > 0 } ?
-                        i.Discounts.Select(d => new SubscriptionSchedulePhaseItemDiscountOptions
-                        {
-                            Coupon = d.CouponId
-                        }).ToList() : null;
-
-                    return new SubscriptionSchedulePhaseItemOptions
-                    {
-                        Price = i.PriceId,
-                        Quantity = i.Quantity,
-                        Discounts = itemDiscounts
-                    };
+                    Price = i.PriceId,
+                    Quantity = i.Quantity,
+                    Discounts = DiscountExtensions.BuildPhaseItemLevelDiscounts(
+                        i.Discounts?.Select(d => d.CouponId) ?? [])
                 })],
                 Discounts = ReusedPhaseDiscounts(subscription),
                 // Only the marker's presence is read; the value is for triage.
@@ -205,16 +199,4 @@ await stripeAdapter.UpdateSubscriptionScheduleAsync(schedule.Id,
         subscription.Discounts is { Count: > 0 }
             ? [.. subscription.Discounts.Select(discount => new SubscriptionSchedulePhaseDiscountOptions { Discount = discount.Id })]
             : null;
-
-    // An item-bound coupon does not travel with the customer or subscription discounts.
-    // Out-of-scope coupons are accepted and applied as zero, so copying can only help.
-    private static List<SubscriptionSchedulePhaseItemDiscountOptions>? ItemDiscounts(SubscriptionItem item)
-    {
-        var discounts = item.Discounts?
-            .Where(discount => !string.IsNullOrEmpty(discount?.Source?.CouponId))
-            .Select(discount => new SubscriptionSchedulePhaseItemDiscountOptions { Coupon = discount!.Source.CouponId })
-            .ToList();
-
-        return discounts is { Count: > 0 } ? discounts : null;
-    }
 }