Skip to content

Added related fields filtering #504

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

Closed
wants to merge 1 commit into from

Conversation

Anton-Shutik
Copy link
Contributor

Fixes #
This simple PR adds fitlering for related querysets. Let's say we have an url like /api/order/1/items/ and I want for filter the items by available=true. There is no way to do it now.
With this PR we can do this by adding filter backend to the OrderviewSet:

class OrderViewSet(ModelViewset):
    related_filter_backends = {'items': [IsAvailableFilter]}

That's it.
Filtering work exactly same way as standard DRF's filtering.

Description of the Change

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • changelog entry added to CHANGELOG.md
  • author name in AUTHORS

@n2ygk
Copy link
Contributor

n2ygk commented Oct 25, 2018 via email

@Anton-Shutik
Copy link
Contributor Author

@n2ygk I think something like that will help better . Didn't tested, just an idea

@Anton-Shutik
Copy link
Contributor Author

Anton-Shutik commented Oct 25, 2018

For this PR it just allows to filter related querysets.
What do you think about it ? If it looks good for you I can add some tests and update the docs.
Thanks

@n2ygk
Copy link
Contributor

n2ygk commented Oct 25, 2018

@Anton-Shutik Actually this might be better than returning an error as you do in #505 if the goal is to reduce the related queryset to those allowed to be returned. Basically the related_filter_backends is a permission filter that only returns items that are permitted to be returned.

The goal I think is to silently not return related data when not permitted (make it invisible) rather than have the request fail. Does that make sense @lcary?

@Anton-Shutik
Copy link
Contributor Author

@n2ygk related_filter_backends is just for filtering. It should not check any permissions or something. It is made same way like DRF does. Filters and permissions are separate things. Let's do not mix it. If you want to hide some items from current customer - add a IsNotHiddenItemFilter and it will just filter out all items that the customer shouldn't see. It you want to check permissions for a given endpoint (or resource) add a IsStuffUser(for example) and that will return response only for stuff users. That's it.

Permissions check is a permission check. It should not do any filtering at all. And filtering is just filtering it should not check anything. That's why I've made 2 separate PRs for that.

Copy link
Member

@sliverc sliverc left a comment

Choose a reason for hiding this comment

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

@sliverc
Copy link
Member

sliverc commented Dec 14, 2018

Closing this for now as the discussion in #496 continues how the issue can be solved.

@sliverc sliverc closed this Dec 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants