BDMS 113 & 114: sample & observation updates & revisions | other updates and revisions#141
Conversation
| samples: Mapped[list["Sample"]] = relationship( # noqa: F821 | ||
| "Sample", | ||
| back_populates="field_event_contact", | ||
| cascade="all, delete-orphan", |
There was a problem hiding this comment.
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.
ksmuczynski
left a comment
There was a problem hiding this comment.
Nice work, looks good!
!!!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:
Samplemodel was determined to be non-normalized, so it was split intoFieldEvent,FieldActivity, andSampleFieldEventHow
Implementation summary - the following was changed / added / removed:
Samplemodel was split intoFieldEvent,FieldActivity, andSampleto better capture when/where/how a sample is created, as well as who was present and who took the sampleFieldEventtablecontacttable would be appropriate. It has all of the requisite information for people. When this gets implemented on the frontend we can add anorganizationquery parameter to GET/contactfor a dropdown menu of eligible participants (if we want to filter by organization. I can definitely add other query parameters, too).activity_typeis stored in theFieldActivitytable that is being used when retrieving/patching different observations rather than prepending the observation class to the observed property.Notes
Any special considerations, workarounds, or follow-up work to note?
staging.contacttable must be associated with asampleI'm not too sure how to proceed.field_event_contact_idbe nullable in thesampletable 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 thecontacttable and have some sort of note saying "legacy data from NM_Aquifer"? Or another flag?MeasuredByfield in theWaterLevelstable will be transferred toContactsbefore creating water levels, thereby not breaking the foreign key constraint?MeasuredByfield in theWaterLevelstable fromNM_AquiferisNULL? Have a contact namedNM Aquifer NULL?FieldEventContactAssociationtable. Will this cause issues with the frontend?FieldEventis created. I plan to allow the user to submit multiple contacts that associate with a field event. After theFieldEventrecord is created, and therefore theFieldEventContactAssociationrecirds are created, they will be in the database and useable when creating/editing asamplerecord./fieldrouter does not yet exist, and therefore theField...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/observationin particular)./fieldcoverage will be my next task.