This is one step in fixing issue #88 #148

meatherly · 2020-08-07T19:32:50Z

It implements the GET '/v1/customers/{id}/sources' endpoint for a customer as described here: https://stripe.com/docs/api/cards/list

Also I've never written python before 😬

Things I still need to implement is supporting the object param but I'm not really sure where to start with that one. Please feel free to give me some feedback on this one.

It implements the /sources endpoint for a customer

adrienverge

Good job for a first Python contribution 👍

About the object parameter, it's OK to implement it in a future PR, if you want. For example you could take inspiration from source in:

localstripe/localstripe/resources.py

Lines 706 to 716 in f0a9400

    
           def _api_add_source(cls, id, source=None, **kwargs): 
        
               if kwargs: 
        
                   raise UserError(400, 'Unexpected ' + ', '.join(kwargs.keys())) 
        
               try: 
        
                   if type(source) is str: 
        
                       assert source[:4] in ('src_', 'tok_') 
        
                   else: 
        
                       assert type(source) is dict 
        
               except AssertionError: 
        
                   raise UserError(400, 'Bad request')

localstripe/resources.py

test.sh

meatherly · 2020-08-13T16:15:53Z

What would you suggest for filtering out sources on their type for the object type param? Would I just initialize a new List then filter out the obj.sources based on their prefix?

something like: (pseudo code)

returnList = List('/v1/customers/' + self.id + '/sources')
returnList._list = obj.sources.filter source.startswith(object)

meatherly · 2020-08-13T16:21:16Z

Looking at their docs it looks like object param can only be card or bank_account

adrienverge

Hey @meatherly,

To save time, I suggest looking at other functions (inside the Customer class, but also other listing methods from other classes). Then you could take inspiration from other parts of the code that list + filter, for example

localstripe/localstripe/resources.py

Lines 503 to 507 in f0a9400

    
           if customer: 
        
               li._list = [c for c in li._list if c.customer == customer] 
        
           if created and created.get('gt'): 
        
               li._list = [c for c in li._list 
        
                           if c.created > try_convert_to_int(created['gt'])]

or

localstripe/localstripe/resources.py

Lines 1267 to 1274 in f0a9400

    
           if customer is not None: 
        
               Customer._api_retrieve(customer)  # to return 404 if not existant 
        
               li._list = [i for i in li._list if i.customer == customer] 
        
           if subscription is not None: 
        
               # to return 404 if not existant 
        
               Subscription._api_retrieve(subscription) 
        
               li._list = [i for i in li._list if i.subscription == subscription] 
        
           li._list.sort(key=lambda i: i.date, reverse=True)

And inside Customer._api_list_sources, maybe something simple can be done, e.g.

localstripe/localstripe/resources.py

Lines 467 to 468 in f0a9400

    
           return Refund._api_list_all('/v1/charges/' + self.id + '/refunds', 
        
                                       charge=self.id)

? (I haven't tested)

Looking at their docs it looks like object param can only be card or bank_account

Right, so you need to validate the input. E.g.:

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

(again, you can look and copy existing code)

adrienverge · 2020-08-13T16:34:43Z

localstripe/resources.py

@@ -678,6 +678,11 @@ def _api_delete(cls, id):
        schedule_webhook(Event('customer.deleted', obj))
        return super()._api_delete(id)

+    @classmethod
+    def _api_retrieve_sources(cls, id):        
+        obj = cls._api_retrieve(id) # return 404 if does not exist


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

I believe so because the sources are set here:

localstripe/localstripe/resources.py

Line 629 in f5a2e0b

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

And then updated here:

localstripe/localstripe/resources.py

Line 739 in f5a2e0b

obj.sources._list.append(source_obj)

and:

localstripe/localstripe/resources.py

Line 754 in f5a2e0b

obj.sources._list.remove(source_obj)

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

localstripe/resources.py

meatherly · 2020-08-13T17:29:10Z

Man can I just say you're awesome for creating this and for being patient with me as I'm working on this!!

overriding Card._api_list_all to filter only customer cards if param is supplied

meatherly · 2020-08-13T19:31:12Z

Alright went off the beaten path did an override on Card._api_list_all to allow filtering of customer cards. If you're not a fan of this I can move that logic into the Customer._api_list_sources.

Let me know what you think

Also not sure why the build keeps failing 😬

adrienverge

Alright went off the beaten path did an override on Card._api_list_all to allow filtering of customer cards. If you're not a fan of this I can move that logic into the Customer._api_list_sources.

Let me know what you think

It's interesting, but it would provide a new API route to list all cards, for every customer (GET /v1/cards). But this API doesn't exist for real (for security reasons maybe?). So it would be better in Customer._api_list_sources 👍

Also not sure why the build keeps failing

You can see errors by clicking on the Travis link -- I think some of your code doesn't pass linting.

adrienverge · 2020-08-18T13:24:46Z

localstripe/resources.py

+        if customer:
+            Customer._api_retrieve(customer)  # to return 404 if not existant
+
+        li = super(Card, cls)._api_list_all(url, limit=limit)


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 😉

adrienverge · 2020-08-18T13:26:20Z

localstripe/resources.py

@@ -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)


Please don't include this in final code.

Good catch!

adrienverge · 2020-08-18T13:29:00Z

localstripe/resources.py

+        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)


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?

This is one step in fixing issue adrienverge#88

7ea29cc

It implements the /sources endpoint for a customer

adrienverge reviewed Aug 8, 2020

View reviewed changes

localstripe/resources.py Outdated Show resolved Hide resolved

test.sh Show resolved Hide resolved

removing redundant api call when listing sources

f5a2e0b

adrienverge reviewed Aug 13, 2020

View reviewed changes

Adding object=card param on customer _api_list_sources

892c1e5

overriding Card._api_list_all to filter only customer cards if param is supplied

adrienverge reviewed Aug 18, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This is one step in fixing issue #88 #148

This is one step in fixing issue #88 #148

meatherly commented Aug 7, 2020

adrienverge left a comment

meatherly commented Aug 13, 2020

meatherly commented Aug 13, 2020

adrienverge left a comment

adrienverge Aug 13, 2020

meatherly Aug 13, 2020

meatherly commented Aug 13, 2020

meatherly commented Aug 13, 2020 •

edited

Loading

adrienverge left a comment

adrienverge Aug 18, 2020

adrienverge Aug 18, 2020

meatherly Aug 19, 2020

adrienverge Aug 18, 2020

	def _api_add_source(cls, id, source=None, **kwargs):
	if kwargs:
	raise UserError(400, 'Unexpected ' + ', '.join(kwargs.keys()))

	try:
	if type(source) is str:
	assert source[:4] in ('src_', 'tok_')
	else:
	assert type(source) is dict
	except AssertionError:
	raise UserError(400, 'Bad request')

	if customer:
	li._list = [c for c in li._list if c.customer == customer]
	if created and created.get('gt'):
	li._list = [c for c in li._list
	if c.created > try_convert_to_int(created['gt'])]

	if customer is not None:
	Customer._api_retrieve(customer) # to return 404 if not existant
	li._list = [i for i in li._list if i.customer == customer]
	if subscription is not None:
	# to return 404 if not existant
	Subscription._api_retrieve(subscription)
	li._list = [i for i in li._list if i.subscription == subscription]
	li._list.sort(key=lambda i: i.date, reverse=True)

	return Refund._api_list_all('/v1/charges/' + self.id + '/refunds',
	charge=self.id)

This is one step in fixing issue #88 #148

Are you sure you want to change the base?

This is one step in fixing issue #88 #148

Conversation

meatherly commented Aug 7, 2020

adrienverge left a comment

Choose a reason for hiding this comment

meatherly commented Aug 13, 2020

meatherly commented Aug 13, 2020

adrienverge left a comment

Choose a reason for hiding this comment

adrienverge Aug 13, 2020

Choose a reason for hiding this comment

meatherly Aug 13, 2020

Choose a reason for hiding this comment

meatherly commented Aug 13, 2020

meatherly commented Aug 13, 2020 • edited Loading

adrienverge left a comment

Choose a reason for hiding this comment

adrienverge Aug 18, 2020

Choose a reason for hiding this comment

adrienverge Aug 18, 2020

Choose a reason for hiding this comment

meatherly Aug 19, 2020

Choose a reason for hiding this comment

adrienverge Aug 18, 2020

Choose a reason for hiding this comment

meatherly commented Aug 13, 2020 •

edited

Loading