BDMS 389: well inventory csv updates#282
Merged
jirhiker merged 14 commits intoDec 10, 2025
Merged
Conversation
update the CSV files to use values restricted by the lexicon
origin_source is freeform, whereas origin_type is a list of pre-defined values
The docker compose file was changed to map Postgres to host port 54321.
These fields were noted with the inline comment "TODO: needs a home"
…V import Both optional and required fields have been added to the CreateWell and CreateThing schemas per the well inventory CSV import requirements. The fields added to CreateThing are applicable to all thing types, while the fields added to CreateWell are specific to well things.
The function add_thing should handle all of the data in CreateWell, so that it can be used in multiple places without duplicating code.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
there can be multiple activities per field event, one of which is the well inventory
jirhiker
requested changes
Dec 9, 2025
jirhiker
left a comment
Member
There was a problem hiding this comment.
looks good. Only issue is the port assignment
| password = os.environ.get("POSTGRES_PASSWORD", "") | ||
| host = os.environ.get("POSTGRES_HOST", "localhost") | ||
| port = os.environ.get("POSTGRES_PORT", "5432") | ||
| port = os.environ.get("POSTGRES_PORT", "54321") |
Member
There was a problem hiding this comment.
54321 is only for testing and should not be default
Contributor
Author
There was a problem hiding this comment.
Because 5432 is used within Docker, but 54321 is used for testing, I set POSTGRES_PORT=5432 in docker-compose.yml and then updated my environment file to use the correct port for testing.
I also updated .env.example to have the other environment variables in use (POSTGRES_PORT and REBUILD_DB)
…docker-compose.yml Inside Docker the app needs to use port 5432 to connect to Postgres, but on the host machine we want to use 54321. This can be set in the .env file, but to prevent 54321 from being used within Docker we set POSTGRES_PORT to 5432.
jirhiker
approved these changes
Dec 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚨 I added the feature file well-inventory-csv.feature to this PR so that the tests can be run against it 🚨
Why
This PR addresses the following problem / context:
/well-inventory-csvendpoint and the feature tests in well-inventory-csv.feature.How
Implementation summary - the following was changed / added / removed:
CreateWellandCreateThingschemas. They were placed in the former if water well-specific and added to the latter if they apply to allthing_typesThing, such asMonitoringFrequencyHistoryandMeasuringPointHistory,Notes, orDataProvenanceare now handled in the function services/thing_helper.py::add_thing. All of this functionality was moved there to keep the code DRY since this is also used in POST/thing/water-well(and eventually otherthing_types).PermissionHistory, are not handled by this function.add_thinghandles the creation ofLocationThingAssociationandGroupThingAssociationalready, so that logic was removed from api/well_inventory.pyWellTransfererwas updated to exclude fields as necessary.Notes
Any special considerations, workarounds, or follow-up work to note?
WellInventoryRowschema has a home in a model. I marked them with the inline commentTODO: needs a home. Should I create new fields for these? If so, some of them can probably be in the polymorphic notes table (target_table=contact), but others I'm not too sure. Thoughts on those fields?LocationThingAssociation.effective_startcorrectly inadd_thing?FieldActivityrecord is added too (and if a water level is determined/chemistry sample taken at the same time they can all be part of the same field event)