Add consolidated water well details endpoint for the well show page#622
Add consolidated water well details endpoint for the well show page#622
Conversation
jirhiker
changed the title
There was a problem hiding this comment.
Pull request overview
Adds a new “water well details” API endpoint that returns a consolidated payload for rendering a well details page (well + related contacts/sensors/deployments/screens and recent groundwater-level observation/sample context).
Changes:
- Added
GET /thing/water-well/{thing_id}/detailsendpoint returning aWellDetailsResponse. - Implemented
get_well_details_payload(...)service to assemble well-related entities and recent groundwater-level observations/latest sample. - Added tests validating the payload shape, latest-observation selection, and 404 behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
api/thing.py |
Adds the new water well “details payload” route wired to the helper + response schema. |
services/well_details_helper.py |
Implements the SQLAlchemy queries to build the consolidated well details payload. |
schemas/well_details.py |
Introduces the Pydantic response model for the details payload. |
tests/test_thing.py |
Adds endpoint tests for payload contents, latest-sample selection, and not-found handling. |
| @router.get( | ||
| "/water-well/{thing_id}/details", | ||
| summary="Get water well details payload", | ||
| status_code=HTTP_200_OK, | ||
| ) | ||
| async def get_well_details( | ||
| user: viewer_dependency, | ||
| thing_id: int, | ||
| session: session_dependency, | ||
| request: Request, | ||
| ) -> WellDetailsResponse: | ||
| """ | ||
| Retrieve the consolidated payload needed to render the well details page. | ||
| Hydrograph series and map layer loading are intentionally handled separately. | ||
| """ | ||
| return get_well_details_payload( | ||
| session=session, | ||
| request=request, | ||
| thing_id=thing_id, | ||
| ) |
There was a problem hiding this comment.
PR title/description refer to GCS upload handling, but this change introduces a new water well details endpoint and related schemas/tests. Please update the PR title and the filled-in PR description to match the actual scope so reviewers can evaluate the right risk/impact.
|
|
||
| groundwater_parameter_id = ( | ||
| session.query(Parameter) | ||
| .filter(Parameter.parameter_name == "groundwater level") |
There was a problem hiding this comment.
Parameter is unique on (parameter_name, matrix) (and this codebase also distinguishes by parameter_type in places), so filtering only by parameter_name and calling .one() can raise MultipleResultsFound if another matrix for "groundwater level" is ever added. Prefer filtering by the expected matrix (e.g., matrix == "groundwater") and/or parameter_type, or reusing a shared helper that returns the groundwater-level parameter id with a clear error when missing.
| .filter(Parameter.parameter_name == "groundwater level") | |
| .filter( | |
| Parameter.parameter_name == "groundwater level", | |
| Parameter.matrix == "groundwater", | |
| ) |
| well_screens: list[WellScreenResponse] = Field(default_factory=list) | ||
| recent_groundwater_level_observations: list[GroundwaterLevelObservationResponse] = ( |
There was a problem hiding this comment.
WellScreenResponse includes a nested thing: WellResponse, while this payload already returns the well as well. That makes the response redundant (the well will be repeated for every screen) and can significantly increase payload size. Consider introducing a slim well-screen response schema for this endpoint that omits the nested thing (or make thing optional) and update WellDetailsResponse.well_screens accordingly.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e9b5af5dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| status_code=HTTP_200_OK, | ||
| ) | ||
| async def get_well_details( | ||
| user: viewer_dependency, |
There was a problem hiding this comment.
This endpoint is guarded by viewer_dependency, but it returns recent_groundwater_level_observations and latest_field_event_sample, which exposes groundwater-observation data through /thing/water-well/{thing_id}/details to users who may not have AMP permissions. In this repo, groundwater observation routes (for example, /observation/groundwater-level in api/observation.py) require amp_viewer_dependency, so this change creates an authorization bypass for any principal that has Viewer but not AMPViewer.
Useful? React with 👍 / 👎.
| .filter(Parameter.parameter_name == "groundwater level") | ||
| .one() |
There was a problem hiding this comment.
The lookup Parameter.parameter_name == "groundwater level" is not unique by schema (the table enforces uniqueness on (parameter_name, matrix)), so this can raise MultipleResultsFound and turn the whole details endpoint into a 500 as soon as another matrix/type variant with the same name is added. Filtering by matrix/type (or otherwise handling multiple matches) avoids a production-breaking failure tied to normal data growth.
Useful? React with 👍 / 👎.
Summary
Adds a singleton endpoint for loading the main water well details page payload without changing hydrograph or map loading behavior.
New endpoint:
GET /thing/water-well/{thing_id}/detailsThe endpoint consolidates the non-asset, non-hydrograph portions of the current frontend fan-out into one response.
What Changed
WellDetailsResponseschema.well_details_helperservice to gather the page payload.GET /thing/water-well/{thing_id}/detailsunder the existingthingrouter.Response Shape
The new endpoint returns:
wellcontactssensorsdeploymentswell_screensrecent_groundwater_level_observationslatest_field_event_sampleExplicitly Not Included
Kept separate on purpose:
This matches the current frontend requirement to keep hydrograph and map loading out of the main well data request, and keeps asset loading separate as requested.
Why
The current well details page fans out across multiple requests for page-scoped data. This endpoint reduces that fan-out for the main content while preserving the heavier or independently refreshed requests.
Validation
blackon touched files.