Skip to content

Conversation

@chasetmartin
Copy link
Contributor

Why

This PR addresses the following problem / context:

  • As a hydrogeologist or data specialist
    I want to upload a CSV file containing well inventory data for multiple wells
    So that many wells can be imported quickly and accurately into the system
  • For discussion in this PR so the feature can be adjusted/broken apart,/expanded upon for future work

How

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

Notes

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

@jirhiker
Copy link
Member

lgtm

@jirhiker
Copy link
Member

@chasetmartin sorry i meant to open a pull request but I push directly to your branch.

These edits are a combination of first running the spec through the Feature File GPT and second, making manual updates to clean up.

@chasetmartin
Copy link
Contributor Author

@jirhiker No worries thanks for the improvements, I'll check them out more Monday morning.

Copy link
Contributor

@jacob-a-brown jacob-a-brown left a comment

Choose a reason for hiding this comment

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

Are 422 status code responses supposed to in the Pydantic style?

Comment on lines 45 to 48
| contact_name |
| contact_organization |
| contact_role |
| contact_type |
Copy link
Contributor

Choose a reason for hiding this comment

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

the user can submit 2 contacts. should this be changed to ..._1 and then replicated for the second contact?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intentionally kept this feature to a single contact to start for simplicity. However, a user Slacked me that "a few" backlogged wells have 2 contacts. So @jirhiker do you think we should add replica fields for a 2nd contact via csv? It will make the csv very wide, but it will also reduce the need for a user to keep track of situations where they have to go back and add a contact after import.

@chasetmartin
Copy link
Contributor Author

@jirhiker Let me know if the changes I just made make sense. I added the communication preference and notes fields Sianin requested and duplicated the contact fields so there is a contact_1 and a contact_2.

@chasetmartin
Copy link
Contributor Author

Added one additional field, sample_possible to record whether it's possible to sample the well. This question is hidden on the 2nd page of the inventory form.

@chasetmartin
Copy link
Contributor Author

Are 422 status code responses supposed to in the Pydantic style?

@jacob-a-brown Sorry just saw this. I think that makes sense for consistency throughout the app. @jirhiker Let me know if you think the feature file should be updated to be specific about this, or if that's just an implementation detail for these 422 responses?

@jirhiker
Copy link
Member

@chasetmartin

@negative @validation @BDMS-??
  Scenario: Upload fails when a row has contact without a contact_role
    Given my CSV file contains a row with a contact but is missing the required "contact_role" field for that contact
    When I upload the file to the bulk upload endpoint
    Then the system returns a 422 Unprocessable Entity status code
    And the system should return a response in JSON format
    And the response includes a validation error indicating the missing "contact_role" field
    And no wells are imported

@jirhiker
Copy link
Member

@chasetmartin

@negative @validation @BDMS-??
  Scenario: Upload fails when a row has an invalid postal code format
    Given my CSV file contains a row  that has an invalid postal code format in contact_1_address_1_postal_code
    When I upload the file to the bulk upload endpoint
    Then the system returns a 422 Unprocessable Entity status code
    And the system should return a response in JSON format
    And the response includes a validation error indicating the invalid postal code format
    And no wells are imported

@jirhiker
Copy link
Member

@chasetmartin There are more @Negative scenarios to define. e.g. invalid state.

@chasetmartin
Copy link
Contributor Author

@jirhiker I added more negative scenarios, let me know of others you encounter as you work. Do you think we should have a negative validation scenario for the binary true/false fields?

Also so this is linked to this feature, here is the Google Sheet template for the csv creation:
https://docs.google.com/spreadsheets/d/1EbENM-WzT7D1YLpqn2i0TgIgK6zfl3C07Bu14GIS41c/edit?usp=sharing

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.

5 participants