BDMS 55: observation coverage & other updates

[jacob-a-brown]

Why

This PR addresses the following problem / context:

• The observation router should be fully covered with GET, GET by ID, POST, PATCH, and DELETE endpoints
• tests impacted by fixtures needed to be updated to use those fixtures
• proper authentication and permissions need to be vetted at endpoints

How

Implementation summary - the following was changed / added / removed:

• Added POST /observation/groundwater-level, /observation/geothermal, /observation/water-chemistry endpoints
• Added PATCH /observation/groundwater-level/{observation_id}, /observation/geothermal/{observation_id}, /observation/water-chemistry/{observation_id} endpoints
  • 404 errors are raised if the observation_id does not exist in the database
  • 404 errors are raised if the observation_id exists in the database but its observation class (e.g. water chemistry) does not correspond to the endpoint. the actual observation class of the record with that ID is specified in the error message
• Added GET /observation/ and /observation/{observation_id}`
  • Returns fields that are not relevant to a particular observation's observation class
• Added GET /observation/groundwater-level, /observation/geothermal, /observation/water-chemistry endpoints
• Added GET /observation/groundwater-level/{observation_id}, /observation/geothermal/{observation_id}, /observation/water-chemistry/{observation_id} endpoints
  • 404 errors are raised if the observation_id does not exist in the database
  • 404 errors are raised if the observation_id exists in the database but its observation class (e.g. water chemistry) does not correspond to the endpoint. the actual observation class of the record with that ID is specified in the error message
• Added DELETE /observation//{observation_id}
• Updated and added new tests

Notes

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

• Different observation_classes (e.g. groundwater level, water chemistry) are defined by observed_property. This is pretty easy when there's only one property per class, like groundwater level, but can become overwhelming and difficult to maintain when there are multiple properties per class, like water chemistry. Maybe this will become a moot point when we make the views, and those can be used?
• add deg C to lexicon for geothermal observed property
• removed __table_args__ and id from Observation model since the primary key is just the id, which is defined in AutoBaseMixin
• removed depth_to_water from the model since that's encapsulated in value and observed_property
• GroundwaterLevelResponse calculates depth to water below ground surface as value - measuring_point_height
  • should the calculated depth_to_water_bgs be called calculated_depth_to_water_bgs for clarity and to be explicit?

[codecov-commenter]

Codecov Report

❌ Patch coverage is 99.61686% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
services/observation_helper.py	98.38%	1 Missing ⚠️
tests/test_observation.py	99.69%	1 Missing ⚠️

Files with missing lines	Coverage Δ
api/observation.py	100.00% <100.00%> (+2.63%)	⬆️
db/observation.py	100.00% <100.00%> (ø)
schemas/observation.py	98.83% <100.00%> (+1.46%)	⬆️
tests/conftest.py	100.00% <100.00%> (ø)
services/observation_helper.py	98.43% <98.38%> (+33.43%)	⬆️
tests/test_observation.py	96.76% <99.69%> (+19.30%)	⬆️

... and 2 files with indirect coverage changes

[jirhiker]

@jacob-a-brown, @ksmuczynski should this be part of the Sample model?

[jacob-a-brown]

The latest update gets the observation_class from the path of the request. This makes the code easier to maintain because that information is contained and retrieved from one location, rather than many different locations.