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.

Sign up for GitHub

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

Jump to bottom

Fetch routes do not limit number of items for non-admin users #276

Open
blenzi opened this issue Aug 7, 2023 · 1 comment · May be fixed by #277
Open

Fetch routes do not limit number of items for non-admin users #276

blenzi opened this issue Aug 7, 2023 · 1 comment · May be fixed by #277
Labels
type: fix Something isn't working

Comments

@blenzi
Copy link
Contributor

blenzi commented Aug 7, 2023

The routes that fetch objects from the db behave differently for admin and non-admin users. For admin, they are limited to 50 items while for non-admin users all the items are returned. For example: https://github.com/pyronear/pyro-api/blob/main/src/app/api/endpoints/alerts.py#L135
@blenzi blenzi added the type: fix Something isn't working label Aug 7, 2023
blenzi pushed a commit that referenced this issue Aug 7, 2023
feat: add limit and offset parameters to routes that fetch lists from db
95cd027 
- add limit and offset parameters with descriptions, to all routes that fetch lists from db (fix #243, #250)
- use crud.fetch_all in all the routes above, to limit the number of items returned for non-admin users (fix #276)
- forbid /installation/site-devices to access site outside of group, instead of filtering devices
- improve test_installations.py (test_get_active_devices_on_site)
- Dockerfile-dev: update FROM image name
@blenzi blenzi linked a pull request Aug 7, 2023 that will close this issue
Open
@frgfm
Copy link
Member

frgfm commented Oct 8, 2023

Indeed, that's a mistake but that's specific to this route because we don't use a crud function here. Hopefully, not too many routes are affected, thanks for spotting it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: fix Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add limit and offset parameters to routes that fetch lists from db pyronear/pyro-api
2 participants
@blenzi @frgfm