-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Task]: Modify search filter schema to require at least one filter be set for a given field #1493
Labels
project: grants.gov
Grants.gov Modernization tickets
Comments
chouinar
added a commit
that referenced
this issue
Mar 25, 2024
chouinar
added a commit
that referenced
this issue
Mar 26, 2024
## Summary Fixes #1493 ### Time to review: __5 mins__ ## Changes proposed Updated search schema logic to disallow: 1. Empty `one_of` lists 2. Empty filter lists ## Context for reviewers These changes remove a few edge cases that have undefined / difficult to define behavior if we allow them (ie. what do we do if a list is empty, `opportunity_status in [ ]`). Rather than try to sort out what the behavior might mean / modify a users search request and interpret what we think they probably mean, we don't allow that type of request and chalk it up to user error in constructing a request. Note that this does not modify the behavior of just not passing filters, that's still perfectly fine, we just want to avoid being passed filters that are half-built. See the additional section below for what is / isn't allowed. ## Additional information An example request looks like: ```json { "filters": { "agency": { "one_of": [ "US-ABC", "HHS" ] }, "applicant_type": { "one_of": [ "state_governments", "county_governments", "individuals" ] }, "funding_category": { "one_of": [ "recovery_act", "arts", "natural_resources" ] }, "funding_instrument": { "one_of": [ "cooperative_agreement", "grant" ] }, "opportunity_status": { "one_of": [ "forecasted", "posted" ] } }, "pagination": { "order_by": "opportunity_id", "page_offset": 1, "page_size": 25, "sort_direction": "descending" }, "query": "research" } ``` Technically only the pagination is required, and so you can also pass us: ```json { "pagination": { "order_by": "opportunity_id", "page_offset": 1, "page_size": 25, "sort_direction": "descending" } } ``` ### Error cases However, the following are now no longer allowed. #### An empty list logically this wouldn't ever return results as the intersection of an empty set always is nothing ```json { "filters": { "agency": { "one_of": [] } }, "pagination": { "order_by": "opportunity_id", "page_offset": 1, "page_size": 25, "sort_direction": "descending" } } ``` Which returns: ```json { "data": {}, "errors": [ { "field": "filters.agency.one_of", "message": "Shorter than minimum length 1.", "type": "min_length" } ], "message": "Validation error", "status_code": 422 } ``` #### An empty set of filter rules for a field this technically doesn't cause issues, but lets us be more flexible in our implementation ```json { "filters": { "agency": { } }, "pagination": { "order_by": "opportunity_id", "page_offset": 1, "page_size": 25, "sort_direction": "descending" } } ``` Which returns: ```json { "data": {}, "errors": [ { "field": "filters.agency", "message": "At least one filter rule must be provided.", "type": "invalid" } ], "message": "Validation error", "status_code": 422 } ``` --------- Co-authored-by: nava-platform-bot <platform-admins@navapbc.com>
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
Right now it would be valid to pass us a filter like:
This doesn't cause any issues necessarily, but is a bit confusing regarding the behavior that would happen, so we should make the filter objects have some sort of validation that says not to do this.
Acceptance criteria
The text was updated successfully, but these errors were encountered: