diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index 1e73392d49..c68b2bd4e1 100644 --- a/corporate/lib/stripe.py +++ b/corporate/lib/stripe.py @@ -34,6 +34,9 @@ log_to_file(logging.getLogger('stripe'), BILLING_LOG_PATH) CallableT = TypeVar('CallableT', bound=Callable[..., Any]) +MIN_INVOICED_SEAT_COUNT = 30 +DEFAULT_INVOICE_DAYS_UNTIL_DUE = 30 + def get_seat_count(realm: Realm) -> int: return UserProfile.objects.filter(realm=realm, is_active=True, is_bot=False).count() @@ -202,7 +205,7 @@ 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) -> None: + 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 @@ -212,6 +215,12 @@ def do_subscribe_customer_to_plan(user: UserProfile, stripe_customer: stripe.Cus "but has an active subscription" % (stripe_customer.id, stripe_plan_id)) raise BillingError('subscribing with existing subscription', BillingError.TRY_RELOADING) customer = Customer.objects.get(stripe_customer_id=stripe_customer.id) + if charge_automatically: + billing_method = 'charge_automatically' + days_until_due = None + else: + billing_method = 'send_invoice' + days_until_due = DEFAULT_INVOICE_DAYS_UNTIL_DUE # Note that there is a race condition here, where if two users upgrade at exactly the # same time, they will have two subscriptions, and get charged twice. We could try to # reduce the chance of it with a well-designed idempotency_key, but it's not easy since @@ -222,7 +231,8 @@ def do_subscribe_customer_to_plan(user: UserProfile, stripe_customer: stripe.Cus # Otherwise we should expect it to throw a stripe.error. stripe_subscription = stripe.Subscription.create( customer=stripe_customer.id, - billing='charge_automatically', + billing=billing_method, + days_until_due=days_until_due, items=[{ 'plan': stripe_plan_id, 'quantity': seat_count, @@ -239,7 +249,8 @@ def do_subscribe_customer_to_plan(user: UserProfile, stripe_customer: stripe.Cus acting_user=user, event_type=RealmAuditLog.STRIPE_PLAN_CHANGED, event_time=timestamp_to_datetime(stripe_subscription.created), - extra_data=ujson.dumps({'plan': stripe_plan_id, 'quantity': seat_count})) + extra_data=ujson.dumps({'plan': stripe_plan_id, 'quantity': seat_count, + 'billing_method': billing_method})) current_seat_count = get_seat_count(customer.realm) if seat_count != current_seat_count: @@ -250,11 +261,14 @@ def do_subscribe_customer_to_plan(user: UserProfile, stripe_customer: stripe.Cus requires_billing_update=True, extra_data=ujson.dumps({'quantity': current_seat_count})) -def process_initial_upgrade(user: UserProfile, plan: Plan, seat_count: int, stripe_token: str) -> None: +def process_initial_upgrade(user: UserProfile, plan: Plan, seat_count: int, + stripe_token: Optional[str]) -> None: customer = Customer.objects.filter(realm=user.realm).first() if customer is None: stripe_customer = do_create_customer(user, stripe_token=stripe_token) - else: + # elif instead of if since we want to avoid doing two round trips to + # stripe if we can + elif stripe_token is not None: stripe_customer = do_replace_payment_source(user, stripe_token) do_subscribe_customer_to_plan( user=user, @@ -263,7 +277,8 @@ def process_initial_upgrade(user: UserProfile, plan: Plan, seat_count: int, stri seat_count=seat_count, # TODO: billing address details are passed to us in the request; # use that to calculate taxes. - tax_percent=0) + tax_percent=0, + charge_automatically=(stripe_token is not None)) do_change_plan_type(user, Realm.STANDARD) def attach_discount_to_realm(user: UserProfile, percent_off: int) -> None: diff --git a/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.create.1.json b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.create.1.json new file mode 100644 index 0000000000..745bca8897 Binary files /dev/null and b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.create.1.json differ diff --git a/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.retrieve.1.json b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.retrieve.1.json new file mode 100644 index 0000000000..b2a12517be Binary files /dev/null and b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.retrieve.1.json differ diff --git a/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.retrieve.2.json b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.retrieve.2.json new file mode 100644 index 0000000000..826f54ba51 Binary files /dev/null and b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Customer.retrieve.2.json differ diff --git a/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Invoice.list.1.json b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Invoice.list.1.json new file mode 100644 index 0000000000..c855e7ecd2 Binary files /dev/null and b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Invoice.list.1.json differ diff --git a/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Subscription.create.1.json b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Subscription.create.1.json new file mode 100644 index 0000000000..fa77251f73 Binary files /dev/null and b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Subscription.create.1.json differ diff --git a/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Subscription.save.1.json b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Subscription.save.1.json new file mode 100644 index 0000000000..479fd1e0b2 Binary files /dev/null and b/corporate/tests/stripe_fixtures/upgrade_billing_by_invoice:Subscription.save.1.json differ diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index b8667db337..a078385f54 100644 --- a/corporate/tests/test_stripe.py +++ b/corporate/tests/test_stripe.py @@ -26,7 +26,8 @@ from corporate.lib.stripe import catch_stripe_errors, \ do_subscribe_customer_to_plan, attach_discount_to_realm, \ get_seat_count, extract_current_subscription, sign_string, unsign_string, \ get_next_billing_log_entry, run_billing_processor_one_step, \ - BillingError, StripeCardError, StripeConnectionError, stripe_get_customer + BillingError, StripeCardError, StripeConnectionError, stripe_get_customer, \ + DEFAULT_INVOICE_DAYS_UNTIL_DUE, MIN_INVOICED_SEAT_COUNT from corporate.models import Customer, Plan, Coupon, BillingProcessor import corporate.urls @@ -206,6 +207,13 @@ class Kandra(object): def __eq__(self, other: Any) -> bool: return True +def process_all_billing_log_entries() -> None: + assert not RealmAuditLog.objects.get(pk=1).requires_billing_update + processor = BillingProcessor.objects.create( + log_row=RealmAuditLog.objects.get(pk=1), realm=None, state=BillingProcessor.DONE) + while run_billing_processor_one_step(processor): + pass + class StripeTest(ZulipTestCase): @mock_stripe("Product.create", "Plan.create", "Coupon.create", generate=False) def setUp(self, mock3: Mock, mock2: Mock, mock1: Mock) -> None: @@ -467,6 +475,30 @@ class StripeTest(ZulipTestCase): self.assert_in_success_response(["Upgrade to Zulip Standard"], response) self.assertEqual(response['error_description'], 'tampered plan') + def test_upgrade_with_insufficient_invoiced_seat_count(self) -> None: + self.login(self.example_email("hamlet")) + # Test invoicing for less than MIN_INVOICED_SEAT_COUNT + response = self.client_post("/upgrade/", { + 'invoiced_seat_count': self.quantity, + 'signed_seat_count': self.signed_seat_count, + 'salt': self.salt, + 'plan': Plan.CLOUD_ANNUAL + }) + self.assert_in_success_response(["Upgrade to Zulip Standard", + "at least %d users" % (MIN_INVOICED_SEAT_COUNT,)], response) + self.assertEqual(response['error_description'], 'lowball seat count') + # Test invoicing for less than your user count + with patch("corporate.views.MIN_INVOICED_SEAT_COUNT", 3): + response = self.client_post("/upgrade/", { + 'invoiced_seat_count': self.quantity - 1, + 'signed_seat_count': self.signed_seat_count, + 'salt': self.salt, + 'plan': Plan.CLOUD_ANNUAL + }) + self.assert_in_success_response(["Upgrade to Zulip Standard", + "at least %d users" % (self.quantity,)], response) + self.assertEqual(response['error_description'], 'lowball seat count') + @patch("corporate.lib.stripe.billing_logger.error") def test_upgrade_with_uncaught_exception(self, mock1: Mock) -> None: self.login(self.example_email("hamlet")) @@ -481,6 +513,69 @@ class StripeTest(ZulipTestCase): "Something went wrong. Please contact"], response) self.assertEqual(response['error_description'], 'uncaught exception during upgrade') + @mock_stripe("Customer.create", "Subscription.create", "Subscription.save", + "Customer.retrieve", "Invoice.list") + def test_upgrade_billing_by_invoice(self, mock5: Mock, mock4: Mock, mock3: Mock, + mock2: Mock, mock1: Mock) -> None: + user = self.example_user("hamlet") + self.login(user.email) + self.client_post("/upgrade/", { + 'invoiced_seat_count': 123, + 'signed_seat_count': self.signed_seat_count, + 'salt': self.salt, + 'plan': Plan.CLOUD_ANNUAL}) + process_all_billing_log_entries() + + # Check that we correctly created a Customer in Stripe + stripe_customer = stripe_get_customer(Customer.objects.get(realm=user.realm).stripe_customer_id) + self.assertEqual(stripe_customer.email, user.email) + # It can take a second for Stripe to attach the source to the + # customer, and in particular it may not be attached at the time + # stripe_get_customer is called above, causing test flakes. + # So commenting the next line out, but leaving it here so future readers know what + # is supposed to happen here (e.g. the default_source is not None as it would be if + # we had not added a Subscription). + # self.assertEqual(stripe_customer.default_source.type, 'ach_credit_transfer') + + # Check that we correctly created a Subscription in Stripe + stripe_subscription = extract_current_subscription(stripe_customer) + self.assertEqual(stripe_subscription.billing, 'send_invoice') + self.assertEqual(stripe_subscription.days_until_due, DEFAULT_INVOICE_DAYS_UNTIL_DUE) + self.assertEqual(stripe_subscription.plan.id, + Plan.objects.get(nickname=Plan.CLOUD_ANNUAL).stripe_plan_id) + self.assertEqual(stripe_subscription.quantity, get_seat_count(user.realm)) + self.assertEqual(stripe_subscription.status, 'active') + # Check that we correctly created an initial Invoice in Stripe + for stripe_invoice in stripe.Invoice.list(customer=stripe_customer.id, limit=1): + self.assertTrue(stripe_invoice.auto_advance) + self.assertEqual(stripe_invoice.billing, 'send_invoice') + self.assertEqual(stripe_invoice.billing_reason, 'subscription_create') + # Transitions to 'open' after 1-2 hours + self.assertEqual(stripe_invoice.status, 'draft') + # Very important. Check that we're invoicing for 123, and not get_seat_count + self.assertEqual(stripe_invoice.amount_due, 8000*123) + + # Check that we correctly updated Realm + realm = get_realm("zulip") + self.assertTrue(realm.has_seat_based_plan) + self.assertEqual(realm.plan_type, Realm.STANDARD) + # Check that we created a Customer in Zulip + self.assertEqual(1, Customer.objects.filter(stripe_customer_id=stripe_customer.id, + realm=realm).count()) + # Check that RealmAuditLog has STRIPE_PLAN_QUANTITY_RESET, and doesn't have STRIPE_CARD_CHANGED + audit_log_entries = list(RealmAuditLog.objects.order_by('-id') + .values_list('event_type', 'event_time', + 'requires_billing_update')[:4])[::-1] + self.assertEqual(audit_log_entries, [ + (RealmAuditLog.STRIPE_CUSTOMER_CREATED, timestamp_to_datetime(stripe_customer.created), False), + (RealmAuditLog.STRIPE_PLAN_CHANGED, timestamp_to_datetime(stripe_subscription.created), False), + (RealmAuditLog.STRIPE_PLAN_QUANTITY_RESET, timestamp_to_datetime(stripe_subscription.created), True), + (RealmAuditLog.REALM_PLAN_TYPE_CHANGED, Kandra(), False), + ]) + self.assertEqual(ujson.loads(RealmAuditLog.objects.filter( + event_type=RealmAuditLog.STRIPE_PLAN_QUANTITY_RESET).values_list('extra_data', flat=True).first()), + {'quantity': self.quantity}) + @patch("stripe.Customer.retrieve", side_effect=mock_customer_with_subscription) def test_redirect_for_billing_home(self, mock_customer_with_subscription: Mock) -> None: user = self.example_user("iago") @@ -532,7 +627,7 @@ class StripeTest(ZulipTestCase): 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) + self.stripe_plan_id, self.quantity, 0, True) def test_sign_string(self) -> None: string = "abc" diff --git a/corporate/views.py b/corporate/views.py index 0265e2d933..c09285ee4e 100644 --- a/corporate/views.py +++ b/corporate/views.py @@ -18,7 +18,8 @@ from zerver.models import UserProfile, Realm from corporate.lib.stripe import STRIPE_PUBLISHABLE_KEY, \ stripe_get_customer, upcoming_invoice_total, get_seat_count, \ extract_current_subscription, process_initial_upgrade, sign_string, \ - unsign_string, BillingError, process_downgrade, do_replace_payment_source + unsign_string, BillingError, process_downgrade, do_replace_payment_source, \ + MIN_INVOICED_SEAT_COUNT from corporate.models import Customer, Plan billing_logger = logging.getLogger('corporate.stripe') @@ -56,7 +57,14 @@ def initial_upgrade(request: HttpRequest) -> HttpResponse: try: plan, seat_count = unsign_and_check_upgrade_parameters( user, request.POST['plan'], request.POST['signed_seat_count'], request.POST['salt']) - process_initial_upgrade(user, plan, seat_count, request.POST['stripeToken']) + if 'invoiced_seat_count' in request.POST: + min_required_seat_count = max(seat_count, MIN_INVOICED_SEAT_COUNT) + if int(request.POST['invoiced_seat_count']) < min_required_seat_count: + raise BillingError( + 'lowball seat count', + "You must invoice for at least %d users." % (min_required_seat_count,)) + seat_count = int(request.POST['invoiced_seat_count']) + process_initial_upgrade(user, plan, seat_count, request.POST.get('stripeToken', None)) except BillingError as e: error_message = e.message error_description = e.description @@ -129,8 +137,12 @@ def billing_home(request: HttpRequest) -> HttpResponse: renewal_amount = 0 payment_method = None - if stripe_customer.default_source is not None: - payment_method = "Card ending in %(last4)s" % {'last4': stripe_customer.default_source.last4} + stripe_source = stripe_customer.default_source + if stripe_source is not None: + if stripe_source.object == 'card': + # To fix mypy error, set Customer.default_source: Union[Source, Card] in stubs and debug + payment_method = "Card ending in %(last4)s" % \ + {'last4': stripe_source.last4} # type: ignore # see above context.update({ 'plan_name': plan_name, diff --git a/stubs/stripe/__init__.pyi b/stubs/stripe/__init__.pyi index f1eeb676be..2e0df42869 100644 --- a/stubs/stripe/__init__.pyi +++ b/stubs/stripe/__init__.pyi @@ -8,7 +8,7 @@ from typing import Optional, Any, Dict, List, Union api_key: Optional[str] class Customer: - default_source: Card + default_source: Source created: int id: str source: str @@ -43,7 +43,12 @@ class Customer: class Invoice: + auto_advance: bool amount_due: int + billing: str + billing_reason: str + default_source: Source + status: str total: int @staticmethod @@ -51,16 +56,22 @@ class Invoice: subscription_items: List[Dict[str, Union[str, int]]]=...) -> Invoice: ... + @staticmethod + def list(customer: str=..., limit: Optional[int]=...) -> List[Invoice]: + ... + class Subscription: created: int status: str canceled_at: int cancel_at_period_end: bool + days_until_due: Optional[int] proration_date: int quantity: int @staticmethod - def create(customer: str=..., billing: str=..., items: List[Dict[str, Any]]=..., + def create(customer: str=..., billing: str=..., days_until_due: Optional[int]=..., + items: List[Dict[str, Any]]=..., prorate: bool=..., tax_percent: float=...) -> Subscription: ... @@ -68,9 +79,15 @@ class Subscription: def save(subscription: Subscription, idempotency_key: str=...) -> Subscription: ... +class Source: + id: str + object: str + type: str + class Card: id: str last4: str + object: str class Plan: id: str