Skip to content

Commit

Permalink
Better handling of syncing deleted customers
Browse files Browse the repository at this point in the history
Credit to @tasn for work done in PR pinax#325 - this PR adds tests

Fixes pinax#268
  • Loading branch information
sprocket-9 committed Sep 5, 2017
1 parent 8a47640 commit f460b62
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 6 deletions.
18 changes: 15 additions & 3 deletions pinax/stripe/actions/customers.py
Expand Up @@ -84,6 +84,12 @@ def get_customer_for_user(user):
return next(iter(models.Customer.objects.filter(user=user)), None)


def purge_local(customer):
customer.user = None
customer.date_purged = timezone.now()
customer.save()


def purge(customer):
"""
Deletes the Stripe customer data and purges the linking of the transaction
Expand All @@ -99,9 +105,7 @@ def purge(customer):
# The exception was thrown because the customer was already
# deleted on the stripe side, ignore the exception
raise
customer.user = None
customer.date_purged = timezone.now()
customer.save()
purge_local(customer)


def link_customer(event):
Expand Down Expand Up @@ -151,8 +155,16 @@ def sync_customer(customer, cu=None):
customer: a Customer object
cu: optionally, data from the Stripe API representing the customer
"""
if customer.date_purged is not None:
return

if cu is None:
cu = customer.stripe_customer

if cu.get('deleted', False):
purge_local(customer)
return

customer.account_balance = utils.convert_amount_for_db(cu["account_balance"], cu["currency"])
customer.currency = cu["currency"] or ""
customer.delinquent = cu["delinquent"]
Expand Down
5 changes: 3 additions & 2 deletions pinax/stripe/management/commands/sync_customers.py
Expand Up @@ -31,5 +31,6 @@ def handle(self, *args, **options):
# This user doesn't exist (might be in test mode)
continue

invoices.sync_invoices_for_customer(customer)
charges.sync_charges_for_customer(customer)
if customer.date_purged is None:
invoices.sync_invoices_for_customer(customer)
charges.sync_charges_for_customer(customer)
25 changes: 25 additions & 0 deletions pinax/stripe/tests/test_actions.py
Expand Up @@ -1175,6 +1175,31 @@ def test_sync_customer_no_cu_provided(self, SyncPaymentSourceMock, SyncSubscript
self.assertTrue(SyncPaymentSourceMock.called)
self.assertTrue(SyncSubscriptionMock.called)

@patch("pinax.stripe.actions.customers.purge_local")
@patch("pinax.stripe.actions.subscriptions.sync_subscription_from_stripe_data")
@patch("pinax.stripe.actions.sources.sync_payment_source_from_stripe_data")
@patch("stripe.Customer.retrieve")
def test_sync_customer_purged_locally(self, RetrieveMock, SyncPaymentSourceMock, SyncSubscriptionMock, PurgeLocalMock):
self.customer.date_purged = timezone.now()
customers.sync_customer(self.customer)
self.assertFalse(RetrieveMock.called)
self.assertFalse(SyncPaymentSourceMock.called)
self.assertFalse(SyncSubscriptionMock.called)
self.assertFalse(PurgeLocalMock.called)

@patch("pinax.stripe.actions.customers.purge_local")
@patch("pinax.stripe.actions.subscriptions.sync_subscription_from_stripe_data")
@patch("pinax.stripe.actions.sources.sync_payment_source_from_stripe_data")
@patch("stripe.Customer.retrieve")
def test_sync_customer_purged_remotely_not_locally(self, RetrieveMock, SyncPaymentSourceMock, SyncSubscriptionMock, PurgeLocalMock):
RetrieveMock.return_value = dict(
deleted=True
)
customers.sync_customer(self.customer)
self.assertFalse(SyncPaymentSourceMock.called)
self.assertFalse(SyncSubscriptionMock.called)
self.assertTrue(PurgeLocalMock.called)

@patch("pinax.stripe.actions.invoices.sync_invoice_from_stripe_data")
@patch("stripe.Customer.retrieve")
def test_sync_invoices_for_customer(self, RetreiveMock, SyncMock):
Expand Down
19 changes: 19 additions & 0 deletions pinax/stripe/tests/test_commands.py
Expand Up @@ -161,3 +161,22 @@ def test_sync_customers_with_unicode_username(self, SyncChargesMock, SyncInvoice
self.assertEqual(SyncChargesMock.call_count, 1)
self.assertEqual(SyncInvoicesMock.call_count, 1)
self.assertEqual(SyncMock.call_count, 1)

@patch("stripe.Customer.retrieve")
@patch("pinax.stripe.actions.invoices.sync_invoices_for_customer")
@patch("pinax.stripe.actions.charges.sync_charges_for_customer")
def test_sync_customers_with_remotely_purged_customer(self, SyncChargesMock, SyncInvoicesMock, RetrieveMock):
customer = Customer.objects.create(
user=self.user,
stripe_id="cus_XXXXX"
)

RetrieveMock.return_value = dict(
deleted=True
)

management.call_command("sync_customers")
self.assertIsNone(Customer.objects.get(stripe_id=customer.stripe_id).user)
self.assertIsNotNone(Customer.objects.get(stripe_id=customer.stripe_id).date_purged)
self.assertEqual(SyncChargesMock.call_count, 0)
self.assertEqual(SyncInvoicesMock.call_count, 0)
2 changes: 1 addition & 1 deletion pinax/stripe/webhooks.py
Expand Up @@ -247,7 +247,7 @@ class CustomerDeletedWebhook(Webhook):
description = "Occurs whenever a customer is deleted."

def process_webhook(self):
customers.purge(self.event.customer)
customers.purge_local(self.event.customer)


class CustomerUpdatedWebhook(Webhook):
Expand Down

1 comment on commit f460b62

@tasn
Copy link

@tasn tasn commented on f460b62 Sep 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,

Thanks for getting to it. Would be better if you kept my commits making the changes (just cherry-pick them) and add your commits adding the tests.

Thanks,
Tom.

Edit: I initially commented on the commit, now I see it shows here in the PR. To maintainers/author: please first include my commit making the actual change, and on top of that include the commit adding the tests so attribution is properly maintained.

Thanks.

Please sign in to comment.