-
-
Notifications
You must be signed in to change notification settings - Fork 96
[feature] Added OpenWispPagination class with suitable default values #587
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| from django.core.exceptions import ImproperlyConfigured | ||
|
|
||
| try: | ||
| from rest_framework.pagination import PageNumberPagination | ||
| except ImportError: # pragma: nocover | ||
| raise ImproperlyConfigured( | ||
| "Django REST Framework is required to use " | ||
| "this feature but it is not installed" | ||
| ) | ||
pushpitkamboj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| class OpenWispPagination(PageNumberPagination): | ||
| """Reusable pagination class with sensible defaults.""" | ||
|
|
||
| page_size = 10 | ||
| max_page_size = 100 | ||
| page_size_query_param = "page_size" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we create a simple DRF view in the test project for any model and then test the pagination on that view? The current tests look very synthetic and do not model real world scenario. I will suggest you to look for test in other modules, e.g. openwisp-notification / openwisp-radius on how we write tests for pagination. Here's one quick instance.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pushpit-kamboj I think @pandafy's suggestion is good, let's make sure the test project has an API view which uses this pagination class so we can also test manually, make sure the tests use that view. |
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.
These examples are good, but let's use a real URL, e.g.
/api/v1/controller/device/.