Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions api/thing.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
UpdateThingIdLink,
UpdateWellScreen,
)
from schemas.well_details import WellDetailsResponse
from services.crud_helper import model_patcher, model_adder, model_deleter
from services.exceptions_helper import PydanticStyleException
from services.lexicon_helper import get_terms_by_category
Expand All @@ -68,6 +69,7 @@
modify_well_descriptor_tables,
WELL_DESCRIPTOR_MODEL_MAP,
)
from services.well_details_helper import get_well_details_payload

router = APIRouter(prefix="/thing", tags=["thing"])

Expand Down Expand Up @@ -177,6 +179,28 @@ async def get_well_by_id(
return get_thing_of_a_thing_type_by_id(session, request, thing_id)


@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,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Require AMPViewer for well details observations payload

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 👍 / 👎.

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,
)
Comment on lines +182 to +201
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.


@router.get(
"/water-well/{thing_id}/well-screen",
summary="Get well screens by water well ID",
Expand Down
22 changes: 22 additions & 0 deletions schemas/well_details.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from pydantic import BaseModel, ConfigDict, Field

from schemas.contact import ContactResponse
from schemas.deployment import DeploymentResponse
from schemas.observation import GroundwaterLevelObservationResponse
from schemas.sample import SampleResponse
from schemas.sensor import SensorResponse
from schemas.thing import WellResponse, WellScreenResponse


class WellDetailsResponse(BaseModel):
model_config = ConfigDict(from_attributes=True)

well: WellResponse
contacts: list[ContactResponse] = Field(default_factory=list)
sensors: list[SensorResponse] = Field(default_factory=list)
deployments: list[DeploymentResponse] = Field(default_factory=list)
well_screens: list[WellScreenResponse] = Field(default_factory=list)
recent_groundwater_level_observations: list[GroundwaterLevelObservationResponse] = (
Comment on lines +18 to +19
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Field(default_factory=list)
)
latest_field_event_sample: SampleResponse | None = None
110 changes: 110 additions & 0 deletions services/well_details_helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
from sqlalchemy import select
from sqlalchemy.orm import Session, joinedload, selectinload

from db import (
Contact,
Deployment,
FieldActivity,
FieldEvent,
FieldEventParticipant,
Observation,
Parameter,
Sample,
Sensor,
ThingContactAssociation,
WellScreen,
)
from services.thing_helper import get_thing_of_a_thing_type_by_id


def get_well_details_payload(
session: Session,
request,
thing_id: int,
recent_observation_limit: int = 100,
):
well = get_thing_of_a_thing_type_by_id(session, request, thing_id)

contacts = session.scalars(
select(Contact)
.join(ThingContactAssociation)
.where(ThingContactAssociation.thing_id == well.id)
.options(
selectinload(Contact.emails),
selectinload(Contact.phones),
selectinload(Contact.addresses),
selectinload(Contact.incomplete_nma_phones),
selectinload(Contact.thing_associations).selectinload(
ThingContactAssociation.thing
),
)
.order_by(Contact.id)
).all()

sensors = session.scalars(
select(Sensor)
.join(Deployment)
.where(Deployment.thing_id == well.id)
.distinct()
.order_by(Sensor.id)
).all()

deployments = session.scalars(
select(Deployment)
.where(Deployment.thing_id == well.id)
.options(selectinload(Deployment.sensor))
.order_by(Deployment.installation_date.desc(), Deployment.id.desc())
).all()

well_screens = session.scalars(
select(WellScreen)
.where(WellScreen.thing_id == well.id)
.order_by(WellScreen.screen_depth_top.asc(), WellScreen.id.asc())
).all()

groundwater_parameter_id = (
session.query(Parameter)
.filter(Parameter.parameter_name == "groundwater level")
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
.filter(Parameter.parameter_name == "groundwater level")
.filter(
Parameter.parameter_name == "groundwater level",
Parameter.matrix == "groundwater",
)

Copilot uses AI. Check for mistakes.
.one()
Comment on lines +67 to +68
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Disambiguate groundwater parameter query before calling one()

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 👍 / 👎.

.id
)

recent_groundwater_level_observations = session.scalars(
select(Observation)
.join(Sample)
.join(FieldActivity)
.join(FieldEvent)
.where(
FieldEvent.thing_id == well.id,
Observation.parameter_id == groundwater_parameter_id,
)
.options(selectinload(Observation.parameter))
.order_by(Observation.observation_datetime.desc(), Observation.id.desc())
.limit(recent_observation_limit)
).all()

latest_field_event_sample = None
if recent_groundwater_level_observations:
latest_sample_id = recent_groundwater_level_observations[0].sample_id
latest_field_event_sample = session.scalar(
select(Sample)
.where(Sample.id == latest_sample_id)
.options(
joinedload(Sample.field_activity)
.joinedload(FieldActivity.field_event)
.joinedload(FieldEvent.thing),
joinedload(Sample.field_event_participant).joinedload(
FieldEventParticipant.participant
),
)
)

return {
"well": well,
"contacts": contacts,
"sensors": sensors,
"deployments": deployments,
"well_screens": well_screens,
"recent_groundwater_level_observations": recent_groundwater_level_observations,
"latest_field_event_sample": latest_field_event_sample,
}
108 changes: 108 additions & 0 deletions tests/test_thing.py
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,114 @@ def test_get_water_well_by_id(water_well_thing, location):
assert data["current_location"] == expected_location


def test_get_water_well_details_payload(
water_well_thing,
field_event,
contact,
email,
phone,
address,
sensor,
sensor_to_water_well_thing_deployment,
thing_id_link,
well_screen,
groundwater_level_sample,
groundwater_level_observation,
):
response = client.get(f"/thing/water-well/{water_well_thing.id}/details")

assert response.status_code == 200
data = response.json()

assert data["well"]["id"] == water_well_thing.id
assert data["well"]["alternate_ids"][0]["id"] == thing_id_link.id
assert data["contacts"][0]["id"] == contact.id
assert data["contacts"][0]["emails"][0]["id"] == email.id
assert data["contacts"][0]["phones"][0]["id"] == phone.id
assert data["contacts"][0]["addresses"][0]["id"] == address.id
assert data["sensors"][0]["id"] == sensor.id
assert data["deployments"][0]["id"] == sensor_to_water_well_thing_deployment.id
assert data["deployments"][0]["sensor"]["id"] == sensor.id
assert data["well_screens"][0]["id"] == well_screen.id
assert (
data["recent_groundwater_level_observations"][0]["id"]
== groundwater_level_observation.id
)
assert data["latest_field_event_sample"]["id"] == groundwater_level_sample.id
assert data["latest_field_event_sample"]["field_event"]["id"] == field_event.id
assert data["latest_field_event_sample"]["contact"]["id"] == contact.id


def test_get_water_well_details_payload_uses_latest_observation_sample(
water_well_thing,
groundwater_level_sample,
groundwater_level_observation,
field_event_participant,
sensor,
):
from db import Observation, Sample

with session_ctx() as session:
later_sample = Sample(
field_activity_id=groundwater_level_sample.field_activity_id,
field_event_participant_id=field_event_participant.id,
sample_date="2025-01-02T12:00:00Z",
sample_name="later groundwater level sample",
sample_matrix="water",
sample_method="Steel-tape measurement",
qc_type="Normal",
notes="later sample",
release_status="draft",
)
session.add(later_sample)
session.commit()
session.refresh(later_sample)

later_observation = Observation(
observation_datetime="2025-01-02T00:04:00Z",
sample_id=later_sample.id,
sensor_id=sensor.id,
parameter_id=groundwater_level_observation.parameter_id,
release_status="draft",
value=9.0,
unit="ft",
measuring_point_height=5.0,
groundwater_level_reason="Water level not affected",
)
session.add(later_observation)
session.commit()
session.refresh(later_observation)
later_sample_id = later_sample.id
later_observation_id = later_observation.id

try:
response = client.get(f"/thing/water-well/{water_well_thing.id}/details")

assert response.status_code == 200
data = response.json()
assert data["latest_field_event_sample"]["id"] == later_sample_id
assert (
data["recent_groundwater_level_observations"][0]["id"]
== later_observation_id
)
finally:
with session_ctx() as session:
later_observation = session.get(Observation, later_observation_id)
if later_observation is not None:
session.delete(later_observation)
later_sample = session.get(Sample, later_sample_id)
if later_sample is not None:
session.delete(later_sample)
session.commit()


def test_get_water_well_details_payload_404_not_found():
response = client.get("/thing/water-well/999999/details")

assert response.status_code == 404
assert response.json()["detail"] == "Thing with ID 999999 not found."


def test_get_water_well_by_id_404_not_found(water_well_thing):
bad_id = 99999
response = client.get(f"/thing/water-well/{bad_id}")
Expand Down
Loading