-
Notifications
You must be signed in to change notification settings - Fork 298
Add filter feature per JSON API specs. #286
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
54cabcd
to
9ce5035
Compare
Current coverage is 91.39% (diff: 94.18%)@@ develop #286 diff @@
==========================================
Files 49 52 +3
Lines 2295 2372 +77
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 2092 2168 +76
- Misses 203 204 +1
Partials 0 0
|
9ce5035
to
a51f8e2
Compare
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.
Does this allow filtering on multiple fields at the same time?
Does this allow filtering on multiple values of the same field?
Does this allow filtering of array, hstore and other complex/DB specific fields?
rest_framework_json_api/filters.py
Outdated
def filter_queryset(self, request, queryset, view): | ||
|
||
filter_class = self.get_filter_class(view, queryset) | ||
print(request.query_params) |
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.
remove print
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.
Done.
return resource_name | ||
|
||
# the name was calculated automatically from the view > pluralize and format | ||
# the name was calculated automatically from the view > pluralize |
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.
Changing all these things is not helpful, even if your dev software did it for you. It only adds noise to the PR.
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.
I agree. However, this project in general needs linting clean up. If the owners agree, we'll agree on a format and clean up the code. I'll also delete these linting changes if required.
from example.models import Comment | ||
|
||
|
||
class CommentFilter(django_filters.FilterSet): |
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 does not seem to be an example of using DRF filters
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.
Can you provide a link that shows an example?
DRF filtering depends on django_filter
and similar examples are found in DRF docs.
Added filter feature. This uses DjangoFilterBackend which either comes from `rest-framework-filter` or `django-filter`. The former is a package which adds more features to the latter. All that is done is reformatting the request query parameters to be compatible with DjangoFilterBackend. The spec recommend the pattern `filter[field]=values`. JsonApiFilterBackend takes that pattern and reformats it to become `field=value` which will then work as typical filtering in DRF. The specs only rule towards filtering is that the `filter` keyword should be reserved. It doesn't however say about the exact format. It only **recommends** `filter[field]=value` but this can also be `filter{field}=value` or `filter(field)=value`. Therefore, a new setting for DJA is introduced: JSON_API_FILTER_KEYWORD. This is a regex which controls the filter format. Its default value is: ``` JSON_API_FILTER_KEYWORD = 'filter\[(?P<field>\w+)\]' ``` It can be changed to anything as long as the `field` keyword is included in the regex. The docs have been updated. Unfortunately, the editor's beautifier was run and made lots of other changes to the code style. No harm was done though, that's why TDD exists :D
a51f8e2
to
d8f848f
Compare
Yes, it is permitted to send a request such as:
The JSON API specs does not get into any of this, it depends on the user implementation. However, this is all possible using |
Added filter feature. This uses DjangoFilterBackend which either
comes from
rest-framework-filter
ordjango-filter
. The formeris a package which adds more features to the latter.
All that is done is reformatting the request query parameters to be
compatible with DjangoFilterBackend. The spec recommend the pattern
filter[field]=values
. JsonApiFilterBackend takes that pattern andreformats it to become
field=value
which will then work as typicalfiltering in DRF.
The specs only rule towards filtering is that the
filter
keywordshould be reserved. It doesn't however say about the exact format.
It only recommends
filter[field]=value
but this can also befilter{field}=value
orfilter(field)=value
. Therefore, a newsetting for DJA is introduced: JSON_API_FILTER_KEYWORD. This is a
regex which controls the filter format. Its default value is:
It can be changed to anything as long as the
field
keyword isincluded in the regex.
The docs have been updated.
Unfortunately, the editor's beautifier was run and made lots of
other changes to the code style. No harm was done though, that's
why TDD exists :D