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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions ADR4.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 18 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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:
Expand Down
30 changes: 30 additions & 0 deletions domain/__init__.py
Original file line number Diff line number Diff line change
@@ -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 =============================================
68 changes: 68 additions & 0 deletions domain/field_staff.py
Original file line number Diff line number Diff line change
@@ -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 =============================================
39 changes: 39 additions & 0 deletions domain/samples.py
Original file line number Diff line number Diff line change
@@ -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 =============================================
45 changes: 45 additions & 0 deletions domain/units.py
Original file line number Diff line number Diff line change
@@ -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 =============================================
56 changes: 56 additions & 0 deletions domain/values.py
Original file line number Diff line number Diff line change
@@ -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 =============================================
Loading
Loading