diff --git a/db/thing.py b/db/thing.py index f5fbff5b..db2419c3 100644 --- a/db/thing.py +++ b/db/thing.py @@ -495,10 +495,15 @@ def measuring_point_height(self) -> int | None: Since measuring_point_history is eagerly loaded, this should not introduce N+1 query issues. """ if self.thing_type == "water well": + if not self.measuring_points: + return None sorted_measuring_point_history = sorted( self.measuring_points, key=lambda x: x.start_date, reverse=True ) - return sorted_measuring_point_history[0].measuring_point_height + for record in sorted_measuring_point_history: + if record.measuring_point_height is not None: + return record.measuring_point_height + return None else: return None @@ -511,10 +516,15 @@ def measuring_point_description(self) -> str | None: Since measuring_point_history is eagerly loaded, this should not introduce N+1 query issues. """ if self.thing_type == "water well": + if not self.measuring_points: + return None sorted_measuring_point_history = sorted( self.measuring_points, key=lambda x: x.start_date, reverse=True ) - return sorted_measuring_point_history[0].measuring_point_description + for record in sorted_measuring_point_history: + if record.measuring_point_description is not None: + return record.measuring_point_description + return None else: return None diff --git a/tests/test_thing.py b/tests/test_thing.py index 00a476d9..6cba4800 100644 --- a/tests/test_thing.py +++ b/tests/test_thing.py @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== -from datetime import timezone +from datetime import date, timezone import pytest @@ -25,7 +25,8 @@ viewer_function, amp_viewer_function, ) -from db import Thing, WellScreen, ThingIdLink +from db import MeasuringPointHistory, Thing, ThingIdLink, WellScreen +from db.engine import session_ctx from main import app from schemas import DT_FMT from schemas.location import LocationResponse @@ -85,6 +86,46 @@ def test_update_well_allows_nma_formation_zone(): assert payload.nma_formation_zone == "FZ-001" +def test_measuring_point_properties_skip_null_history(): + with session_ctx() as session: + well = Thing( + name="Null MP Height Well", + thing_type="water well", + release_status="draft", + ) + session.add(well) + session.commit() + session.refresh(well) + + old_history = MeasuringPointHistory( + thing_id=well.id, + measuring_point_height=2.5, + measuring_point_description="old mp", + start_date=date(2020, 1, 1), + end_date=None, + release_status="draft", + ) + new_history = MeasuringPointHistory( + thing_id=well.id, + measuring_point_height=None, + measuring_point_description=None, + start_date=date(2021, 1, 1), + end_date=None, + release_status="draft", + ) + session.add_all([old_history, new_history]) + session.commit() + session.refresh(well) + + assert well.measuring_point_height == 2.5 + assert well.measuring_point_description == "old mp" + + session.delete(new_history) + session.delete(old_history) + session.delete(well) + session.commit() + + # this is not a valid test because measuring_point_height is not related to hole_depth # def test_validate_mp_height_hole_depth(): # with pytest.raises( diff --git a/transfers/well_transfer.py b/transfers/well_transfer.py index 5d459c23..d8e1c200 100644 --- a/transfers/well_transfer.py +++ b/transfers/well_transfer.py @@ -569,16 +569,9 @@ def _build_well_payload(self, row) -> CreateWell | None: mpheight = row.MPHeight mpheight_description = row.MeasuringPoint - if mpheight is None: - mphs = self._measuring_point_estimator.estimate_measuring_point_height( - row - ) - if mphs: - try: - mpheight = mphs[0][0] - mpheight_description = mphs[1][0] - except IndexError: - pass + if mpheight is None or isna(mpheight): + # Treat missing/NaN MPHeight as unknown during migration. + mpheight = None completion_date, completion_date_parse_failed = _normalize_completion_date( row.CompletionDate @@ -736,22 +729,9 @@ 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) - added_measuring_point = False - for mph, mph_desc, start_date, end_date in zip(*mphs): - session.add( - MeasuringPointHistory( - thing_id=well.id, - measuring_point_height=mph, - measuring_point_description=mph_desc, - start_date=start_date, - end_date=end_date, - ) - ) - added_measuring_point = True - - # Preserve transfer intent even when no MP height can be measured/estimated. - if not added_measuring_point: + raw_mpheight = getattr(row, "MPHeight", None) + if raw_mpheight is None or isna(raw_mpheight): + # No estimator for NaN/missing MPHeight; persist NULL history row. raw_desc = getattr(row, "MeasuringPoint", None) mp_desc = None if isna(raw_desc) else raw_desc session.add( @@ -763,6 +743,34 @@ def _add_histories(self, session: Session, row, well: Thing) -> None: end_date=None, ) ) + else: + mphs = self._measuring_point_estimator.estimate_measuring_point_height(row) + added_measuring_point = False + for mph, mph_desc, start_date, end_date in zip(*mphs): + session.add( + MeasuringPointHistory( + thing_id=well.id, + measuring_point_height=mph, + measuring_point_description=mph_desc, + start_date=start_date, + end_date=end_date, + ) + ) + added_measuring_point = True + + # Preserve transfer intent even when no MP height can be measured/estimated. + if not added_measuring_point: + raw_desc = getattr(row, "MeasuringPoint", None) + mp_desc = None if isna(raw_desc) else raw_desc + session.add( + MeasuringPointHistory( + thing_id=well.id, + measuring_point_height=None, + measuring_point_description=mp_desc, + start_date=datetime.now(tz=UTC).date(), + end_date=None, + ) + ) target_id = well.id target_table = "thing"