Skip to content

Get queryset #528

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

Merged
merged 6 commits into from
Mar 31, 2019
Merged

Get queryset #528

merged 6 commits into from
Mar 31, 2019

Conversation

zbyte64
Copy link
Collaborator

@zbyte64 zbyte64 commented Oct 5, 2018

My attempt at adding support for DjangoObjectType.get_queryset as described as Option 1 for #79 ; Tried to match up the method call as much as possible but dropped request and args as I wanted get_node to also be able to call get_queryset .

@coveralls
Copy link

coveralls commented Oct 5, 2018

Coverage Status

Coverage increased (+0.03%) to 94.547% when pulling ad2688f on zbyte64:get_queryset into f76f38e on graphql-python:master.

@zbyte64
Copy link
Collaborator Author

zbyte64 commented Oct 18, 2018

After reviewing graphql-python/graphene#846 I think this would be better served by a permission class. In the future a DjangoConnectionField could consult it's permissions for more specific permission actions like filtering objects on a connection.

@maarcingebala
Copy link

This is something that I wanted to implement in my project and then I found this PR. There is a lot of cases when I'd like to be able to limit the queryset used underneath when resolving nodes so I think it's a must-have for graphene-django.

I took a look at the permissions PR you linked above and I think the solution you proposed is fine for now. Overriding a function that returns the queryset is a common pattern used in class-based views or DRF; it's also simple to use and explicit.

Copy link
Member

@jkimbo jkimbo left a comment

Choose a reason for hiding this comment

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

This seems like a good idea!

@jkimbo jkimbo merged commit 0a5020b into graphql-python:master Mar 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants