stripe: Fix legacy plans not being invoiced.

Earlier, in 'migrate_customer_to_legacy_plan`, we set
'next_invoice_date' to None for legacy plans.
This will result in legacy plans not getting invoiced.
We need to invoice legacy plans on their end date to
either downgrade them or switch to a new plan.

We set next_invoice_date for legacy plans to end_date.

corporate/lib/stripe.py

@@ -854,7 +854,6 @@ class BillingSession(ABC):
             customer=customer,
             tier=CustomerPlan.TIER_SELF_HOSTED_LEGACY,
             status=status,
-            next_invoice_date=None,
         ).first()
 
     def get_formatted_remote_server_legacy_plan_end_date(
@@ -2418,7 +2417,10 @@ class BillingSession(ABC):
             raise NotImplementedError(
                 "Plan with invoicing_status==STARTED needs manual resolution."
             )
-        if not plan.customer.stripe_customer_id:
+        if (
+            plan.tier != CustomerPlan.TIER_SELF_HOSTED_LEGACY
+            and not plan.customer.stripe_customer_id
+        ):
             raise BillingError(
                 f"Customer has a paid plan without a Stripe customer ID: {plan.customer!s}"
             )
@@ -2483,6 +2485,7 @@ class BillingSession(ABC):
                 plan.invoiced_through = ledger_entry
                 plan.invoicing_status = CustomerPlan.INVOICING_STATUS_STARTED
                 plan.save(update_fields=["invoicing_status", "invoiced_through"])
+                assert plan.customer.stripe_customer_id is not None
                 stripe.InvoiceItem.create(
                     currency="usd",
                     customer=plan.customer.stripe_customer_id,
@@ -2918,10 +2921,10 @@ class BillingSession(ABC):
             "tier": CustomerPlan.TIER_SELF_HOSTED_LEGACY,
             # End when the new plan starts.
             "end_date": end_date,
+            "next_invoice_date": end_date,
             # The primary mechanism for preventing charges under this
-            # plan is setting a null `next_invoice_date`, but setting
-            # a 0 price is useful defense in depth here.
-            "next_invoice_date": None,
+            # plan is setting 'invoiced_through' to last ledger_entry below,
+            # but setting a 0 price is useful defense in depth here.
             "price_per_license": 0,
             "billing_schedule": CustomerPlan.BILLING_SCHEDULE_ANNUAL,
             "automanage_licenses": True,
@@ -4433,7 +4436,7 @@ def get_plan_renewal_or_end_date(plan: CustomerPlan, event_time: datetime) -> da
 
 
 def invoice_plans_as_needed(event_time: Optional[datetime] = None) -> None:
-    if event_time is None:  # nocoverage
+    if event_time is None:
         event_time = timezone_now()
     for plan in CustomerPlan.objects.filter(next_invoice_date__lte=event_time):
         remote_server: Optional[RemoteZulipServer] = None

corporate/migrations/0035_update_legacy_plan_next_invoice_date.py

@@ -0,0 +1,28 @@
+# Generated by Django 4.2.8 on 2024-01-25 05:49
+
+from django.db import migrations
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+from django.db.models import F
+
+
+def update_legacy_plan_next_invoice_date(
+    apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
+) -> None:
+    CustomerPlan = apps.get_model("corporate", "CustomerPlan")
+    CustomerPlan.TIER_SELF_HOSTED_LEGACY = 101
+
+    # For legacy plans, set next_invoice_date = end_date.
+    CustomerPlan.objects.filter(tier=CustomerPlan.TIER_SELF_HOSTED_LEGACY).update(
+        next_invoice_date=F("end_date")
+    )
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("corporate", "0034_customer_discount_required_tier"),
+    ]
+
+    operations = [
+        migrations.RunPython(update_legacy_plan_next_invoice_date),
+    ]

corporate/tests/test_stripe.py

@@ -7359,10 +7359,21 @@ class TestRemoteServerBillingFlow(StripeTestCase, RemoteServerTestCase):
         assert customer is not None
         plan = get_current_plan_by_customer(customer)
         assert plan is not None
+        self.assertEqual(plan.end_date, plan_end_date)
+        self.assertEqual(plan.next_invoice_date, plan_end_date)
+        self.assertEqual(plan.status, CustomerPlan.ACTIVE)
         self.assertEqual(
             self.remote_server.plan_type, RemoteZulipServer.PLAN_TYPE_SELF_MANAGED_LEGACY
         )
-        self.billing_session.make_end_of_cycle_updates_if_needed(plan, plan_end_date)
+
+        with mock.patch("stripe.Invoice.create") as invoice_create, time_machine.travel(
+            plan_end_date, tick=False
+        ):
+            send_server_data_to_push_bouncer(consider_usage_statistics=False)
+            invoice_plans_as_needed()
+            # The legacy plan is downgraded, no invoice created.
+            invoice_create.assert_not_called()
+
         plan.refresh_from_db()
         self.remote_server.refresh_from_db()
         self.assertEqual(self.remote_server.plan_type, RemoteZulipServer.PLAN_TYPE_SELF_MANAGED)