Skip to content
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

[Issue #1493] Update search schema to disallow empty filters #1534

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

chouinar
Copy link
Collaborator

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:

{
  "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:

{
  "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

{
  "filters": {
    "agency": {
      "one_of": []
    }
  },

  "pagination": {
    "order_by": "opportunity_id",
    "page_offset": 1,
    "page_size": 25,
    "sort_direction": "descending"
  }
}

Which returns:

{
  "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

{
  "filters": {
    "agency": {
    }
  },

  "pagination": {
    "order_by": "opportunity_id",
    "page_offset": 1,
    "page_size": 25,
    "sort_direction": "descending"
  }
}

Which returns:

{
  "data": {},
  "errors": [
    {
      "field": "filters.agency",
      "message": "At least one filter rule must be provided.",
      "type": "invalid"
    }
  ],
  "message": "Validation error",
  "status_code": 422
}

Copy link
Collaborator

@coilysiren coilysiren left a comment

Choose a reason for hiding this comment

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

Good thinking 👍🏼 and thanks for the PR description examples, I was confused until I saw them.

@chouinar chouinar merged commit 56f01bd into main Mar 26, 2024
@chouinar chouinar deleted the chouinar/1493-search-schema-adds branch March 26, 2024 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Modify search filter schema to require at least one filter be set for a given field
3 participants