Skip to content

kas-bdms-749-thru-753-refactor-water-level-importer#656

Merged
ksmuczynski merged 8 commits into
stagingfrom
kas-bdms-749-thru-753-refactor-water-level-importer
Apr 17, 2026
Merged

kas-bdms-749-thru-753-refactor-water-level-importer#656
ksmuczynski merged 8 commits into
stagingfrom
kas-bdms-749-thru-753-refactor-water-level-importer

Conversation

@ksmuczynski

Copy link
Copy Markdown
Contributor

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

  • Create or reuse FieldEventParticipant rows for field_staff, field_staff_2, and field_staff_3 during water-level import.
  • Link imported groundwater-level samples to the measuring participant through sample.field_event_participant_id.
  • Fail a row with a human-readable error if measuring_person does not resolve to exactly one participant for the field event.
    • Keep participant creation idempotent so rerunning the same import does not create duplicate participant rows.
  • Stop storing staff/sampler boilerplate in field_event.notes and field_activity.notes; keep only freeform water-level notes there.

API

  • Use the existing sample and well-details payloads as the authoritative path for measuring staff:
    • latest_field_event_sample.contact
    • field_event_participants[]
  • Add regression coverage to verify imported water-level staff is exposed through /thing/water-well/{thing_id}/details.

Testing

  • Added/updated importer service tests for:
    • participant creation
    • sample-to-participant linkage
    • rerun/idempotent participant reuse
    • ambiguous measuring_person `failure behavior
    • notes now containing only freeform text
  • Added API regression coverage for imported water-level staff in well details.
  • Local test runs passed:
    • pytest tests/test_water_level_csv_service.py
    • pytest tests/test_thing.py
    • pytest tests/test_cli_commands.py

Notes

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

  • No schema migration was needed.
  • Existing API payload shapes were preserved; this change fills structured participant/contact fields and removes the old staff/sampler note boilerplate.

…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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 FieldEventParticipant rows for field_staff/field_staff_2/field_staff_3 during import and link the imported Sample to 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 /details payload 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.

Comment thread services/water_level_csv.py Outdated
…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.
Copilot AI review requested due to automatic review settings April 16, 2026 15:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 FieldEventParticipant records for field_staff / field_staff_2 / field_staff_3 and link Sample.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.

Comment thread services/water_level_csv.py Outdated
raw={**normalized},
well=well,
field_staff=model.field_staff,
field_staff_2=model.field_staff_2,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My only concern is that this is brittle and would not handle more than three participants. Thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.
@ksmuczynski ksmuczynski requested a review from jirhiker April 16, 2026 21:07
@ksmuczynski ksmuczynski merged commit a664e98 into staging Apr 17, 2026
8 checks passed
@ksmuczynski ksmuczynski deleted the kas-bdms-749-thru-753-refactor-water-level-importer branch April 17, 2026 19:28
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.

3 participants