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

POLIO-1797 paginate dashboard endpoints #1902

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

quang-le
Copy link
Member

@quang-le quang-le commented Jan 6, 2025

endpoints consumed for ETL are not always paginated by default

Related JIRA tickets : POLIO-1797

Self proofreading checklist

  • Did I use eslint and black formatters
  • Is my code clear enough and well documented
  • Are my typescript files well typed
  • New translations have been added or updated if new strings have been introduced in the frontend
  • My migrations file are included
  • Are there enough tests
  • Documentation has been included (for new feature)

Doc

NA

Changes

  • group /polio/dashboards/ endpoints in same folder
  • Make all viewsets inherit EtlModelViewset
  • add test on default pagination query params

How to test

With Django API, test endpoints: pagination (limit=20&page=1) should be added to the API if not passed as queryparams directly

Print screen / video

NA

Notes

commit:

fix: paginate dashboard endpoints

refs: POLIO-1797

- group in same folder
- add test on default pagination query params
Copy link
Collaborator

@mathvdh mathvdh left a comment

Choose a reason for hiding this comment

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

I think it's very cleaning with the use of EtlModelViewset.
We might want to rename it to ForcedPaginatedViewset or something as it is not specific to ETL ( We force pagination in other places like vaccine stock documents) but that's a detail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants