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

Jump to bottom

Closed

abdulhaq-e wants to merge 1 commit into django-json-api:develop from abdulhaq-e:filter_feature

Closed

Add filter feature per JSON API specs. #286

abdulhaq-e wants to merge 1 commit into django-json-api:develop from abdulhaq-e:filter_feature

Conversation

abdulhaq-e

Copy link

Contributor

@abdulhaq-e abdulhaq-e commented Oct 7, 2016

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

@abdulhaq-e abdulhaq-e force-pushed the filter_feature branch 2 times, most recently from 54cabcd to 9ce5035 Compare

October 7, 2016 21:22

@codecov-io

Copy link

codecov-io commented Oct 7, 2016 •

edited

Loading

Current coverage is 91.39% (diff: 94.18%)

Merging #286 into develop will increase coverage by 0.24%

@@ 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

Powered by Codecov. Last update 1623942...a51f8e2

@abdulhaq-e abdulhaq-e force-pushed the filter_feature branch from 9ce5035 to a51f8e2 Compare

October 7, 2016 21:49

khornberg

khornberg reviewed

Oct 21, 2016

View reviewed changes

Copy link

@khornberg khornberg left a 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)

Copy link

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

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

remove print

Copy link

Contributor Author

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

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

Done.

rest_framework_json_api/utils.py

return resource_name

# the name was calculated automatically from the view > pluralize and format

# the name was calculated automatically from the view > pluralize

Copy link

@khornberg khornberg Oct 21, 2016

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.

Copy link

Contributor Author

@abdulhaq-e abdulhaq-e Oct 21, 2016

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.

example/filters.py

from example.models import Comment

class CommentFilter(django_filters.FilterSet):

Copy link

@khornberg khornberg Oct 21, 2016

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

Copy link

Contributor Author

@abdulhaq-e abdulhaq-e Oct 21, 2016

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.

@abdulhaq-e


 Add filter feature per JSON API specs.

d8f848f

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

@abdulhaq-e abdulhaq-e force-pushed the filter_feature branch from a51f8e2 to d8f848f Compare

October 21, 2016 13:57

@abdulhaq-e

Copy link

Contributor Author

abdulhaq-e commented Oct 21, 2016

Does this allow filtering on multiple fields at the same time?

Yes, it is permitted to send a request such as:

GET http://localhost/?filter[author]=1&filter[blog]=10

Does this allow filtering on multiple values of the same field?
Does this allow filtering of array, hstore and other complex/DB specific fields?

The JSON API specs does not get into any of this, it depends on the user implementation. However, this is all possible using django_filter.

@abdulhaq-e abdulhaq-e closed this

Feb 3, 2017

@abdulhaq-e abdulhaq-e deleted the filter_feature branch

February 3, 2017 06:58

Labels

None yet

3 participants

@abdulhaq-e @codecov-io @khornberg

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add filter feature per JSON API specs. #286

Add filter feature per JSON API specs. #286

Uh oh!

Conversation

@abdulhaq-e abdulhaq-e commented Oct 7, 2016

Uh oh!

codecov-io commented Oct 7, 2016 •

edited

Loading

Uh oh!

Current coverage is 91.39% (diff: 94.18%)

Uh oh!

@khornberg khornberg left a comment

Choose a reason for hiding this comment

Uh oh!

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

abdulhaq-e commented Oct 21, 2016

Uh oh!

Uh oh!

Add filter feature per JSON API specs. #286

Add filter feature per JSON API specs. #286

Uh oh!

Conversation

@abdulhaq-e abdulhaq-e commented Oct 7, 2016

Uh oh!

codecov-io commented Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 91.39% (diff: 94.18%)

Uh oh!

@khornberg khornberg left a comment

Choose a reason for hiding this comment

Uh oh!

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@khornberg khornberg Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

@abdulhaq-e abdulhaq-e Oct 21, 2016

Choose a reason for hiding this comment

Uh oh!

abdulhaq-e commented Oct 21, 2016

Uh oh!

Uh oh!

codecov-io commented Oct 7, 2016 •

edited

Loading