diff --git a/alembic/versions/c4d5e6f7a8b9_add_measuring_point_height_is_assumed.py b/alembic/versions/c4d5e6f7a8b9_add_measuring_point_height_is_assumed.py new file mode 100644 index 000000000..583a0f527 --- /dev/null +++ b/alembic/versions/c4d5e6f7a8b9_add_measuring_point_height_is_assumed.py @@ -0,0 +1,52 @@ +"""Add measuring_point_height_is_assumed to measuring_point_history. + +Revision ID: c4d5e6f7a8b9 +Revises: e71807682f57 +Create Date: 2026-02-16 12:00:00.000000 +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op +from sqlalchemy import inspect + +# revision identifiers, used by Alembic. +revision: str = "c4d5e6f7a8b9" +down_revision: Union[str, Sequence[str], None] = "e71807682f57" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + + if inspector.has_table("measuring_point_history"): + columns = { + col["name"] for col in inspector.get_columns("measuring_point_history") + } + if "measuring_point_height_is_assumed" not in columns: + op.add_column( + "measuring_point_history", + sa.Column( + "measuring_point_height_is_assumed", + sa.Boolean(), + nullable=False, + server_default=sa.text("false"), + ), + ) + + +def downgrade() -> None: + bind = op.get_bind() + inspector = inspect(bind) + + if inspector.has_table("measuring_point_history"): + columns = { + col["name"] for col in inspector.get_columns("measuring_point_history") + } + if "measuring_point_height_is_assumed" in columns: + op.drop_column( + "measuring_point_history", "measuring_point_height_is_assumed" + ) diff --git a/db/measuring_point_history.py b/db/measuring_point_history.py index 7d23518a1..15159d8cf 100644 --- a/db/measuring_point_history.py +++ b/db/measuring_point_history.py @@ -14,7 +14,7 @@ from typing import TYPE_CHECKING -from sqlalchemy import Integer, ForeignKey, Date, Text, Numeric +from sqlalchemy import Integer, ForeignKey, Date, Text, Numeric, Boolean, false from sqlalchemy.orm import relationship, Mapped, mapped_column from db.base import Base, AutoBaseMixin, ReleaseMixin @@ -40,6 +40,13 @@ class MeasuringPointHistory(Base, AutoBaseMixin, ReleaseMixin): nullable=False, comment="The official, surveyed height of the measuring point relative to ground surface (in feet).", ) + measuring_point_height_is_assumed: Mapped[bool] = mapped_column( + Boolean, + nullable=False, + default=False, + server_default=false(), + comment="True when measuring point height is assumed/defaulted rather than explicitly measured.", + ) measuring_point_description: Mapped[str] = mapped_column( Text, nullable=True, diff --git a/db/thing.py b/db/thing.py index a0f3db3b6..1234d4ff5 100644 --- a/db/thing.py +++ b/db/thing.py @@ -125,7 +125,6 @@ class Thing( info={"unit": "feet below ground surface"}, comment="Depth of the well casing from ground surface to the bottom of the casing (in feet).", ) - well_completion_date: Mapped[date] = mapped_column( nullable=True, comment="the date the well was completed if known" ) @@ -498,6 +497,8 @@ def measuring_point_height(self) -> int | None: sorted_measuring_point_history = sorted( self.measuring_points, key=lambda x: x.start_date, reverse=True ) + if not sorted_measuring_point_history: + return 0 return sorted_measuring_point_history[0].measuring_point_height else: return None @@ -518,6 +519,23 @@ def measuring_point_description(self) -> str | None: else: return None + @property + def measuring_point_height_is_assumed(self) -> bool | None: + """ + Returns whether the most recent measuring point height is assumed. + + Since measuring_point_history is eagerly loaded, this should not introduce N+1 query issues. + """ + if self.thing_type == "water well": + sorted_measuring_point_history = sorted( + self.measuring_points, key=lambda x: x.start_date, reverse=True + ) + if not sorted_measuring_point_history: + return True + return sorted_measuring_point_history[0].measuring_point_height_is_assumed + else: + return None + @property def well_depth_source(self) -> str | None: return self._get_data_provenance_attribute("well_depth", "origin_type") diff --git a/schemas/thing.py b/schemas/thing.py index 60dfce426..0d940c512 100644 --- a/schemas/thing.py +++ b/schemas/thing.py @@ -66,25 +66,6 @@ def validate_values(self): elif self.hole_depth is not None and self.well_pump_depth > self.hole_depth: raise ValueError("well pump depth must be less than hole depth") - # if self.measuring_point_height is not None: - # if ( - # self.hole_depth is not None - # and self.measuring_point_height >= self.hole_depth - # ): - # raise ValueError("measuring point height must be less than hole depth") - # elif ( - # self.well_casing_depth is not None - # and self.measuring_point_height >= self.well_casing_depth - # ): - # raise ValueError( - # "measuring point height must be less than well casing depth" - # ) - # elif ( - # self.well_depth is not None - # and self.measuring_point_height >= self.well_depth - # ): - # raise ValueError("measuring point height must be less than well depth") - return self @@ -145,7 +126,9 @@ class CreateWell(CreateBaseThing, ValidateWell): default=None, gt=0, description="Well casing depth in feet" ) well_casing_materials: list[CasingMaterial] | None = None - measuring_point_height: float = Field(description="Measuring point height in feet") + measuring_point_height: float | None = Field( + default=0, description="Measuring point height in feet" + ) measuring_point_description: str | None = None well_completion_date: PastOrTodayDate | None = None well_completion_date_source: str | None = None @@ -260,13 +243,14 @@ class WellResponse(BaseThingResponse): well_status: str | None open_status: str | None datalogger_suitability_status: str | None - measuring_point_height: float + measuring_point_height: float | None = None + measuring_point_height_is_assumed: bool | None = None measuring_point_height_unit: str = "ft" measuring_point_description: str | None aquifers: list[dict] = [] water_notes: list[NoteResponse] = [] construction_notes: list[NoteResponse] = [] - permissions: list[PermissionHistoryResponse] + permissions: list[PermissionHistoryResponse] = [] formation_completion_code: FormationCode | None nma_formation_zone: str | None @@ -336,8 +320,7 @@ class SpringResponse(BaseThingResponse): class ThingResponse(WellResponse, SpringResponse): - # required fields for wells that don't apply to other thing types - measuring_point_height: float | None + pass class WellScreenResponse(BaseResponseModel): diff --git a/services/thing_helper.py b/services/thing_helper.py index cc2fbf6e2..c06cbddc6 100644 --- a/services/thing_helper.py +++ b/services/thing_helper.py @@ -14,6 +14,7 @@ # limitations under the License. # =============================================================================== from datetime import datetime +import math from zoneinfo import ZoneInfo from fastapi import Request, HTTPException @@ -193,7 +194,9 @@ def add_thing( # Extract data for related tables # Normalize Pydantic models to dictionaries so we can safely mutate with .pop() if isinstance(data, BaseModel): - data = data.model_dump() + # Preserve "unset" semantics so defaults (e.g., measuring_point_height=0) + # don't mask whether a value was actually provided by the caller. + data = data.model_dump(exclude_unset=True) # --------- # BEGIN UNIVERSAL THING RELATED TABLES @@ -232,6 +235,8 @@ def add_thing( # measuring point info measuring_point_height = data.pop("measuring_point_height", None) + if isinstance(measuring_point_height, float) and math.isnan(measuring_point_height): + measuring_point_height = None measuring_point_description = data.pop("measuring_point_description", None) # data provenance info @@ -263,17 +268,21 @@ def add_thing( if thing_type == WATER_WELL_THING_TYPE: - # Create MeasuringPointHistory record if measuring_point_height provided - if measuring_point_height is not None: - measuring_point_history = MeasuringPointHistory( - thing_id=thing.id, - measuring_point_height=measuring_point_height, - measuring_point_description=measuring_point_description, - start_date=datetime.now(tz=ZoneInfo("UTC")), - end_date=None, - ) - audit_add(user, measuring_point_history) - session.add(measuring_point_history) + # Always create a MeasuringPointHistory record for water wells. + # If the value is missing, default to 0 and mark as assumed. + measuring_point_height_is_assumed = measuring_point_height is None + measuring_point_history = MeasuringPointHistory( + thing_id=thing.id, + measuring_point_height=( + 0 if measuring_point_height is None else measuring_point_height + ), + measuring_point_height_is_assumed=measuring_point_height_is_assumed, + measuring_point_description=measuring_point_description, + start_date=datetime.now(tz=ZoneInfo("UTC")), + end_date=None, + ) + audit_add(user, measuring_point_history) + session.add(measuring_point_history) if well_completion_date_source is not None: dp = DataProvenance( diff --git a/services/well_inventory_csv.py b/services/well_inventory_csv.py index 247091a2b..dfbc3ad50 100644 --- a/services/well_inventory_csv.py +++ b/services/well_inventory_csv.py @@ -585,7 +585,6 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) monitoring_frequencies=monitoring_frequencies, alternate_ids=alternate_ids, ) - well_data = data.model_dump() """ Developer's notes @@ -605,7 +604,7 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) """ well = add_thing( session=session, - data=well_data, + data=data, user=user, thing_type="water well", commit=False, diff --git a/tests/test_thing.py b/tests/test_thing.py index 713b7444b..c1c4e0e7b 100644 --- a/tests/test_thing.py +++ b/tests/test_thing.py @@ -25,7 +25,8 @@ viewer_function, amp_viewer_function, ) -from db import Thing, WellScreen, ThingIdLink +from db import Thing, WellScreen, ThingIdLink, MeasuringPointHistory +from db.engine import session_ctx from main import app from schemas import DT_FMT from schemas.location import LocationResponse @@ -190,6 +191,65 @@ def test_add_water_well_with_measuring_point(location, group): cleanup_post_test(Thing, data["id"]) +def test_add_water_well_missing_measuring_point_height_sets_assumed(location, group): + payload = { + "location_id": location.id, + "group_id": group.id, + "release_status": "draft", + "name": "Test Well Missing MP Height", + } + + response = client.post("/thing/water-well", json=payload) + assert response.status_code == 201 + data = response.json() + assert data["measuring_point_height"] == 0 + assert data["measuring_point_height_is_assumed"] is True + + with session_ctx() as session: + mph = ( + session.query(MeasuringPointHistory) + .filter( + MeasuringPointHistory.thing_id == data["id"], + MeasuringPointHistory.end_date.is_(None), + ) + .one() + ) + assert float(mph.measuring_point_height) == 0.0 + assert mph.measuring_point_height_is_assumed is True + + cleanup_post_test(Thing, data["id"]) + + +def test_add_water_well_explicit_measuring_point_height_not_assumed(location, group): + payload = { + "location_id": location.id, + "group_id": group.id, + "release_status": "draft", + "name": "Test Well Explicit MP Height", + "measuring_point_height": 2.5, + } + + response = client.post("/thing/water-well", json=payload) + assert response.status_code == 201 + data = response.json() + assert data["measuring_point_height"] == 2.5 + assert data["measuring_point_height_is_assumed"] is False + + with session_ctx() as session: + mph = ( + session.query(MeasuringPointHistory) + .filter( + MeasuringPointHistory.thing_id == data["id"], + MeasuringPointHistory.end_date.is_(None), + ) + .one() + ) + assert float(mph.measuring_point_height) == 2.5 + assert mph.measuring_point_height_is_assumed is False + + cleanup_post_test(Thing, data["id"]) + + @pytest.mark.skip("Needs to be updated per changes made from feature files") def test_add_water_well_409_bad_group_id(location): bad_group_id = 9999 diff --git a/tests/transfers/test_contact_with_multiple_wells.py b/tests/transfers/test_contact_with_multiple_wells.py index 835aafb3f..92ec1772d 100644 --- a/tests/transfers/test_contact_with_multiple_wells.py +++ b/tests/transfers/test_contact_with_multiple_wells.py @@ -22,7 +22,7 @@ def _run_contact_transfer(pointids: list[str]): wt = WellTransferer(pointids=pointids) - wt.transfer() + wt.transfer_parallel() ct = ContactTransfer(pointids=pointids) ct.transfer() diff --git a/transfers/contact_transfer.py b/transfers/contact_transfer.py index 0acedb57f..e69742df9 100644 --- a/transfers/contact_transfer.py +++ b/transfers/contact_transfer.py @@ -232,7 +232,7 @@ def _add_first_contact( role = "Owner" release_status = "private" - name = _make_name(row.FirstName, row.LastName) + name = _safe_make_name(row.FirstName, row.LastName, row.OwnerKey, organization) contact_data = { "thing_id": thing.id, @@ -326,6 +326,19 @@ def _add_first_contact( return contact +def _safe_make_name( + first: str | None, last: str | None, ownerkey: str, organization: str | None +) -> str: + name = _make_name(first, last) + if name is None and organization is None: + logger.warning( + f"Missing both first and last name and organization for OwnerKey {ownerkey}; " + f"using OwnerKey as fallback name." + ) + return ownerkey + return name + + def _add_second_contact( session: Session, row: pd.Series, thing: Thing, organization: str, added: list ) -> None: diff --git a/transfers/util.py b/transfers/util.py index d358937ce..29fdc4052 100644 --- a/transfers/util.py +++ b/transfers/util.py @@ -106,11 +106,17 @@ def estimate_measuring_point_height( ) -> tuple[float, str, datetime | None, datetime | None]: mph = row.MPHeight mph_desc = row.MeasuringPoint + # Treat NaN as missing the same as None. + if notna(mph): + mph_is_missing = False + else: + mph = None + mph_is_missing = True try: df = self._grouped.get_group(row.PointID) except KeyError: df = None - if mph is None: + if mph_is_missing: if self.verbose: logger.info( f"No MPHeight found for PointID: {row.PointID}. Estimating from measurements." @@ -765,6 +771,14 @@ def map_value(self, value, default=None) -> str: return default raise KeyError(f"No mapping found for {value}") + def valid_values_for_type(self, lu_type: str) -> list[str]: + prefix = f"{lu_type}:" + return [ + v.split(":", 1)[1] + for v in self._make_lu_to_lexicon_mapper() + if v.startswith(prefix) + ] + def _make_lu_to_lexicon_mapper(self) -> dict[str, str]: """ Lookup tables intentionally skipped (kept for documentation only) diff --git a/transfers/well_transfer.py b/transfers/well_transfer.py index a6fa64089..6fbd0cbbd 100644 --- a/transfers/well_transfer.py +++ b/transfers/well_transfer.py @@ -84,6 +84,7 @@ "well_casing_materials", "measuring_point_height", "measuring_point_description", + "measuring_point_height_is_assumed", "well_completion_date_source", "well_construction_method_source", "well_depth_source", @@ -548,7 +549,7 @@ def _build_well_payload(self, row) -> CreateWell | None: [], ) - mpheight = row.MPHeight + mpheight = row.MPHeight if notna(row.MPHeight) else None mpheight_description = row.MeasuringPoint if mpheight is None: mphs = self._measuring_point_estimator.estimate_measuring_point_height( @@ -718,11 +719,26 @@ def _add_notes_and_provenance( def _add_histories(self, session: Session, row, well: Thing) -> None: mphs = self._measuring_point_estimator.estimate_measuring_point_height(row) - for mph, mph_desc, start_date, end_date in zip(*mphs): + heights, descriptions, start_dates, end_dates = mphs + is_assumed = not notna(row.MPHeight) + + # Always persist at least one measuring point history record. + if not heights: + heights = [0] + descriptions = [ + "Defaulted to 0 because measuring point height was not provided." + ] + start_dates = [datetime.now(tz=UTC)] + end_dates = [None] + + for mph, mph_desc, start_date, end_date in zip( + heights, descriptions, start_dates, end_dates + ): session.add( MeasuringPointHistory( thing_id=well.id, measuring_point_height=mph, + measuring_point_height_is_assumed=is_assumed, measuring_point_description=mph_desc, start_date=start_date, end_date=end_date, @@ -774,6 +790,15 @@ def _add_histories(self, session: Session, row, well: Thing) -> None: ) ) except KeyError: + logger.warning( + "Unknown status code '%s' for well %s. Skipping status history record.", + sv, + well.name, + ) + logger.warning( + "Valid status codes for LU_Status: %s", + lexicon_mapper.valid_values_for_type("LU_Status"), + ) self._capture_error(well.name, f"Unknown status code: {sv}", "Status") if notna(row.OpenWellLoggerOK):