billing: Try paying invoices when user puts a new card on file.

Previously, when users got a "payment failed" email from Stripe (e.g. if
their card failed on renewal), they would enter in a new card on
/billing#payment-method, and wouldn't find out if the card worked till
Stripe retried the payment 4 days later.

corporate/lib/stripe.py

@@ -194,16 +194,23 @@ def do_create_stripe_customer(user: UserProfile, stripe_token: Optional[str]=Non
     return customer
 
 @catch_stripe_errors
-def do_replace_payment_source(user: UserProfile, stripe_token: str) -> stripe.Customer:
+def do_replace_payment_source(user: UserProfile, stripe_token: str,
+                              pay_invoices: bool=False) -> stripe.Customer:
     stripe_customer = stripe_get_customer(Customer.objects.get(realm=user.realm).stripe_customer_id)
     stripe_customer.source = stripe_token
     # Deletes existing card: https://stripe.com/docs/api#update_customer-source
-    # This can also have other side effects, e.g. it will try to pay certain past-due
-    # invoices: https://stripe.com/docs/api#update_customer
     updated_stripe_customer = stripe.Customer.save(stripe_customer)
     RealmAuditLog.objects.create(
         realm=user.realm, acting_user=user, event_type=RealmAuditLog.STRIPE_CARD_CHANGED,
         event_time=timezone_now())
+    if pay_invoices:
+        for stripe_invoice in stripe.Invoice.list(
+                billing='charge_automatically', customer=stripe_customer.id, status='open'):
+            # The user will get either a receipt or a "failed payment" email, but the in-app
+            # messaging could be clearer here (e.g. it could explictly tell the user that there
+            # were payment(s) and that they succeeded or failed).
+            # Worth fixing if we notice that a lot of cards end up failing at this step.
+            stripe.Invoice.pay(stripe_invoice)
     return updated_stripe_customer
 
 # event_time should roughly be timezone_now(). Not designed to handle

corporate/tests/test_stripe.py

@@ -153,6 +153,8 @@ def normalize_fixture_data(decorated_function: CallableT,
                 file_content = file_content.replace(match, normalized_values[pattern][match])
         file_content = re.sub(r'(?<="risk_score": )(\d+)', '00', file_content)
         file_content = re.sub(r'(?<="times_redeemed": )(\d+)', '00', file_content)
+        file_content = re.sub(r'(?<="idempotency-key": )"([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f-]*)"',
+                              '"00000000-0000-0000-0000-000000000000"', file_content)
         # Dates
         file_content = re.sub(r'(?<="Date": )"(.* GMT)"', '"NORMALIZED DATETIME"', file_content)
         file_content = re.sub(r'[0-3]\d [A-Z][a-z]{2} 20[1-2]\d', 'NORMALIZED DATE', file_content)
@@ -168,7 +170,7 @@ MOCKED_STRIPE_FUNCTION_NAMES = ["stripe.{}".format(name) for name in [
     "Charge.create", "Charge.list",
     "Coupon.create",
     "Customer.create", "Customer.retrieve", "Customer.save",
-    "Invoice.create", "Invoice.finalize_invoice", "Invoice.list", "Invoice.upcoming",
+    "Invoice.create", "Invoice.finalize_invoice", "Invoice.list", "Invoice.pay", "Invoice.upcoming",
     "InvoiceItem.create", "InvoiceItem.list",
     "Plan.create",
     "Product.create",
@@ -819,35 +821,57 @@ class StripeTest(StripeTestCase):
         user = self.example_user("hamlet")
         self.login(user.email)
         self.upgrade()
-        # Try replacing with a valid card
-        stripe_token = stripe_create_token(card_number='5555555555554444').id
+        # Create an open invoice
+        stripe_customer_id = Customer.objects.first().stripe_customer_id
+        stripe.InvoiceItem.create(amount=5000, currency='usd', customer=stripe_customer_id)
+        stripe_invoice = stripe.Invoice.create(customer=stripe_customer_id)
+        stripe.Invoice.finalize_invoice(stripe_invoice)
+        RealmAuditLog.objects.filter(event_type=RealmAuditLog.STRIPE_CARD_CHANGED).delete()
+
+        # Replace with an invalid card
+        stripe_token = stripe_create_token(card_number='4000000000009987').id
+        with patch("corporate.lib.stripe.billing_logger.error") as mock_billing_logger:
+            with patch("stripe.Invoice.list") as mock_invoice_list:
                 response = self.client_post("/json/billing/sources/change",
                                             {'stripe_token': ujson.dumps(stripe_token)})
-        self.assert_json_success(response)
-        number_of_sources = 0
-        for stripe_source in stripe_get_customer(Customer.objects.first().stripe_customer_id).sources:
-            self.assertEqual(cast(stripe.Card, stripe_source).last4, '4444')
-            number_of_sources += 1
-        self.assertEqual(number_of_sources, 1)
-        audit_log_entry = RealmAuditLog.objects.order_by('-id') \
-                                               .values_list('acting_user', 'event_type').first()
-        self.assertEqual(audit_log_entry, (user.id, RealmAuditLog.STRIPE_CARD_CHANGED))
-        RealmAuditLog.objects.filter(acting_user=user).delete()
+        mock_billing_logger.assert_called()
+        mock_invoice_list.assert_not_called()
+        self.assertEqual(ujson.loads(response.content)['error_description'], 'card error')
+        self.assert_json_error_contains(response, 'Your card was declined')
+        for stripe_source in stripe_get_customer(stripe_customer_id).sources:
+            self.assertEqual(cast(stripe.Card, stripe_source).last4, '4242')
+        self.assertFalse(RealmAuditLog.objects.filter(event_type=RealmAuditLog.STRIPE_CARD_CHANGED).exists())
 
-        # Try replacing with an invalid card
-        stripe_token = stripe_create_token(card_number='4000000000009987').id
+        # Replace with a card that's valid, but charging the card fails
+        stripe_token = stripe_create_token(card_number='4000000000000341').id
         with patch("corporate.lib.stripe.billing_logger.error") as mock_billing_logger:
             response = self.client_post("/json/billing/sources/change",
                                         {'stripe_token': ujson.dumps(stripe_token)})
         mock_billing_logger.assert_called()
         self.assertEqual(ujson.loads(response.content)['error_description'], 'card error')
         self.assert_json_error_contains(response, 'Your card was declined')
+        for stripe_source in stripe_get_customer(stripe_customer_id).sources:
+            self.assertEqual(cast(stripe.Card, stripe_source).last4, '0341')
+        self.assertEqual(len(list(stripe.Invoice.list(customer=stripe_customer_id, status='open'))), 1)
+        self.assertEqual(1, RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STRIPE_CARD_CHANGED).count())
+
+        # Replace with a valid card
+        stripe_token = stripe_create_token(card_number='5555555555554444').id
+        response = self.client_post("/json/billing/sources/change",
+                                    {'stripe_token': ujson.dumps(stripe_token)})
+        self.assert_json_success(response)
         number_of_sources = 0
-        for stripe_source in stripe_get_customer(Customer.objects.first().stripe_customer_id).sources:
+        for stripe_source in stripe_get_customer(stripe_customer_id).sources:
             self.assertEqual(cast(stripe.Card, stripe_source).last4, '4444')
             number_of_sources += 1
+        # Verify that we replaced the previous card, rather than adding a new one
         self.assertEqual(number_of_sources, 1)
-        self.assertFalse(RealmAuditLog.objects.filter(event_type=RealmAuditLog.STRIPE_CARD_CHANGED).exists())
+        # Ideally we'd also test that we don't pay invoices with billing=='send_invoice'
+        for stripe_invoice in stripe.Invoice.list(customer=stripe_customer_id):
+            self.assertEqual(stripe_invoice.status, 'paid')
+        self.assertEqual(2, RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.STRIPE_CARD_CHANGED).count())
 
 class RequiresBillingAccessTest(ZulipTestCase):
     def setUp(self) -> None:

corporate/views.py

@@ -218,7 +218,7 @@ def downgrade(request: HttpRequest, user: UserProfile) -> HttpResponse:  # nocov
 def replace_payment_source(request: HttpRequest, user: UserProfile,
                            stripe_token: str=REQ("stripe_token", validator=check_string)) -> HttpResponse:
     try:
-        do_replace_payment_source(user, stripe_token)
+        do_replace_payment_source(user, stripe_token, pay_invoices=True)
     except BillingError as e:
         return json_error(e.message, data={'error_description': e.description})
     return json_success()

stubs/stripe/__init__.pyi

@@ -67,7 +67,8 @@ class Invoice:
         ...
 
     @staticmethod
-    def list(customer: str=..., limit: Optional[int]=...) -> List[Invoice]:
+    def list(billing: str=..., customer: str=..., limit: Optional[int]=...,
+             status: str=...) -> List[Invoice]:
         ...
 
     @staticmethod
@@ -79,6 +80,10 @@ class Invoice:
     def finalize_invoice(invoice: Invoice) -> Invoice:
         ...
 
+    @staticmethod
+    def pay(invoice: Invoice) -> Invoice:
+        ...
+
     def get(self, key: str) -> Any:
         ...