From 7d97f478ec639ea64d55a17a418fb5406de81e17 Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Fri, 17 Oct 2025 09:42:18 -0600 Subject: [PATCH 01/11] feat: Create new `Notes` table A polymorphic Notes table was needed to store all unstructured notes. This commit creates the necessary polymorphic Notes table. Its purpose is to store all unstructured notes, categorized by a note_type. It 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 single, intrinsic attribute of the record itself. --- db/__init__.py | 1 + db/notes.py | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 db/notes.py 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/notes.py b/db/notes.py new file mode 100644 index 000000000..0125ee2b5 --- /dev/null +++ b/db/notes.py @@ -0,0 +1,81 @@ +""" +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 single, +intrinsic attribute of the record itself. +""" + +from typing import TYPE_CHECKING + +from sqlalchemy import Integer, Text, and_, Index +from sqlalchemy.orm import relationship, Mapped, mapped_column, foreign + +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).", + ) + # TODO: create new lexicon category and lexicon terms for notable_type + notable_type: Mapped[str] = lexicon_term( + nullable=False, + comment="The type of the note associated with this record.", + ) + + # --- Columns --- + # TODO: create new lexicon category and lexicon terms for note_type + 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(notable_id) == Thing.id, notable_type == "Thing"), + viewonly=True, + ) + _location_target: Mapped["Location"] = relationship( + "Location", + primaryjoin=and_( + foreign(notable_id) == Location.id, 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"),) From 8ab40ff30d90717bb73b3ebe309e9dc18df42d2b Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Mon, 20 Oct 2025 11:23:14 -0600 Subject: [PATCH 02/11] doc: Updated `Notes` model docstring for clarity. --- db/notes.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/notes.py b/db/notes.py index 0125ee2b5..fee5e5cf3 100644 --- a/db/notes.py +++ b/db/notes.py @@ -7,8 +7,8 @@ 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 single, -intrinsic attribute of the record itself. +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 7aff37db6fbd47cb72e26705634aa9919e16bb7f Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Mon, 20 Oct 2025 11:57:56 -0600 Subject: [PATCH 03/11] feat: Add 'notable_type' category and values This commit introduces a new category, `notable_type`, to the `lexicon.json` file. This category will serve as the central, authoritative source for the controlled vocabulary used in the polymorphic `noteable_type` columns --- core/lexicon.json | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/core/lexicon.json b/core/lexicon.json index e1ffd9cf4..2ac67c5c6 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": "noteable_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": ["noteable_type"], "term": "Access", "definition": "Access instructions, gate codes, permission requirements, etc."}, + {"categories": ["noteable_type"], "term": "Construction", "definition": "Construction details, well development, drilling notes, etc. Could create separate `types` for each of these if needed."}, + {"categories": ["noteable_type"], "term": "Maintenance", "definition": "Maintenance observations and issues."}, + {"categories": ["noteable_type"], "term": "Historical", "definition": "Historical information or context about the well or location."}, + {"categories": ["noteable_type"], "term": "Other", "definition": "Other types of notes that do not fit into the predefined categories."} ] } \ No newline at end of file From 1c9db25d0406de60fb25600730a4eadc0d12ea93 Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Mon, 20 Oct 2025 11:59:45 -0600 Subject: [PATCH 04/11] doc: Remove TODO comments Removed the TODO comments about updating the lexicon file with 'notable_type' category and values. They have been added. --- db/notes.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/db/notes.py b/db/notes.py index fee5e5cf3..4fbfb1cec 100644 --- a/db/notes.py +++ b/db/notes.py @@ -35,14 +35,12 @@ class Notes(Base, AutoBaseMixin, ReleaseMixin): nullable=False, comment="The ID of the parent record this note is about (e.g., a `thing_id`, `location_id`, etc).", ) - # TODO: create new lexicon category and lexicon terms for notable_type notable_type: Mapped[str] = lexicon_term( nullable=False, comment="The type of the note associated with this record.", ) # --- Columns --- - # TODO: create new lexicon category and lexicon terms for note_type 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`, ", From 8936e1aec345b4ff15b93dff1f0dcdb8b37b7e5d Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Mon, 20 Oct 2025 12:32:28 -0600 Subject: [PATCH 05/11] feat: Add polymorphic NotesMixin to support multi-category notes Added NotesMixin to db/base.py that provides polymorphic note capabilities to any model that inherits from it. The mixin offers: - A polymorphic one-to-many relationship to the Notes table using the notable_id/notable_type pattern - Efficient loading with selectin strategy to prevent N+1 query issues - A convenient add_note() factory method for creating new notes with proper polymorphic associations - Clean separation between read (relationship) and write (method) operations This mixin enables consistent note-taking across multiple entity types while maintaining type safety and relationship integrity. --- db/base.py | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/db/base.py b/db/base.py index ba2a45be8..3414842bb 100644 --- a/db/base.py +++ b/db/base.py @@ -54,8 +54,14 @@ ) from sqlalchemy_searchable import make_searchable from sqlalchemy_continuum import make_versioned +from sqlalchemy.inspection import inspect import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from db.notes import Notes + make_versioned() @@ -210,6 +216,64 @@ 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. + """ + # Dynamically get the primary key column name of the inheriting class + pk_name = inspect(cls).primary_key[0].name + + return relationship( + "Notes", + primaryjoin=f"and_({cls.__name__}.{pk_name}==Notes.notable_id, " + f"Notes.notable_type=='{cls.__name__}')", + lazy="selectin", # A good default for eager loading on demand + 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. + + NOTE: This method creates and returns a new Note object but does *not* add + it to the database session. The caller is responsible for session management. + + Args: + content: The text content of the note. + note_type: The categorized type of the note (from a controlled vocabulary). + created_by: The user or process creating the note. + + Returns: + A new, unsaved Notes object linked to this parent. + """ + # This import is inside the method to avoid circular import issues at runtime. + from db.notes import Notes + + # Dynamically get the primary key value of this specific instance + pk_name = inspect(self.__class__).primary_key[0].name + pk_value = getattr(self, pk_name) + + return Notes( + content=content, + note_type=note_type, + notable_id=pk_value, + notable_type=self.__class__.__name__, + ) + + class User(Base): """Represents a user in the system.""" From d0a7b408890d444ed6c516e2e1abd31c9aba5efd Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Mon, 20 Oct 2025 12:49:14 -0600 Subject: [PATCH 06/11] feat: Integrate NotesMixin into Location and Thing models Added the NotesMixin to both Location and Thing models to enable polymorphic note capabilities. This allows: - Associating multiple categorized notes with locations and things - Consistent note handling across entity types - Type-safe access to notes through relationship loading - Efficient loading of notes with selectin strategy The NotesMixin provides both the notes relationship and add_note() method for these models, enabling a clean API for both reading and creating notes. --- db/location.py | 5 +++-- db/thing.py | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) 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/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. From c905273cf68b5db0666df8f260955b398cdc59c1 Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Wed, 22 Oct 2025 09:42:39 -0600 Subject: [PATCH 07/11] feat: Add Notes schema and polymorphic relationships to Thing and Location models - Created new Pydantic schema for the Notes model - Added polymorphic relationship `notes: List[NoteResponse] = []` to Thing and Location schemas for flexible data modeling. --- schemas/location.py | 14 ++++++++++++- schemas/notes.py | 48 +++++++++++++++++++++++++++++++++++++++++++++ schemas/thing.py | 4 ++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 schemas/notes.py diff --git a/schemas/location.py b/schemas/location.py index 9340b8cac..9f47be5d8 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,6 +42,8 @@ class CreateLocation(BaseCreateModel, ValidateLocation): """ # name: str | None = None + # TODO: remove `notes` field once polymorphic Notes are successfully implemented. + # I didn't want to remove yet in case it breaks something with the transfer. notes: str | None = None point: str # point is required and should be in WKT format elevation: float @@ -66,6 +70,9 @@ class LocationResponse(BaseResponseModel): """ # name: str | None + # TODO: remove `notes` field once polymorphic Notes are successfully implemented. + # I didn't want to remove yet in case it breaks something with the transfer. + notes: str | None = None notes: str | None point: str elevation: float | None @@ -80,6 +87,9 @@ class LocationResponse(BaseResponseModel): county: str | None quad_name: str | None + # The new relationship to the polymorphic Notes table + notes: List[NoteResponse] = [] + @field_validator("point", mode="before") def point_to_wkt(cls, value): if isinstance(value, WKBElement): @@ -104,10 +114,12 @@ 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 + # TODO: remove `notes` field once polymorphic Notes are successfully implemented. + # I didn't want to remove yet in case it breaks something with the transfer. notes: str | None = None point: str | None = None elevation: float | 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..a3014cd98 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 ---------- @@ -142,6 +143,9 @@ class BaseThingResponse(BaseResponseModel): current_location: LocationResponse | None first_visit_date: PastDate | None + # The new relationship to the polymorphic Notes table + notes: List[NoteResponse] = [] + class WellResponse(BaseThingResponse): """ From 550accec40c7c16aa3070accb8570612f95ef30d Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Wed, 22 Oct 2025 10:13:07 -0600 Subject: [PATCH 08/11] fix: Resolve circular import in Notes model - Resolved SQLAlchemy inspection error caused by circular import between Notes and Thing/Location models - Updated relationship definitions in notes.py to use string-based SQL expressions instead of direct class references - Maintained TYPE_CHECKING imports for proper type hinting while avoiding runtime circular dependencies --- db/notes.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/db/notes.py b/db/notes.py index 4fbfb1cec..2793dfa25 100644 --- a/db/notes.py +++ b/db/notes.py @@ -13,8 +13,8 @@ from typing import TYPE_CHECKING -from sqlalchemy import Integer, Text, and_, Index -from sqlalchemy.orm import relationship, Mapped, mapped_column, foreign +from sqlalchemy import Integer, Text, Index +from sqlalchemy.orm import relationship, Mapped, mapped_column from db.base import Base, AutoBaseMixin, ReleaseMixin, lexicon_term @@ -51,14 +51,12 @@ class Notes(Base, AutoBaseMixin, ReleaseMixin): # These are viewonly relationships used by the 'target' property below. _thing_target: Mapped["Thing"] = relationship( "Thing", - primaryjoin=and_(foreign(notable_id) == Thing.id, notable_type == "Thing"), + primaryjoin="and_(foreign(Notes.notable_id) == Thing.id, Notes.notable_type == 'Thing')", viewonly=True, ) _location_target: Mapped["Location"] = relationship( "Location", - primaryjoin=and_( - foreign(notable_id) == Location.id, notable_type == "Location" - ), + primaryjoin="and_(foreign(Notes.notable_id) == Location.id, Notes.notable_type == 'Location')", viewonly=True, ) From f18b715ab8365911b8e7074f694944dc0100f28b Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Wed, 22 Oct 2025 13:32:31 -0600 Subject: [PATCH 09/11] fix: Resolve undefined pk_value in NotesMixin.add_note method - Fixed the NotesMixin.add_note method by replacing the undefined pk_value reference with self.id - This resolves the "Unresolved reference 'pk_value'" error in base.py - Ensures proper note creation through the convenient factory method - Complements previous circular import fixes in the polymorphic relationship structure The change maintains the intended functionality of creating notes associated with model instances while fixing a reference error that was preventing proper execution. --- db/base.py | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/db/base.py b/db/base.py index 3414842bb..82cb66323 100644 --- a/db/base.py +++ b/db/base.py @@ -54,7 +54,6 @@ ) from sqlalchemy_searchable import make_searchable from sqlalchemy_continuum import make_versioned -from sqlalchemy.inspection import inspect import re from typing import TYPE_CHECKING @@ -232,14 +231,14 @@ def notes(cls): PERFORMANCE NOTE: Use with `selectinload` in queries to prevent the N+1 query problem when accessing notes for multiple parent objects. """ - # Dynamically get the primary key column name of the inheriting class - pk_name = inspect(cls).primary_key[0].name + # All parent tables use 'id' as their primary key. + pk_name = "id" return relationship( "Notes", - primaryjoin=f"and_({cls.__name__}.{pk_name}==Notes.notable_id, " + primaryjoin=f"and_({cls.__name__}.{pk_name}==foreign(Notes.notable_id), " f"Notes.notable_type=='{cls.__name__}')", - lazy="selectin", # A good default for eager loading on demand + lazy="selectin", viewonly=True, ) @@ -247,29 +246,14 @@ 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. - - NOTE: This method creates and returns a new Note object but does *not* add - it to the database session. The caller is responsible for session management. - - Args: - content: The text content of the note. - note_type: The categorized type of the note (from a controlled vocabulary). - created_by: The user or process creating the note. - - Returns: - A new, unsaved Notes object linked to this parent. """ # This import is inside the method to avoid circular import issues at runtime. from db.notes import Notes - # Dynamically get the primary key value of this specific instance - pk_name = inspect(self.__class__).primary_key[0].name - pk_value = getattr(self, pk_name) - return Notes( content=content, note_type=note_type, - notable_id=pk_value, + notable_id=self.id, notable_type=self.__class__.__name__, ) From 6425d06f479738aebcc9a8b3ade68d64f5817c13 Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Wed, 22 Oct 2025 14:56:59 -0600 Subject: [PATCH 10/11] refactor: Update 'noteable_type' category to `note_type` The lexicon incorrectly categorized a type of note as `noteable_type`. It has been corrected to `note_type` This resolves the error where tests for the location model were unable to locate the proper "note_type". --- core/lexicon.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core/lexicon.json b/core/lexicon.json index 2ac67c5c6..07dbfd8f1 100644 --- a/core/lexicon.json +++ b/core/lexicon.json @@ -23,7 +23,7 @@ {"name": "limit_type", "description": null}, {"name": "measurement_method", "description": null}, {"name": "monitoring_status", "description": null}, - {"name": "noteable_type", "description": null}, + {"name": "note_type", "description": null}, {"name": "parameter_name", "description": null}, {"name": "organization", "description": null}, {"name": "parameter_type", "description": null}, @@ -527,10 +527,10 @@ {"categories": ["sensor_status"], "term": "In Repair", "definition": "In Repair"}, {"categories": ["sensor_status"], "term": "Retired", "definition": "Retired"}, {"categories": ["sensor_status"], "term": "Lost", "definition": "Lost"}, - {"categories": ["noteable_type"], "term": "Access", "definition": "Access instructions, gate codes, permission requirements, etc."}, - {"categories": ["noteable_type"], "term": "Construction", "definition": "Construction details, well development, drilling notes, etc. Could create separate `types` for each of these if needed."}, - {"categories": ["noteable_type"], "term": "Maintenance", "definition": "Maintenance observations and issues."}, - {"categories": ["noteable_type"], "term": "Historical", "definition": "Historical information or context about the well or location."}, - {"categories": ["noteable_type"], "term": "Other", "definition": "Other types of notes that do not fit into the predefined categories."} + {"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 From e814635e7259ea017a6bba5b5fc48af087efe829 Mon Sep 17 00:00:00 2001 From: ksmuczynski Date: Wed, 22 Oct 2025 15:10:46 -0600 Subject: [PATCH 11/11] fix: Standardize location notes as List[NoteResponse] across schemas - Comment out string-type notes fields from CreateLocation and UpdateLocation schemas - Ensure LocationResponse consistently uses notes as List[NoteResponse] - Fix validation error where API was receiving string notes but expecting list type - Complete transition to polymorphic notes relationship for location entities - Update test data to create proper Note objects instead of using string values This resolves the ResponseValidationError where the API expected notes to be a list but received a string value ('these are some test notes'). --- schemas/location.py | 22 +++++++++------------- schemas/thing.py | 1 - tests/test_location.py | 26 +++++++++++++++++++++----- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/schemas/location.py b/schemas/location.py index 9f47be5d8..6f60b648a 100644 --- a/schemas/location.py +++ b/schemas/location.py @@ -42,9 +42,9 @@ class CreateLocation(BaseCreateModel, ValidateLocation): """ # name: str | None = None - # TODO: remove `notes` field once polymorphic Notes are successfully implemented. - # I didn't want to remove yet in case it breaks something with the transfer. - 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" @@ -70,10 +70,9 @@ class LocationResponse(BaseResponseModel): """ # name: str | None - # TODO: remove `notes` field once polymorphic Notes are successfully implemented. - # I didn't want to remove yet in case it breaks something with the transfer. - notes: str | None = 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" @@ -87,9 +86,6 @@ class LocationResponse(BaseResponseModel): county: str | None quad_name: str | None - # The new relationship to the polymorphic Notes table - notes: List[NoteResponse] = [] - @field_validator("point", mode="before") def point_to_wkt(cls, value): if isinstance(value, WKBElement): @@ -118,9 +114,9 @@ class UpdateLocation(BaseUpdateModel, ValidateLocation): """ # name: str | None = None - # TODO: remove `notes` field once polymorphic Notes are successfully implemented. - # I didn't want to remove yet in case it breaks something with the transfer. - 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/thing.py b/schemas/thing.py index a3014cd98..64710dbd3 100644 --- a/schemas/thing.py +++ b/schemas/thing.py @@ -142,7 +142,6 @@ 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] = [] 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