Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7k
Let SearchFilter subclasses dynamically set search fields#6279
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
carltongibson left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There's the obvious "docs & tests" but this seems like a nice/small enough adjustment.
allanbreyes commentedOct 25, 2018
👍 Happy to add any unit tests to ensure forward compatibility for consumers who might subclass it. |
allanbreyes commentedOct 26, 2018
I added a test that ensures that subclasses can properly inherit from both |
Uh oh!
There was an error while loading.Please reload this page.
allanbreyes commentedNov 21, 2018
@carltongibson@rpkilby any more thoughts on this? Thank you! |
rpkilby left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A couple of minor nitpicks, but this otherwise looks good to me 👍
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
allanbreyes commentedDec 1, 2018
Apologies for the delay; been a bit busy at work. Thanks for reviewing,@rpkilby! |
rpkilby commentedDec 3, 2018
Thanks! |
carltongibson left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hi@allanbreyes.
Super. Thank you. Welcome aboard! 🎁
| To dynamically change search fields based on request content, it's possible to subclass the`SearchFilter` and override the`get_search_fields()` function. For example, the following subclass will only search on`title` if the query parameter`title_only` is in the request: | ||
| class CustomSearchFilter(self, view, request): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Did you mean:
class CustomSearchFilter(SearchFilter): def get_search_fields(self, view, request): ...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks, someone else noticed this in#6487.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for catching! Apologies for the mistake :(
| search_fields= ('$title','$text') | ||
| view=SearchListView.as_view() | ||
| request=factory.get('/', {'search':'^\w{3}$'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Use raw strings to avoid Python warnings.
Uh oh!
There was an error while loading.Please reload this page.
Description
This pull request refactors
#filter_querysetand extracts a#get_search_fieldsmethod, passing in the request in as a second argument (currently unused). This allows subclasses to override this method and dynamically set search fields (vs. overriding the entire#filter_querysetmethod). A few example use cases:Suppose you have 5 search fields, with
emailbeing one of them. If you run the search terms through a regex and recognize an email, you can dynamically set the search fields only toemail. (This avoids wasteful searches on the other 4 fields.)Suppose the default search fields are
("^book_title",). In combination with overriding the#get_search_termsmethod, a subclass might implement a light lexer/parser to recognize when a search term is encased in quotes, e.g."Green Eggs and Ham"to translate to anexact search of that string onbook_title. This is how Google searches (roughly) work. Otherwise, the existing behavior would do a prefix search on['"Green", 'Eggs', 'and', 'Ham"'].Handling more complex search configurations, e.g.Google advanced search.
Thanks! ❤️ DRF!