From 026eb37c2828a12f732b6fd3d8a908011184c2c6 Mon Sep 17 00:00:00 2001 From: Prakhar Pratyush Date: Mon, 22 Jan 2024 18:50:49 +0530 Subject: [PATCH] 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 | 15 ++++++---- ...35_update_legacy_plan_next_invoice_date.py | 28 +++++++++++++++++++ corporate/tests/test_stripe.py | 13 ++++++++- 3 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 corporate/migrations/0035_update_legacy_plan_next_invoice_date.py diff --git a/corporate/lib/stripe.py b/corporate/lib/stripe.py index aa52b01450..cc3852a7d8 100644 --- a/corporate/lib/stripe.py +++ b/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 diff --git a/corporate/migrations/0035_update_legacy_plan_next_invoice_date.py b/corporate/migrations/0035_update_legacy_plan_next_invoice_date.py new file mode 100644 index 0000000000..c636713a27 --- /dev/null +++ b/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), + ] diff --git a/corporate/tests/test_stripe.py b/corporate/tests/test_stripe.py index c9e0515162..cb7efdac6a 100644 --- a/corporate/tests/test_stripe.py +++ b/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)