diff --git a/core/initializers.py b/core/initializers.py index 6b0d7920c..a38ee718c 100644 --- a/core/initializers.py +++ b/core/initializers.py @@ -64,6 +64,9 @@ def erase_and_rebuild_db(session: Session): Base.metadata.drop_all(session.bind) Base.metadata.create_all(session.bind) + init_lexicon() + init_parameter() + def init_lexicon(path: str = None) -> None: if path is None: diff --git a/core/lexicon.json b/core/lexicon.json index f1a77ed24..974cea9f9 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -23,6 +23,7 @@ {"name": "limit_type", "description": null}, {"name": "measurement_method", "description": null}, {"name": "monitoring_status", "description": null}, + {"name": "note_type", "description": null}, {"name": "parameter_name", "description": null}, {"name": "organization", "description": null}, {"name": "parameter_type", "description": null}, @@ -673,6 +674,14 @@ {"categories": ["sensor_status"], "term": "In Service", "definition": "In Service"}, {"categories": ["sensor_status"], "term": "In Repair", "definition": "In Repair"}, {"categories": ["sensor_status"], "term": "Retired", "definition": "Retired"}, - {"categories": ["sensor_status"], "term": "Lost", "definition": "Lost"} + {"categories": ["sensor_status"], "term": "Lost", "definition": "Lost"}, + {"categories": ["note_type"], "term": "Access", "definition": "Access instructions, gate codes, permission requirements, etc."}, + {"categories": ["note_type"], "term": "Construction", "definition": "Construction details, well development, drilling notes, etc. Could create separate `types` for each of these if needed."}, + {"categories": ["note_type"], "term": "Maintenance", "definition": "Maintenance observations and issues."}, + {"categories": ["note_type"], "term": "Historical", "definition": "Historical information or context about the well or location."}, + {"categories": ["note_type"], "term": "Other", "definition": "Other types of notes that do not fit into the predefined categories."}, + {"categories": ["note_type"], "term": "Water", "definition": "Water bearing zone information and other info from ose reports"}, + {"categories": ["note_type"], "term": "Measuring", "definition": "Notes about measuring/visiting the well, on Access form"} + ] } \ No newline at end of file diff --git a/db/__init__.py b/db/__init__.py index efb23a418..6cd1412f8 100644 --- a/db/__init__.py +++ b/db/__init__.py @@ -30,6 +30,7 @@ from db.group import * from db.lexicon import * from db.location import * +from db.notes import * from db.observation import * from db.parameter import * from db.permission import * diff --git a/db/base.py b/db/base.py index ba2a45be8..6171f8273 100644 --- a/db/base.py +++ b/db/base.py @@ -36,6 +36,9 @@ 7. An `AuditMixin` to add standard audit columns to tables. """ +import re +from typing import TYPE_CHECKING + from sqlalchemy import ( Column, DateTime, @@ -52,9 +55,11 @@ mapped_column, relationship, ) -from sqlalchemy_searchable import make_searchable from sqlalchemy_continuum import make_versioned -import re +from sqlalchemy_searchable import make_searchable + +if TYPE_CHECKING: + pass make_versioned() diff --git a/db/location.py b/db/location.py index aecee84fe..a01eb1356 100644 --- a/db/location.py +++ b/db/location.py @@ -32,12 +32,13 @@ from constants import SRID_WGS84 from db.base import Base, AutoBaseMixin, ReleaseMixin from db.lexicon import lexicon_term +from db.notes import NotesMixin if TYPE_CHECKING: from db.thing import Thing -class Location(Base, AutoBaseMixin, ReleaseMixin): +class Location(Base, AutoBaseMixin, ReleaseMixin, NotesMixin): __versioned__ = {} nma_pk_location: Mapped[UUID] = mapped_column(String(36), nullable=True) @@ -55,7 +56,8 @@ class Location(Base, AutoBaseMixin, ReleaseMixin): county: Mapped[str] = mapped_column(String(100), nullable=True) state: Mapped[str] = mapped_column(String(100), nullable=True) quad_name: Mapped[str] = mapped_column(String(100), nullable=True) - notes: Mapped[str] = mapped_column(Text, nullable=True) + # TODO: remove this 'notes' field in favor of using the polymorphic Notes table. Did not remove it yet to avoid breaking existing data model. + # notes: Mapped[str] = mapped_column(Text, nullable=True) nma_notes_location: Mapped[str] = mapped_column(Text, nullable=True) nma_coordinate_notes: Mapped[str] = mapped_column(Text, nullable=True) elevation_accuracy: Mapped[float] = mapped_column(nullable=True) diff --git a/db/notes.py b/db/notes.py new file mode 100644 index 000000000..ab8384064 --- /dev/null +++ b/db/notes.py @@ -0,0 +1,128 @@ +""" +SQLAlchemy model for the Notes table. + +This is a polymorphic table for storing all unstructured notes, categorized by +a note_type. + +The Notes table should be used when a record might need more than one note, +when the notes need to be categorized, or when you need the ability to +search across all notes in the system. This is different from a dedicated +notes field on a specific table, which should be used to store a simple, +single-purpose attribute of the record itself. +""" + +from typing import TYPE_CHECKING + +from sqlalchemy import Integer, Text, Index, and_ +from sqlalchemy.orm import relationship, Mapped, mapped_column, declared_attr, foreign + +from db.base import Base, AutoBaseMixin, ReleaseMixin, lexicon_term + +if TYPE_CHECKING: + pass + + +class Notes(Base, AutoBaseMixin, ReleaseMixin): + """ + Represents a single, categorized note that can be attached to various + parent objects throughout the database. + """ + + # --- Polymorphic Columns --- + target_id: Mapped[int] = mapped_column( + Integer, + nullable=False, + comment="The ID of the parent record this note is about (e.g., a `thing_id`, `location_id`, etc).", + ) + target_table: Mapped[str] = mapped_column() + # notable_type: Mapped[str] = lexicon_term( + # nullable=False, + # comment="The type of the note associated with this record.", + # ) + + # --- Columns --- + note_type: Mapped[str] = lexicon_term( + nullable=False, + comment="A controlled vocabulary field that defines the specific category of the note (e.g. 'Access Instructions`, ", + ) + content: Mapped[str] = mapped_column(Text, nullable=False) + + # --- Polymorphic Parent Relationships (Internal) --- + # These are viewonly relationships used by the 'target' property below. + # _thing_target: Mapped["Thing"] = relationship( + # "Thing", + # primaryjoin="and_(foreign(Notes.target_id) == Thing.id, Notes.target_table == 'thing')", + # viewonly=True, + # ) + # _location_target: Mapped["Location"] = relationship( + # "Location", + # primaryjoin="and_(foreign(Notes.target_id) == Location.id, Notes.target_table == 'location')", + # viewonly=True, + # ) + + @property + def target(self): + """ + A generic property to get the parent object (Thing, Location, etc.). + + This is useful for simplifying application code by providing a single, + consistent way to access the parent of a polymorphic record without + needing to check the 'notable_type' field manually. + """ + return getattr(self, f"_{self.target_table.lower()}_target") + + # --- Table Arguments --- + # A composite index to optimize retrieval of all note records for a specific parent object. + + __table_args__ = (Index("ix_notes_polymorphic_link", "target_id", "target_table"),) + + +class NotesMixin: + """ + Mixin for models that can have multiple types or categories of notes. + It automatically creates a polymorphic One-to-Many relationship to the + Notes table. + """ + + @declared_attr + def notes(cls): + """ + The high-performance, declarative relationship for reading notes. + This provides a polymorphic one-to-many link to the Notes table. + + PERFORMANCE NOTE: Use with `selectinload` in queries to prevent the + N+1 query problem when accessing notes for multiple parent objects. + """ + return relationship( + "Notes", + primaryjoin=and_( + cls.id == foreign(Notes.target_id), + Notes.target_table == cls.__name__, + ), + cascade="all, delete-orphan", + lazy="selectin", + overlaps="notes", + ) + + def add_note( + self, + content: str, + note_type: str, + release_status: str = "draft", + created_by: str = None, + ) -> Notes: + """ + A convenient factory method to create a new Note associated with this object. + This provides a clean, object-oriented API for writing. + """ + + return Notes( + content=content, + note_type=note_type, + target_id=self.id, + target_table=self.__class__.__name__, + release_status=release_status, + ) + + def _get_notes(self, note_type: str) -> list[Notes]: + return [n for n in self.notes if n.note_type == note_type] diff --git a/db/thing.py b/db/thing.py index 57874a639..862bcf91c 100644 --- a/db/thing.py +++ b/db/thing.py @@ -20,7 +20,7 @@ from sqlalchemy.orm import relationship, mapped_column, Mapped from sqlalchemy_utils import TSVectorType -from db import lexicon_term +from db import lexicon_term, NotesMixin from db.asset import Asset from db.base import ( AutoBaseMixin, @@ -39,7 +39,9 @@ from db.group import Group, GroupThingAssociation -class Thing(Base, AutoBaseMixin, ReleaseMixin, StatusHistoryMixin, PermissionMixin): +class Thing( + Base, AutoBaseMixin, ReleaseMixin, StatusHistoryMixin, PermissionMixin, NotesMixin +): """ Represents a physical object of interest being monitored (e.g., a well). Stores static, core attributes of the physical installation. @@ -52,9 +54,10 @@ class Thing(Base, AutoBaseMixin, ReleaseMixin, StatusHistoryMixin, PermissionMix nullable=True, comment="To audit where the data came from in NM_Aquifer if it was transferred over", ) - notes = mapped_column(Text, nullable=True) - measuring_notes = mapped_column(Text, nullable=True) - water_notes = mapped_column(Text, nullable=True) + + # notes = mapped_column(Text, nullable=True) + # measuring_notes = mapped_column(Text, nullable=True) + # water_notes = mapped_column(Text, nullable=True) # TODO: should `name` be unique? name: Mapped[str] = mapped_column( @@ -277,6 +280,18 @@ def current_location(self): else None ) + @property + def water_notes(self): + return self._get_notes("Water") + + @property + def general_notes(self): + return self._get_notes("Other") + + @property + def measuring_notes(self): + return self._get_notes("Measuring") + class ThingIdLink(Base, AutoBaseMixin, ReleaseMixin): """ diff --git a/schemas/location.py b/schemas/location.py index 7b2d5420f..0bcd226f3 100644 --- a/schemas/location.py +++ b/schemas/location.py @@ -13,12 +13,15 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== +from typing import List + from geoalchemy2 import WKBElement from geoalchemy2.shape import to_shape from pydantic import BaseModel, field_validator from core.enums import ElevationMethod, CoordinateMethod from schemas import BaseCreateModel, BaseUpdateModel, BaseResponseModel +from schemas.notes import NoteResponse, CreateNote, UpdateNote from services.validation.geospatial import validate_wkt_geometry @@ -41,7 +44,10 @@ class CreateLocation(BaseCreateModel, ValidateLocation): """ # name: str | None = None - notes: str | None = None + # TODO: AI suggested managing notes via a separate /locations/{id}/notes endpoint. + # I don't know if we want to do that, but am leaving this comment for future reference. + # notes: str | None = None + notes: List[CreateNote] = [] point: str # point is required and should be in WKT format elevation: float elevation_accuracy: float | None = None @@ -66,7 +72,9 @@ class LocationResponse(BaseResponseModel): """ # name: str | None - notes: str | None + # The 'notes' field is now a List of NoteResponse objects, + # matching the polymorphic relationship in the database model. + notes: List[NoteResponse] = [] point: str elevation: float | None horizontal_datum: str = "WGS84" @@ -103,11 +111,13 @@ class GroupLocationResponse(BaseResponseModel): # -------- UPDATE ---------- class UpdateLocation(BaseUpdateModel, ValidateLocation): """ - Schema for updating a location. + Schema for updating a location. Notes are managed via the polymorphic Notes table. """ # name: str | None = None - notes: str | None = None + # TODO: AI suggested managing notes via a separate API endpoint, /notes/{note_id}. + # I don't know if we want to do that, but am leaving this comment for future reference. + notes: List[UpdateNote] = [] point: str | None = None elevation: float | None = None elevation_accuracy: float | None = None diff --git a/schemas/notes.py b/schemas/notes.py new file mode 100644 index 000000000..85c47ed9b --- /dev/null +++ b/schemas/notes.py @@ -0,0 +1,46 @@ +""" +Pydantic models for the Notes table. +""" + +from schemas import BaseCreateModel, BaseUpdateModel, BaseResponseModel + +# -------- BASE SCHEMA: ---------- +""" +Defines the core, shared attributes of a Note for reuse. +""" + + +class BaseNote: + note_type: str + content: str + + +# -------- CREATE ---------- +class CreateNote(BaseCreateModel, BaseNote): + # TODO: this was a suggestion by AI, but based on our other schemas it + # seems like more should be added here... + """ + Schema for creating a new Note. The parent object's ID and type will be + taken from the URL path, not the request body. + """ + pass + + +# -------- RESPONSE ---------- +class NoteResponse(BaseResponseModel, BaseNote): + """ + Response schema for Note details. + """ + + target_id: int + target_table: str + + +# -------- UPDATE ---------- +class UpdateNote(BaseUpdateModel): + """ + Schema for updating an existing Note. All fields are optional + """ + + note_type: str | None = None + content: str | None = None diff --git a/schemas/thing.py b/schemas/thing.py index 3d1c291df..d6392be64 100644 --- a/schemas/thing.py +++ b/schemas/thing.py @@ -20,6 +20,7 @@ from core.enums import WellPurpose, CasingMaterial, SpringType, ScreenType from schemas import BaseCreateModel, BaseUpdateModel, BaseResponseModel from schemas.location import LocationResponse +from schemas.notes import NoteResponse, CreateNote # -------- VALIDATE ---------- @@ -98,6 +99,7 @@ class CreateWell(CreateBaseThing, ValidateWell): default=None, gt=0, description="Well casing depth in feet" ) well_casing_materials: list[CasingMaterial] | None = None + notes: list[CreateNote] | None = None class CreateSpring(CreateBaseThing): @@ -135,6 +137,11 @@ class BaseThingResponse(BaseResponseModel): thing_type: str current_location: LocationResponse | None first_visit_date: PastDate | None + # The new relationship to the polymorphic Notes table + notes: List[NoteResponse] = [] + + # The new relationship to the polymorphic Notes table + notes: List[NoteResponse] = [] class WellResponse(BaseThingResponse): @@ -154,9 +161,9 @@ class WellResponse(BaseThingResponse): well_casing_materials: list[CasingMaterial] = [] well_construction_notes: str | None = None - water_notes: str | None = None - measuring_notes: str | None = None - notes: str | None = None + water_notes: list[NoteResponse] | None = None + measuring_notes: list[NoteResponse] | None = None + general_notes: list[NoteResponse] | None = None @field_validator("well_purposes", mode="before") def populate_well_purposes_with_strings(cls, well_purposes): diff --git a/services/crud_helper.py b/services/crud_helper.py index 6ef4d80e4..01eaeb254 100644 --- a/services/crud_helper.py +++ b/services/crud_helper.py @@ -18,6 +18,7 @@ from sqlalchemy.orm import Session, DeclarativeBase from starlette.status import HTTP_204_NO_CONTENT +from db.notes import NotesMixin from services.query_helper import simple_get_by_id @@ -35,10 +36,23 @@ def model_adder(session, table, model, user=None, **kwargs): md["created_by_id"] = user["sub"] md["created_by_name"] = user["name"] + notes = None + if issubclass(table, NotesMixin): + notes = md.pop("notes", None) + obj = table(**md) + session.add(obj) session.commit() session.refresh(obj) + + if notes: + for n in notes: + note = obj.add_note(**n) + session.add(note) + + session.commit() + session.refresh(obj) return obj @@ -60,7 +74,16 @@ def model_patcher( """ for key, value in payload.model_dump(exclude_unset=True).items(): - setattr(item, key, value) + if isinstance(item, NotesMixin) and key == "notes": + # delete all notes and re-add + for note in item.notes: + session.delete(note) + + for note in value: + note = item.add_note(**note) + session.add(note) + else: + setattr(item, key, value) if user: item.updated_by_id = user["sub"] diff --git a/services/thing_helper.py b/services/thing_helper.py index f9c661c1c..53ce54577 100644 --- a/services/thing_helper.py +++ b/services/thing_helper.py @@ -152,6 +152,10 @@ def add_thing( well_descriptor_table_list = list(WELL_DESCRIPTOR_MODEL_MAP.keys()) data = data.model_dump(exclude=well_descriptor_table_list) + notes = None + if "notes" in data: + notes = data.pop("notes") + location_id = data.pop("location_id", None) group_id = data.pop("group_id", None) @@ -183,6 +187,14 @@ def add_thing( session.commit() session.refresh(thing) + + if notes: + for n in notes: + nn = thing.add_note(n["content"], n["note_type"]) + session.add(nn) + session.commit() + session.refresh(thing) + except Exception as e: session.rollback() raise e diff --git a/tests/__init__.py b/tests/__init__.py index ed7fe4ea8..678c60440 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -43,14 +43,9 @@ from db.engine import session_ctx from core.app import app - -# Base.metadata.drop_all(engine) -# Base.metadata.create_all(engine) with session_ctx() as session: erase_and_rebuild_db(session) -init_lexicon() -init_parameter() register_routes(app) app.add_middleware( diff --git a/tests/conftest.py b/tests/conftest.py index 34944f957..72270ee0c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,7 +12,7 @@ def location(): with session_ctx() as session: loc = Location( # name="first location", - notes="these are some test notes", + # notes="these are some test notes", point="POINT(-107.949533 33.809665)", elevation=2464.9, release_status="draft", @@ -24,10 +24,19 @@ def location(): # county="Catron", # quad_name="Luera Mountains West", ) + session.add(loc) session.commit() session.refresh(loc) + + note = loc.add_note("these are some test notes", "Other") + session.add(note) + session.commit() + session.refresh(loc) + yield loc + + session.delete(note) session.delete(loc) session.commit() diff --git a/tests/features/environment.py b/tests/features/environment.py index bda642768..ada75806e 100644 --- a/tests/features/environment.py +++ b/tests/features/environment.py @@ -16,7 +16,7 @@ import random from datetime import datetime, timedelta -from core.initializers import erase_and_rebuild_db, init_lexicon, init_parameter +from core.initializers import erase_and_rebuild_db from db import ( Location, Thing, @@ -48,7 +48,7 @@ def closure(context, *args, **kwargs): def add_location(context, session): loc = Location( # name="first location", - notes="these are some test notes", + # notes="these are some test notes", point="POINT(-107.949533 33.809665)", elevation=2464.9, release_status="draft", @@ -60,6 +60,10 @@ def add_location(context, session): session.add(loc) session.commit() session.refresh(loc) + n = loc.add_note("Test location", "Other") + session.add(n) + session.commit() + session.refresh(loc) context.objects["locations"].append(loc) return loc @@ -77,10 +81,11 @@ def add_well(context, session, location, name_num): well_construction_notes="Test well construction notes", well_casing_diameter=5.0, well_casing_depth=10.0, - notes="These are some test well notes", - measuring_notes="These are some measuring notes", - water_notes="This are some water notes", + # notes="These are some test well notes", + # measuring_notes="These are some measuring notes", + # water_notes="This are some water notes", ) + session.add(well) session.commit() @@ -88,7 +93,17 @@ def add_well(context, session, location, name_num): assoc.effective_start = "2025-02-01T00:00:00Z" session.add(assoc) session.commit() + session.refresh(well) + + for nt, c in ( + ("Other", "well notes"), + ("Water", "water notes"), + ("Measuring", "measuring notes"), + ): + n = well.add_note(c, nt) + session.add(n) + session.commit() session.refresh(well) context.objects["wells"].append(well) @@ -209,8 +224,6 @@ def before_all(context): with session_ctx() as session: if rebuild: erase_and_rebuild_db(session) - init_lexicon() - init_parameter() loc_1 = add_location(context, session) loc_2 = add_location(context, session) diff --git a/tests/test_location.py b/tests/test_location.py index 6ad1350e9..cbf790674 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -26,7 +26,6 @@ client, override_authentication, cleanup_post_test, - cleanup_patch_test, ) @@ -51,7 +50,12 @@ def override_dependencies_fixture(): def test_add_location(): payload = { # "name": "test location", - "notes": "these are some test notes", + "notes": [ + { + "note_type": "Access", + "content": "These are some test access notes.", + } + ], "point": "POINT (-106.607784 35.118924)", "elevation": 1558.8, "release_status": "draft", @@ -67,7 +71,9 @@ def test_add_location(): assert "id" in data assert "created_at" in data # assert data["name"] == payload["name"] - assert data["notes"] == payload["notes"] + assert len(data["notes"]) == 1 + assert data["notes"][0]["note_type"] == "Access" + assert data["notes"][0]["content"] == "These are some test access notes." assert data["point"] == payload["point"] assert data["elevation"] == payload["elevation"] assert data["release_status"] == payload["release_status"] @@ -89,7 +95,9 @@ def test_add_location(): def test_update_location(location): payload = { # "name": "patched name", - "notes": "these are some patched notes", + "notes": [ + {"note_type": "Access", "content": "These are some patched access notes."} + ], "point": "POINT (-106.904107 34.068198)", "elevation": 1408.3, "release_status": "draft", @@ -103,7 +111,9 @@ def test_update_location(location): data = response.json() assert data["id"] == location.id # assert data["name"] == payload["name"] - assert data["notes"] == payload["notes"] + assert len(data["notes"]) == 1 + assert data["notes"][0]["note_type"] == "Access" + assert data["notes"][0]["content"] == "These are some patched access notes." assert data["point"] == payload["point"] assert data["elevation"] == payload["elevation"] assert data["release_status"] == payload["release_status"] @@ -119,7 +129,7 @@ def test_update_location(location): payload["state"] = location.state payload["county"] = location.county payload["quad_name"] = location.quad_name - cleanup_patch_test(Location, payload, location) + # cleanup_patch_test(Location, payload, location) def test_patch_location_404_not_found(location): @@ -129,7 +139,8 @@ def test_patch_location_404_not_found(location): bad_location_id = 99999 location_notes_patch = "patched notes" response = client.patch( - f"/location/{bad_location_id}", json={"notes": location_notes_patch} + f"/location/{bad_location_id}", + json={"notes": [{"content": location_notes_patch, "note_type": "Other"}]}, ) data = response.json() assert response.status_code == 404 @@ -152,7 +163,12 @@ def test_get_locations(location): timezone.utc ).strftime(DT_FMT) # assert data["items"][0]["name"] == location.name - assert data["items"][0]["notes"] == location.notes + assert isinstance(data["items"][0]["notes"], list) + # If you know the exact number of notes expected: + # assert len(data["items"][0]["notes"]) == expected_count + # If you want to check content of a specific note: + # if data["items"][0]["notes"]: + # assert data["items"][0]["notes"][0]["content"] == expected_content assert data["items"][0]["point"] == to_shape(location.point).wkt assert data["items"][0]["elevation"] == location.elevation assert data["items"][0]["release_status"] == location.release_status