diff --git a/core/lexicon.json b/core/lexicon.json index e1ffd9cf4..07dbfd8f1 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}, @@ -525,6 +526,11 @@ {"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."} ] } \ No newline at end of file diff --git a/db/__init__.py b/db/__init__.py index 7a7b20fa6..82c6637a4 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..82cb66323 100644 --- a/db/base.py +++ b/db/base.py @@ -56,6 +56,11 @@ from sqlalchemy_continuum import make_versioned import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from db.notes import Notes + make_versioned() @@ -210,6 +215,49 @@ def permissions(self): ) +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. + """ + # All parent tables use 'id' as their primary key. + pk_name = "id" + + return relationship( + "Notes", + primaryjoin=f"and_({cls.__name__}.{pk_name}==foreign(Notes.notable_id), " + f"Notes.notable_type=='{cls.__name__}')", + lazy="selectin", + viewonly=True, + ) + + def add_note(self, content: str, note_type: str, created_by: str) -> "Notes": + """ + A convenient factory method to create a new Note associated with this object. + This provides a clean, object-oriented API for writing. + """ + # This import is inside the method to avoid circular import issues at runtime. + from db.notes import Notes + + return Notes( + content=content, + note_type=note_type, + notable_id=self.id, + notable_type=self.__class__.__name__, + ) + + class User(Base): """Represents a user in the system.""" diff --git a/db/location.py b/db/location.py index b1113eaad..292e2dc1b 100644 --- a/db/location.py +++ b/db/location.py @@ -31,14 +31,14 @@ from sqlalchemy.ext.associationproxy import association_proxy, AssociationProxy from constants import SRID_WGS84 -from db.base import Base, AutoBaseMixin, ReleaseMixin +from db.base import Base, AutoBaseMixin, ReleaseMixin, NotesMixin from db.lexicon import lexicon_term 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) @@ -56,6 +56,7 @@ 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) + # 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) diff --git a/db/notes.py b/db/notes.py new file mode 100644 index 000000000..2793dfa25 --- /dev/null +++ b/db/notes.py @@ -0,0 +1,77 @@ +""" +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 +from sqlalchemy.orm import relationship, Mapped, mapped_column + +from db.base import Base, AutoBaseMixin, ReleaseMixin, lexicon_term + +if TYPE_CHECKING: + from db.thing import Thing + from db.location import Location + + +class Notes(Base, AutoBaseMixin, ReleaseMixin): + """ + Represents a single, categorized note that can be attached to various + parent objects throughout the database. + """ + + # --- Polymorphic Columns --- + notable_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).", + ) + 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.notable_id) == Thing.id, Notes.notable_type == 'Thing')", + viewonly=True, + ) + _location_target: Mapped["Location"] = relationship( + "Location", + primaryjoin="and_(foreign(Notes.notable_id) == Location.id, Notes.notable_type == '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.notable_type.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", "notable_id", "notable_type"),) diff --git a/db/thing.py b/db/thing.py index 13ce81bbb..ea4dc9efc 100644 --- a/db/thing.py +++ b/db/thing.py @@ -26,6 +26,7 @@ ReleaseMixin, StatusHistoryMixin, PermissionMixin, + NotesMixin, ) from typing import List, TYPE_CHECKING @@ -39,7 +40,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. diff --git a/schemas/location.py b/schemas/location.py index 9340b8cac..6f60b648a 100644 --- a/schemas/location.py +++ b/schemas/location.py @@ -18,6 +18,8 @@ from pydantic import BaseModel, field_validator from schemas import BaseCreateModel, BaseUpdateModel, BaseResponseModel +from schemas.notes import NoteResponse +from typing import List from services.validation.geospatial import validate_wkt_geometry @@ -40,7 +42,9 @@ 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 point: str # point is required and should be in WKT format elevation: float release_status: str | None = "draft" @@ -66,7 +70,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" @@ -104,11 +110,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: str | None = None point: str | None = None elevation: float | None = None release_status: str | None = None diff --git a/schemas/notes.py b/schemas/notes.py new file mode 100644 index 000000000..9ec50b2ff --- /dev/null +++ b/schemas/notes.py @@ -0,0 +1,48 @@ +""" +Pydantic models for the Notes table. +""" + +from pydantic import BaseModel +from schemas import BaseCreateModel, BaseUpdateModel, BaseResponseModel + +# -------- BASE SCHEMA: ---------- +""" +Defines the core, shared attributes of a Note for reuse. +""" + + +class BaseNote(BaseModel): + 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. + """ + + note_id: int + notable_id: int + notable_type: 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 6bf0befc1..64710dbd3 100644 --- a/schemas/thing.py +++ b/schemas/thing.py @@ -19,6 +19,7 @@ from schemas import BaseCreateModel, BaseUpdateModel, BaseResponseModel from schemas.location import LocationResponse +from schemas.notes import NoteResponse # -------- VALIDATE ---------- @@ -141,6 +142,8 @@ 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] = [] class WellResponse(BaseThingResponse): diff --git a/tests/test_location.py b/tests/test_location.py index 628c1c352..755cd8f89 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -43,7 +43,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", @@ -59,7 +64,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"] @@ -81,7 +88,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", @@ -95,7 +104,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"] @@ -144,7 +155,12 @@ def test_get_locations(location): "+00:00", "Z" ) # 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