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

This is one step in fixing issue #88 #148

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions localstripe/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,22 @@ def _attaching_is_declined(self):
def _charging_is_declined(self):
return PaymentMethod._charging_is_declined(self)

@classmethod
def _api_list_all(cls, url, customer=None, limit=10):
try:
if customer is not None:
assert type(customer) is str and customer.startswith('cus_')
except AssertionError:
raise UserError(400, 'Bad request')

if customer:
Customer._api_retrieve(customer) # to return 404 if not existant

li = super(Card, cls)._api_list_all(url, limit=limit)
Copy link
Owner

Choose a reason for hiding this comment

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

I think super() is enough.

Anyway, if you move this code inside the Customer class, you can simply call Card._api_list_all() which is even clearer 😉

if customer:
li._list = [c for c in li._list if c.customer == customer]
return li


class Charge(StripeObject):
object = 'charge'
Expand Down Expand Up @@ -678,6 +694,28 @@ def _api_delete(cls, id):
schedule_webhook(Event('customer.deleted', obj))
return super()._api_delete(id)

@classmethod
def _api_list_sources(cls, id, object=None, limit=10):
print(object)
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't include this in final code.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch!

try:
if object is not None:
assert object in ('card', 'bank_account')
except AssertionError:
raise UserError(400, 'Bad request')

obj = cls._api_retrieve(id) # return 404 if does not exist
Copy link
Owner

Choose a reason for hiding this comment

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

Does this code really retrieve a list of sources that belong to a customer?

Copy link
Author

Choose a reason for hiding this comment

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

I believe so because the sources are set here:

self.sources = List('/v1/customers/' + self.id + '/sources')

And then updated here:

obj.sources._list.append(source_obj)

and:
obj.sources._list.remove(source_obj)

So I'm just returning that value from the customer object


url = '/v1/customers/' + obj.id + '/sources'

if object is 'card':
return Card._api_list_all(url, customer=obj.id)

#TODO: implement bank accounts
if object is 'bank_account':
return List(url, limit=limit)
Comment on lines +708 to +715
Copy link
Owner

Choose a reason for hiding this comment

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

I propose to simplify all this and don't use the object argument. Just check it, but don't use it, and return all sources. What do you think?


return obj.sources

@classmethod
def _api_retrieve_source(cls, id, source_id, **kwargs):
if kwargs:
Expand Down Expand Up @@ -818,6 +856,9 @@ def _api_update_subscription(cls, id, subscription_id, **data):


extra_apis.extend((
# Retrieve list of sources:
('GET', '/v1/customers/{id}/sources', Customer._api_list_sources),
# Add a source
('POST', '/v1/customers/{id}/sources', Customer._api_add_source),
# Retrieve single source by id:
('GET', '/v1/customers/{id}/sources/{source_id}',
Expand Down
14 changes: 14 additions & 0 deletions test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,20 @@ res=$(
| grep -oE $card)
[ -n "$res" ]

# observe new card in customer sources response
res=$(
curl -sSf -u $SK: $HOST/v1/customers/$cus/sources \
| grep -oE $card)
[ -n "$res" ]
meatherly marked this conversation as resolved.
Show resolved Hide resolved


# observe new card in customer sources response when requesting object=card
res=$(
curl -sSfG -u $SK: $HOST/v1/customers/$cus/sources \
-d object=card \
| grep -oE $card)
[ -n "$res" ]

# delete the card
curl -sSf -u $SK: $HOST/v1/customers/$cus/sources/$card \
-X DELETE
Expand Down