Skip to content

BDMS-195: jir-kas-notes-model#241

Merged
jirhiker merged 27 commits into
stagingfrom
jir-kas-notes-model
Nov 18, 2025
Merged

BDMS-195: jir-kas-notes-model#241
jirhiker merged 27 commits into
stagingfrom
jir-kas-notes-model

Conversation

@jirhiker

Copy link
Copy Markdown
Member

Why

This PR addresses the following problem / context:

  • Use bullet points here

How

Implementation summary - the following was changed / added / removed:

  • Use bullet points here

Notes

Any special considerations, workarounds, or follow-up work to note?

  • Use bullet points here

ksmuczynski and others added 13 commits October 17, 2025 09:42
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.
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
Removed the TODO comments about updating the lexicon file with 'notable_type' category and values. They have been added.
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.
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.
…ation models

- Created new Pydantic schema for the Notes model
- Added polymorphic relationship `notes: List[NoteResponse] = []` to Thing and
  Location schemas for flexible data modeling.
- 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
- 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.
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".
- 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').
@jirhiker jirhiker changed the title jir-kas-notes-model BDMS-195: jir-kas-notes-model Nov 10, 2025
@codecov-commenter

codecov-commenter commented Nov 10, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.17476% with 6 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
services/thing_helper.py 44.44% 5 Missing ⚠️
db/notes.py 95.23% 1 Missing ⚠️
Files with missing lines Coverage Δ
core/initializers.py 87.95% <100.00%> (+1.93%) ⬆️
db/__init__.py 97.72% <100.00%> (+0.05%) ⬆️
db/base.py 96.66% <100.00%> (+0.05%) ⬆️
db/location.py 88.37% <100.00%> (-6.63%) ⬇️
db/thing.py 100.00% <100.00%> (ø)
schemas/location.py 92.85% <100.00%> (+0.26%) ⬆️
schemas/notes.py 100.00% <100.00%> (ø)
schemas/thing.py 97.79% <100.00%> (+0.11%) ⬆️
services/crud_helper.py 97.95% <100.00%> (+0.98%) ⬆️
tests/__init__.py 82.60% <ø> (-17.40%) ⬇️
... and 4 more

... and 17 files with indirect coverage changes

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread services/crud_helper.py Outdated
Comment thread services/crud_helper.py
Comment thread db/notes.py Outdated
Comment thread schemas/notes.py
Comment thread schemas/notes.py
Comment thread services/crud_helper.py
Comment thread services/crud_helper.py
jacob-a-brown and others added 8 commits November 11, 2025 22:14
…ation models

- Created new Pydantic schema for the Notes model
- Added polymorphic relationship `notes: List[NoteResponse] = []` to Thing and
  Location schemas for flexible data modeling.
- 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').
Comment thread db/notes.py Outdated
Comment thread db/base.py Outdated
…table in Notes model; reintroduce NotesMixin for polymorphic relationships
@jirhiker jirhiker merged commit 574869d into staging Nov 18, 2025
6 checks passed
@jirhiker jirhiker deleted the jir-kas-notes-model branch December 3, 2025 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants