BDMS-112 Add missing fields to Thing model, schema, and tests#166
Merged
Conversation
…anized code into labeled sections
…umns` code section
feat: add missing relationships to `Thing` model
feat: add relationships to `GroupThingAssociation` table. feat: add proxy to `Group` table
…hips in the Thing model
…ship in Thing model
…AMP folks on 2025-08-29.
Codecov Report✅ All modified and coverable lines are covered by tests.
|
…le from `foreign(permissible_id) == Thing.thing_id` to `foreign(permissible_id) == Thing.id`
…StatusHistoryMixin` The previous logic dynamically generated a primary key name based on the class name (e.g., "thing_id"), which failed for models using the AutoBaseMixin that provides a standard "id" column. This change modifies the f-string to use the correct ".id" attribute, resolving the AttributeError during mapper configuration.
The polymorphic relationships `_thing_target` and `_location_target` caused a `NameError` at runtime because they referenced the `Thing` and `Location` models directly. These models were only imported within a `TYPE_CHECKING` block to avoid circular dependencies, making them unavailable when the code was executed. This commit refactors the `primaryjoin` condition to be a string. This allows SQLAlchemy to defer the evaluation of the join until all models are fully loaded, resolving the `NameError` and making the relationship definitions more robust.
…, and addresses relationships in `Contact` model
Columns, relationships, and proxies were randomly added to the `Contact` model. It has been reorganized so like things are grouped together under a designed header.
SQLAlchemy was raising an `ArgumentError` because it could not infer the direction of the one-to-many polymorphic relationships defined in the `StatusHistoryMixin` and `PermissionMixin`. Without a formal `ForeignKey` constraint on the child columns (`statusable_id` and `permissible_id`), SQLAlchemy could not determine which side of the join was the foreign key. This commit resolves the ambiguity by wrapping these child columns with the `foreign()` annotation in their respective `primaryjoin` conditions. This explicitly marks them as the foreign-key side of the join, allowing the mapper to configure correctly.
jirhiker
reviewed
Oct 1, 2025
The `Thing` model's search_vector referenced the `well_casing_description` column, but it was not defined in the schema, causing an `AttributeError` on startup. The `well_casing_description` column was renamed `well_casing_material` so the search_vector was updated accordingly.
`first_visit_date` was missing from the `schemas/thing.py` UPDATE model. It has been added. Renamed instances of 'well_type' to 'well_purpose' Commented out the `Thing.description` field
The `well_type` property was renamed to `well_purposed` in the `make_well_response` function.
Instances of ondelete, CASCADE should be a str e.g. ondelete="CASCADE". The CASCADE from tkinter and tkinter itself was removed.
`first_visit_date` field should be of type `PastDate` to ensure a future date is not entered.
…n-nullable) When creating a new `Thing` record, users should be required to enter a value for `first_visit_date`. This applies to new records being created - the database itself will still allow NULLS in order to accommodate legacy data.
The `thing_groups` relationship was updated to `group_associations` for clarity. It more descriptive of the relationship and is also consistent with the naming convention used in the group table (`group_associations)`. The back_populates property reference in the group.thing relationship was updated from "thing_groups" to "group_associations" to reflect the update to the relationship name in the Thing table.
# noqa: F821 calls were removed since TYPE_CHECKING is invoked.
…dEventParticipants`. Update instances of `field_event_contact_associations` to `field_event_participants`
The foreign key on `field_event_contact_id` field was incorrectly named "field_event_participant.id". It was updated to "field_event_participants.id"
Change data type for `first_visit_date` from `date` to `PastDate`
jirhiker
approved these changes
Oct 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
This PR addresses the following problem / context:
Thingmodel needed to inherit the new tables that have been created.Thingmodel needed afirst_visit_datefield to store the date of NMBGMR's first recorded interaction with a specific Thing.well_purposefield was needed to capture the primary function of a well (Monitoring, Irrigation, etc)How
Implementation summary - the following was changed / added / removed:
StatusHistoryMixinandPermissionMixinwere imported and inherited by theThingmodel.Thingmodel and related models.first_visit_datefield was added.well_purposefield was added.well_typefield existed in schema files, test files, and the lexicon.json. Thewell_typefield served the same purpose as thewell_purposefield, despite not existing in theThingmodel. Instances ofwell_typewere replaced bywell_purposebased on AMP feedback received on 2025-08-29.Notes
Any special considerations, workarounds, or follow-up work to note?