From e06957bef59dfbcdce88a39e01a7fb6782d2c859 Mon Sep 17 00:00:00 2001 From: Vishnu Ks Date: Fri, 27 Jul 2018 21:17:03 +0530 Subject: [PATCH] billing: Raise exceptions instead of returning errors in upgrade flow. --- zilencer/lib/stripe.py | 13 +++++++------ zilencer/tests/test_stripe.py | 5 +++-- zilencer/views.py | 15 ++++++++++----- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/zilencer/lib/stripe.py b/zilencer/lib/stripe.py index f7c175cde0..12d464b698 100644 --- a/zilencer/lib/stripe.py +++ b/zilencer/lib/stripe.py @@ -67,6 +67,9 @@ def unsign_string(signed_string: str, salt: str) -> str: class StripeError(JsonableError): pass +class BillingError(Exception): + pass + def catch_stripe_errors(func: CallableT) -> CallableT: @wraps(func) def wrapped(*args: Any, **kwargs: Any) -> Any: @@ -147,7 +150,7 @@ def do_subscribe_customer_to_plan(stripe_customer: stripe.Customer, stripe_plan_ billing_logger.error("Stripe customer %s trying to subscribe to %s, " "but has an active subscription" % (stripe_customer.id, stripe_plan_id)) # TODO: Change to an error sent to the frontend - raise AssertionError("Customer already has an active subscription.") + raise BillingError("Your organization has an existing active subscription.") stripe_subscription = stripe.Subscription.create( customer=stripe_customer.id, billing='charge_automatically', @@ -180,17 +183,17 @@ def do_subscribe_customer_to_plan(stripe_customer: stripe.Customer, stripe_plan_ extra_data=ujson.dumps({'quantity': current_seat_count})) def process_initial_upgrade(user: UserProfile, plan: str, signed_seat_count: str, - salt: str, stripe_token: str) -> Optional[str]: + salt: str, stripe_token: str) -> None: if plan not in [Plan.CLOUD_ANNUAL, Plan.CLOUD_MONTHLY]: billing_logger.warning("Tampered plan during realm upgrade. user: %s, realm: %s (%s)." % (user.id, user.realm.id, user.realm.string_id)) - return "Something went wrong. Please contact support@zulipchat.com" + raise BillingError("Something went wrong. Please contact support@zulipchat.com") try: seat_count = int(unsign_string(signed_seat_count, salt)) except signing.BadSignature: billing_logger.warning("Tampered seat count during realm upgrade. user: %s, realm: %s (%s)." % (user.id, user.realm.id, user.realm.string_id)) - return "Something went wrong. Please contact support@zulipchat.com" + raise BillingError("Something went wrong. Please contact support@zulipchat.com") stripe_customer = do_create_customer_with_payment_source(user, stripe_token) do_subscribe_customer_to_plan( @@ -200,5 +203,3 @@ def process_initial_upgrade(user: UserProfile, plan: str, signed_seat_count: str # TODO: billing address details are passed to us in the request; # use that to calculate taxes. tax_percent=0) - # TODO: check for errors and raise/send to frontend - return None diff --git a/zilencer/tests/test_stripe.py b/zilencer/tests/test_stripe.py index 58bd9f4da0..ae8e5ec8b5 100644 --- a/zilencer/tests/test_stripe.py +++ b/zilencer/tests/test_stripe.py @@ -14,7 +14,8 @@ from zerver.lib.timestamp import timestamp_to_datetime from zerver.models import Realm, UserProfile, get_realm, RealmAuditLog from zilencer.lib.stripe import StripeError, catch_stripe_errors, \ do_create_customer_with_payment_source, do_subscribe_customer_to_plan, \ - get_seat_count, extract_current_subscription, sign_string, unsign_string + get_seat_count, extract_current_subscription, sign_string, unsign_string, \ + BillingError from zilencer.models import Customer, Plan fixture_data_file = open(os.path.join(os.path.dirname(__file__), 'stripe_fixtures.json'), 'r') @@ -264,7 +265,7 @@ class StripeTest(ZulipTestCase): @mock.patch("stripe.Customer.retrieve", side_effect=mock_customer_with_active_subscription) def test_subscribe_customer_to_second_plan(self, mock_customer_with_active_subscription: mock.Mock) -> None: - with self.assertRaisesRegex(AssertionError, "Customer already has an active subscription."): + with self.assertRaisesRegex(BillingError, "Your organization has an existing active subscription."): do_subscribe_customer_to_plan(stripe.Customer.retrieve(), # type: ignore # Mocked out function call self.stripe_plan_id, self.quantity, 0) diff --git a/zilencer/views.py b/zilencer/views.py index 93137dcacc..3b667a46b5 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -26,7 +26,8 @@ from zerver.models import UserProfile, Realm from zerver.views.push_notifications import validate_token from zilencer.lib.stripe import STRIPE_PUBLISHABLE_KEY, StripeError, \ get_stripe_customer, get_upcoming_invoice, get_seat_count, \ - extract_current_subscription, process_initial_upgrade, sign_string + extract_current_subscription, process_initial_upgrade, sign_string, \ + BillingError from zilencer.models import RemotePushDeviceToken, RemoteZulipServer, \ Customer, Plan @@ -169,10 +170,14 @@ def initial_upgrade(request: HttpRequest) -> HttpResponse: return HttpResponseRedirect(reverse('zilencer.views.billing_home')) if request.method == 'POST': - error_message = process_initial_upgrade(user, request.POST['plan'], - request.POST['signed_seat_count'], - request.POST['salt'], request.POST['stripeToken']) or "" - if not error_message: + try: + process_initial_upgrade(user, request.POST['plan'], request.POST['signed_seat_count'], + request.POST['salt'], request.POST['stripeToken']) + except (BillingError, StripeError) as e: + error_message = str(e) + except Exception: + error_message = "Something went wrong. Please contact support@zulipchat.com." + else: return HttpResponseRedirect(reverse('zilencer.views.billing_home')) seat_count = get_seat_count(user.realm)