Skip to content
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

webhooks: customer.created should create local Customer #467

Closed
blueyed opened this issue Nov 2, 2017 · 9 comments · Fixed by #531 or lock8/pinax-stripe#108
Closed

webhooks: customer.created should create local Customer #467

blueyed opened this issue Nov 2, 2017 · 9 comments · Fixed by #531 or lock8/pinax-stripe#108

Comments

@blueyed
Copy link
Contributor

blueyed commented Nov 2, 2017

When you create a customer through the Stripe Dashboard, it sends the "customer.created" event, but pinax-stripe does not create the local Customer object then.

Do you agree?

@blueyed
Copy link
Contributor Author

blueyed commented Nov 2, 2017

I think this could get handled in link_customer then, where it should also get ensured - i.e. a event with customer info should always link a customer, and create it if it does not exist.

@blueyed
Copy link
Contributor Author

blueyed commented Nov 2, 2017

@blueyed
Copy link
Contributor Author

blueyed commented Nov 2, 2017

Affects other events also, e.g. customer.subscription.updated.
We should allow for a customer to be deleted locally always, right?

@blueyed
Copy link
Contributor Author

blueyed commented Nov 13, 2017

@paltman @lukeburden
Any input on this?

@lukeburden
Copy link
Contributor

If there's a customer, I think the local cache should know about it - so I'd be for properly adding a customer on receipt of a customer.created event. Worth noting it will be orphaned though, in the sense that it won't be associated with a local user.

I also agree that if a customer is deleted on Stripe's side, this deletion should be reflected in the local cache.

@blueyed
Copy link
Contributor Author

blueyed commented Nov 14, 2017

What do you think about doing this in link_customer in general?

@lukeburden
Copy link
Contributor

That seems reasonable to me.

We might need to see how it performs practically though? In one project I have, I currently get some weird race conditions around customer existence for source creation when making charges, which I haven't been able to work out yet. So I'm a little hesitant to create a race condition between:

  • an event being processed creating a customer
  • a web request making a charge that also needs to create a customer

But I think that should be addressed separately and that it's reasonable to create a customer record in the link_customer function.

Unrelated: @blueyed, would you be open to jumping onto the Pinax Slack team? @paltman and I regularly expedite discussions with a quick chat there, and it'd be cool to pick your brains on a couple of other projects etc.

@lukeburden
Copy link
Contributor

@ticosax you'd be welcome to join the Slack team too, if you wanted!

@blueyed
Copy link
Contributor Author

blueyed commented Nov 15, 2017

Just for reference, we use the following decorator, which could be adopted:

def recreate_missing_stripe_customer(f):
    @functools.wraps(f)
    def wrapper(*args, **kwargs):
        callargs = inspect.getcallargs(f, *args, **kwargs)
        user = callargs['self']
        foo = callargs['foo']

        try:
            return f(*args, **kwargs)
        except stripe.error.InvalidRequestError as exc:
            if (exc.param == 'customer' and
                    exc.args[0].startswith('No such customer:')):
                customer, _ = user.get_or_create_customer(foo)
                customer.delete()
                return f(*args, **kwargs)
            raise exc
    return wrapper

@paltman paltman added this to the Rosie milestone Nov 23, 2017
@paltman paltman modified the milestones: Rosie, December 2017 Sprint Dec 1, 2017
blueyed added a commit to blueyed/pinax-stripe that referenced this issue Dec 4, 2017
This also fixes some event fixtures.

Fixes pinax#467.
blueyed added a commit to blueyed/pinax-stripe that referenced this issue Dec 4, 2017
This also fixes some event fixtures.

Fixes pinax#467.
@blueyed blueyed reopened this Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants