-
Notifications
You must be signed in to change notification settings - Fork 301
Document how to use rest_framework.filters.SearchFilter
#476
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
Conversation
Includes updates to the example app and tests.
example/tests/test_filters.py
Outdated
""" | ||
test for `filter[search]=keyword1...` (keyword1 [AND keyword2...]) | ||
""" | ||
for keywords in ("research", "chemistry", "nonesuch", |
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.
This test is a bit difficult to comprehend. It would be much easier to write it with pytest parametrize.
I ask myself though do we really need a test for SearchFilter? It is DRF functionality and as such it is already tested in DRF itself and we just do same work again. Hence I think we could remove this test.
@sliverc My reason for the test is because it is testing (demonstrating) the
example app’s usage. The tests after all are in `example/tests`.
w.r.t to pytest parameterize, I'm not sure this would be any more clear. Let me take a crack at better documenting the test, which was written to be somewhat independent of the fixture content, in case someone changes it, but of course still dependent on my given lest of test keywords being present.
If after the next commit you still think it should be removed, we can do that.
|
@n2ygk if this is the case then the test should be simplified where only one keyword is searched for and a simple |
@sliverc Sorry, I didn't notice your followup comment until just now... Please take a look at the test case again before we kill it:-(. I am also happy to add the result.json() assertion with static test values but still think it's valuable to demonstrate the multi-keyword searching across ORM-related fields. |
…fragility of this test. Any future changes to unrelated parts will break it.
OK. Let's leave it this way and merge. Thanks. |
Fixes #432
Description of the Change
Documents how to use
rest_framework.filters.SearchFilter
along with the other filter backends.Includes updates to the example app to demonstrate usage and test cases.
CHANGELOG.md remains unchanged because this is included in: "Add optional jsonapi-style filter backends"
I also documented some
filterset_fields
to complement thesearch_fields
in the usage document showing how to configure filter backends.Checklist
CHANGELOG.md
AUTHORS