Skip to content

BDMS 389: well inventory csv updates#282

Merged
jirhiker merged 14 commits into
well-inventory-csvfrom
bdms-389-well-inventory-csv-updates
Dec 10, 2025
Merged

BDMS 389: well inventory csv updates#282
jirhiker merged 14 commits into
well-inventory-csvfrom
bdms-389-well-inventory-csv-updates

Conversation

@jacob-a-brown

@jacob-a-brown jacob-a-brown commented Dec 9, 2025

Copy link
Copy Markdown
Contributor

🚨 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:

  • Fields that have been added for the implementation of well-core-information.feature, well-additional-information.feature, and well-notes.feature need to be handled by the /well-inventory-csv endpoint and the feature tests in well-inventory-csv.feature.

How

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

  • New fields were mapped in both CreateWell and CreateThing schemas. They were placed in the former if water well-specific and added to the latter if they apply to all thing_types
  • All models that relate to a Thing, such as MonitoringFrequencyHistory and MeasuringPointHistory, Notes, or DataProvenance are 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 other thing_types).
    • Models that are dependent on the creation of objects from other models, like PermissionHistory, are not handled by this function.
    • add_thing handles the creation of LocationThingAssociation and GroupThingAssociation already, so that logic was removed from api/well_inventory.py
  • The WellTransferer was updated to exclude fields as necessary.
  • The csv tables used for the tests were updated to use restricted vocabulary from the lexicon.

Notes

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

  • Not every field in the WellInventoryRow schema has a home in a model. I marked them with the inline comment TODO: 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?
    • Please make comments about them in schemas/well_inventory.py where I made the inline comments.
  • The default port for the DB engine was changed to 54321 to correspond with the new Docker setup.
  • Am I handling LocationThingAssociation.effective_start correctly in add_thing?
  • @ksmuczynski, @chasetmartin and I determined that inventorying a well is a field activity, so a FieldActivity record 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)

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.
@chatgpt-codex-connector

Copy link
Copy Markdown

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 jirhiker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. Only issue is the port assignment

Comment thread db/engine.py Outdated
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")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

54321 is only for testing and should not be default

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 jirhiker merged commit 872ae4b into well-inventory-csv Dec 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants