NO TICKET: revert value_reason to groundwater_level_reason

[jacob-a-brown]

Why

This PR addresses the following problem / context:

• After more discussions it was determined that different types of observations, like groundwater levels and water chemistry, need different "reason" fields to describe the observation

How

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

• removed value_reason as a non-nullable field for all observations
• added groundwater_level_reason for groundwater level observations as a nullable field
  • while it is nullable in the database it is necessary for adding new records
  • the response is nullable since there are null values from NM_Aquifer

Notes

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

• In light of focusing on water levels no other "reasons" will be added until necessary (e.g. we work on water chemistry)
• Now that we are using the word "reason" instead of "status" the not-affected term was changed from Water level not affected by status to Water level not affected

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines	Coverage Δ
db/observation.py	100.00% <100.00%> (ø)
schemas/observation.py	98.41% <100.00%> (ø)
tests/conftest.py	100.00% <ø> (ø)
tests/test_observation.py	95.39% <100.00%> (ø)

[jirhiker]

Why do different observation types need separate fields? Are all the fields going to be string fields? It should be clear from the observation type what the reason represents. This seems like a case of the storage layer driving design which is something we should avoid.

[jacob-a-brown]

I think the reason that this was split into separate fields depending on the observation type is because different types of observations can have different numbers of "reason" fields (water chemistry will likely need more than one - perhaps one for the field and one (or more) for the lab). This will also allow us to more easily restrict possible reasons by observation type. The following reason, for example, doesn't really apply to non-groundwater level observations: "Water level affected by atmospheric pressure"

We could create a separate "reason" table to allow multiple reasons to be associated with a particular observation, negating the necessity of multiple fields in the observation table. That's a bit more up-front work (and another table), but then the reasons will all be standardized and normalized.

[ksmuczynski]

@jacob-a-brown @jirhiker Yes, Jacob is correct, we did discuss a separate Remark table. I think we agreed it is the preferred choice from a design and normalization perspective, but we were weighing the benefit of meeting our sprint goals with the time it would take to create and implement a new table. @jacob-a-brown please let me know if I misinterpreted our conversation or if I left anything out.

[jacob-a-brown]

That sounds correct to me. We can just have this going for water levels for the time being, then when water chemistry is incorporated we can refactor. Or would it make more sense to refactor now to prevent undoing/redoing work later on?

[jirhiker]

That all makes sense, thanks.