adrienverge / localstripe

A fake but stateful Stripe server that you can run locally, for testing purposes.
GNU General Public License v3.0
196 stars 58 forks source link

Fix incorrect invoice lines on subscription plan upgrade #137

Closed adrienverge closed 4 years ago

adrienverge commented 4 years ago

InvoiceItem: Include 'plan' and 'subscription' info if available

That's what real Stripe does, e.g. on a plan upgrade:

[
  {
    "id": "il_1GbU4aKxWWXl12QBOzUyETIM",
    "type": "invoiceitem",
    "subscription_item": "si_H9nhRadSKnHfBu",
    "invoice_item": "ii_1GbU4bKxWWXl12QBhGX2ce3u",
    "amount": -1000,
    "description": "Unused time on Plan after 24 Apr 2020",
    "plan": { "id": "monthly-plan", ... },
    "proration": true
  },
  {
    "id": "il_1GbU4bKxWWXl12QCMJnxK6Ws",
    "type": "subscription",
    "subscription_item": "si_H9nhRadSKnHfBu",
    "amount": 9900,
    "currency": "eur",
    "description": "1 × Plan (at €99.00 / year)",
    "plan": { "id": "yearly-plan", ... },
    "proration": false
  }
]

Invoice: Create SubscriptionItems (not InvoiceItems) for upcoming

This fixes a bug when a subscription is upgraded to another plan with a different billing cycle (e.g. monthly to yearly). In such a case, the invoice is generated when the user creates the invoice then calls API /pay, but in this case the invoice is made from Invoice._get_next_invoice(), not from Subscription._create_invoice().

Subsciption items need to be created as SubscriptionItem, not InvoiceItem (since commit a80dda2 "feat: Support invoice line items (il_) from Stripe API 2019-12-03").

H--o-l commented 4 years ago

Hey @adrienverge !

I propose to add the following unit test inside one of the two commit (feel free to change or improve it!):

--- a/test.sh
+++ b/test.sh
@@ -469,6 +469,29 @@ curl -sSf -u $SK: $HOST/v1/subscriptions/$sub \

 curl -sSf -u $SK: $HOST/v1/invoices?customer=$cus

+cus=$(curl -sSf -u $SK: $HOST/v1/customers \
+           -d description='This customer will switch from a yearly to another yearly plan' \
+           -d email=switch@bar.com \
+      | grep -oE 'cus_\w+' | head -n 1)
+
+curl -sSf -u $SK: $HOST/v1/customers/$cus/sources \
+     -d source=$tok
+
+sub=$(curl -sSf -u $SK: $HOST/v1/subscriptions \
+           -d customer=$cus \
+           -d items[0][plan]=basique-annuel)
+sub_id=$(echo "$sub" | grep -oE 'sub_\w+' | head -n 1)
+sub_item_id=$(echo "$sub" | grep -oE 'si_\w+' | head -n 1)
+
+sub=$(curl -sSf -u $SK: $HOST/v1/subscriptions/$sub_id \
+           -d items[0][plan]=pro-annuel \
+           -d items[0][id]=$sub_item_id)
+
+in=$(curl -sSf -u $SK: $HOST/v1/invoices \
+          -d customer=$cus)
+echo $in | grep "Abonnement PRO (annuel)"
+echo $in | grep "Abonnement basique (annuel)"
+
 cus=$(curl -sSf -u $SK: $HOST/v1/customers \
            -d email=john.malkovich@example.com \
       | grep -oE 'cus_\w+' | head -n 1)

I checked that before the PR it fails, and on it, it works.

adrienverge commented 4 years ago

I propose to add the following unit test inside one of the two commit (feel free to change or improve it!):

So nice!!!

I took it as is, and just changed this for a better display (otherwise it's very hard to read and debug), otherwise, thanks :ok_hand:

-echo $in | grep "Abonnement PRO (annuel)"
-echo $in | grep "Abonnement basique (annuel)"
+grep -q "Abonnement PRO (annuel)" <<<"$in"
+grep -q "Abonnement basique (annuel)" <<<"$in"