From fe7fba2558b1a4d17aa13c23ddda9d45fc799d11 Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 14:16:41 -0600 Subject: [PATCH 1/7] feat(well inventory): require measuring_point_height_ft or mp_height_ft for non-null observations Either of these should be required when a non-null observation is being added to the DB so that dtw bgs can be calculated --- services/well_inventory_csv.py | 25 +++++- tests/test_well_inventory.py | 154 ++++++++++++++++++++++++++++++++- 2 files changed, 174 insertions(+), 5 deletions(-) diff --git a/services/well_inventory_csv.py b/services/well_inventory_csv.py index 049eddea..172ac4f2 100644 --- a/services/well_inventory_csv.py +++ b/services/well_inventory_csv.py @@ -681,6 +681,22 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) } ) + if ( + model.mp_height + and model.measuring_point_height_ft + and model.mp_height != model.measuring_point_height_ft + ): + raise ValueError( + "Conflicting values for measuring point height: mp_height and measuring_point_height_ft" + ) + + if model.measuring_point_height_ft: + universal_mp_height = model.measuring_point_height_ft + elif model.mp_height: + universal_mp_height = model.mp_height + else: + universal_mp_height = None + data = CreateWell( location_id=loc.id, group_id=group.id, @@ -689,7 +705,7 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) well_depth=model.total_well_depth_ft, well_depth_source=model.depth_source, well_casing_diameter=model.casing_diameter_ft, - measuring_point_height=model.measuring_point_height_ft, + measuring_point_height=universal_mp_height, measuring_point_description=model.measuring_point_description, well_completion_date=model.date_drilled, well_completion_date_source=model.completion_source, @@ -821,6 +837,11 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) session.add(sample) session.flush() + if model.depth_to_water_ft and not universal_mp_height: + raise ValueError( + "measuring_point_height_ft or mp_height is required when depth_to_water_ft is provided for a non-null observation" + ) + # create Observation # TODO: groundwater_level_reason may be conditionally required for null depth_to_water_ft - handle accordingly observation = Observation( @@ -829,7 +850,7 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) value=model.depth_to_water_ft, unit="ft", observation_datetime=model.measurement_date_time, - measuring_point_height=model.mp_height, + measuring_point_height=universal_mp_height, groundwater_level_reason=( model.level_status.value if hasattr(model.level_status, "value") diff --git a/tests/test_well_inventory.py b/tests/test_well_inventory.py index 08d2573b..24493012 100644 --- a/tests/test_well_inventory.py +++ b/tests/test_well_inventory.py @@ -481,7 +481,7 @@ def test_well_inventory_db_contents_with_waterlevels(tmp_path): "sample_method": "Steel-tape measurement", "data_quality": "Water level accurate to within two hundreths of a foot", "water_level_notes": "Attempted measurement", - "mp_height_ft": 2.5, + "mp_height_ft": 3.5, "level_status": "Water level not affected", } ) @@ -529,6 +529,154 @@ def test_well_inventory_db_contents_with_waterlevels(tmp_path): assert observation.sample == sample +def test_measuring_point_height_ft_used_for_thing_and_observation(tmp_path): + """When both mp_height and measuring_point_height_ft are provided, measuring_point_height_ft should be used for the thing's and observation's measuring_point_height.""" + row = _minimal_valid_well_inventory_row() + row.update( + { + "measuring_point_height_ft": 3.5, + "water_level_date_time": "2025-02-15T10:30:00", + "depth_to_water_ft": "8", + "sample_method": "Steel-tape measurement", + "data_quality": "Water level accurate to within two hundreths of a foot", + "water_level_notes": "Attempted measurement", + } + ) + + file_path = tmp_path / "well-inventory-blank-depth.csv" + with file_path.open("w", encoding="utf-8", newline="") as f: + writer = csv.DictWriter(f, fieldnames=list(row.keys())) + writer.writeheader() + writer.writerow(row) + + result = well_inventory_csv(file_path) + assert result.exit_code == 0, result.stderr + + with session_ctx() as session: + things = session.query(Thing).all() + observations = session.query(Observation).all() + + assert len(things) == 1 + assert things[0].measuring_point_height == 3.5 + assert len(observations) == 1 + assert observations[0].measuring_point_height == 3.5 + + +def test_mp_height_used_for_thing_and_observation_when_mp_height_ft_blank(tmp_path): + """When depth to water is provided but measuring_point_height_ft is blank, the mp_height value should be used for the thing's and observation's measuring_point_height.""" + row = _minimal_valid_well_inventory_row() + row.update( + { + "measuring_point_height_ft": "", + "water_level_date_time": "2025-02-15T10:30:00", + "depth_to_water_ft": "8", + "sample_method": "Steel-tape measurement", + "data_quality": "Water level accurate to within two hundreths of a foot", + "water_level_notes": "Attempted measurement", + "mp_height": 4.0, + } + ) + + file_path = tmp_path / "well-inventory-blank-depth.csv" + with file_path.open("w", encoding="utf-8", newline="") as f: + writer = csv.DictWriter(f, fieldnames=list(row.keys())) + writer.writeheader() + writer.writerow(row) + + result = well_inventory_csv(file_path) + assert result.exit_code == 0, result.stderr + + with session_ctx() as session: + things = session.query(Thing).all() + observations = session.query(Observation).all() + + assert len(things) == 1 + assert things[0].measuring_point_height == 4.0 + assert len(observations) == 1 + assert observations[0].measuring_point_height == 4.0 + + +def test_null_observation_allows_blank_mp_height(tmp_path): + """When depth to water is not provided, a blank measuring_point_height_ft should be allowed and result in a null measuring_point_height on the thing and observation and no associated measuring point height for the well.""" + row = _minimal_valid_well_inventory_row() + row.update( + { + "measuring_point_height_ft": "", + "water_level_date_time": "2025-02-15T10:30:00", + "depth_to_water_ft": "", + "sample_method": "Steel-tape measurement", + "data_quality": "Water level accurate to within two hundreths of a foot", + "water_level_notes": "Attempted measurement", + } + ) + + file_path = tmp_path / "well-inventory-blank-depth.csv" + with file_path.open("w", encoding="utf-8", newline="") as f: + writer = csv.DictWriter(f, fieldnames=list(row.keys())) + writer.writeheader() + writer.writerow(row) + + result = well_inventory_csv(file_path) + assert result.exit_code == 0, result.stderr + + with session_ctx() as session: + things = session.query(Thing).all() + observations = session.query(Observation).all() + + assert len(things) == 1 + assert things[0].measuring_point_height is None + assert len(observations) == 1 + assert observations[0].measuring_point_height is None + + +def test_conflicting_mp_heights_raises_error(tmp_path): + row = _minimal_valid_well_inventory_row() + row.update( + { + "measuring_point_height_ft": 3.5, + "mp_height": 4.0, + } + ) + + file_path = tmp_path / "well-inventory-blank-depth.csv" + with file_path.open("w", encoding="utf-8", newline="") as f: + writer = csv.DictWriter(f, fieldnames=list(row.keys())) + writer.writeheader() + writer.writerow(row) + + result = well_inventory_csv(file_path) + assert result.exit_code == 1, result.stderr + assert ( + result.payload["validation_errors"][0]["error"] + == "Conflicting values for measuring point height: mp_height and measuring_point_height_ft" + ) + + +def test_no_mp_height_raises_error_when_depth_to_water_provided(tmp_path): + row = _minimal_valid_well_inventory_row() + row.update( + { + "water_level_date_time": "2025-02-15T10:30:00", + "measuring_point_height_ft": "", + "mp_height": "", + "depth_to_water_ft": "8", + } + ) + + file_path = tmp_path / "well-inventory-no-mp-height.csv" + with file_path.open("w", encoding="utf-8", newline="") as f: + writer = csv.DictWriter(f, fieldnames=list(row.keys())) + writer.writeheader() + writer.writerow(row) + + result = well_inventory_csv(file_path) + assert result.exit_code == 1, result.stderr + assert ( + result.payload["validation_errors"][0]["error"] + == "measuring_point_height_ft or mp_height is required when depth_to_water_ft is provided for a non-null observation" + ) + + def test_blank_depth_to_water_still_creates_water_level_records(tmp_path): """Blank depth-to-water is treated as missing while preserving the attempted measurement.""" row = _minimal_valid_well_inventory_row() @@ -539,7 +687,7 @@ def test_blank_depth_to_water_still_creates_water_level_records(tmp_path): "sample_method": "Steel-tape measurement", "data_quality": "Water level accurate to within two hundreths of a foot", "water_level_notes": "Attempted measurement", - "mp_height_ft": 2.5, + "mp_height_ft": 3.5, } ) @@ -563,7 +711,7 @@ def test_blank_depth_to_water_still_creates_water_level_records(tmp_path): "2025-02-15T10:30:00Z" ) assert observations[0].value is None - assert observations[0].measuring_point_height == 2.5 + assert observations[0].measuring_point_height == 3.5 def test_rerunning_same_well_inventory_csv_is_idempotent(): From 1763df28badc5e1043e7034d4e4f7db483b52ee1 Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 14:49:15 -0600 Subject: [PATCH 2/7] fix(test): make test name more accurate --- tests/test_well_inventory.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_well_inventory.py b/tests/test_well_inventory.py index 24493012..646e36af 100644 --- a/tests/test_well_inventory.py +++ b/tests/test_well_inventory.py @@ -562,7 +562,9 @@ def test_measuring_point_height_ft_used_for_thing_and_observation(tmp_path): assert observations[0].measuring_point_height == 3.5 -def test_mp_height_used_for_thing_and_observation_when_mp_height_ft_blank(tmp_path): +def test_mp_height_used_for_thing_and_observation_when_measuring_point_height_ft_blank( + tmp_path, +): """When depth to water is provided but measuring_point_height_ft is blank, the mp_height value should be used for the thing's and observation's measuring_point_height.""" row = _minimal_valid_well_inventory_row() row.update( From 4c0db46958cabd0d86fe0ae7c04abf45cd292650 Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 14:50:59 -0600 Subject: [PATCH 3/7] fix(well inventory): test if mp height not None to avoid truthiness trap --- services/well_inventory_csv.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/well_inventory_csv.py b/services/well_inventory_csv.py index 172ac4f2..ed545157 100644 --- a/services/well_inventory_csv.py +++ b/services/well_inventory_csv.py @@ -682,17 +682,17 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) ) if ( - model.mp_height - and model.measuring_point_height_ft + model.mp_height is not None + and model.measuring_point_height_ft is not None and model.mp_height != model.measuring_point_height_ft ): raise ValueError( "Conflicting values for measuring point height: mp_height and measuring_point_height_ft" ) - if model.measuring_point_height_ft: + if model.measuring_point_height_ft is not None: universal_mp_height = model.measuring_point_height_ft - elif model.mp_height: + elif model.mp_height is not None: universal_mp_height = model.mp_height else: universal_mp_height = None From aac5c9521d4f8d1d7eaf84a139cebd17d78cb56b Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 14:52:00 -0600 Subject: [PATCH 4/7] fix(well inventory): check for Nones to avoid truthiness traps --- services/well_inventory_csv.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/well_inventory_csv.py b/services/well_inventory_csv.py index ed545157..89ece733 100644 --- a/services/well_inventory_csv.py +++ b/services/well_inventory_csv.py @@ -837,7 +837,7 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) session.add(sample) session.flush() - if model.depth_to_water_ft and not universal_mp_height: + if model.depth_to_water_ft is not None and universal_mp_height is None: raise ValueError( "measuring_point_height_ft or mp_height is required when depth_to_water_ft is provided for a non-null observation" ) From b6e5d800433de1d630e5c014e3766bf8b5b72c36 Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 15:36:59 -0600 Subject: [PATCH 5/7] fix(test): make docstring more accurate --- tests/test_well_inventory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_well_inventory.py b/tests/test_well_inventory.py index 646e36af..b7f1fcab 100644 --- a/tests/test_well_inventory.py +++ b/tests/test_well_inventory.py @@ -530,7 +530,7 @@ def test_well_inventory_db_contents_with_waterlevels(tmp_path): def test_measuring_point_height_ft_used_for_thing_and_observation(tmp_path): - """When both mp_height and measuring_point_height_ft are provided, measuring_point_height_ft should be used for the thing's and observation's measuring_point_height.""" + """When measuring_point_height_ft is provided it is used for the thing's and observation's measuring_point_height.""" row = _minimal_valid_well_inventory_row() row.update( { From 4a0f0daf8eabfd8785924f9742ffe2bff8c26cdc Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 15:39:53 -0600 Subject: [PATCH 6/7] fix(test): clarify docstrings --- tests/test_well_inventory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_well_inventory.py b/tests/test_well_inventory.py index b7f1fcab..69edee32 100644 --- a/tests/test_well_inventory.py +++ b/tests/test_well_inventory.py @@ -530,7 +530,7 @@ def test_well_inventory_db_contents_with_waterlevels(tmp_path): def test_measuring_point_height_ft_used_for_thing_and_observation(tmp_path): - """When measuring_point_height_ft is provided it is used for the thing's and observation's measuring_point_height.""" + """When measuring_point_height_ft is provided it is used for the thing's (MeasuringPointHistory) and observation's measuring_point_height values.""" row = _minimal_valid_well_inventory_row() row.update( { From 6df12f6e70f86286de01c244cdb20022cf0bd86a Mon Sep 17 00:00:00 2001 From: jacob-a-brown Date: Wed, 18 Mar 2026 15:54:56 -0600 Subject: [PATCH 7/7] fix(test): clarify docstrings --- tests/test_well_inventory.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_well_inventory.py b/tests/test_well_inventory.py index 69edee32..fd27e395 100644 --- a/tests/test_well_inventory.py +++ b/tests/test_well_inventory.py @@ -565,7 +565,7 @@ def test_measuring_point_height_ft_used_for_thing_and_observation(tmp_path): def test_mp_height_used_for_thing_and_observation_when_measuring_point_height_ft_blank( tmp_path, ): - """When depth to water is provided but measuring_point_height_ft is blank, the mp_height value should be used for the thing's and observation's measuring_point_height.""" + """When depth to water is provided and measuring_point_height_ft is blank the mp_height value should be used for the thing's (MeasuringPointHistory) and observation's measuring_point_height.""" row = _minimal_valid_well_inventory_row() row.update( { @@ -599,7 +599,7 @@ def test_mp_height_used_for_thing_and_observation_when_measuring_point_height_ft def test_null_observation_allows_blank_mp_height(tmp_path): - """When depth to water is not provided, a blank measuring_point_height_ft should be allowed and result in a null measuring_point_height on the thing and observation and no associated measuring point height for the well.""" + """When depth to water is not provided (ie null), blank measuring_point_height_ft and mp_height fields should be allowed and result in a null measuring_point_height for the observation and no associated measuring point height (MeasuringPointHistory) for the well.""" row = _minimal_valid_well_inventory_row() row.update( { @@ -632,7 +632,11 @@ def test_null_observation_allows_blank_mp_height(tmp_path): def test_conflicting_mp_heights_raises_error(tmp_path): + """ + When both measuring_point_height_ft and mp_height are provided, an inequality (conflict) should raise an error. + """ row = _minimal_valid_well_inventory_row() + row.update( { "measuring_point_height_ft": 3.5,