From 7d1f858273380eaa0ba6d4f42e333ade42d12bdb Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Mon, 29 Apr 2024 09:16:54 +0000 Subject: [PATCH] stripe: Ensure required_plan_tier is set before setting discount. --- corporate/lib/stripe.py | 38 ++++++----- corporate/tests/test_stripe.py | 22 ++++++- corporate/tests/test_support_views.py | 66 +++++++++++++------ .../support/sponsorship_forms_support.html | 57 +++++++++------- 4 files changed, 119 insertions(+), 64 deletions(-) diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index eb110a3f8e..66fdb2cce0 100644 --- a/corporate/lib/stripe.py +++ b/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: - old_discount = customer.default_discount - customer.default_discount = new_discount - customer.flat_discounted_months = 0 - customer.save(update_fields=["default_discount", "flat_discounted_months"]) - else: - customer = self.update_or_create_customer( - defaults={"default_discount": new_discount, "flat_discounted_months": 0} - ) + + # We set required plan tier before setting a discount for the customer, so it's always defined. + assert customer is not None + assert customer.required_plan_tier is not None + + old_discount = customer.default_discount + customer.default_discount = new_discount + customer.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. - # Make this a check on CustomerPlan.SWITCH_PLAN_TIER_AT_PLAN_END status - # if we support this for other plans. - next_plan = self.get_legacy_remote_server_next_plan(customer) - if next_plan is not None: # nocoverage - self.apply_discount_to_plan(next_plan, new_discount) + # 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 + # if we support this for other plans. + next_plan = self.get_legacy_remote_server_next_plan(customer) + if next_plan is not None and next_plan.tier == customer.required_plan_tier: + 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} ) diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index 63b1d20a8b..d37d0d4759 100644 --- a/corporate/tests/test_stripe.py +++ b/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() diff --git a/corporate/tests/test_support_views.py b/corporate/tests/test_support_views.py index 93afddb417..b034096b82 100644 --- a/corporate/tests/test_support_views.py +++ b/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"} ) diff --git a/templates/corporate/support/sponsorship_forms_support.html b/templates/corporate/support/sponsorship_forms_support.html index aff8e74371..acc095770d 100644 --- a/templates/corporate/support/sponsorship_forms_support.html +++ b/templates/corporate/support/sponsorship_forms_support.html @@ -20,31 +20,6 @@ {% endif %} -
- Discount percentage:
- Updates will change pre-existing plans and scheduled upgrades.
- Any prorated licenses for the current billing cycle will be billed at the updated discounted rate.
- {{ csrf_input }} - - {% if has_fixed_price %} - - - {% else %} - - - {% endif %} -
- -{% if not has_fixed_price and (sponsorship_data.default_discount or sponsorship_data.minimum_licenses) %} -
- Minimum licenses:
- {{ csrf_input }} - - - -
-{% endif %} -
Required plan tier for discounts and fixed prices:
Updates will not change any pre-existing plans or scheduled upgrades.
@@ -62,6 +37,38 @@
+
+ Discount percentage:
+ Needs required plan tier to be set.
+ Updates will change pre-existing plans and scheduled upgrades.
+ Any prorated licenses for the current billing cycle will be billed at the updated discounted rate.
+ {{ csrf_input }} + + {% if has_fixed_price %} + + + {% else %} + + + {% endif %} +
+ +{% if not has_fixed_price and (sponsorship_data.default_discount or sponsorship_data.minimum_licenses) %} +
+ Minimum licenses:
+ {{ csrf_input }} + + + +
+{% endif %} + {% if sponsorship_data.sponsorship_pending %} {% with %} {% set latest_sponsorship_request = sponsorship_data.latest_sponsorship_request %}