feat(tests): relax validation rules and expand enum coverage in well-inventory-csv feature#548
Conversation
…inventory-csv feature - Move site_name, elevation_ft, elevation_method, and measuring_point_height_ft from required to optional - Replace "both contact_name and contact_organization required" rule with "at least one required" rule - Remove all-or-nothing water level rule; water_level_date_time now required only when depth_to_water_ft is provided, all other water level fields are independent and optional - Add negative scenarios for invalid address_type, state abbreviation, well_hole_status, monitoring_status, and well_pump_type with allowed values specified - Add well_notes, well_measuring_notes, water_notes, historical_notes, well_hole_status, and monitoring_status to optional fields
marissafichera
requested review from
Copilot and
ksmuczynski
and removed request for
Copilot
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56f6cbf678
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @negative @validation @BDMS-TBD | ||
| 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 | ||
| Given my CSV file contains a row that has an invalid postal code format in contact_1_address_1_postal_code |
There was a problem hiding this comment.
This edit changes the step text from row that has... to row that has..., but the registered Given step in tests/features/steps/well-inventory-csv-given.py still uses the original double-space phrase, so this scenario no longer matches any step definition. In a behave run, that makes the scenario undefined and prevents the validation assertion from executing.
Useful? React with 👍 / 👎.
|
|
||
| @negative @validation @BDMS-TBD | ||
| Scenario: Upload fails when a row has a contact with an invalid "address_type" | ||
| Given my CSV file contains a row with an address_type value that is not one of: Work, Personal, Mailing, Physical |
There was a problem hiding this comment.
The new enum/contact/water-level scenarios introduced in this section rely on Given/Then phrases that currently have no matching step decorators in the well-inventory step files (tests/features/steps/well-inventory-csv-given.py and tests/features/steps/well-inventory-csv-validation-error.py). Because these steps are undefined, the added scenarios fail at step resolution time and do not provide executable coverage.
Useful? React with 👍 / 👎.
|
@marissafichera Two comments below related to potential future work, but I agree overall with the updates. Approved! Regarding point no. 1 in the HOW section of the PR:
|
jirhiker
left a comment
jirhiker
left a comment
There was a problem hiding this comment.
Looks good. Remove instances of " And the system should return a response in JSON format" and this should be ready to proceed to implementation.
Since this updates the spec without updating the implementation, there are (at least) two options for keeping the tests passing on staging. 1) do not merge into staging. Whoever is working on the implementation should create a new branch from marissa, make the tests pass, then open a PR to staging 2) remove the @production tag from this file so that CI skips this test. once implementation is complete @production tag can be re-added
…inventory-csv feature - removed production tag - removed instances of returning in JSON format - Move site_name, elevation_ft, elevation_method, and measuring_point_height_ft from required to optional - Replace "both contact_name and contact_organization required" rule with "at least one required" rule - Remove all-or-nothing water level rule; water_level_date_time now required only when depth_to_water_ft is provided, all other water level fields are independent and optional - Add negative scenarios for invalid address_type, state abbreviation, well_hole_status, monitoring_status, and well_pump_type with allowed values specified - Add well_notes, well_measuring_notes, water_notes, historical_notes, well_hole_status, and monitoring_status to optional fields
…yle field requirements - Update water-level feature scenarios to mirror well-inventory behavior for requested headers - Keep only base required fields (`field_staff`, `well_name_point_id`, `field_event_date_time`) - Move water-level measurement fields to optional set (`water_level_date_time`, `measuring_person`, `sample_method`, `mp_height`, `level_status`, `depth_to_water_ft`, `data_quality`, `water_level_notes`) - Add timezone-naive ISO 8601 expectation for `field_event_date_time` and `water_level_date_time` - Fix required-field examples and column-order scenario to match new required set - Normalize numeric validation wording to `mp_height` (not `mp_height_ft`)`
@BDMS-608
Why
How
site_name,elevation_ft,elevation_method, andmeasuring_point_height_ftfrom required to optionalwater_level_date_timeis now required only whendepth_to_water_ftis provided, all other water level fields are independent and optionaladdress_type, state abbreviation,well_hole_status,monitoring_status, andwell_pump_typewith allowed values specified in each Given stepwell_notes,well_measuring_notes,water_notes,historical_notes,well_hole_status, andmonitoring_statusto optional fieldsNotes
WellInventoryRow) are required for these rules to take effect in the backend — this PR covers the feature spec only