pinax / pinax-stripe-light

a payments Django app for Stripe
MIT License
686 stars 286 forks source link

Subscriptions are not deleted on pinax-stripe when immediately deleted on stripe dashboard. #275

Open gusreyes01 opened 8 years ago

gusreyes01 commented 8 years ago

I believe the error is on line 152 of customers.py file.

        for subscription in cu["subscriptions"]["data"]:
            subscriptions.sync_subscription_from_stripe_data(customer, subscription)

Since customer subscription doesn't exist anymore ( it was immediately deleted ) it wont be synced, leaving a stale subscription object on the django side.

gusreyes01 commented 8 years ago

I solved it by validating there is subscription data on the stripe response and deleting all customer subscription objects if there is none.

    if cu["subscriptions"]["data"]:
        for subscription in cu["subscriptions"]["data"]:
            subscriptions.sync_subscription_from_stripe_data(customer, subscription)
    else:
        Subscription.objects.filter(customer=customer).delete()
ossanna16 commented 8 years ago

Thanks for reporting and solving this issue @gusreyes01! Can this be closed?

gusreyes01 commented 8 years ago

I'll try to post a PR later this week.

paltman commented 8 years ago

Thank you @gusreyes01. Looking forward to it.

johndark commented 8 years ago

Just got same problem with immediately subscription cancelation on stripe side. I see event in pinax_stripe_event table with customer.subscription.deleted, but still have subscription record in pinax_stripe_subscription for same user with canceled_at is NULL. Not sure if it's good idea to delete subscription record b/c its linked to invoiceitem and invoice and customer should see payment history information.

danolsen commented 8 years ago

I agree with @johndark. Stripe is sending information about the subscription being cancelled. The code should see this and update the subscription record in the database to show it was cancelled. That would be a clean way to keep things in sync with Stripe.

danolsen commented 8 years ago
if self.event.customer:
    customers.sync_customer(self.event.customer, self.event.customer.stripe_customer)

needs to be changed to:

subscriptions.sync_subscription_from_stripe_data(self.event.customer,  self.event.validated_message["data"]["object"])
if self.event.customer:
    customers.sync_customer(self.event.customer, self.event.customer.stripe_customer)

This is in the process_webhook method in the CustomerSubscriptionWebhook class.

That way when Stripe sends the information about the canceled subscription we actually process the subscription cancelation.

I am not sure if there are some bad side effects to doing this but with some initial testing it looks like it works correctly.

danolsen commented 8 years ago

279 The pull request to address this issue. Feedback is appreciated.

paltman commented 8 years ago

Thanks! I'll take a look very soon.

Sent from my iPhone

On Sep 17, 2016, at 11:33 AM, Dan Olsen notifications@github.com wrote:

279 The pull request to address this issue. Feedback is appreciated.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

danolsen commented 8 years ago

280 this is the updated pull request.

danolsen commented 8 years ago

When can we expect a new version to be rolled out to include this fix?

paltman commented 8 years ago

@danolsen within next several weeks but latest at end of October as part of the next Pinax distribution

tbell511 commented 7 years ago

I am having the same issue. Immediately canceling a subscription on the stripe dashboard, does not remove the subscription on django side. It's been about 8 months since this was brought up and was wondering if this was supposed to fixed? Thanks!

blueyed commented 7 years ago

To me it looked like the customer.subscription.deleted event is not handled at all?! (i.e. it does not have to be canceled immediately, but also days later) The event was processed (no exception), have not investigated otherwise yet.