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

manage.py sync_customers broken - AttributeError: invoices #582

Open
mcastle opened this issue Aug 7, 2018 · 5 comments · Fixed by #622
Open

manage.py sync_customers broken - AttributeError: invoices #582

mcastle opened this issue Aug 7, 2018 · 5 comments · Fixed by #622

Comments

@mcastle
Copy link
Contributor

mcastle commented Aug 7, 2018

Running manage.py sync_customers is broken and throws an AttributeError: invoices.

File "/pinax/stripe/actions/invoices.py", line 134, in sync_invoices_for_customer
    for invoice in customer.stripe_customer.invoices().data:
  File "/stripe/stripe_object.py", line 85, in __getattr__
    raise AttributeError(*err.args)
AttributeError: invoices

I first noticed this after upgrading to pinax-stripe 4.4.0 and stripe-python 2.4.0.

@mcastle
Copy link
Contributor Author

mcastle commented Aug 7, 2018

Reverting to pinax-stripe 4.3.1 and stripe-python 1.84.2 fixes issue.

@bonidjukic
Copy link

bonidjukic commented Sep 9, 2018

@mcastle If you're interested, this is a quick hack we came up with to be able to support latest pinax-stripe and stripe-python versions: bonidjukic@80e618e

Please keep in mind that we only tested the above mentioned sync_customers, we still have to test if this works generally.

BTW, this is the stripe-python "deprecation" commit which seems to have introduced regressions in pinax-stripe: stripe/stripe-python@d416e9e

@blueyed
Copy link
Contributor

blueyed commented Sep 9, 2018

@bonidjukic
Please consider creating a PR, then it should also be visible if this code is covered in tests (probably not).

@streeter
Copy link
Contributor

streeter commented Dec 5, 2018

The problem is that the tests mock the call out to invoices.sync_invoices_for_customer. Or, in the tests for the function itself, that is also mocked. The mocks aren't mocking a specific version of the stripe-python library, they are mocking whatever you want them to.

Basically, a fix would involve replicating the code to fetch all the invoices for the given user, doing the same thing the removed code did: stripe/stripe-python@d416e9e#diff-0399a8a9696e3c2e5288d50b66a3c81cL30

def get_invoices_for_customer(customer):
	params = {customer_id: customer.id}
	invoices = Invoice.list(customer.api_key, **params)
	return invoices

@streeter
Copy link
Contributor

streeter commented Dec 5, 2018

Here's actually how I solved it, which I also think should be backwards compatible with the previous version of the stripe-python library:

def sync_invoices_for_customer(customer):
    stripe_customer = customer.stripe_customer
    stripe_invoices = stripe.Invoice.list(customer=stripe_customer.id)
    for invoice in stripe_invoices.data:
        sync_invoice_from_stripe_data(invoice, send_receipt=False)

streeter added a commit to streeter/pinax-stripe that referenced this issue Dec 5, 2018
The invoices method was removed on the stripe.Customer object in stripe/stripe-python@d416e9e

This replaces that same logic to fetch the invoices and sync them and
does it within the method itself. This should be backwards compatible
with stripe-python < 2.0 as well as the 2.0 and up release.

Fixes pinax#582
streeter added a commit to streeter/pinax-stripe that referenced this issue Dec 5, 2018
The invoices method was removed on the stripe.Customer object in stripe/stripe-python@d416e9e

This replaces that same logic to fetch the invoices and sync them and
does it within the method itself. This should be backwards compatible
with stripe-python < 2.0 as well as the 2.0 and up release.

Fixes pinax#582
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants