billing: Fix the type annotation of Customer.stripe_customer_id.

This also fixes a bug in void_all_open_invoices function. If a realm
with a local Customer object but without an associated stripe.Customer
is passed to void_all_open_invoices, then the function will end up
voiding the last 10 invoices created by billing system instead of voiding
no invoices at all. This is because stripe.Invoice.list(customer=None)
return last 10 invoices across all customers.

But this bug won't cauuse any issue in production since
void_all_open_invoices can be only invoked from /support page. And we
show the option to void invoices in support page only if the realm
has a paid plan. And it's not really possible for a realm to have
a paid plan without having an associated stripe_customer_id. Plus I
went through the void events in stripe stream since the PR to add
void invoices was merged and there does not seems to be any suspicious
events.
This commit is contained in:
Vishnu KS 2021-06-18 19:10:45 +00:00 committed by Tim Abbott
parent 127d3de125
commit 1d579ec567
17 changed files with 66 additions and 6 deletions

View File

@ -319,6 +319,7 @@ def do_replace_payment_source(
) -> stripe.Customer:
customer = get_customer_by_realm(user.realm)
assert customer is not None # for mypy
assert customer.stripe_customer_id is not None # for mypy
stripe_customer = stripe_get_customer(customer.stripe_customer_id)
stripe_customer.source = stripe_token
@ -533,6 +534,8 @@ def process_initial_upgrade(
) -> None:
realm = user.realm
customer = update_or_create_stripe_customer(user, stripe_token=stripe_token)
assert customer.stripe_customer_id is not None # for mypy
charge_automatically = stripe_token is not None
free_trial = is_free_trial_offer_enabled()
@ -710,6 +713,11 @@ def update_license_ledger_if_needed(realm: Realm, event_time: datetime) -> None:
def invoice_plan(plan: CustomerPlan, event_time: datetime) -> None:
if plan.invoicing_status == CustomerPlan.STARTED:
raise NotImplementedError("Plan with invoicing_status==STARTED needs manual resolution.")
if not plan.customer.stripe_customer_id:
raise BillingError(
f"Realm {plan.customer.realm.string_id} has a paid plan without a Stripe customer."
)
make_end_of_cycle_updates_if_needed(plan, event_time)
if plan.invoicing_status == CustomerPlan.INITIAL_INVOICE_TO_BE_SENT:
@ -957,6 +965,8 @@ def void_all_open_invoices(realm: Realm) -> int:
customer = get_customer_by_realm(realm)
if customer is None:
return 0
if customer.stripe_customer_id is None:
return 0
invoices = stripe.Invoice.list(customer=customer.stripe_customer_id)
voided_invoices_count = 0
for invoice in invoices:

View File

@ -16,7 +16,7 @@ class Customer(models.Model):
"""
realm: Realm = models.OneToOneField(Realm, on_delete=CASCADE)
stripe_customer_id: str = models.CharField(max_length=255, null=True, unique=True)
stripe_customer_id: Optional[str] = models.CharField(max_length=255, null=True, unique=True)
sponsorship_pending: bool = models.BooleanField(default=False)
# A percentage, like 85.
default_discount: Optional[Decimal] = models.DecimalField(

View File

@ -1990,6 +1990,7 @@ class StripeTest(StripeTestCase):
monthly_plan.refresh_from_db()
self.assertEqual(monthly_plan.next_invoice_date, None)
assert customer.stripe_customer_id
[invoice0, invoice1, invoice2] = stripe.Invoice.list(customer=customer.stripe_customer_id)
[invoice_item0, invoice_item1] = invoice0.get("lines")
@ -2154,6 +2155,7 @@ class StripeTest(StripeTestCase):
self.assertEqual(annual_plan.next_invoice_date, add_months(self.next_month, 12))
self.assertEqual(annual_plan.invoicing_status, CustomerPlan.DONE)
assert customer.stripe_customer_id
[invoice0, invoice1] = stripe.Invoice.list(customer=customer.stripe_customer_id)
[invoice_item] = invoice0.get("lines")
@ -2632,12 +2634,17 @@ class StripeTest(StripeTestCase):
@mock_stripe()
def test_void_all_open_invoices(self, *mock: Mock) -> None:
iago = self.example_user("iago")
self.assertEqual(void_all_open_invoices(iago.realm), 0)
customer = update_or_create_stripe_customer(iago)
king = self.lear_user("king")
self.assertEqual(void_all_open_invoices(iago.realm), 0)
zulip_customer = update_or_create_stripe_customer(iago)
lear_customer = update_or_create_stripe_customer(king)
assert zulip_customer.stripe_customer_id
stripe.InvoiceItem.create(
currency="usd",
customer=customer.stripe_customer_id,
customer=zulip_customer.stripe_customer_id,
description="Zulip standard upgrade",
discountable=False,
unit_amount=800,
@ -2646,14 +2653,45 @@ class StripeTest(StripeTestCase):
stripe_invoice = stripe.Invoice.create(
auto_advance=True,
billing="send_invoice",
customer=customer.stripe_customer_id,
customer=zulip_customer.stripe_customer_id,
days_until_due=30,
statement_descriptor="Zulip Standard",
)
stripe.Invoice.finalize_invoice(stripe_invoice)
assert lear_customer.stripe_customer_id
stripe.InvoiceItem.create(
currency="usd",
customer=lear_customer.stripe_customer_id,
description="Zulip standard upgrade",
discountable=False,
unit_amount=800,
quantity=8,
)
stripe_invoice = stripe.Invoice.create(
auto_advance=True,
billing="send_invoice",
customer=lear_customer.stripe_customer_id,
days_until_due=30,
statement_descriptor="Zulip Standard",
)
stripe.Invoice.finalize_invoice(stripe_invoice)
self.assertEqual(void_all_open_invoices(iago.realm), 1)
invoices = stripe.Invoice.list(customer=customer.stripe_customer_id)
invoices = stripe.Invoice.list(customer=zulip_customer.stripe_customer_id)
self.assert_length(invoices, 1)
for invoice in invoices:
self.assertEqual(invoice.status, "void")
lear_stripe_customer_id = lear_customer.stripe_customer_id
lear_customer.stripe_customer_id = None
lear_customer.save(update_fields=["stripe_customer_id"])
self.assertEqual(void_all_open_invoices(king.realm), 0)
lear_customer.stripe_customer_id = lear_stripe_customer_id
lear_customer.save(update_fields=["stripe_customer_id"])
self.assertEqual(void_all_open_invoices(king.realm), 1)
invoices = stripe.Invoice.list(customer=lear_customer.stripe_customer_id)
self.assert_length(invoices, 1)
for invoice in invoices:
self.assertEqual(invoice.status, "void")
@ -3207,6 +3245,17 @@ class InvoiceTest(StripeTestCase):
with self.assertRaises(NotImplementedError):
invoice_plan(CustomerPlan.objects.first(), self.now)
def test_invoice_plan_without_stripe_customer(self) -> None:
self.local_upgrade(self.seat_count, True, CustomerPlan.ANNUAL)
plan = get_current_plan_by_realm(get_realm("zulip"))
assert plan and plan.customer
plan.customer.stripe_customer_id = None
plan.customer.save(update_fields=["stripe_customer_id"])
with self.assertRaisesRegex(
BillingError, "Realm zulip has a paid plan without a Stripe customer"
):
invoice_plan(plan, timezone_now())
@mock_stripe()
def test_invoice_plan(self, *mocks: Mock) -> None:
user = self.example_user("hamlet")

View File

@ -317,6 +317,7 @@ def billing_home(request: HttpRequest) -> HttpResponse:
)
renewal_cents = renewal_amount(plan, now)
charge_automatically = plan.charge_automatically
assert customer.stripe_customer_id is not None # for mypy
stripe_customer = stripe_get_customer(customer.stripe_customer_id)
if charge_automatically:
payment_method = payment_method_string(stripe_customer)