Add pagination to Client Grants, Resource Servers and Rules #110
Conversation
Codecov Report
@@ Coverage Diff @@
## master #110 +/- ##
==========================================
+ Coverage 93.46% 93.47% +0.01%
==========================================
Files 32 32
Lines 643 644 +1
==========================================
+ Hits 601 602 +1
Misses 42 42
Continue to review full report at Codecov.
|
auth0/v3/management/client_grants.py
Outdated
| params = {'audience': audience or None} | ||
| params = { | ||
| 'audience': audience or None, | ||
| 'per_page': per_page, |
There was a problem hiding this comment.
Match the order of the function params, IMHO
auth0/v3/management/client_grants.py
Outdated
| 'audience': audience or None, | ||
| 'per_page': per_page, | ||
| 'page': page, | ||
| 'include_totals': str(include_totals).lower() |
There was a problem hiding this comment.
This is a boolean argument in the docs ... why a string here?
There was a problem hiding this comment.
this are query params. Sending them as Strings is OK.
| c.all() | ||
|
|
||
| args, kwargs = mock_instance.get.call_args | ||
|
|
||
| self.assertEqual('https://domain/api/v2/client-grants', args[0]) | ||
| self.assertEqual(kwargs['params'], {'audience': None}) | ||
| self.assertEqual(kwargs['params'], { |
| self.assertEqual('https://domain/api/v2/client-grants', args[0]) | ||
| self.assertEqual(kwargs['params'], { | ||
| 'audience': 'http://domain.auth0.com/api/v2/', | ||
| 'per_page': None, |
There was a problem hiding this comment.
Minuscule but inconsistent order here (page first)
auth0/v3/management/client_grants.py
Outdated
| """Retrieves all client grants. | ||
|
|
||
| Args: | ||
| audience (str, optional): URL Encoded audience of a Resource Server | ||
| to filter | ||
| audience (str, optional): URL Encoded audience of a Resource Server |
| @@ -24,15 +24,28 @@ def _url(self, id=None): | |||
| return url + '/' + id | |||
| return url | |||
|
|
|||
| def all(self, audience=None): | |||
| def all(self, audience=None, page=None, per_page=None, include_totals=False): | |||
| """Retrieves all client grants. | |||
|
|
|||
| Args: | |||
There was a problem hiding this comment.
Should these blocks include a link to the v2 API doc?
There was a problem hiding this comment.
Only for create and update now.
auth0/v3/management/client_grants.py
Outdated
| """ | ||
|
|
||
| params = {'audience': audience or None} | ||
| params = { | ||
| 'audience': audience or None, |
There was a problem hiding this comment.
I'm guessing that you're just keeping what was there but why not just audience here? Could audience ever be a value where None is used or even makes sense?
There was a problem hiding this comment.
if audience is empty I don't want to send the parameter at all.
>>> audience = '1'
>>> audience or None
'1'
>>> audience = ''
>>> audience or None
>>>
auth0/v3/management/client_grants.py
Outdated
| page (int, optional): The result's page number (zero based). | ||
|
|
||
| per_page (int, optional): The amount of entries per page. | ||
| Default: 50. Max value: 100 |
There was a problem hiding this comment.
I thought we talked through including these values in docs and landed on not including them because they were apt to change.
There was a problem hiding this comment.
this is a copy paste issue and is present on most of the new endpoints implementing pagination that default to None. I'll check the rest as well. Nice catch :)
lbalmaceda
force-pushed
the
add-pagination
branch
from
8ae6c1b
to
b4b9e6f
Compare
What
This PR adds pagination support on MGMT API entities that were missing.
I kept this free from breaking changes by providing an
include_totals=Falsevalue (since if that's true, the response is an Object rather than an Array).Scope
How it was tested
✅ Unit tests asserting the params with which the function was called. Similar to the other pagination PR.