Conversation
Regarding query behaviour via
Tricky. The original search interface was designed to be 'pretty' (e.g. URLs like
Let's not change the API parameter name if possible. Public bodies can have multiple categories and it should be possible to filter by multiple categories, so the plural makes sense.
I think this is mainly because exposing a user interface for these search parameters does not serve a common use cases for web browser users, but for API users it's helpful (I built the feature because I needed it). If we can have the same search/filter machinery for API and UI while keeping the UI simple and the API stable, let's do it. |
4bccf5b to
b3ee1d9
Compare
b3ee1d9 to
2919cec
Compare
|
Following up on Stefan's comment, I aimed to unify the implementation as much as possible while still keeping the parts separate that should stay separate. The main change to the API search is that it now uses the I also removed the old guard that ignored queries shorter than three characters in the API search. If this turns out to cause performance issues, we can always add it back. |
| def is_valid(self): | ||
| # Trigger form validation to populate cleaned_data and errors, | ||
| # but always return True to prevent DjangoFilterBackend from | ||
| # raising a 400 response for invalid filter values. |
There was a problem hiding this comment.
Currently, filters like ?jurisdiction=123456 do result in a 400 error, and I think they should. Does this change that behavior?
There was a problem hiding this comment.
You're right when it comes to the publicbody endpoint. However, the publicbody/search endpoint currently returns an empty list when queried with invalid filter values, and I wanted to preserve that behavior.
I agree that returning a 400 error would be better, and it would also make it consistent with what e.g. the request/search endpoint does.
@stefanw Do you see any reason not to change this?
Search is currently handled differently in regular views and API views, which leads to inconsistent search results depending on whether a search is performed via the search form or through the API.
This is especially the case for
PublicBody(see https://github.com/fragdenstaat/issues/issues/293). In the API view,get_searchquerysetis overridden with a custom search and filtering configuration. As a result, recent changes to the search analyzers are not applied there. UsingPublicBodyFilterSetinstead, as in the view, could resolve this.However, I noticed several differences between
PublicBodysearch in views and in the API (which is also why the tests I added are currently failing):multi_matchsearch, while the rest of the application appears to usesimple_query_string.PublicBodyFilterSetuses slugs (which seems to be consistent with other API endpoints, e.g. forFoiRequest).categoriesin the API butcategoryinPublicBodyFilterSet.regionsandregions_kindexist in the API, but not inPublicBodyFilterSet.I’m unsure how to resolve these differences without breaking the API, and which parts are still intentional vs. legacy behaviour.
@stefanw Could you take a look?