From 5944e0d371a7aa5c8460109b8a5ccd590aec71d0 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 25 Sep 2025 12:57:47 -0600 Subject: [PATCH 1/5] refactor: remove field_event_id & field_event_contact_id from sample response --- schemas/sample.py | 4 ++-- tests/test_sample.py | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/schemas/sample.py b/schemas/sample.py index aec115d9a..44a29aef9 100644 --- a/schemas/sample.py +++ b/schemas/sample.py @@ -124,8 +124,8 @@ class SampleResponse(BaseResponseModel): field_event: FieldEventResponse field_activity: FieldActivityResponse contact: ContactResponse - field_activity_id: int - field_event_contact_id: int + # field_activity_id: int + # field_event_contact_id: int sample_date: Annotated[AwareDatetime, PastDatetime()] sample_name: str sample_matrix: str diff --git a/tests/test_sample.py b/tests/test_sample.py index b1b495afe..f7380cd26 100644 --- a/tests/test_sample.py +++ b/tests/test_sample.py @@ -54,7 +54,7 @@ def test_validate_sample_top_and_bottom(): # ============= Post tests for samples ============================================= def test_add_sample( - groundwater_level_field_activity, water_well_thing, field_event_contact + groundwater_level_field_activity, water_well_thing, contact, field_event_contact ): """ Test adding a sample. @@ -84,9 +84,7 @@ def test_add_sample( assert data["thing"]["id"] == water_well_thing.id assert data["field_event"]["id"] == groundwater_level_field_activity.field_event_id assert data["field_activity"]["id"] == groundwater_level_field_activity.id - assert data["field_activity_id"] == payload["field_activity_id"] - assert data["contact"]["id"] == field_event_contact.contact_id - assert data["field_event_contact_id"] == payload["field_event_contact_id"] + assert data["contact"]["id"] == contact.id assert data["sample_date"] == payload["sample_date"] assert data["sample_name"] == payload["sample_name"] assert data["sample_matrix"] == payload["sample_matrix"] @@ -183,7 +181,6 @@ def test_patch_sample(water_chemistry_sample, groundwater_level_field_activity): """ payload = { "field_activity_id": groundwater_level_field_activity.id, - # "field_event_contact_id": third_contact.id, "sample_date": "2025-01-02T00:00:00Z", "sample_name": "patched sample name", "sample_matrix": "soil", @@ -199,6 +196,8 @@ def test_patch_sample(water_chemistry_sample, groundwater_level_field_activity): data = response.json() for key, value in payload.items(): + if key in ["field_event_contact_id", "field_activity_id"]: + continue assert data[key] == value # rollback after updating the sample @@ -288,9 +287,7 @@ def test_get_samples(water_chemistry_sample, groundwater_level_sample): assert "thing" in item assert "field_event" in item assert "field_activity" in item - assert "field_activity_id" in item assert "contact" in item - assert "field_event_contact_id" in item assert "sample_date" in item assert "sample_name" in item assert "sample_matrix" in item @@ -307,7 +304,7 @@ def test_get_sample_by_id( water_chemistry_field_activity, field_event, water_well_thing, - field_event_contact, + contact, ): """ Test retrieving a sample by its ID. @@ -322,8 +319,7 @@ def test_get_sample_by_id( assert data["thing"]["id"] == water_well_thing.id assert data["field_event"]["id"] == field_event.id assert data["field_activity"]["id"] == water_chemistry_field_activity.id - assert data["field_activity_id"] == water_chemistry_field_activity.id - assert data["field_event_contact_id"] == field_event_contact.id + assert data["contact"]["id"] == contact.id assert data["sample_date"] == water_chemistry_sample.sample_date assert data["sample_name"] == water_chemistry_sample.sample_name assert data["sample_matrix"] == water_chemistry_sample.sample_matrix From 532b71b0586945005aaf33a2a6806091467388bf Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 25 Sep 2025 13:10:37 -0600 Subject: [PATCH 2/5] fix: eagerly load related tables to contact for speed --- api/contact.py | 17 +++-------------- services/contact_helper.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/api/contact.py b/api/contact.py index e7ea8ed4e..c38f52e26 100644 --- a/api/contact.py +++ b/api/contact.py @@ -27,7 +27,7 @@ amp_editor_dependency, amp_viewer_dependency, ) -from db import ThingContactAssociation, Thing, Contact, Email, Phone, Address +from db import Contact, Email, Phone, Address from schemas.contact import ( CreateContact, CreateAddress, @@ -43,14 +43,11 @@ UpdateAddress, ) from services.crud_helper import model_patcher, model_deleter, model_adder -from services.contact_helper import ( - add_contact, -) +from services.contact_helper import add_contact, get_db_contacts from services.lexicon_helper import get_terms_by_category from services.query_helper import ( simple_get_by_id, paginated_all_getter, - order_sort_filter, ) from services.exceptions_helper import PydanticStyleException @@ -484,15 +481,7 @@ async def get_contacts( :param session: :return: """ - if thing_id: - sql = select(Contact) - sql = sql.join(ThingContactAssociation).join(Thing) - sql = sql.where(Thing.id == thing_id) - - sql = order_sort_filter(sql, Contact, sort=sort, order=order, filter_=filter_) - return paginate(query=sql, conn=session) - else: - return paginated_all_getter(session, Contact, sort, order, filter_) + return get_db_contacts(session, thing_id, sort, order, filter_) @router.get("/{contact_id}", summary="Get contact by ID") diff --git a/services/contact_helper.py b/services/contact_helper.py index 3f7031999..f50a3b3e1 100644 --- a/services/contact_helper.py +++ b/services/contact_helper.py @@ -17,8 +17,36 @@ from schemas.contact import ( CreateContact, ) +from services.query_helper import order_sort_filter from services.audit_helper import audit_add -from sqlalchemy.orm import Session + +from fastapi_pagination.ext.sqlalchemy import paginate +from sqlalchemy.orm import Session, joinedload + + +def get_db_contacts( + session: Session, + thing_id: int | None = None, + sort: str | None = None, + order: str | None = None, + filter_: str | None = None, +): + sql = session.query(Contact).options( + # eagerly load related tables to avoid N+1 problems + joinedload(Contact.emails), + joinedload(Contact.phones), + joinedload(Contact.addresses), + joinedload(Contact.thing_associations).joinedload( + ThingContactAssociation.thing + ), + ) + + if thing_id: + sql = sql.join(ThingContactAssociation) + sql = sql.where(ThingContactAssociation.thing_id == thing_id) + + sql = order_sort_filter(sql, Contact, sort, order, filter_) + return paginate(sql) def add_contact(session: Session, data: CreateContact | dict, user: dict) -> Contact: From 3099e4aef477a030bc6add06bc7401e368634d29 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 25 Sep 2025 13:26:02 -0600 Subject: [PATCH 3/5] feat: add thing_id query param test for /sample --- tests/test_sample.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/test_sample.py b/tests/test_sample.py index f7380cd26..4f5a301ee 100644 --- a/tests/test_sample.py +++ b/tests/test_sample.py @@ -299,6 +299,22 @@ def test_get_samples(water_chemistry_sample, groundwater_level_sample): assert "release_status" in item +def test_get_samples_by_thing_id( + water_chemistry_sample, groundwater_level_sample, water_well_thing +): + response = client.get(f"/sample?thing_id={water_well_thing.id}") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 2 + + data_ids = [d["id"] for d in data["items"]] + sorted_data_ids = sorted(data_ids) + + assert sorted_data_ids == sorted( + [water_chemistry_sample.id, groundwater_level_sample.id] + ) + + def test_get_sample_by_id( water_chemistry_sample, water_chemistry_field_activity, From dd7d3fbf0bbd2ad90b3c49e419d6c83aa79022f2 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 25 Sep 2025 13:33:07 -0600 Subject: [PATCH 4/5] feat: add thing_id query parameter to /sample --- api/sample.py | 3 ++- services/sample_helper.py | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/api/sample.py b/api/sample.py index f26b9c6db..22003a122 100644 --- a/api/sample.py +++ b/api/sample.py @@ -116,6 +116,7 @@ async def update_sample( async def get_samples( session: session_dependency, user: viewer_dependency, + thing_id: int | None = None, sort: str = None, order: str = None, filter_: str = Query(alias="filter", default=None), @@ -123,7 +124,7 @@ async def get_samples( """ Endpoint to retrieve samples. """ - return get_db_samples(session, sort=sort, order=order, filter_=filter_) + return get_db_samples(session, thing_id, sort=sort, order=order, filter_=filter_) @router.get("/{sample_id}", summary="Get Sample by ID") diff --git a/services/sample_helper.py b/services/sample_helper.py index dc02506e0..421cb3336 100644 --- a/services/sample_helper.py +++ b/services/sample_helper.py @@ -7,6 +7,7 @@ def get_db_samples( session: Session, + thing_id: int | None = None, order: str | None = None, sort: str | None = None, filter_: str | None = None, @@ -21,6 +22,11 @@ def get_db_samples( ), # Eagerly load related Contact ) + if thing_id: + query = query.join(FieldActivity) + query = query.join(FieldEvent) + query = query.where(FieldEvent.thing_id == thing_id) + query = order_sort_filter(query, Sample, sort, order, filter_) return paginate(query) From 8b68d508721622926c78df16f32f40af98ac21b7 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Thu, 25 Sep 2025 13:41:05 -0600 Subject: [PATCH 5/5] test: add test for thing_id query param for /contact --- tests/test_contact.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/test_contact.py b/tests/test_contact.py index 22b5ad65a..67e81ea43 100644 --- a/tests/test_contact.py +++ b/tests/test_contact.py @@ -410,6 +410,15 @@ def test_get_contacts(contact, email, address, phone): assert data["items"][0]["addresses"][0]["release_status"] == address.release_status +def test_get_contacts_by_thing_id(contact, second_contact, water_well_thing): + response = client.get(f"/contact?thing_id={water_well_thing.id}") + data = response.json() + + assert response.status_code == 200 + assert data["total"] == 1 + assert data["items"][0]["id"] == contact.id + + def test_get_contact_by_id(contact, email, address, phone): response = client.get(f"/contact/{contact.id}") assert response.status_code == 200