From 34a226564ff8ac12c9d5f3a23d57675df3e1723f Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 16:31:05 -0600 Subject: [PATCH 01/10] test: setup location tests for all functions except DELETE --- tests/test_location.py | 107 ++++++++++++++++++++++++++++++++--------- 1 file changed, 85 insertions(+), 22 deletions(-) diff --git a/tests/test_location.py b/tests/test_location.py index a73910077..96630d867 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -13,40 +13,44 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== +from geoalchemy2.shape import to_shape import pytest +from db import Location +from db.engine import session_ctx from tests import client +# ============= Post tests for locations ====================================== + def test_add_location(): - response = client.post( - "/location", - json={ - "name": "Test Location 3", - "point": "POINT(10.1 10.1)", - # "visible": True, - }, - ) - assert response.status_code == 201 - data = response.json() - assert "id" in data + payload = { + "name": "test location", + "point": "POINT (10.1 10.1)", + "release_status": "draft", + } + response = client.post("/location", json=payload) - response = client.post( - "/location", - json={ - "name": "Test Location 4", - "point": "POINT(50.0 50.0)", - # "visible": False, - }, - ) assert response.status_code == 201 data = response.json() assert "id" in data + assert data["name"] == payload["name"] + assert data["point"] == payload["point"] + assert data["release_status"] == payload["release_status"] + + # cleanup after test + with session_ctx() as session: + session.delete(session.get(Location, data["id"])) + session.commit() + +# ============= Patch tests for locations ===================================== -def test_update_location(): + +def test_update_location(location): + location_id = location.id response = client.patch( - "/location/1", + f"/location/{location_id}", json={ "point": "POINT (10.1 20.2)", "release_status": "draft", @@ -54,10 +58,69 @@ def test_update_location(): ) assert response.status_code == 200 data = response.json() - assert "id" in data + assert data["id"] == location_id assert data["point"] == "POINT (10.1 20.2)" assert data["release_status"] == "draft" + # cleanup after test + with session_ctx() as session: + updated_location = session.get(Location, location_id) + updated_location.point = location.point + updated_location.release_status = location.release_status + session.commit() + + +def test_patch_location_404_not_found(location): + """ + Testing updating a location that does not exist + """ + bad_location_id = 999999999999 + location_name_patch = "another test name" + response = client.patch( + f"/location/{location.id}", json={"name": location_name_patch} + ) + data = response.json() + assert response.status_code == 404 + assert data["detail"] == f"Location with ID {bad_location_id} not found" + + +# ============= GET tests for locations ======================================= + + +def test_get_locations(location): + """ + Test retrieving locations + """ + response = client.get("/location") + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["items"][0]["id"] == location.id + assert data["items"][0]["name"] == location.name + assert data["items"][0]["point"] == to_shape(location.point).wkt + assert data["items"][0]["release_status"] == location.release_status + + +def test_get_location_by_id(location): + response = client.get(f"/location/{location.id}") + assert response.status_code == 200 + data = response.json() + assert data["id"] == location.id + assert data["name"] == location.name + assert data["point"] == to_shape(location.point).wkt + assert data["release_status"] == location.release_status + + +def test_get_sample_by_id_404_not_found(location): + bad_location_id = 999999999 + response = client.get(f"/location/{bad_location_id}") + data = response.json() + assert response.status_code == 404 + assert data["detail"] == f"Location with ID {bad_location_id} not found." + + +# ============= skipped tests for locations =================================== + @pytest.mark.skip def test_get_locations_expand(): From 0c77bc6b5d6eef2e046300987cab291a0bb8abf9 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 16:49:10 -0600 Subject: [PATCH 02/10] fix: fix patch 404 test for location --- tests/test_location.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_location.py b/tests/test_location.py index 96630d867..ce39e9fc1 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -74,14 +74,14 @@ def test_patch_location_404_not_found(location): """ Testing updating a location that does not exist """ - bad_location_id = 999999999999 + bad_location_id = 99999 location_name_patch = "another test name" response = client.patch( - f"/location/{location.id}", json={"name": location_name_patch} + f"/location/{bad_location_id}", json={"name": location_name_patch} ) data = response.json() assert response.status_code == 404 - assert data["detail"] == f"Location with ID {bad_location_id} not found" + assert data["detail"] == f"Location with ID {bad_location_id} not found." # ============= GET tests for locations ======================================= From 454892f6c0cb697d595f723b80ec15f96455b699 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 16:51:51 -0600 Subject: [PATCH 03/10] feat: raise 404 error for /location/{location_id} if not found --- api/location.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/api/location.py b/api/location.py index 1d63b23c5..065329d3d 100644 --- a/api/location.py +++ b/api/location.py @@ -29,7 +29,7 @@ from schemas.location import CreateLocation, LocationResponse, UpdateLocation from schemas.thing import LocationWellResponse from services.geospatial_helper import make_within_wkt -from services.query_helper import make_query, order_sort_filter +from services.query_helper import make_query, order_sort_filter, simple_get_by_id from services.crud_helper import model_patcher from fastapi import APIRouter @@ -177,13 +177,7 @@ async def get_location_by_id( """ Retrieve a sample location by ID from the database. """ - sql = select(Location).where(Location.id == location_id) - - result = session.execute(sql) - location = result.scalar_one_or_none() - - if not location: - return {"message": "Location not found"} + location = simple_get_by_id(session, Location, location_id) response_klass = LocationResponse if expand == "well": From cca5800ea92ec44ac4a774a27da2a32693e3378e Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 17:01:29 -0600 Subject: [PATCH 04/10] test: add delete location tests --- tests/test_location.py | 43 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/test_location.py b/tests/test_location.py index ce39e9fc1..2bbcccaa7 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -20,6 +20,24 @@ from db.engine import session_ctx from tests import client +# ============= module & function fixtures ======================================= + + +@pytest.fixture(scope="function") +def second_location(): + with session_ctx() as session: + location = Location( + name="second location", + point="POINT (10.2 10.2)", + release_status="draft", + ) + session.add(location) + session.commit() + yield location + session.delete(location) + session.commit() + + # ============= Post tests for locations ====================================== @@ -119,6 +137,31 @@ def test_get_sample_by_id_404_not_found(location): assert data["detail"] == f"Location with ID {bad_location_id} not found." +# ============= DELETE tests for locations ==================================== + + +def test_delete_location(second_location): + response = client.delete(f"/location/{second_location.id}") + assert response.status_code == 204 + + # Verify the location is deleted + response = client.get(f"/location/{second_location.id}") + assert response.status_code == 404 + data = response.json() + assert data["detail"] == f"Location with ID {second_location.id} not found." + + +def test_delete_location_404_not_found(second_location): + """ + Testing deleting a location that does not exist + """ + bad_location_id = 99999 + response = client.delete(f"/location/{bad_location_id}") + data = response.json() + assert response.status_code == 404 + assert data["detail"] == f"Location with ID {bad_location_id} not found." + + # ============= skipped tests for locations =================================== From b3d04612e9904fe5e754ee985bc38a96b062c7e4 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 17:03:17 -0600 Subject: [PATCH 05/10] feat: add general/reusable model_delete function --- services/crud_helper.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/services/crud_helper.py b/services/crud_helper.py index ac9f37a95..fe26c5ce1 100644 --- a/services/crud_helper.py +++ b/services/crud_helper.py @@ -13,10 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== -from fastapi import HTTPException from pydantic import BaseModel from sqlalchemy.orm import Session, DeclarativeBase -from starlette.status import HTTP_404_NOT_FOUND from services.query_helper import simple_get_by_id @@ -35,4 +33,11 @@ def model_patcher( return item +def model_deleter(session: Session, model: DeclarativeBase, item_id: int): + # simple_get_by_id raises HTTP_404_NOT_FOUND if the item is not found + item = simple_get_by_id(session, model, item_id) + session.delete(item) + session.commit() + + # ============= EOF ============================================= From f88f6207d6af9d3aa86eee80043805628698806c Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 17:03:39 -0600 Subject: [PATCH 06/10] feat: implement DELETE endpoint for location --- api/location.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/api/location.py b/api/location.py index 065329d3d..3ed66097b 100644 --- a/api/location.py +++ b/api/location.py @@ -15,7 +15,7 @@ # =============================================================================== from typing import Union -from fastapi import Depends, Query +from fastapi import Depends, Query, Response from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select, func from sqlalchemy.orm import Session @@ -30,7 +30,7 @@ from schemas.thing import LocationWellResponse from services.geospatial_helper import make_within_wkt from services.query_helper import make_query, order_sort_filter, simple_get_by_id -from services.crud_helper import model_patcher +from services.crud_helper import model_patcher, model_deleter from fastapi import APIRouter @@ -186,4 +186,15 @@ async def get_location_by_id( return response_klass.model_validate(location) +@router.delete("/{location_id}", summary="Delete location by ID") +async def delete_location( + location_id: int, session: Session = Depends(get_db_session) +) -> None: + """ + Delete a sample location by ID from the database. + """ + model_deleter(session, Location, location_id) + return Response(status_code=204) + + # ============= EOF ============================================= From 2bcf5fd447355564b6af49d9bae5e902d66a3316 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 17:07:34 -0600 Subject: [PATCH 07/10] refactor: remove expand from location endpoints --- api/location.py | 27 +++++---------------------- tests/test_location.py | 29 ----------------------------- 2 files changed, 5 insertions(+), 51 deletions(-) diff --git a/api/location.py b/api/location.py index 3ed66097b..19c9cde61 100644 --- a/api/location.py +++ b/api/location.py @@ -13,8 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== -from typing import Union - from fastapi import Depends, Query, Response from fastapi_pagination.ext.sqlalchemy import paginate from sqlalchemy import select, func @@ -27,7 +25,6 @@ from db.location import Location from db.engine import get_db_session from schemas.location import CreateLocation, LocationResponse, UpdateLocation -from schemas.thing import LocationWellResponse from services.geospatial_helper import make_within_wkt from services.query_helper import make_query, order_sort_filter, simple_get_by_id from services.crud_helper import model_patcher, model_deleter @@ -132,11 +129,10 @@ async def get_location( nearby_distance_km: float = 1, within: str = None, query: str = None, - expand: str = None, sort: str = None, order: str = None, filter_: str = Query(alias="filter", default=None), -) -> CustomPage[Union[LocationResponse, LocationWellResponse]]: +) -> CustomPage[LocationResponse]: """ Retrieve all wells from the database. """ @@ -154,17 +150,9 @@ async def get_location( elif within: sql = make_within_wkt(sql, within) - if expand == "well": - pass - - def transformer(items): - if expand == "well": - return [LocationWellResponse.model_validate(item) for item in items] - return [LocationResponse.model_validate(item) for item in items] - sql = order_sort_filter(sql, Location, sort, order, filter_) - return paginate(query=sql, conn=session, transformer=transformer) + return paginate(query=sql, conn=session) @router.get( @@ -172,18 +160,13 @@ def transformer(items): summary="Get location by ID", ) async def get_location_by_id( - location_id: int, expand: str = None, session: Session = Depends(get_db_session) -) -> LocationResponse | LocationWellResponse: + location_id: int, session: Session = Depends(get_db_session) +) -> LocationResponse: """ Retrieve a sample location by ID from the database. """ location = simple_get_by_id(session, Location, location_id) - - response_klass = LocationResponse - if expand == "well": - response_klass = LocationWellResponse - - return response_klass.model_validate(location) + return location @router.delete("/{location_id}", summary="Delete location by ID") diff --git a/tests/test_location.py b/tests/test_location.py index 2bbcccaa7..ee98770d1 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -162,33 +162,4 @@ def test_delete_location_404_not_found(second_location): assert data["detail"] == f"Location with ID {bad_location_id} not found." -# ============= skipped tests for locations =================================== - - -@pytest.mark.skip -def test_get_locations_expand(): - response = client.get("/base/location?expand=well") - assert response.status_code == 200 - data = response.json() - assert "items" in data - assert len(data["items"]) > 0 - for item in data["items"]: - assert "id" in item - assert "point" in item - assert "well" in item - - -@pytest.mark.skip -def test_get_location_expand(): - response = client.get("/base/location/1", params={"expand": "well"}) - assert response.status_code == 200 - data = response.json() - assert "id" in data - assert data["id"] == 1 - assert "point" in data - assert data["point"] == "POINT (10.1 10.1)" - assert "well" in data - assert len(data["well"]) == 1 - - # ============= EOF ============================================= From db010211051a61191811bc68df523372b0b6c6d0 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Mon, 11 Aug 2025 17:14:20 -0600 Subject: [PATCH 08/10] fix: make tests more independent --- tests/test_geospatial.py | 7 +++++++ tests/test_location.py | 1 + tests/test_thing.py | 8 +++----- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/test_geospatial.py b/tests/test_geospatial.py index e9cff5d34..11d5f9060 100644 --- a/tests/test_geospatial.py +++ b/tests/test_geospatial.py @@ -57,6 +57,13 @@ def populate(): session.add(LocationThingAssociation(location=loc2, thing=thing2)) session.commit() + yield + + # Cleanup + session.delete(loc1) + session.delete(loc2) + session.commit() + def test_get_geojson(): response = client.get("/geospatial", params={"format": "geojson"}) diff --git a/tests/test_location.py b/tests/test_location.py index ee98770d1..4e0ccdfea 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -112,6 +112,7 @@ def test_get_locations(location): response = client.get("/location") assert response.status_code == 200 data = response.json() + print(data) assert data["total"] == 1 assert data["items"][0]["id"] == location.id assert data["items"][0]["name"] == location.name diff --git a/tests/test_thing.py b/tests/test_thing.py index f8e79132b..181dcf481 100644 --- a/tests/test_thing.py +++ b/tests/test_thing.py @@ -13,8 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== -import pytest - from tests import client from main import app from core.dependencies import well_user_function @@ -46,7 +44,7 @@ def test_add_group(): assert data["name"] == "collabnet" -def test_add_well(): +def test_add_well(location): # response = client.post( # "/lexicon/add", json={"term": "Monitoring", "definition": "Monitoring Well"} # ) @@ -60,7 +58,7 @@ def test_add_well(): "/thing", json={ "thing_type": "water well", - "location_id": 1, + "location_id": location.id, "name": "Test Well", "api_id": "1001-0001", "ose_pod_id": "RA-0001", @@ -79,7 +77,7 @@ def test_add_well(): "/thing", json={ "thing_type": "water well", - "location_id": 2, + "location_id": location.id, "name": "Test Well 2", "api_id": "1001-0002", "ose_pod_id": "RA-0002", From 062e94e467589727d1a419bdc6861d64257b9729 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 12 Aug 2025 08:31:16 -0600 Subject: [PATCH 09/10] fix: remove print debugging statement --- tests/test_location.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_location.py b/tests/test_location.py index 4e0ccdfea..ee98770d1 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -112,7 +112,6 @@ def test_get_locations(location): response = client.get("/location") assert response.status_code == 200 data = response.json() - print(data) assert data["total"] == 1 assert data["items"][0]["id"] == location.id assert data["items"][0]["name"] == location.name From 686c2f1ce1f2c3599aa36ff4630ceda4a7420d77 Mon Sep 17 00:00:00 2001 From: Jacob Brown Date: Tue, 12 Aug 2025 08:56:43 -0600 Subject: [PATCH 10/10] refactor: address PR 79 feedback Return 204 Response from model_deleter --- api/location.py | 5 ++--- schemas/location.py | 2 +- services/crud_helper.py | 3 +++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/api/location.py b/api/location.py index 19c9cde61..666e09fe3 100644 --- a/api/location.py +++ b/api/location.py @@ -172,12 +172,11 @@ async def get_location_by_id( @router.delete("/{location_id}", summary="Delete location by ID") async def delete_location( location_id: int, session: Session = Depends(get_db_session) -) -> None: +) -> Response: """ Delete a sample location by ID from the database. """ - model_deleter(session, Location, location_id) - return Response(status_code=204) + return model_deleter(session, Location, location_id) # ============= EOF ============================================= diff --git a/schemas/location.py b/schemas/location.py index af48e8f9d..b81795cec 100644 --- a/schemas/location.py +++ b/schemas/location.py @@ -21,7 +21,7 @@ from schemas import ORMBaseModel """ -REFACTOR TODO +TODO Create common validator classes to be shared amongst create and update schemas. Since many fields are optional in the update schemas set check_fields=False in the field_validator. diff --git a/services/crud_helper.py b/services/crud_helper.py index fe26c5ce1..623abfce1 100644 --- a/services/crud_helper.py +++ b/services/crud_helper.py @@ -13,8 +13,10 @@ # See the License for the specific language governing permissions and # limitations under the License. # =============================================================================== +from fastapi import Response from pydantic import BaseModel from sqlalchemy.orm import Session, DeclarativeBase +from starlette.status import HTTP_204_NO_CONTENT from services.query_helper import simple_get_by_id @@ -38,6 +40,7 @@ def model_deleter(session: Session, model: DeclarativeBase, item_id: int): item = simple_get_by_id(session, model, item_id) session.delete(item) session.commit() + return Response(status_code=HTTP_204_NO_CONTENT) # ============= EOF =============================================