kas-bdms-749-thru-753-refactor-water-level-importer#656
Conversation
…and sampler validation - create or reuse field event participants for imported staff columns - link imported samples to the measuring participant - fail a row when measuring_person does not resolve to exactly one participant
- Keep `water_level_notes` as freeform notes - Stop duplicating field staff and sampler into field event/activity notes - Treat structured participants as the authoritative staff source
- Verify the well details payload exposes the measuring person on latest_field_event_sample.contact - Verify the latest field event participants list includes imported staff
There was a problem hiding this comment.
Pull request overview
Refactors the water-level CSV importer to persist measuring staff as structured FieldEventParticipant/Contact data and link imported groundwater-level samples to the measuring participant, enabling downstream/API consumers to rely on participant/contact fields instead of note text.
Changes:
- Create/reuse
FieldEventParticipantrows forfield_staff/field_staff_2/field_staff_3during import and link the importedSampleto the measuring participant. - Stop writing staff/sampler boilerplate into
FieldEvent.notes/FieldActivity.notes(retain only freeform water-level notes). - Add/adjust regression tests covering participant creation/reuse, sample linkage, ambiguous measuring-person failure behavior, and
/detailspayload exposure.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| services/water_level_csv.py | Adds participant/contact creation + sample-to-participant linkage; removes legacy note boilerplate. |
| tests/test_water_level_csv_service.py | Adds importer tests for participants, linkage, rerun idempotency, ambiguity failures, and note behavior. |
| tests/test_thing.py | Adds API regression test asserting imported staff appear in well details payload fields. |
| tests/test_cli_commands.py | Updates CLI E2E assertions to reflect notes no longer include staff boilerplate. |
…tion `Contact` rows are unique on `name + organization`, so the water-level importer now uses that same key when resolving field staff contacts. This avoids missing an existing contact with a different contact_type and attempting a duplicate insert. Also adds regression coverage for reusing an existing same-name/same-organization contact during import.
… tests Water-level importer tests were leaving participant contacts behind in the shared test database, which caused test_get_contacts to fail during full-suite runs. Update fixture and CLI test cleanup so importer-created staff contacts are removed after tests finish, while keeping the contact API assertions strict.
There was a problem hiding this comment.
Pull request overview
Refactors the water-level CSV importer to persist measuring staff as structured FieldEventParticipant/Contact data, link imported groundwater-level samples to the measuring participant, and ensure API consumers can access staff via existing well-details payload fields (instead of parsing notes).
Changes:
- Create/reuse
FieldEventParticipantrecords forfield_staff/field_staff_2/field_staff_3and linkSample.field_event_participant. - Stop writing staff/sampler boilerplate into
field_event.notes/field_activity.notes(retain only freeform water-level notes). - Add/extend regression tests covering participant creation/reuse, ambiguity failures, API payload exposure, and test DB cleanup for importer-created contacts.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| services/water_level_csv.py | Implements participant/contact creation & reuse, measuring participant resolution, sample linkage, and removes legacy notes behavior. |
| tests/test_water_level_csv_service.py | Adds importer tests for participant creation, idempotent reuse, ambiguity handling, and sample-to-participant linkage. |
| tests/test_thing.py | Adds API regression test ensuring /thing/water-well/{id}/details exposes imported staff via structured payload fields. |
| tests/test_cli_commands.py | Updates CLI end-to-end assertions for notes behavior and adds cleanup for importer-created participant contacts. |
| tests/conftest.py | Enhances water_well_thing fixture teardown to remove importer-created participant contacts and avoid cross-test leakage. |
| raw={**normalized}, | ||
| well=well, | ||
| field_staff=model.field_staff, | ||
| field_staff_2=model.field_staff_2, |
There was a problem hiding this comment.
My only concern is that this is brittle and would not handle more than three participants. Thoughts?
There was a problem hiding this comment.
That limit is mostly set by the current input csv/schema contract, which only supports three staff columns. That said, I agree the current shape is a bit brittle if we ever expand beyond three participants. It would prob be cleaner to normalize those fields into a list and iterate that, so we’re not hardcoding the slots in the importer. Let me know if you'd like me to implement something like that.
There was a problem hiding this comment.
Cool. Yep that's what I was thinking, keep the input form unmodified but normalize staff_1,staff_2 etc into a list. Please implement that then it should be good to merge
There was a problem hiding this comment.
Done. Re-requested your review.
…ipant creation Normalize the fixed field_staff CSV columns into a single iterable shape after validation, so participant creation no longer hardcodes each slot in the importer. This keeps the input CSV/schema unchanged while making the participant logic easier to read and easier to extend later if the staff-field shape changes.
Summary
Update the water-level importer to create and reuse structured field-event participants, link imported samples to the measuring participant, and expose water-level staff through existing API payloads instead of relying on
note text.
Why
This PR addresses the following problem / context:
Frontend and reporting consumers need reliable access to water-level measuring staff. Before this change, the importer stored staff and sampler information in notes, which made the data hard to query and easy to misuse. If left unchanged, downstream consumers would continue depending on note text instead of structured participant/contact data.
How
Implementation summary - the following was changed / added / removed:
Backend
FieldEventParticipantrows forfield_staff,field_staff_2, andfield_staff_3during water-level import.sample.field_event_participant_id.measuring_persondoes not resolve to exactly one participant for the field event.field_event.notesandfield_activity.notes; keep only freeform water-level notes there.API
latest_field_event_sample.contactfield_event_participants[]/thing/water-well/{thing_id}/details.Testing
measuring_person`failure behaviorpytest tests/test_water_level_csv_service.pypytest tests/test_thing.pypytest tests/test_cli_commands.pyNotes
Any special considerations, workarounds, or follow-up work to note?