stripe: Ensure required_plan_tier is set before setting discount.

corporate/lib/stripe.py

@@ -1278,26 +1278,25 @@ class BillingSession(ABC):
     def attach_discount_to_customer(self, new_discount: Decimal) -> str:
         # Remove flat discount if giving customer a percentage discount.
         customer = self.get_customer()
-        old_discount = None
+
-        if customer is not None:
+        # We set required plan tier before setting a discount for the customer, so it's always defined.
-            old_discount = customer.default_discount
+        assert customer is not None
-            customer.default_discount = new_discount
+        assert customer.required_plan_tier is not None
-            customer.flat_discounted_months = 0
+
-            customer.save(update_fields=["default_discount", "flat_discounted_months"])
+        old_discount = customer.default_discount
-        else:
+        customer.default_discount = new_discount
-            customer = self.update_or_create_customer(
+        customer.flat_discounted_months = 0
-                defaults={"default_discount": new_discount, "flat_discounted_months": 0}
+        customer.save(update_fields=["default_discount", "flat_discounted_months"])
-            )
         plan = get_current_plan_by_customer(customer)
-        if plan is not None:
+        if plan is not None and plan.tier == customer.required_plan_tier:
             self.apply_discount_to_plan(plan, new_discount)
 
-            # If the customer has a next plan, apply discount to that plan as well.
+        # If the customer has a next plan, apply discount to that plan as well.
-            # Make this a check on CustomerPlan.SWITCH_PLAN_TIER_AT_PLAN_END status
+        # Make this a check on CustomerPlan.SWITCH_PLAN_TIER_AT_PLAN_END status
-            # if we support this for other plans.
+        # if we support this for other plans.
-            next_plan = self.get_legacy_remote_server_next_plan(customer)
+        next_plan = self.get_legacy_remote_server_next_plan(customer)
-            if next_plan is not None:  # nocoverage
+        if next_plan is not None and next_plan.tier == customer.required_plan_tier:
-                self.apply_discount_to_plan(next_plan, new_discount)
+            self.apply_discount_to_plan(next_plan, new_discount)
 
         self.write_to_audit_log(
             event_type=AuditLogEventType.DISCOUNT_CHANGED,
@@ -1366,10 +1365,15 @@ class BillingSession(ABC):
             raise SupportRequestError(f"Invalid plan tier for {self.billing_entity_display_name}.")
 
         if customer is not None:
+            if new_plan_tier is None and customer.default_discount:
+                raise SupportRequestError(
+                    f"Discount for {self.billing_entity_display_name} must be 0 before setting required plan tier to None."
+                )
             previous_required_plan_tier = customer.required_plan_tier
             customer.required_plan_tier = new_plan_tier
             customer.save(update_fields=["required_plan_tier"])
         else:
+            assert new_plan_tier is not None
             customer = self.update_or_create_customer(
                 defaults={"required_plan_tier": new_plan_tier}
             )

corporate/tests/test_stripe.py

@@ -4531,6 +4531,7 @@ class StripeTest(StripeTestCase):
         billing_session = RealmBillingSession(
             user=self.example_user("iago"), realm=realm, support_session=True
         )
+        billing_session.set_required_plan_tier(CustomerPlan.TIER_CLOUD_STANDARD)
         billing_session.attach_discount_to_customer(Decimal(20))
         rows.append(Row(realm, Realm.PLAN_TYPE_SELF_HOSTED, None, None, 0, False))
 
@@ -6025,6 +6026,16 @@ class TestSupportBillingHelpers(StripeTestCase):
         support_admin = self.example_user("iago")
         user = self.example_user("hamlet")
         billing_session = RealmBillingSession(support_admin, realm=user.realm, support_session=True)
+
+        # Cannot attach discount without a required_plan_tier set.
+        with self.assertRaises(AssertionError):
+            billing_session.attach_discount_to_customer(Decimal(85))
+        billing_session.update_or_create_customer()
+
+        with self.assertRaises(AssertionError):
+            billing_session.attach_discount_to_customer(Decimal(85))
+
+        billing_session.set_required_plan_tier(CustomerPlan.TIER_CLOUD_STANDARD)
         billing_session.attach_discount_to_customer(Decimal(85))
         realm_audit_log = RealmAuditLog.objects.filter(
             event_type=RealmAuditLog.REALM_DISCOUNT_CHANGED
@@ -6055,6 +6066,7 @@ class TestSupportBillingHelpers(StripeTestCase):
         plan.status = CustomerPlan.ENDED
         plan.save(update_fields=["status"])
         billing_session = RealmBillingSession(support_admin, realm=user.realm, support_session=True)
+        billing_session.set_required_plan_tier(CustomerPlan.TIER_CLOUD_STANDARD)
         billing_session.attach_discount_to_customer(Decimal(25))
         with time_machine.travel(self.now, tick=False):
             self.add_card_and_upgrade(
@@ -6122,6 +6134,7 @@ class TestSupportBillingHelpers(StripeTestCase):
         ):
             billing_session.process_support_view_request(support_view_request)
 
+        billing_session.set_required_plan_tier(CustomerPlan.TIER_CLOUD_STANDARD)
         billing_session.attach_discount_to_customer(Decimal(50))
         message = billing_session.process_support_view_request(support_view_request)
         self.assertEqual("Minimum licenses for zulip changed to 25 from 0.", message)
@@ -6197,10 +6210,17 @@ class TestSupportBillingHelpers(StripeTestCase):
         with self.assertRaisesRegex(SupportRequestError, "Invalid plan tier for zulip."):
             billing_session.process_support_view_request(support_view_request)
 
-        # Set plan tier to None and check that discount is applied to all plan tiers
+        # Cannot set required plan tier to None before setting discount to 0.
         support_view_request = SupportViewRequest(
             support_type=SupportType.update_required_plan_tier, required_plan_tier=0
         )
+        with self.assertRaisesRegex(
+            SupportRequestError,
+            "Discount for zulip must be 0 before setting required plan tier to None.",
+        ):
+            billing_session.process_support_view_request(support_view_request)
+
+        billing_session.attach_discount_to_customer(Decimal(0))
         message = billing_session.process_support_view_request(support_view_request)
         self.assertEqual("Required plan tier for zulip set to None.", message)
         customer.refresh_from_db()

corporate/tests/test_support_views.py

@@ -523,7 +523,39 @@ class TestRemoteServerSupportEndpoint(ZulipTestCase):
         iago = self.example_user("iago")
         self.login_user(iago)
 
+        # A required plan tier for a customer can be set when an upgrade
+        # is scheduled which is different than the one customer is upgrading to,
+        # but won't change either pre-existing plan tiers.
+        self.assertIsNone(customer.required_plan_tier)
+        result = self.client_post(
+            "/activity/remote/support",
+            {
+                "remote_realm_id": f"{remote_realm.id}",
+                "required_plan_tier": f"{CustomerPlan.TIER_SELF_HOSTED_BUSINESS}",
+            },
+        )
+        self.assert_in_success_response(
+            ["Required plan tier for realm-name-4 set to Zulip Business."],
+            result,
+        )
+        customer.refresh_from_db()
+        next_plan.refresh_from_db()
+        self.assertEqual(customer.required_plan_tier, CustomerPlan.TIER_SELF_HOSTED_BUSINESS)
+        self.assertEqual(plan.tier, CustomerPlan.TIER_SELF_HOSTED_LEGACY)
+        self.assertEqual(next_plan.tier, CustomerPlan.TIER_SELF_HOSTED_BASIC)
+
         # A default discount can be added when an upgrade is scheduled.
+        result = self.client_post(
+            "/activity/remote/support",
+            {
+                "remote_realm_id": f"{remote_realm.id}",
+                "required_plan_tier": f"{CustomerPlan.TIER_SELF_HOSTED_BASIC}",
+            },
+        )
+        self.assert_in_success_response(
+            ["Required plan tier for realm-name-4 set to Zulip Basic."],
+            result,
+        )
         result = self.client_post(
             "/activity/remote/support",
             {"remote_realm_id": f"{remote_realm.id}", "discount": "50"},
@@ -535,7 +567,8 @@ class TestRemoteServerSupportEndpoint(ZulipTestCase):
         plan.refresh_from_db()
         next_plan.refresh_from_db()
         self.assertEqual(customer.default_discount, Decimal(50))
-        self.assertEqual(plan.discount, Decimal(50))
+        # Discount for current plan stays None since it is not the same as required tier for discount.
+        self.assertEqual(plan.discount, None)
         self.assertEqual(next_plan.discount, Decimal(50))
         self.assertEqual(plan.tier, CustomerPlan.TIER_SELF_HOSTED_LEGACY)
         self.assertEqual(next_plan.tier, CustomerPlan.TIER_SELF_HOSTED_BASIC)
@@ -556,26 +589,6 @@ class TestRemoteServerSupportEndpoint(ZulipTestCase):
         customer.refresh_from_db()
         self.assertIsNone(customer.minimum_licenses)
 
-        # A required plan tier for a customer can be set when an upgrade
-        # is scheduled, but won't change either pre-existing plan tiers.
-        self.assertIsNone(customer.required_plan_tier)
-        result = self.client_post(
-            "/activity/remote/support",
-            {
-                "remote_realm_id": f"{remote_realm.id}",
-                "required_plan_tier": f"{CustomerPlan.TIER_SELF_HOSTED_BUSINESS}",
-            },
-        )
-        self.assert_in_success_response(
-            ["Required plan tier for realm-name-4 set to Zulip Business."],
-            result,
-        )
-        customer.refresh_from_db()
-        next_plan.refresh_from_db()
-        self.assertEqual(customer.required_plan_tier, CustomerPlan.TIER_SELF_HOSTED_BUSINESS)
-        self.assertEqual(plan.tier, CustomerPlan.TIER_SELF_HOSTED_LEGACY)
-        self.assertEqual(next_plan.tier, CustomerPlan.TIER_SELF_HOSTED_BASIC)
-
     def test_support_deactivate_remote_server(self) -> None:
         iago = self.example_user("iago")
         self.login_user(iago)
@@ -1104,6 +1117,17 @@ class TestSupportEndpoint(ZulipTestCase):
         iago = self.example_user("iago")
         self.login_user(iago)
 
+        result = self.client_post(
+            "/activity/support",
+            {
+                "realm_id": f"{lear_realm.id}",
+                "required_plan_tier": f"{CustomerPlan.TIER_CLOUD_STANDARD}",
+            },
+        )
+        self.assert_in_success_response(
+            ["Required plan tier for lear set to Zulip Cloud Standard."],
+            result,
+        )
         result = self.client_post(
             "/activity/support", {"realm_id": f"{lear_realm.id}", "discount": "25"}
         )

templates/corporate/support/sponsorship_forms_support.html

@@ -20,31 +20,6 @@
 </form>
 {% endif %}
 
-<form method="POST" class="remote-form">
-    <b>Discount percentage</b>:<br />
-    <i>Updates will change pre-existing plans and scheduled upgrades.</i><br />
-    <i>Any prorated licenses for the current billing cycle will be billed at the updated discounted rate.</i><br />
-    {{ csrf_input }}
-    <input type="hidden" name="{{ remote_type }}" value="{{ remote_id }}" />
-    {% if has_fixed_price %}
-    <input type="number" name="discount" value="{{ format_discount(sponsorship_data.default_discount) }}" step="0.01" min="0" max="99.99" disabled />
-    <button type="submit" class="support-submit-button" disabled>Update</button>
-    {% else %}
-    <input type="number" name="discount" value="{{ format_discount(sponsorship_data.default_discount) }}" step="0.01" min="0" max="99.99" required />
-    <button type="submit" class="support-submit-button">Update</button>
-    {% endif %}
-</form>
-
-{% if not has_fixed_price and (sponsorship_data.default_discount or sponsorship_data.minimum_licenses) %}
-<form method="POST" class="remote-form">
-    <b>Minimum licenses</b>:<br />
-    {{ csrf_input }}
-    <input type="hidden" name="{{ remote_type }}" value="{{ remote_id }}" />
-    <input type="number" name="minimum_licenses" value="{{ sponsorship_data.minimum_licenses }}" required />
-    <button type="submit" class="support-submit-button">Update</button>
-</form>
-{% endif %}
-
 <form method="POST" class="remote-form">
     <b>Required plan tier for discounts and fixed prices</b>:<br />
     <i>Updates will not change any pre-existing plans or scheduled upgrades.</i><br />
@@ -62,6 +37,38 @@
     <button type="submit" class="support-submit-button">Update</button>
 </form>
 
+<form method="POST" class="remote-form">
+    <b>Discount percentage</b>:<br />
+    <i>Needs required plan tier to be set.</i><br />
+    <i>Updates will change pre-existing plans and scheduled upgrades.</i><br />
+    <i>Any prorated licenses for the current billing cycle will be billed at the updated discounted rate.</i><br />
+    {{ csrf_input }}
+    <input type="hidden" name="{{ remote_type }}" value="{{ remote_id }}" />
+    {% if has_fixed_price %}
+    <input type="number" name="discount" value="{{ format_discount(sponsorship_data.default_discount) }}" step="0.01" min="0" max="99.99" disabled />
+    <button type="submit" class="support-submit-button" disabled>Update</button>
+    {% else %}
+    <input type="number" name="discount" value="{{ format_discount(sponsorship_data.default_discount) }}" step="0.01" min="0" max="99.99"
+      {% if sponsorship_data.required_plan_tier %}
+      required
+      {% else %}
+      disabled
+      {% endif %}
+      />
+    <button type="submit" class="support-submit-button">Update</button>
+    {% endif %}
+</form>
+
+{% if not has_fixed_price and (sponsorship_data.default_discount or sponsorship_data.minimum_licenses) %}
+<form method="POST" class="remote-form">
+    <b>Minimum licenses</b>:<br />
+    {{ csrf_input }}
+    <input type="hidden" name="{{ remote_type }}" value="{{ remote_id }}" />
+    <input type="number" name="minimum_licenses" value="{{ sponsorship_data.minimum_licenses }}" required />
+    <button type="submit" class="support-submit-button">Update</button>
+</form>
+{% endif %}
+
 {% if sponsorship_data.sponsorship_pending %}
     {% with %}
         {% set latest_sponsorship_request = sponsorship_data.latest_sponsorship_request %}