From 698aeddc85e0763453950ea22dd0047f4f28362b Mon Sep 17 00:00:00 2001 From: Rishi Gupta Date: Tue, 27 Nov 2018 15:20:58 -0800 Subject: [PATCH] billing: Delete test_subscribe_customer_to_second_plan. This used to be a more likely codepath, before we introduced Customer.has_billing_relationship. It is no longer literally impossible to hit this race condition, so I'm not deleting the code, but it's unlikely enough that it's not worth figuring out how to test it. --- corporate/lib/stripe.py | 9 ++++----- corporate/tests/test_stripe.py | 6 ------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index 9aa799f519..f57a777267 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -205,11 +205,10 @@ def do_replace_coupon(user: UserProfile, coupon: Coupon) -> stripe.Customer: @catch_stripe_errors def do_subscribe_customer_to_plan(user: UserProfile, stripe_customer: stripe.Customer, stripe_plan_id: str, seat_count: int, tax_percent: float, charge_automatically: bool) -> None: - if extract_current_subscription(stripe_customer) is not None: - # Most likely due to two people in the org going to the billing page, - # and then both upgrading their plan. We don't send clients - # real-time event updates for the billing pages, so this is more - # likely than it would be in other parts of the app. + if extract_current_subscription(stripe_customer) is not None: # nocoverage + # Unlikely race condition from two people upgrading (clicking "Make payment") + # at exactly the same time. Doesn't fully resolve the race condition, but having + # a check here reduces the likelihood. billing_logger.error("Stripe customer %s trying to subscribe to %s, " "but has an active subscription" % (stripe_customer.id, stripe_plan_id)) raise BillingError('subscribing with existing subscription', BillingError.TRY_RELOADING) diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index e7a2841639..36c7ec5527 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -643,12 +643,6 @@ class StripeTest(ZulipTestCase): do_deactivate_user(user2) self.assertEqual(get_seat_count(realm), initial_count) - def test_subscribe_customer_to_second_plan(self) -> None: - with self.assertRaisesRegex(BillingError, 'subscribing with existing subscription'): - do_subscribe_customer_to_plan(self.example_user("iago"), - mock_customer_with_subscription(), - self.stripe_plan_id, self.quantity, 0, True) - def test_sign_string(self) -> None: string = "abc" signed_string, salt = sign_string(string)