billing: Fix RealmAuditLog accounting for downgrade.

zilencer/lib/stripe.py

@@ -299,8 +299,21 @@ def process_downgrade(user: UserProfile) -> None:
         stripe_customer.account_balance = stripe_customer.account_balance + subscription_balance
     stripe_subscription = extract_current_subscription(stripe_customer)
     # Wish these two could be transaction.atomic
-    stripe_subscription.delete()
+    stripe_subscription = stripe_subscription.delete()
     stripe_customer.save()
+    with transaction.atomic():
+        user.realm.has_seat_based_plan = False
+        user.realm.save(update_fields=['has_seat_based_plan'])
+        RealmAuditLog.objects.create(
+            realm=user.realm,
+            acting_user=user,
+            event_type=RealmAuditLog.STRIPE_PLAN_CHANGED,
+            event_time=timestamp_to_datetime(stripe_subscription.canceled_at),
+            extra_data=ujson.dumps({'plan': None, 'quantity': stripe_subscription.quantity}))
+    # Doing this last, since it results in user-visible confirmation (via
+    # product changes) that the downgrade succeeded.
+    # Keeping it out of the transaction.atomic block because it will
+    # eventually have a lot of stuff going on.
     do_change_plan_type(user, Realm.LIMITED)
 
 ## Process RealmAuditLog
@@ -384,6 +397,9 @@ def run_billing_processor_one_step(processor: BillingProcessor) -> bool:
         process_billing_log_entry(processor, log_row)
         return True
     except Exception as e:
+        # Possible errors include processing subscription quantity entries
+        # after downgrade, since the downgrade code doesn't check that
+        # billing processor is up to date
         billing_logger.error("Error on log_row.realm=%s, event_type=%s, log_row.id=%s, "
                              "processor.id=%s, processor.realm=%s" % (
                                  processor.log_row.realm.string_id, processor.log_row.event_type,

zilencer/tests/test_stripe.py

@@ -405,17 +405,25 @@ class StripeTest(ZulipTestCase):
     def test_downgrade(self, mock_retrieve_customer: mock.Mock, mock_upcoming_invoice: mock.Mock,
                        mock_save_customer: mock.Mock, mock_delete_subscription: mock.Mock) -> None:
         realm = get_realm('zulip')
+        realm.has_seat_based_plan = True
         realm.plan_type = Realm.PREMIUM
-        realm.save(update_fields=['plan_type'])
+        realm.save(update_fields=['has_seat_based_plan', 'plan_type'])
         Customer.objects.create(
             realm=realm, stripe_customer_id=self.stripe_customer_id, has_billing_relationship=True)
-        self.login(self.example_email('iago'))
+        user = self.example_user('iago')
+        self.login(user.email)
         response = self.client_post("/json/billing/downgrade", {})
         self.assert_json_success(response)
 
         mock_delete_subscription.assert_called()
         mock_save_customer.assert_called()
         realm = get_realm('zulip')
+        self.assertFalse(realm.has_seat_based_plan)
+        audit_log_entries = list(RealmAuditLog.objects.filter(acting_user=user)
+                                 .values_list('event_type', flat=True).order_by('id'))
+        # TODO: once we have proper mocks, test for event_time and extra_data in STRIPE_PLAN_CHANGED
+        self.assertEqual(audit_log_entries, [RealmAuditLog.STRIPE_PLAN_CHANGED,
+                                             RealmAuditLog.REALM_PLAN_TYPE_CHANGED])
         self.assertEqual(realm.plan_type, Realm.LIMITED)
 
     @mock.patch("stripe.Customer.save")