-
Notifications
You must be signed in to change notification settings - Fork 0
Add pagination support to GET /locations endpoint (#75) #93
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
Changes from 3 commits
9e8c47a
395a3ae
7e44355
63ff328
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Collaborator
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. Still looking for pagination tests for router and service |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| from fastapi import APIRouter, Depends | ||
| from fastapi import APIRouter, Depends, Query | ||
| from src.core.authentication import authenticate_admin, authenticate_staff_or_admin | ||
| from src.modules.location.location_model import ( | ||
| Location, | ||
|
|
@@ -13,16 +13,37 @@ | |
|
|
||
| @location_router.get("/", response_model=PaginatedLocationResponse) | ||
| async def get_locations( | ||
| page: int | None = Query(default=None, ge=1), | ||
| size: int | None = Query(default=None, ge=1), | ||
| location_service: LocationService = Depends(), | ||
| _=Depends(authenticate_staff_or_admin), | ||
| ): | ||
| locations = await location_service.get_locations() | ||
|
|
||
| total = len(locations) | ||
|
|
||
| # default — return everything | ||
| if page is None or size is None: | ||
| return PaginatedLocationResponse( | ||
| items=locations, | ||
| total_records=total, | ||
| page_number=1, | ||
| page_size=total, | ||
| total_pages=1, | ||
| ) | ||
|
|
||
| start = (page - 1) * size | ||
| end = start + size | ||
| sliced = locations[start:end] | ||
|
|
||
| total_pages = (total + size - 1) // size if total > 0 else 1 | ||
|
|
||
| return PaginatedLocationResponse( | ||
| items=locations, | ||
| total_records=len(locations), | ||
| page_number=1, | ||
| page_size=len(locations), | ||
| total_pages=1, | ||
| items=sliced, | ||
| total_records=total, | ||
| page_number=page, | ||
| page_size=size, | ||
| total_pages=total_pages, | ||
|
Comment on lines
+97
to
+121
Collaborator
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. This still does not mirror other modules. Other modules do not paginate in memory. You should have a service function that handles limit and offset, and then the router handles pagination metadata |
||
| ) | ||
|
|
||
|
|
||
|
|
@@ -61,7 +82,6 @@ async def update_location( | |
| ): | ||
| location = await location_service.get_location_by_id(location_id) | ||
|
|
||
| # If place_id changed, fetch new address data; otherwise use existing location | ||
| if location.google_place_id != data.google_place_id: | ||
| address_data = await location_service.get_place_details(data.google_place_id) | ||
| location_data = LocationData.from_address( | ||
|
|
||
|
Collaborator
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. All current changes in this file are outside the scope of this ticket |
Uh oh!
There was an error while loading. Please reload this page.