New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of syncing deleted customers #358

Merged
merged 4 commits into from Sep 21, 2017

Conversation

Projects
None yet
3 participants
@spr0cketeer
Contributor

spr0cketeer commented Sep 5, 2017

What's this PR do?

Modifies deleted customer webhook to purge customer locally and not attempt to delete customer again on Stripe.

Modifies customer syncing actions and management command to handle situation where customer is deleted remotely but not locally.

Any background context you want to provide?

What ticket or issue # does this fix?

Closes #268

Definition of Done (check if considered and/or addressed):

  • Are all backwards incompatible changes documented in this PR?
  • Have all new dependencies been documented in this PR?
  • Has the appropriate documentation been updated (if applicable)?
  • Have you written tests to prove this change works (if applicable)?
@spr0cketeer

This comment has been minimized.

Show comment
Hide comment
@spr0cketeer

spr0cketeer Sep 5, 2017

Contributor

@tasn used cherry pick on your commits and resubmitted this PR - does this look OK to you?

Contributor

spr0cketeer commented Sep 5, 2017

@tasn used cherry pick on your commits and resubmitted this PR - does this look OK to you?

@tasn

This comment has been minimized.

Show comment
Hide comment
@tasn

tasn Sep 5, 2017

Contributor

This looks great. Thank you.

Also, extra thank you for adding the tests to finally get this PR merged in!

Edit: forgot to mention, I'll now abandon my PR in favour of this one.

Contributor

tasn commented Sep 5, 2017

This looks great. Thank you.

Also, extra thank you for adding the tests to finally get this PR merged in!

Edit: forgot to mention, I'll now abandon my PR in favour of this one.

@spr0cketeer

This comment has been minimized.

Show comment
Hide comment
@spr0cketeer

spr0cketeer Sep 5, 2017

Contributor

@tasn great! Learned a lot of new git skills doing this too 👍

Contributor

spr0cketeer commented Sep 5, 2017

@tasn great! Learned a lot of new git skills doing this too 👍

@tasn

This comment has been minimized.

Show comment
Hide comment
@tasn

tasn Sep 5, 2017

Contributor

I was going to explain to you how to do it, but since you said you were new to git, I thought it may be too complex.

Glad you managed to do it on your own and learn new tricks on the way. Git is a powerful tool, you can do very cool things with it.

Contributor

tasn commented Sep 5, 2017

I was going to explain to you how to do it, but since you said you were new to git, I thought it may be too complex.

Glad you managed to do it on your own and learn new tricks on the way. Git is a powerful tool, you can do very cool things with it.

tasn and others added some commits Mar 10, 2017

@spr0cketeer

This comment has been minimized.

Show comment
Hide comment
@spr0cketeer

spr0cketeer Sep 21, 2017

Contributor

@paltman added a commit to use double quotes so this PR is good to go.

Contributor

spr0cketeer commented Sep 21, 2017

@paltman added a commit to use double quotes so this PR is good to go.

@paltman paltman merged commit 158948b into pinax:master Sep 21, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@tasn

This comment has been minimized.

Show comment
Hide comment
@tasn

tasn Sep 22, 2017

Contributor

Thanks everyone. Happy it's finally in!

Contributor

tasn commented Sep 22, 2017

Thanks everyone. Happy it's finally in!

@paltman paltman added this to the Samwise milestone Oct 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment