Skip to content

BDMS 113 & 114: sample & observation updates & revisions | other updates and revisions#141

Merged
jirhiker merged 39 commits into
stagingfrom
jab-observation-sample-field-revisions
Sep 24, 2025
Merged

BDMS 113 & 114: sample & observation updates & revisions | other updates and revisions#141
jirhiker merged 39 commits into
stagingfrom
jab-observation-sample-field-revisions

Conversation

@jacob-a-brown
Copy link
Copy Markdown
Contributor

@jacob-a-brown jacob-a-brown commented Sep 19, 2025

!!!ALERT!!! There is a lot in this PR. There have been a number of big and consequential design choices and changes to the database and therefore the API. Please reach out to me here, Slack, or elsewhere with any questions or if you need clarification.

Why

This PR addresses the following problem / context:

  • The Sample model was determined to be non-normalized, so it was split into FieldEvent, FieldActivity, and Sample
  • AMP colleagues want to know all personnel who were present at a particular FieldEvent
  • lexicon terms should be plain-text and not contain extra information that ought to be retrieved from the database and subsequently used by the API
  • geothermal observations/samples are not ready to be implemented

How

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

  • The Sample model was split into FieldEvent, FieldActivity, and Sample to better capture when/where/how a sample is created, as well as who was present and who took the sample
    • AMP colleagues specifically asked that the collecting organization be recorded, which is occurring in the FieldEvent table
    • AMP colleagues specifically asked that all present at the field event, even if they are not the event leader nor a participant, be recorded. @ksmuczynski and I spoke through the best way to do this and we determined that using the contact table would be appropriate. It has all of the requisite information for people. When this gets implemented on the frontend we can add an organization query parameter to GET /contact for a dropdown menu of eligible participants (if we want to filter by organization. I can definitely add other query parameters, too).
  • Because the activity_type is stored in the FieldActivity table that is being used when retrieving/patching different observations rather than prepending the observation class to the observed property.
  • the contacts that are associated with a sample are now restricted to those present in the many-to-many relationship between contact and field event
  • all references to geothermal have been deprecated until we can get feedback from colleagues. this is not currently used and has been inhibiting development.

Notes

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

  • @jirhiker and @chasetmartin because of the significant database revisions @ksmuczynski should provide feedback/approval for this PR before being merged into staging.
  • this PR does not include the full updates to the transfer scripts. It's mostly there, but now that someone in the contact table must be associated with a sample I'm not too sure how to proceed.
    • Should field_event_contact_id be nullable in the sample table so that the records can be transferred to the new database? This can be updated to be non-nullable in the future, at which point we'd need to figure out how to correctly associate old records. Perhaps put all of them into the contact table and have some sort of note saying "legacy data from NM_Aquifer"? Or another flag?
    • Should that field remain non-nullable and then the MeasuredBy field in the WaterLevels table will be transferred to Contacts before creating water levels, thereby not breaking the foreign key constraint?
      • What do we do if the MeasuredBy field in the WaterLevels table from NM_Aquifer is NULL? Have a contact named NM Aquifer NULL?
  • to restrict contact associations to a sample (ie the sampler) to those that participated in the field event we use the FieldEventContactAssociation table. Will this cause issues with the frontend?
    • This association table will be populated when a new FieldEvent is created. I plan to allow the user to submit multiple contacts that associate with a field event. After the FieldEvent record is created, and therefore the FieldEventContactAssociation recirds are created, they will be in the database and useable when creating/editing a sample record.
  • the /field router does not yet exist, and therefore the Field... are not fully covered (if at all). This will come in a future PR. I wanted to open this PR, though, to get the updated models into staging (sample/observation in particular). /field coverage will be my next task.

Comment thread db/field.py Outdated
Comment thread db/field.py Outdated
samples: Mapped[list["Sample"]] = relationship( # noqa: F821
"Sample",
back_populates="field_event_contact",
cascade="all, delete-orphan",
Copy link
Copy Markdown
Contributor

@ksmuczynski ksmuczynski Sep 22, 2025

Choose a reason for hiding this comment

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

Remove this cascade parameter. The data stored in the FieldEventContactAssociation table is essentially administrative metadata. Deleting an admin record should never automatically trigger the deletion of core scientific data.

For example, say someone notices Ethan Mamer is listed as a field participant during an event he wasn't at and they delete the record. If cascade="all, delete-orphan" were active, the database would say, "Okay, Ethan Mamer's participation record is gone. Now find and delete every Sample record that he is listed as the collector for." The database would delete all associated Sample records and then, because the Sample table should have a cascade to Observation table, the database would delete all the lab results associated with those samples.

Comment thread db/field.py Outdated
Comment thread db/field.py
Comment thread db/field.py
Comment thread db/field.py Outdated
Comment thread db/field.py
Comment thread db/field.py
Comment thread db/field.py Outdated
Comment thread db/field.py
Comment thread db/observation.py
Comment thread db/observation.py Outdated
Comment thread db/observation.py Outdated
Comment thread db/sample.py Outdated
Comment thread db/sample.py Outdated
Comment thread db/sample.py Outdated
Comment thread db/sample.py Outdated
Comment thread db/sample.py Outdated
Comment thread db/sensor.py Outdated
Copy link
Copy Markdown
Contributor

@ksmuczynski ksmuczynski left a comment

Choose a reason for hiding this comment

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

Nice work, looks good!

@jirhiker jirhiker merged commit 9a70756 into staging Sep 24, 2025
3 checks passed
@TylerAdamMartinez TylerAdamMartinez deleted the jab-observation-sample-field-revisions branch February 5, 2026 18:06
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.

5 participants