From 3f10ddb8412afb7b01f3d1558c895e65be63d03a Mon Sep 17 00:00:00 2001 From: jakeross Date: Fri, 7 Aug 2026 01:08:05 -0700 Subject: [PATCH] refactor(domain): extract CSV importer rules into a domain layer services/ was documented as "business logic and database interactions" and did both in the same functions, so rules could not be exercised without a database and drifted between callers. The groundwater-level sample name was written out three times across two files; the field staff contact lookup had two different WHERE clauses. Add a domain/ package holding those rules as plain functions over plain values. Modules there import nothing from api/, db/, schemas/, or services/, and no fastapi, sqlalchemy, pydantic, or httpx. services/ keeps its orchestration role: load rows, call the rule, persist the result. - domain/units.py holds the foot/meter conversions, moved out of services/util.py, which re-exports them so existing imports keep working. Importing them from services/util.py previously dragged in httpx, pyproj, and SQLAlchemy. - domain/wells.py, domain/water_levels.py, domain/samples.py, domain/field_staff.py, and domain/values.py hold the rules extracted from the two CSV importers. - Domain errors subclass ValueError, because the importers already treat a ValueError raised on a row as a per-row validation failure. Both importers keep every existing function signature, so the tests that import their private helpers still work. 67 new tests cover the extracted rules with no database and no fixtures. Aligning the two field staff contact lookups fixes a defect: well inventory also filtered on contact_type, so it missed an existing contact created with a different type and then failed on the duplicate insert. Contact enforces uniqueness on (name, organization), and both paths now use that key. This changes import behavior -- well inventory now reuses a contact where it previously errored. See ADR4.md for the layering rationale and for what was deliberately left alone. Co-Authored-By: Claude Opus 5 --- ADR4.md | 80 +++++++++ CLAUDE.md | 19 +- domain/__init__.py | 30 ++++ domain/field_staff.py | 68 ++++++++ domain/samples.py | 39 +++++ domain/units.py | 45 +++++ domain/values.py | 56 ++++++ domain/water_levels.py | 98 +++++++++++ domain/wells.py | 214 +++++++++++++++++++++++ services/util.py | 25 ++- services/water_level_csv.py | 90 ++++------ services/well_inventory_csv.py | 276 ++++++++++-------------------- tests/test_domain_values.py | 109 ++++++++++++ tests/test_domain_water_levels.py | 138 +++++++++++++++ tests/test_domain_wells.py | 210 +++++++++++++++++++++++ tests/test_well_inventory.py | 32 ++-- 16 files changed, 1256 insertions(+), 273 deletions(-) create mode 100644 ADR4.md create mode 100644 domain/__init__.py create mode 100644 domain/field_staff.py create mode 100644 domain/samples.py create mode 100644 domain/units.py create mode 100644 domain/values.py create mode 100644 domain/water_levels.py create mode 100644 domain/wells.py create mode 100644 tests/test_domain_values.py create mode 100644 tests/test_domain_water_levels.py create mode 100644 tests/test_domain_wells.py diff --git a/ADR4.md b/ADR4.md new file mode 100644 index 000000000..00bd92cdb --- /dev/null +++ b/ADR4.md @@ -0,0 +1,80 @@ +# ADR4: A Domain Layer for Import Rules + +## Status + +Accepted, partially applied. The `domain/` package exists and the two CSV +importers use it. The rest of `services/` is untouched and stays that way until +someone has a reason to open those files. + +## Context + +`services/` is documented as "business logic and database interactions", and it +does both in the same functions. The clearest example is +`services/well_inventory_csv.py`: a single call to `_add_csv_row` mixed unit +conversion, cross-column validation, note formatting, and `session.add(...)`. + +Three consequences: + +1. **Rules could not be tested without a database.** Verifying that a + measuring point height conflict is rejected meant standing up PostGIS, + building a `Thing`, and running an import. +2. **Rules drifted between callers.** The groundwater-level sample name was + written out three times across two files. The foot/meter conversion was + duplicated until BDMS-284 consolidated it. Field staff contact lookup had + two different WHERE clauses, one of which was wrong (see below). +3. **There was no obvious home for a new rule.** `services/util.py` had quietly + become one — it holds the unit conversions — but nothing named it as such, so + the next rule went wherever it was first needed. + +## Decision + +Add a `domain/` package holding business rules as plain functions over plain +values. Modules there import nothing from `api/`, `db/`, `schemas/`, or +`services/`, and no `fastapi`, `sqlalchemy`, `pydantic`, or `httpx`. + +`services/` keeps its orchestration role: load rows, call the rule, persist the +result, translate errors into the transport's shape. + +Domain errors subclass `ValueError`, because the importers already treat a +`ValueError` raised while handling a row as a per-row validation failure rather +than an aborted run. + +### What we did *not* decide + +This is not an adoption of hexagonal architecture or DDD. There are no entities, +repositories, aggregates, or mapping layers, and `services/` still talks to +SQLAlchemy models directly. The cost of a full restructure is not justified at +this size, and a half-applied one — domain objects that quietly hold a session — +is worse than none. + +Extraction is opportunistic: when you open an importer to change a rule, move +the rule. There is no migration plan for the remaining service modules. + +## Consequences + +**Good.** The extracted rules have 67 tests that need no database and run in +seconds. `services/util.py` no longer has to be imported to convert feet to +meters, which previously dragged in `httpx`, `pyproj`, and SQLAlchemy. + +**Cost.** One more package, and a rule now lives one call away from where it is +used. For a rule with a single caller this is pure overhead; extract when a rule +is shared, subtle, or expensive to test in place, not by default. + +**Watch for.** `services/util.py` re-exports the unit conversions for backwards +compatibility. That re-export is a transition aid, not a pattern — new code +should import from `domain.units`. + +## Notes + +Aligning the two field-staff contact lookups surfaced a real defect. +`services/water_level_csv.py` looked contacts up on `(name, organization)` with a +comment explaining that `Contact` enforces uniqueness on exactly that pair, while +`services/well_inventory_csv.py` also filtered on `contact_type`. The second form +misses an existing contact created with a different type and then fails on the +duplicate insert. Both now use the `(name, organization)` key. + +Two remaining copies of the enum-unwrapping idiom in +`services/well_inventory_csv.py` (`groundwater_level_reason`, `nma_data_quality`) +were left alone: each treats a falsy non-enum value slightly differently from +`domain.values.enum_value`, and reconciling them is a behavior change that wants +its own ticket. diff --git a/CLAUDE.md b/CLAUDE.md index d193e6a7c..77cb84105 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -117,8 +117,9 @@ Location (geographic point) ├── db/ # SQLAlchemy models (one file per table/resource) │ ├── engine.py # Database connection configuration │ └── ... +├── domain/ # Business rules as plain functions (no DB, no HTTP) ├── schemas/ # Pydantic schemas (validation, serialization) -├── services/ # Business logic and database interactions +├── services/ # Orchestration: load, call domain rules, persist ├── tests/ # Pytest test suite │ ├── conftest.py # Shared fixtures (test data setup) │ └── __init__.py # Sets test database (ocotilloapi_test) @@ -129,6 +130,22 @@ Location (geographic point) └── main.py # Application entry point ``` +### Domain Rules + +`domain/` holds business rules as plain functions over plain values -- unit +conversion, cross-column validation, deterministic naming. Modules there import +nothing from `api/`, `db/`, `schemas/`, or `services/`, and no `fastapi`, +`sqlalchemy`, `pydantic`, or `httpx`, so the rules are testable without a +database. + +`services/` loads the data, calls the rule, and persists the result. Domain +errors subclass `ValueError` because the CSV importers treat a `ValueError` +raised on a row as a per-row validation failure. + +Extraction is opportunistic, not a migration: move a rule into `domain/` when +you are already editing it and it is shared, subtle, or awkward to test in +place. Read **`ADR4.md`** before extending the layer. + ### Authentication & Authorization The system uses **Authentik** for OAuth2 authentication with role-based access control: diff --git a/domain/__init__.py b/domain/__init__.py new file mode 100644 index 000000000..49d8c64b9 --- /dev/null +++ b/domain/__init__.py @@ -0,0 +1,30 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +""" +Domain rules: business knowledge expressed as plain Python. + +Modules in this package must not import ``fastapi``, ``sqlalchemy``, ``pydantic``, +``httpx``, or anything from ``api/``, ``db/``, ``schemas/``, or ``services/``. +That restriction is the point: everything here is callable, and testable, without +a database session, an HTTP request, or a network round trip. + +Callers in ``services/`` are responsible for loading data, calling into these +rules, and persisting the result. + +See ``ADR4.md`` for the layering rationale. +""" + +# ============= EOF ============================================= diff --git a/domain/field_staff.py b/domain/field_staff.py new file mode 100644 index 000000000..6bc00e6ef --- /dev/null +++ b/domain/field_staff.py @@ -0,0 +1,68 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +""" +Field staff rules shared by the CSV importers. + +Both importers read the same three fixed staff columns and both create the same +kind of contact for a name they have not seen before. Keeping the roles and the +contact defaults here stops the two from drifting. +""" + +LEAD_ROLE = "Lead" +PARTICIPANT_ROLE = "Participant" + +FIELD_STAFF_CONTACT_TYPE = "Field Event Participant" +FIELD_STAFF_ORGANIZATION = "NMBGMR" +FIELD_STAFF_CONTACT_ROLE = "Technician" + + +def field_staff_entries( + lead: str | None, + second: str | None, + third: str | None, +) -> tuple[tuple[str, str], ...]: + """ + Normalize the three fixed staff columns into ``(name, role)`` pairs. + + The first column is the lead; the other two are participants. Blank columns + are dropped, so a row that names only a lead yields a single entry. + """ + specs = ( + (lead, LEAD_ROLE), + (second, PARTICIPANT_ROLE), + (third, PARTICIPANT_ROLE), + ) + return tuple((name, role) for name, role in specs if name) + + +def field_staff_contact_payload(name: str) -> dict: + """ + Build the contact payload used when an imported staff name has no contact yet. + + Callers must look the contact up on ``(name, organization)`` -- the pair + ``Contact`` enforces uniqueness on. Including ``contact_type`` in the lookup + misses an existing row that was created with a different type and then fails + on the duplicate insert. + """ + return { + "name": name, + "role": FIELD_STAFF_CONTACT_ROLE, + "organization": FIELD_STAFF_ORGANIZATION, + "contact_type": FIELD_STAFF_CONTACT_TYPE, + } + + +# ============= EOF ============================================= diff --git a/domain/samples.py b/domain/samples.py new file mode 100644 index 000000000..27cbc2f3e --- /dev/null +++ b/domain/samples.py @@ -0,0 +1,39 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +"""Sample naming rules.""" + +from datetime import datetime + +WATER_LEVEL_SAMPLE_TOKEN = "WL" +WATER_LEVEL_SAMPLE_TIMESTAMP_FORMAT = "%Y%m%d%H%M" + + +def water_level_sample_name(well_name: str, measured_at: datetime) -> str: + """ + Build the deterministic sample identifier for a groundwater-level measurement. + + Both CSV importers use this name to decide whether a measurement has already + been imported, so the two must agree exactly: the well inventory importer + writes the name and later looks a well up by it, while the water level + importer matches on it to update in place instead of inserting a duplicate. + A drift between the two formats would silently turn every re-import into a + new sample. + """ + stamp = measured_at.strftime(WATER_LEVEL_SAMPLE_TIMESTAMP_FORMAT) + return f"{well_name}-{WATER_LEVEL_SAMPLE_TOKEN}-{stamp}" + + +# ============= EOF ============================================= diff --git a/domain/units.py b/domain/units.py new file mode 100644 index 000000000..66fd5d917 --- /dev/null +++ b/domain/units.py @@ -0,0 +1,45 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +""" +Unit conversion. + +This is the single definition of the foot/meter relationship for application +code. ``services/util.py`` re-exports these names, so existing imports continue +to work; new code should import from here. + +Alembic revisions deliberately keep their own copy of the constant. A migration +must reproduce the arithmetic it ran with at the time it was written, so it +cannot track a moving import. +""" + +METERS_TO_FEET = 3.28084 + + +def convert_ft_to_m(feet: float | None, ndigits: int = 6) -> float | None: + """Convert a length from feet to meters.""" + if feet is None: + return None + return round(feet / METERS_TO_FEET, ndigits) + + +def convert_m_to_ft(meters: float | None, ndigits: int = 6) -> float | None: + """Convert a length from meters to feet.""" + if meters is None: + return None + return round(meters * METERS_TO_FEET, ndigits) + + +# ============= EOF ============================================= diff --git a/domain/values.py b/domain/values.py new file mode 100644 index 000000000..eba6d1ea5 --- /dev/null +++ b/domain/values.py @@ -0,0 +1,56 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +"""Small value helpers shared by the domain rules.""" + +from typing import Any + + +def enum_value(value: Any, default: Any = None) -> Any: + """ + Unwrap an ``Enum``-like value to its ``.value``. + + CSV rows reach the importers with fields that may be a validated enum member + or a bare string, depending on which Pydantic schema produced them, so the + ``x.value if hasattr(x, "value") else x`` idiom was repeated at roughly a + dozen call sites. + + Non-enum values pass through unchanged. When ``default`` is supplied, a falsy + non-enum value (``None``, ``""``) is replaced by it; when ``default`` is + omitted, falsy values are returned as-is. + """ + if hasattr(value, "value"): + return value.value + if default is not None and not value: + return default + return value + + +def build_notes(candidates) -> list[dict]: + """ + Turn ``(content, note_type)`` pairs into note payloads, dropping empty content. + + ``candidates`` is any iterable of two-tuples. Order is preserved, and a pair + whose content is ``None`` is skipped -- an empty string is *not* skipped, + matching the importers' existing ``is not None`` check. + """ + return [ + {"content": content, "note_type": note_type} + for content, note_type in candidates + if content is not None + ] + + +# ============= EOF ============================================= diff --git a/domain/water_levels.py b/domain/water_levels.py new file mode 100644 index 000000000..5939a51f1 --- /dev/null +++ b/domain/water_levels.py @@ -0,0 +1,98 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +""" +Water level rules used when importing measurement spreadsheets. + +The importer resolves the well and its measuring point history from the database +and then hands the plain numbers to these functions. Messages are returned +without a row prefix; the caller adds ``Row N:`` so the same rule can be reported +from a per-row importer or from a single-record API call. +""" + +MEASUREMENT_UNIT = "ft" +SAMPLE_MATRIX = "groundwater" +SAMPLE_QC_TYPE = "Normal" +GROUNDWATER_LEVEL_ACTIVITY_TYPE = "groundwater level" + + +def reconcile_measuring_point_height( + csv_mp_height: float | None, + existing_mp_height: float | int | None, +) -> tuple[float | int | None, float | int | None, bool]: + """ + Decide which measuring point height applies to a measurement. + + Returns ``(resolved, existing, differs)``. A height given in the CSV wins over + the well's recorded history, because the field crew measured it on the day of + the reading; ``differs`` reports that the two disagreed so the caller can warn + without rejecting the row. + + ``existing_mp_height`` arrives as whatever the database column yields, often a + ``Decimal``, and is coerced to ``float`` so callers compare and render like + values. + """ + if existing_mp_height is not None: + existing_mp_height = float(existing_mp_height) + + if csv_mp_height is not None: + differs = existing_mp_height is not None and csv_mp_height != existing_mp_height + return csv_mp_height, existing_mp_height, differs + + return existing_mp_height, existing_mp_height, False + + +def measuring_point_height_conflict_message( + csv_mp_height: float | None, + existing_mp_height: float | int | None, +) -> str: + """Describe a CSV height that overrides a different recorded height.""" + return ( + f"CSV mp_height ({csv_mp_height}) differs from existing measuring point " + f"height ({existing_mp_height}); CSV value will be used" + ) + + +def depth_to_water_error( + depth_to_water_ft: float | None, + resolved_mp_height: float | int | None, + well_depth: float | int | None, +) -> str | None: + """ + Reject a reading that puts the water table below the bottom of the well. + + ``depth_to_water_ft`` is measured from the measuring point, which sits above + the ground surface, while ``well_depth`` is measured from the ground surface, + so the two are only comparable after subtracting the measuring point height. + + Returns ``None`` when the check does not apply -- any of the three inputs may + be missing, and an unknown well depth is not evidence of a bad reading. + """ + if depth_to_water_ft is None or resolved_mp_height is None or well_depth is None: + return None + + well_depth = float(well_depth) + corrected_depth_to_water = depth_to_water_ft - resolved_mp_height + if corrected_depth_to_water >= well_depth: + return ( + f"depth_to_water_ft minus measuring point height " + f"({corrected_depth_to_water}) must be less than well depth " + f"({well_depth})" + ) + + return None + + +# ============= EOF ============================================= diff --git a/domain/wells.py b/domain/wells.py new file mode 100644 index 000000000..fcabf0ba4 --- /dev/null +++ b/domain/wells.py @@ -0,0 +1,214 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +""" +Well rules used when importing the well inventory spreadsheet. + +Every function here takes plain values and returns plain values, so the rules can +be exercised without a database. ``services/well_inventory_csv.py`` supplies the +values from a validated ``WellInventoryRow`` and persists whatever comes back. + +Errors subclass ``ValueError`` because the importer already treats a ``ValueError`` +raised while handling a row as a per-row validation failure rather than an aborted +import. +""" + +import re + +from core.constants import SRID_UTM_ZONE_12N, SRID_UTM_ZONE_13N +from domain.units import convert_ft_to_m +from domain.values import enum_value + +AUTOGEN_DEFAULT_PREFIX = "NM-" +AUTOGEN_PREFIX_REGEX = re.compile(r"^[A-Z]{2,3}-$", re.IGNORECASE) +AUTOGEN_TOKEN_REGEX = re.compile( + r"^(?P[A-Z]{2,3})\s*-\s*(?:x{4}|X{4})$", re.IGNORECASE +) + +# TODO: this needs to be more sophisticated in the future. Likely more than 13N +# and 12N will be used. +UTM_ZONE_SRIDS = { + "13N": SRID_UTM_ZONE_13N, + "12N": SRID_UTM_ZONE_12N, +} + +RELEASE_STATUS_PUBLIC = "public" +RELEASE_STATUS_PRIVATE = "private" +RELEASE_STATUS_DRAFT = "draft" + +UNKNOWN_DEPTH_SOURCE = "unknown" + +SITE_NAME_ORGANIZATION = "NMBGMR" +OSE_WELL_RECORD_ORGANIZATION = "NMOSE" +ALTERNATE_ID_RELATION = "same_as" + + +class UnsupportedUtmZone(ValueError): + """Raised when a row carries a UTM zone the importer cannot project from.""" + + +class ConflictingMeasuringPointHeight(ValueError): + """Raised when a row gives two different measuring point heights.""" + + +def autogen_prefix(well_id: str | None) -> str | None: + """ + Return the normalized auto-generation prefix for a placeholder well id. + + Returns ``None`` when the value is a real well id and should be used as-is. + + Supported placeholder forms: + + - ``XY-`` / ``ABC-`` -- a bare 2-3 letter prefix + - ``WL-XXXX`` / ``SAC-xxxx`` -- a prefix with a placeholder number, with + optional spaces around the dash + - blank -- uses the default ``NM-`` prefix + """ + value = (well_id or "").strip() + + if not value: + return AUTOGEN_DEFAULT_PREFIX + + if AUTOGEN_PREFIX_REGEX.match(value): + return f"{value[:-1].upper()}-" + + match = AUTOGEN_TOKEN_REGEX.match(value) + if match: + return f"{match.group('prefix').upper()}-" + + return None + + +def srid_for_utm_zone(utm_zone: str | None) -> int: + """Return the EPSG code for a supported UTM zone label.""" + try: + return UTM_ZONE_SRIDS[utm_zone] + except KeyError: + raise UnsupportedUtmZone(f"Unsupported UTM zone: {utm_zone}") from None + + +def elevation_m_from_ft(elevation_ft: float | str | None) -> float: + """ + Convert a reported elevation to the meters the ``Location`` row stores. + + A missing elevation becomes ``0.0`` rather than ``NULL``: ``Location.elevation`` + is not nullable, and the inventory sheet leaves the column blank for wells + whose elevation has not been surveyed yet. + """ + if elevation_ft is None: + return 0.0 + return convert_ft_to_m(float(elevation_ft)) + + +def release_status(public_availability_acknowledgement: bool | None) -> str: + """ + Map the public-availability acknowledgement to a location release status. + + The acknowledgement is deliberately three-state. An unanswered question is + not the same as a refusal, so it holds the location in ``draft`` instead of + publishing or hiding it. + """ + if public_availability_acknowledgement is True: + return RELEASE_STATUS_PUBLIC + if public_availability_acknowledgement is False: + return RELEASE_STATUS_PRIVATE + return RELEASE_STATUS_DRAFT + + +def resolve_measuring_point_height( + mp_height: float | None, + measuring_point_height_ft: float | None, +) -> float | None: + """ + Reconcile the two columns that can carry a measuring point height. + + The sheet grew a second spelling of the same measurement. Either may be + supplied, but when both are they must agree -- guessing which one is + authoritative would silently bias every water level computed against it. + """ + if ( + mp_height is not None + and measuring_point_height_ft is not None + and mp_height != measuring_point_height_ft + ): + raise ConflictingMeasuringPointHeight( + "Conflicting values for measuring point height: " + "mp_height and measuring_point_height_ft" + ) + + if measuring_point_height_ft is not None: + return measuring_point_height_ft + return mp_height + + +def historic_depth_to_water_source(depth_source) -> str: + """ + Return the source to credit for a historic depth-to-water reading. + + Developer's note: Laila said the depth source is almost always the source for + the historic depth to water, and that reusing it here is acceptable. + """ + if not depth_source: + return UNKNOWN_DEPTH_SOURCE + return str(enum_value(depth_source)).lower() + + +def historic_depth_to_water_note( + historic_depth_to_water_ft: float | None, + depth_source, +) -> str | None: + """ + Render the historic depth-to-water note, or ``None`` when there is no reading. + + The value is recorded as a note rather than a measurement because it is + hearsay from the well owner, not something the field crew observed. + """ + if historic_depth_to_water_ft is None: + return None + source = historic_depth_to_water_source(depth_source) + return ( + f"historic depth to water: {historic_depth_to_water_ft} ft - source: {source}" + ) + + +def well_purposes(*purposes) -> list: + """Collapse the fixed well-purpose columns into a list, dropping blanks.""" + return [purpose for purpose in purposes if purpose] + + +def alternate_ids(site_name: str | None, ose_well_record_id: str | None) -> list[dict]: + """ + Build the alternate-id payloads for the identifiers other agencies use. + + ``thing_id`` is a placeholder; the caller replaces it once the ``Thing`` has + been flushed and has an id. + """ + pairs = ( + (site_name, SITE_NAME_ORGANIZATION), + (ose_well_record_id, OSE_WELL_RECORD_ORGANIZATION), + ) + return [ + { + "thing_id": -1, + "alternate_id": alternate_id, + "alternate_organization": organization, + "relation": ALTERNATE_ID_RELATION, + } + for alternate_id, organization in pairs + if alternate_id is not None + ] + + +# ============= EOF ============================================= diff --git a/services/util.py b/services/util.py index aeeaae807..dbf88f98e 100644 --- a/services/util.py +++ b/services/util.py @@ -11,8 +11,17 @@ from core.constants import SRID_WGS84 +# Re-exported so the many existing ``from services.util import convert_ft_to_m`` +# imports keep working. The definitions live in ``domain/units.py``; importing +# them from here drags in httpx, pyproj, and SQLAlchemy, which is exactly what +# the domain layer exists to avoid. +from domain.units import ( # noqa: F401 + METERS_TO_FEET, + convert_ft_to_m, + convert_m_to_ft, +) + TRANSFORMERS = {} -METERS_TO_FEET = 3.28084 DEFAULT_HTTP_TIMEOUT = 10.0 DEFAULT_HTTP_RETRIES = 3 DEFAULT_HTTP_BACKOFF = 0.5 @@ -120,20 +129,6 @@ def convert_dt_tz_naive_to_tz_aware( return dt_aware -def convert_ft_to_m(feet: float | None, ndigits: int = 6) -> float | None: - """Convert a length from feet to meters.""" - if feet is None: - return None - return round(feet / METERS_TO_FEET, ndigits) - - -def convert_m_to_ft(meters: float | None, ndigits: int = 6) -> float | None: - """Convert a length from meters to feet.""" - if meters is None: - return None - return round(meters * METERS_TO_FEET, ndigits) - - def get_tiger_data( lon: float, lat: float, layer: int, outfields: str = "*" ) -> dict | None: diff --git a/services/water_level_csv.py b/services/water_level_csv.py index a9f4198d0..848db529a 100644 --- a/services/water_level_csv.py +++ b/services/water_level_csv.py @@ -35,6 +35,21 @@ FieldEventParticipant, ) from db.engine import session_ctx +from domain.field_staff import ( + FIELD_STAFF_ORGANIZATION, + field_staff_contact_payload, + field_staff_entries, +) +from domain.samples import water_level_sample_name +from domain.water_levels import ( + GROUNDWATER_LEVEL_ACTIVITY_TYPE, + MEASUREMENT_UNIT, + SAMPLE_MATRIX, + SAMPLE_QC_TYPE, + depth_to_water_error, + measuring_point_height_conflict_message, + reconcile_measuring_point_height, +) from pydantic import ValidationError from schemas.water_level_csv import ( WaterLevelCsvRow, @@ -323,30 +338,16 @@ def _normalize_field_staff_entries( model: WaterLevelCsvRow, ) -> tuple[tuple[str, str], ...]: """Normalize fixed staff columns into an iterable participant list.""" - participant_specs = ( - (model.field_staff, "Lead"), - (model.field_staff_2, "Participant"), - (model.field_staff_3, "Participant"), - ) - return tuple( - (staff_name, role) for staff_name, role in participant_specs if staff_name + return field_staff_entries( + model.field_staff, model.field_staff_2, model.field_staff_3 ) def _resolve_measuring_point_height( well: Thing, csv_mp_height: float | None ) -> tuple[float | int | None, float | int | None, bool]: - existing_mp_height = well.measuring_point_height - if existing_mp_height is not None: - existing_mp_height = float(existing_mp_height) - if csv_mp_height is not None: - return ( - csv_mp_height, - existing_mp_height, - (existing_mp_height is not None and csv_mp_height != existing_mp_height), - ) - - return existing_mp_height, existing_mp_height, False + """Read the well's recorded height and apply the reconciliation rule.""" + return reconcile_measuring_point_height(csv_mp_height, well.measuring_point_height) def _validate_depth_to_water_against_well( @@ -355,22 +356,11 @@ def _validate_depth_to_water_against_well( depth_to_water_ft: float | None, resolved_mp_height: float | int | None, ) -> str | None: - well_depth = well.well_depth - if well_depth is not None: - well_depth = float(well_depth) - - if depth_to_water_ft is None or resolved_mp_height is None or well_depth is None: + """Apply the depth-to-water rule to a well, tagging any message with its row.""" + error = depth_to_water_error(depth_to_water_ft, resolved_mp_height, well.well_depth) + if error is None: return None - - corrected_depth_to_water = depth_to_water_ft - resolved_mp_height - if corrected_depth_to_water >= well_depth: - return ( - f"Row {row_index}: depth_to_water_ft minus measuring point height " - f"({corrected_depth_to_water}) must be less than well depth " - f"({well_depth})" - ) - - return None + return f"Row {row_index}: {error}" def _create_records( @@ -395,7 +385,7 @@ def _create_records( ) field_activity = FieldActivity( field_event=field_event, - activity_type="groundwater level", + activity_type=GROUNDWATER_LEVEL_ACTIVITY_TYPE, # Measuring staff now lives on structured participants and the # sample participant link, not in field_activity.notes. notes=None, @@ -433,10 +423,10 @@ def _create_records( if row.mp_height_differs_from_history: errors.append( - "Row " - f"{row.row_index}: CSV mp_height ({row.mp_height}) differs " - "from existing measuring point height " - f"({row.existing_mp_height}); CSV value will be used" + f"Row {row.row_index}: " + + measuring_point_height_conflict_message( + row.mp_height, row.existing_mp_height + ) ) created.append( @@ -463,7 +453,7 @@ def _create_records( def _build_sample_name(row: _ValidatedRow) -> str: """Build the deterministic sample identifier used for create/update matching.""" - return f"{row.well.name}-WL-{row.measurement_dt.strftime('%Y%m%d%H%M')}" + return water_level_sample_name(row.well.name, row.measurement_dt) def _find_existing_imported_sample( @@ -482,7 +472,7 @@ def _find_existing_imported_sample( .where( Thing.name == row.well.name, Thing.thing_type == "water well", - FieldActivity.activity_type == "groundwater level", + FieldActivity.activity_type == GROUNDWATER_LEVEL_ACTIVITY_TYPE, Sample.sample_name == sample_name, ) .order_by(Sample.id.asc()) @@ -537,25 +527,19 @@ def _ensure_field_event_participants( def _get_or_create_field_staff_contact(session: Session, staff_name: str) -> Contact: """Resolve or create the contact record used by field event participants.""" - contact_type = "Field Event Participant" - organization = "NMBGMR" # Contact uniqueness is enforced on (name, organization), so the lookup # must use the same key to avoid missing an existing row with a different # contact_type and attempting a duplicate insert. contact = session.scalars( select(Contact) .where(Contact.name == staff_name) - .where(Contact.organization == organization) + .where(Contact.organization == FIELD_STAFF_ORGANIZATION) ).first() if contact is None: - payload = { - "name": staff_name, - "role": "Technician", - "organization": organization, - "contact_type": contact_type, - } - contact = add_contact(session, payload, None, commit=False) + contact = add_contact( + session, field_staff_contact_payload(staff_name), None, commit=False + ) return contact @@ -592,9 +576,9 @@ def _apply_sample_values(sample: Sample, row: _ValidatedRow, sample_name: str) - """Apply normalized sample values from the validated CSV row.""" sample.sample_date = row.measurement_dt sample.sample_name = sample_name - sample.sample_matrix = "groundwater" + sample.sample_matrix = SAMPLE_MATRIX sample.sample_method = row.sample_method_term - sample.qc_type = "Normal" + sample.qc_type = SAMPLE_QC_TYPE sample.notes = row.water_level_notes @@ -605,7 +589,7 @@ def _apply_observation_values( observation.observation_datetime = row.measurement_dt observation.parameter_id = parameter_id observation.value = row.depth_to_water_ft - observation.unit = "ft" + observation.unit = MEASUREMENT_UNIT observation.measuring_point_height = row.resolved_mp_height observation.groundwater_level_reason = row.level_status observation.nma_data_quality = row.data_quality diff --git a/services/well_inventory_csv.py b/services/well_inventory_csv.py index 18e9a4f53..ccb2863b5 100644 --- a/services/well_inventory_csv.py +++ b/services/well_inventory_csv.py @@ -29,7 +29,7 @@ from sqlalchemy.orm import Session from starlette.status import HTTP_400_BAD_REQUEST -from core.constants import SRID_UTM_ZONE_13N, SRID_UTM_ZONE_12N, SRID_WGS84 +from core.constants import SRID_WGS84 from db import ( Group, Location, @@ -46,53 +46,40 @@ Parameter, ) from db.engine import session_ctx +from domain.field_staff import ( + FIELD_STAFF_ORGANIZATION, + LEAD_ROLE, + PARTICIPANT_ROLE, + field_staff_contact_payload, +) +from domain.samples import water_level_sample_name +from domain.values import build_notes, enum_value +from domain.water_levels import ( + GROUNDWATER_LEVEL_ACTIVITY_TYPE, + MEASUREMENT_UNIT, + SAMPLE_MATRIX, +) +from domain.wells import ( + alternate_ids, + autogen_prefix, + elevation_m_from_ft, + historic_depth_to_water_note, + release_status, + resolve_measuring_point_height, + srid_for_utm_zone, + well_purposes, +) from pydantic import ValidationError from schemas.thing import CreateWell from schemas.well_inventory import WellInventoryRow from services.contact_helper import add_contact from services.exceptions_helper import PydanticStyleException from services.thing_helper import add_thing, find_water_wells_by_name -from services.util import transform_srid, convert_ft_to_m +from services.util import transform_srid -AUTOGEN_DEFAULT_PREFIX = "NM-" -AUTOGEN_PREFIX_REGEX = re.compile(r"^[A-Z]{2,3}-$", re.IGNORECASE) -AUTOGEN_TOKEN_REGEX = re.compile( - r"^(?P[A-Z]{2,3})\s*-\s*(?:x{4}|X{4})$", re.IGNORECASE -) PROGRESS_INTERVAL = 25 -def _extract_autogen_prefix(well_id: str | None) -> str | None: - """ - Return normalized auto-generation prefix when a placeholder token is provided. - - Supported forms: - - ``XY-`` (existing behavior) - - ``WL-XXXX`` / ``SAC-XXXX`` / ``ABC-XXXX`` (2-3 uppercase letter prefixes) - - blank value (uses default ``NM-`` prefix) - """ - # Normalize input - value = (well_id or "").strip() - - # Blank / missing value -> use default prefix - if not value: - return AUTOGEN_DEFAULT_PREFIX - - # Direct prefix form, e.g. "XY-" or "ABC-" - if AUTOGEN_PREFIX_REGEX.match(value): - # Ensure normalized trailing dash and uppercase - prefix = value[:-1].upper() - return f"{prefix}-" - - # Token form, e.g. "WL-XXXX", "SAC-xxxx", with optional spaces around "-" - m = AUTOGEN_TOKEN_REGEX.match(value) - if m: - prefix = m.group("prefix").upper() - return f"{prefix}-" - - return None - - def import_well_inventory_csv(*args, **kw) -> dict: with session_ctx() as session: return _import_well_inventory_csv(session, *args, **kw) @@ -363,47 +350,28 @@ def _extract_field_from_value_error(error_text: str) -> str: def _make_location(model) -> Location: point = Point(model.utm_easting, model.utm_northing) - # TODO: this needs to be more sophisticated in the future. Likely more than 13N and 12N will be used - if model.utm_zone == "13N": - source_srid = SRID_UTM_ZONE_13N - elif model.utm_zone == "12N": - source_srid = SRID_UTM_ZONE_12N - else: - raise ValueError(f"Unsupported UTM zone: {model.utm_zone}") - # Convert the point to a WGS84 coordinate system transformed_point = transform_srid( - point, source_srid=source_srid, target_srid=SRID_WGS84 + point, + source_srid=srid_for_utm_zone(model.utm_zone), + target_srid=SRID_WGS84, ) - elevation_ft = model.elevation_ft - elevation_m = ( - convert_ft_to_m(float(elevation_ft)) if elevation_ft is not None else 0.0 - ) - - release_status = "draft" - if model.public_availability_acknowledgement is True: - release_status = "public" - elif model.public_availability_acknowledgement is False: - release_status = "private" - loc = Location( + return Location( point=transformed_point.wkt, - elevation=elevation_m, - release_status=release_status, + elevation=elevation_m_from_ft(model.elevation_ft), + release_status=release_status(model.public_availability_acknowledgement), ) - return loc - def _make_contact(model: WellInventoryRow, well: Thing, idx) -> dict: # add contact - notes = [] - for content, note_type in ( - (model.result_communication_preference, "Communication"), - (model.contact_special_requests_notes, "General"), - ): - if content is not None: - notes.append({"content": content, "note_type": note_type}) + notes = build_notes( + ( + (model.result_communication_preference, "Communication"), + (model.contact_special_requests_notes, "General"), + ) + ) emails = [] phones = [] @@ -517,9 +485,8 @@ def _find_existing_imported_well( session: Session, model: WellInventoryRow ) -> Thing | None: if model.measurement_date_time is not None: - sample_name = ( - f"{model.well_name_point_id}-WL-" - f"{model.measurement_date_time.strftime('%Y%m%d%H%M')}" + sample_name = water_level_sample_name( + model.well_name_point_id, model.measurement_date_time ) existing = session.scalars( select(Thing) @@ -529,7 +496,7 @@ def _find_existing_imported_well( .where( Thing.name == model.well_name_point_id, Thing.thing_type == "water well", - FieldActivity.activity_type == "groundwater level", + FieldActivity.activity_type == GROUNDWATER_LEVEL_ACTIVITY_TYPE, Sample.sample_name == sample_name, ) .order_by(Thing.id.asc()) @@ -567,13 +534,11 @@ def _make_row_models(rows, session, progress_callback=None): raise ValueError("Field required") well_id = row.get("well_name_point_id") - autogen_prefix = _extract_autogen_prefix(well_id) - if autogen_prefix is not None: - offset = offsets.get(autogen_prefix, 0) - well_id, offset = _generate_autogen_well_id( - session, autogen_prefix, offset - ) - offsets[autogen_prefix] = offset + prefix = autogen_prefix(well_id) + if prefix is not None: + offset = offsets.get(prefix, 0) + well_id, offset = _generate_autogen_well_id(session, prefix, offset) + offsets[prefix] = offset row["well_name_point_id"] = well_id elif not well_id: raise ValueError("Field required") @@ -644,18 +609,19 @@ def _make_row_models(rows, session, progress_callback=None): def _add_field_staff( session: Session, fs: str, field_event: FieldEvent, role: str, user: str ) -> None: - ct = "Field Event Participant" - org = "NMBGMR" + # Contact uniqueness is enforced on (name, organization), so the lookup must + # use the same key. Adding contact_type here misses an existing row created + # with a different type and then fails on the duplicate insert. contact = session.scalars( select(Contact) .where(Contact.name == fs) - .where(Contact.organization == org) - .where(Contact.contact_type == ct) + .where(Contact.organization == FIELD_STAFF_ORGANIZATION) ).first() if not contact: - payload = dict(name=fs, role="Technician", organization=org, contact_type=ct) - contact = add_contact(session, payload, user, commit=False) + contact = add_contact( + session, field_staff_contact_payload(fs), user, commit=False + ) fec = FieldEventParticipant( field_event=field_event, contact_id=contact.id, participant_role=role @@ -694,16 +660,11 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) session.add(directions_note) # add data provenance records - elevation_method = ( - model.elevation_method.value - if hasattr(model.elevation_method, "value") - else (model.elevation_method or "Unknown") - ) dp = DataProvenance( target_id=loc.id, target_table="location", field_name="elevation", - collection_method=elevation_method, + collection_method=enum_value(model.elevation_method, "Unknown"), ) session.add(dp) @@ -712,67 +673,29 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) # -------------------- # add Thing - """ - Developer's note - - Laila said that the depth source is almost always the source for the historic depth to water. - She indicated that it would be acceptable to use the depth source for the historic depth to water source. - """ - if model.depth_source: - historic_depth_to_water_source = ( - model.depth_source.value - if hasattr(model.depth_source, "value") - else model.depth_source - ).lower() - else: - historic_depth_to_water_source = "unknown" + historic_depth_note = historic_depth_to_water_note( + model.historic_depth_to_water_ft, model.depth_source + ) - if model.historic_depth_to_water_ft is not None: - historic_depth_note = f"historic depth to water: {model.historic_depth_to_water_ft} ft - source: {historic_depth_to_water_source}" - else: - historic_depth_note = None - - well_notes = [] - for note_content, note_type in ( - (model.specific_location_of_well, "Access"), - (model.contact_special_requests_notes, "General"), - (model.well_measuring_notes, "Sampling Procedure"), - (model.sampling_scenario_notes, "Sampling Procedure"), - (model.well_notes, "General"), - (model.water_notes, "Water"), - (historic_depth_note, "Historical"), + well_notes = build_notes( ( + (model.specific_location_of_well, "Access"), + (model.contact_special_requests_notes, "General"), + (model.well_measuring_notes, "Sampling Procedure"), + (model.sampling_scenario_notes, "Sampling Procedure"), + (model.well_notes, "General"), + (model.water_notes, "Water"), + (historic_depth_note, "Historical"), ( - f"Sample possible: {model.sample_possible}" - if model.sample_possible is not None - else None + ( + f"Sample possible: {model.sample_possible}" + if model.sample_possible is not None + else None + ), + "Sampling Procedure", ), - "Sampling Procedure", - ), - ): - if note_content is not None: - well_notes.append({"content": note_content, "note_type": note_type}) - - alternate_ids = [] - for alternate_id, alternate_organization in ( - (model.site_name, "NMBGMR"), - (model.ose_well_record_id, "NMOSE"), - ): - if alternate_id is not None: - alternate_ids.append( - { - "thing_id": -1, - "alternate_id": alternate_id, - "alternate_organization": alternate_organization, - "relation": "same_as", - } - ) - - well_purposes = [] - if model.well_purpose: - well_purposes.append(model.well_purpose) - if model.well_purpose_2: - well_purposes.append(model.well_purpose_2) + ) + ) monitoring_frequencies = [] if model.monitoring_frequency: @@ -783,21 +706,9 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) } ) - if ( - 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 is not None: - universal_mp_height = model.measuring_point_height_ft - elif model.mp_height is not None: - universal_mp_height = model.mp_height - else: - universal_mp_height = None + universal_mp_height = resolve_measuring_point_height( + model.mp_height, model.measuring_point_height_ft + ) data = CreateWell( location_id=loc.id, @@ -815,20 +726,12 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) well_pump_depth=model.well_pump_depth_ft, is_suitable_for_datalogger=model.datalogger_possible, is_open=model.is_open, - well_status=( - model.well_status.value - if hasattr(model.well_status, "value") - else model.well_status - ), - monitoring_status=( - model.monitoring_status.value - if hasattr(model.monitoring_status, "value") - else model.monitoring_status - ), + well_status=enum_value(model.well_status), + monitoring_status=enum_value(model.monitoring_status), notes=well_notes, - well_purposes=well_purposes, + well_purposes=well_purposes(model.well_purpose, model.well_purpose_2), monitoring_frequencies=monitoring_frequencies, - alternate_ids=alternate_ids, + alternate_ids=alternate_ids(model.site_name, model.ose_well_record_id), ) well_data = data.model_dump() @@ -878,9 +781,9 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) # add field staff for fsi, role in ( - (model.field_staff, "Lead"), - (model.field_staff_2, "Participant"), - (model.field_staff_3, "Participant"), + (model.field_staff, LEAD_ROLE), + (model.field_staff_2, PARTICIPANT_ROLE), + (model.field_staff_3, PARTICIPANT_ROLE), ): if not fsi: continue @@ -920,24 +823,19 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) # create FieldActivity gwl_field_activity = FieldActivity( field_event=fe, - activity_type="groundwater level", + activity_type=GROUNDWATER_LEVEL_ACTIVITY_TYPE, notes="Groundwater level measurement activity conducted during well inventory field event.", ) session.add(gwl_field_activity) session.flush() # create Sample - sample_method = ( - model.sample_method.value - if hasattr(model.sample_method, "value") - else (model.sample_method or "Unknown") - ) sample = Sample( field_activity_id=gwl_field_activity.id, sample_date=model.measurement_date_time, - sample_name=f"{well.name}-WL-{model.measurement_date_time.strftime('%Y%m%d%H%M')}", - sample_matrix="groundwater", - sample_method=sample_method, + sample_name=water_level_sample_name(well.name, model.measurement_date_time), + sample_matrix=SAMPLE_MATRIX, + sample_method=enum_value(model.sample_method, "Unknown"), notes=model.water_level_notes, ) session.add(sample) @@ -949,7 +847,7 @@ def _add_csv_row(session: Session, group: Group, model: WellInventoryRow, user) sample_id=sample.id, parameter_id=parameter.id, value=model.depth_to_water_ft, - unit="ft", + unit=MEASUREMENT_UNIT, observation_datetime=model.measurement_date_time, measuring_point_height=universal_mp_height, groundwater_level_reason=( diff --git a/tests/test_domain_values.py b/tests/test_domain_values.py new file mode 100644 index 000000000..a638b777b --- /dev/null +++ b/tests/test_domain_values.py @@ -0,0 +1,109 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +"""Shared value helpers and field staff rules. No database, no fixtures.""" + +from enum import Enum + +from domain.field_staff import ( + FIELD_STAFF_CONTACT_TYPE, + FIELD_STAFF_ORGANIZATION, + field_staff_contact_payload, + field_staff_entries, +) +from domain.values import build_notes, enum_value + + +class _Method(Enum): + STEEL_TAPE = "Steel Tape" + + +# -------------------------------------------------------------------------- +# enum_value +# -------------------------------------------------------------------------- +def test_enum_value_unwraps_an_enum(): + assert enum_value(_Method.STEEL_TAPE) == "Steel Tape" + + +def test_enum_value_passes_a_plain_string_through(): + assert enum_value("Steel Tape") == "Steel Tape" + + +def test_enum_value_without_a_default_returns_falsy_values_unchanged(): + assert enum_value(None) is None + assert enum_value("") == "" + + +def test_enum_value_substitutes_the_default_for_falsy_values(): + assert enum_value(None, "Unknown") == "Unknown" + assert enum_value("", "Unknown") == "Unknown" + + +def test_enum_value_default_does_not_override_an_enum(): + assert enum_value(_Method.STEEL_TAPE, "Unknown") == "Steel Tape" + + +# -------------------------------------------------------------------------- +# build_notes +# -------------------------------------------------------------------------- +def test_build_notes_keeps_order_and_drops_missing_content(): + assert build_notes( + ( + ("locked gate", "Access"), + (None, "General"), + ("call ahead", "Communication"), + ) + ) == [ + {"content": "locked gate", "note_type": "Access"}, + {"content": "call ahead", "note_type": "Communication"}, + ] + + +def test_build_notes_keeps_an_empty_string(): + # Only None means "no note"; the importers never filtered on truthiness. + assert build_notes((("", "General"),)) == [{"content": "", "note_type": "General"}] + + +def test_build_notes_of_nothing_is_empty(): + assert build_notes(()) == [] + + +# -------------------------------------------------------------------------- +# field staff +# -------------------------------------------------------------------------- +def test_field_staff_entries_assigns_lead_then_participants(): + assert field_staff_entries("A Lopez", "B Chen", "C Diaz") == ( + ("A Lopez", "Lead"), + ("B Chen", "Participant"), + ("C Diaz", "Participant"), + ) + + +def test_field_staff_entries_drops_blank_columns(): + assert field_staff_entries("A Lopez", None, "") == (("A Lopez", "Lead"),) + assert field_staff_entries(None, "B Chen", None) == (("B Chen", "Participant"),) + assert field_staff_entries(None, None, None) == () + + +def test_field_staff_contact_payload_uses_the_shared_defaults(): + assert field_staff_contact_payload("A Lopez") == { + "name": "A Lopez", + "role": "Technician", + "organization": FIELD_STAFF_ORGANIZATION, + "contact_type": FIELD_STAFF_CONTACT_TYPE, + } + + +# ============= EOF ============================================= diff --git a/tests/test_domain_water_levels.py b/tests/test_domain_water_levels.py new file mode 100644 index 000000000..64ea0ed74 --- /dev/null +++ b/tests/test_domain_water_levels.py @@ -0,0 +1,138 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +"""Water level and sample-naming rules. No database, no fixtures.""" + +from datetime import datetime +from decimal import Decimal + +from domain.samples import water_level_sample_name +from domain.water_levels import ( + depth_to_water_error, + measuring_point_height_conflict_message, + reconcile_measuring_point_height, +) + + +# -------------------------------------------------------------------------- +# reconcile_measuring_point_height +# -------------------------------------------------------------------------- +def test_reconcile_prefers_the_csv_height_and_reports_the_difference(): + resolved, existing, differs = reconcile_measuring_point_height(4.0, 3.5) + + assert resolved == 4.0 + assert existing == 3.5 + assert differs is True + + +def test_reconcile_falls_back_to_the_recorded_height(): + resolved, existing, differs = reconcile_measuring_point_height(None, 3.5) + + assert resolved == 3.5 + assert existing == 3.5 + assert differs is False + + +def test_reconcile_coerces_a_decimal_history_value(): + resolved, existing, differs = reconcile_measuring_point_height(None, Decimal("3.5")) + + assert resolved == 3.5 + assert isinstance(existing, float) + assert differs is False + + +def test_reconcile_allows_both_missing(): + assert reconcile_measuring_point_height(None, None) == (None, None, False) + + +def test_reconcile_does_not_flag_a_matching_height(): + _, _, differs = reconcile_measuring_point_height(3.5, Decimal("3.5")) + + assert differs is False + + +def test_reconcile_does_not_flag_a_csv_height_with_no_history(): + resolved, existing, differs = reconcile_measuring_point_height(4.0, None) + + assert resolved == 4.0 + assert existing is None + assert differs is False + + +def test_measuring_point_height_conflict_message_names_both_values(): + assert measuring_point_height_conflict_message(1.5, 2.0) == ( + "CSV mp_height (1.5) differs from existing measuring point height (2.0); " + "CSV value will be used" + ) + + +# -------------------------------------------------------------------------- +# depth_to_water_error +# -------------------------------------------------------------------------- +def test_depth_to_water_error_rejects_a_reading_below_the_well_bottom(): + assert depth_to_water_error(12.5, 1.0, 10.0) == ( + "depth_to_water_ft minus measuring point height (11.5) " + "must be less than well depth (10.0)" + ) + + +def test_depth_to_water_error_accepts_a_reading_inside_the_well(): + assert depth_to_water_error(8.0, 1.0, 10.0) is None + + +def test_depth_to_water_error_rejects_water_exactly_at_the_bottom(): + # The corrected depth must be strictly less than the well depth. + assert depth_to_water_error(11.0, 1.0, 10.0) is not None + + +def test_depth_to_water_error_subtracts_the_measuring_point_height(): + # Without the correction this reading would look like it was past the bottom. + assert depth_to_water_error(10.5, 1.0, 10.0) is None + + +def test_depth_to_water_error_coerces_a_decimal_well_depth(): + assert depth_to_water_error(12.5, 1.0, Decimal("10.0")) == ( + "depth_to_water_ft minus measuring point height (11.5) " + "must be less than well depth (10.0)" + ) + + +def test_depth_to_water_error_skips_when_an_input_is_missing(): + assert depth_to_water_error(None, 1.0, 10.0) is None + assert depth_to_water_error(12.5, None, 10.0) is None + assert depth_to_water_error(12.5, 1.0, None) is None + + +# -------------------------------------------------------------------------- +# water_level_sample_name +# -------------------------------------------------------------------------- +def test_water_level_sample_name_is_deterministic(): + measured_at = datetime(2026, 3, 4, 9, 5) + + assert water_level_sample_name("AR0001", measured_at) == "AR0001-WL-202603040905" + + +def test_water_level_sample_name_ignores_sub_minute_precision(): + # Both importers must agree on the name for re-import matching to work, so + # seconds are deliberately not part of it. + with_seconds = datetime(2026, 3, 4, 9, 5, 42) + without_seconds = datetime(2026, 3, 4, 9, 5) + + assert water_level_sample_name("AR0001", with_seconds) == water_level_sample_name( + "AR0001", without_seconds + ) + + +# ============= EOF ============================================= diff --git a/tests/test_domain_wells.py b/tests/test_domain_wells.py new file mode 100644 index 000000000..5429f10d8 --- /dev/null +++ b/tests/test_domain_wells.py @@ -0,0 +1,210 @@ +# =============================================================================== +# Copyright 2026 ross +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# =============================================================================== +"""Well rules. No database, no fixtures.""" + +from enum import Enum + +import pytest + +from core.constants import SRID_UTM_ZONE_12N, SRID_UTM_ZONE_13N +from domain.wells import ( + AUTOGEN_DEFAULT_PREFIX, + ConflictingMeasuringPointHeight, + UnsupportedUtmZone, + alternate_ids, + autogen_prefix, + elevation_m_from_ft, + historic_depth_to_water_note, + historic_depth_to_water_source, + release_status, + resolve_measuring_point_height, + srid_for_utm_zone, + well_purposes, +) + + +class _DepthSource(Enum): + DRILLER = "Driller" + + +# -------------------------------------------------------------------------- +# autogen_prefix +# -------------------------------------------------------------------------- +@pytest.mark.parametrize( + "well_id, expected", + [ + ("", AUTOGEN_DEFAULT_PREFIX), + (" ", AUTOGEN_DEFAULT_PREFIX), + (None, AUTOGEN_DEFAULT_PREFIX), + ("XY-", "XY-"), + ("xy-", "XY-"), + ("ABC-", "ABC-"), + ("WL-XXXX", "WL-"), + ("SAC-xxxx", "SAC-"), + ("WL - XXXX", "WL-"), + (" WL-XXXX ", "WL-"), + ], +) +def test_autogen_prefix_recognizes_placeholders(well_id, expected): + assert autogen_prefix(well_id) == expected + + +@pytest.mark.parametrize( + "well_id", + ["AR0001", "WL-0001", "A-", "ABCD-", "WL-XXX", "WL-XXXXX", "NM-1234"], +) +def test_autogen_prefix_leaves_real_ids_alone(well_id): + assert autogen_prefix(well_id) is None + + +# -------------------------------------------------------------------------- +# srid_for_utm_zone +# -------------------------------------------------------------------------- +def test_srid_for_utm_zone_maps_supported_zones(): + assert srid_for_utm_zone("13N") == SRID_UTM_ZONE_13N + assert srid_for_utm_zone("12N") == SRID_UTM_ZONE_12N + + +@pytest.mark.parametrize("zone", ["11N", "13n", "", None]) +def test_srid_for_utm_zone_rejects_unsupported_zones(zone): + with pytest.raises(UnsupportedUtmZone, match=f"Unsupported UTM zone: {zone}"): + srid_for_utm_zone(zone) + + +def test_unsupported_utm_zone_is_a_value_error(): + # The importer catches ValueError to fail a single row rather than the run. + assert issubclass(UnsupportedUtmZone, ValueError) + + +# -------------------------------------------------------------------------- +# elevation_m_from_ft +# -------------------------------------------------------------------------- +def test_elevation_m_from_ft_converts(): + assert elevation_m_from_ft(10) == 3.048 + assert elevation_m_from_ft("10") == 3.048 + + +def test_elevation_m_from_ft_defaults_missing_to_zero(): + # Location.elevation is not nullable and the sheet leaves it blank. + assert elevation_m_from_ft(None) == 0.0 + + +# -------------------------------------------------------------------------- +# release_status +# -------------------------------------------------------------------------- +def test_release_status_is_three_state(): + assert release_status(True) == "public" + assert release_status(False) == "private" + assert release_status(None) == "draft" + + +# -------------------------------------------------------------------------- +# resolve_measuring_point_height +# -------------------------------------------------------------------------- +def test_resolve_measuring_point_height_prefers_the_explicit_ft_column(): + assert resolve_measuring_point_height(None, 2.5) == 2.5 + assert resolve_measuring_point_height(2.5, 2.5) == 2.5 + + +def test_resolve_measuring_point_height_falls_back_to_mp_height(): + assert resolve_measuring_point_height(1.5, None) == 1.5 + + +def test_resolve_measuring_point_height_allows_both_missing(): + assert resolve_measuring_point_height(None, None) is None + + +def test_resolve_measuring_point_height_rejects_disagreement(): + with pytest.raises(ConflictingMeasuringPointHeight) as exc: + resolve_measuring_point_height(1.5, 2.5) + + assert str(exc.value) == ( + "Conflicting values for measuring point height: " + "mp_height and measuring_point_height_ft" + ) + + +def test_conflicting_measuring_point_height_is_a_value_error(): + assert issubclass(ConflictingMeasuringPointHeight, ValueError) + + +def test_resolve_measuring_point_height_accepts_a_shared_zero(): + # 0.0 is a real height, not a missing one. + assert resolve_measuring_point_height(0.0, 0.0) == 0.0 + + +# -------------------------------------------------------------------------- +# historic depth to water +# -------------------------------------------------------------------------- +def test_historic_depth_to_water_source_lowercases_an_enum(): + assert historic_depth_to_water_source(_DepthSource.DRILLER) == "driller" + + +def test_historic_depth_to_water_source_lowercases_a_string(): + assert historic_depth_to_water_source("Driller") == "driller" + + +@pytest.mark.parametrize("depth_source", [None, ""]) +def test_historic_depth_to_water_source_defaults_to_unknown(depth_source): + assert historic_depth_to_water_source(depth_source) == "unknown" + + +def test_historic_depth_to_water_note_renders_value_and_source(): + assert ( + historic_depth_to_water_note(42.5, _DepthSource.DRILLER) + == "historic depth to water: 42.5 ft - source: driller" + ) + + +def test_historic_depth_to_water_note_is_none_without_a_reading(): + assert historic_depth_to_water_note(None, _DepthSource.DRILLER) is None + + +# -------------------------------------------------------------------------- +# well_purposes / alternate_ids +# -------------------------------------------------------------------------- +def test_well_purposes_drops_blanks_and_keeps_order(): + assert well_purposes("Monitoring", "Domestic") == ["Monitoring", "Domestic"] + assert well_purposes("Monitoring", None) == ["Monitoring"] + assert well_purposes(None, "Domestic") == ["Domestic"] + assert well_purposes(None, None) == [] + + +def test_alternate_ids_credits_the_right_organization(): + assert alternate_ids("SITE-1", "OSE-9") == [ + { + "thing_id": -1, + "alternate_id": "SITE-1", + "alternate_organization": "NMBGMR", + "relation": "same_as", + }, + { + "thing_id": -1, + "alternate_id": "OSE-9", + "alternate_organization": "NMOSE", + "relation": "same_as", + }, + ] + + +def test_alternate_ids_skips_missing_identifiers(): + assert alternate_ids(None, None) == [] + assert [ + entry["alternate_organization"] for entry in alternate_ids(None, "OSE-9") + ] == ["NMOSE"] + + +# ============= EOF ============================================= diff --git a/tests/test_well_inventory.py b/tests/test_well_inventory.py index 23686fd79..aa16afddb 100644 --- a/tests/test_well_inventory.py +++ b/tests/test_well_inventory.py @@ -1281,29 +1281,31 @@ def test_generate_autogen_well_id_with_offset(self): def test_extract_autogen_prefix_pattern(self): """Test auto-generation prefix extraction for supported placeholders.""" - from services.well_inventory_csv import _extract_autogen_prefix + # The rule itself now lives in domain/wells.py; see + # tests/test_domain_wells.py for the database-free version of this. + from domain.wells import autogen_prefix # Existing supported form - assert _extract_autogen_prefix("XY-") == "XY-" - assert _extract_autogen_prefix("AB-") == "AB-" + assert autogen_prefix("XY-") == "XY-" + assert autogen_prefix("AB-") == "AB-" # Placeholder tokens are accepted case-insensitively and normalized. - assert _extract_autogen_prefix("WL-XXXX") == "WL-" - assert _extract_autogen_prefix("SAC-XXXX") == "SAC-" - assert _extract_autogen_prefix("ABC -xxxx") == "ABC-" - assert _extract_autogen_prefix("wl-xxxx") == "WL-" - assert _extract_autogen_prefix("abc - XXXX") == "ABC-" + assert autogen_prefix("WL-XXXX") == "WL-" + assert autogen_prefix("SAC-XXXX") == "SAC-" + assert autogen_prefix("ABC -xxxx") == "ABC-" + assert autogen_prefix("wl-xxxx") == "WL-" + assert autogen_prefix("abc - XXXX") == "ABC-" # Blank values use default prefix - assert _extract_autogen_prefix("") == "NM-" - assert _extract_autogen_prefix(" ") == "NM-" + assert autogen_prefix("") == "NM-" + assert autogen_prefix(" ") == "NM-" # Unsupported forms - assert _extract_autogen_prefix("XY-001") is None - assert _extract_autogen_prefix("XYZ-") == "XYZ-" - assert _extract_autogen_prefix("X-") is None - assert _extract_autogen_prefix("123-") is None - assert _extract_autogen_prefix("USER-XXXX") is None + assert autogen_prefix("XY-001") is None + assert autogen_prefix("XYZ-") == "XYZ-" + assert autogen_prefix("X-") is None + assert autogen_prefix("123-") is None + assert autogen_prefix("USER-XXXX") is None def test_make_row_models_missing_well_name_point_id_column_errors(self): """Missing well_name_point_id column should fail validation (blank cell is separate)."""