-
Notifications
You must be signed in to change notification settings - Fork 286
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
fix charges/invoices listing by using stripe API #630
base: original
Are you sure you want to change the base?
fix charges/invoices listing by using stripe API #630
Conversation
I see no backwards incompatible changes that should be documented in this PR. |
Codecov Report
@@ Coverage Diff @@
## master #630 +/- ##
========================================
- Coverage 99.21% 99% -0.21%
========================================
Files 33 33
Lines 1911 1911
Branches 175 175
========================================
- Hits 1896 1892 -4
- Misses 7 9 +2
- Partials 8 10 +2
Continue to review full report at Codecov.
|
def test_sync_invoices_for_customer(self, RetreiveMock, SyncMock): | ||
RetreiveMock().invoices().data = [Mock()] | ||
def test_sync_invoices_for_customer(self, RetrieveMock, SyncMock): | ||
RetrieveMock.return_value = [Mock()] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is correct. I believe that stripe.Customer.retrieve
isn't actually called anymore. You would need to keep the @patch("pinax.stripe.actions.invoices.sync_invoice_from_stripe_data")
and remove the @patch("stripe.Customer.retrieve")
patch, which would mean that stripe.Invoice.auto_paging_iter
is set to the RetrieveMock
and pinax.stripe.actions.invoices.sync_invoice_from_stripe_data
is set to the SyncMock
.
@patch("stripe.Customer.retrieve") | ||
def test_sync_charges_for_customer(self, RetreiveMock, SyncMock): | ||
RetreiveMock().charges().data = [Mock()] | ||
def test_sync_charges_for_customer(self, RetrieveMock, SyncMock): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment above should also apply here.
I had attempted to fix the invoices method removal (in #622), but didn't know about the |
What's this PR do?
As described in #623
invoices
is no longer available in the pulled stripe customer data, thus pulling invoices list from the stripe API separately with customer id. Suggested by @polski-g in his comment: https://github.com/pinax/pinax-stripe/issues/623\#issuecomment-455814540Same goes for
charges
as noted by the user in the comment.Any background context you want to provide?
What ticket or issue # does this fix?
Closes #623
Definition of Done (check if considered and/or addressed):