Open pors opened 4 years ago
@pors that is expected since graphene.List
is part of the graphene library and so has no Django specific code. You can use DjangoListField
instead to which does take into account the get_queryset
method:
from graphene import ObjectType
from graphene_django.fields import DjangoListField
class Query(ObjectType):
all_currencies = DjangoListLField(CurrencyType)
I have just realised that unfortunately DjangoListField
is not currently exported from the top level graphene_django
module (which I'll fix) and it's not currently documented either (🤦♂️) so it's completely understandable that you didn't find it! I'll try and fix both of these issues as soon as possible.
Also just a heads up: the DjangoListField
only calls the get_queryset
method on the DjangoObjectType if the resolver for that field doesn't return anything. See: https://github.com/graphql-python/graphene-django/blob/3483428f70efc619b7f138a709486c91ba67abe8/graphene_django/fields.py#L42-L51)
So if you define a resolver on the parent object (like you have in your example) the get_queryset
method will not be called. I can see that might be a bit unexpected and I'm open to suggestions on how the API could be improved to allow resolvers to modify querysets rather than just override them. Any suggestions?
Ok @pors I've implemented a fix for your issue here: graphql-python/graphene#929 (and added some documentation). Could you take a look at the tests and make sure that they cover your usecase?
Awesome @jkimbo! I have it working now for the example I provided (I don't need a resolver on the query level here). I think it actually makes sense as it is called the "Default QuerySet".
Something that still doesn't work as I hoped is the following: When Currency
from my example is a related field in another table, let's say Fund
, a query for funds does not call get_queryset
in the CurrencyType
class.
If that would be possible it provides an easy way to protect from returning too much data for specific tables (this is especially important for the User table being a related field).
Ah we were commenting at the same time :) I'l have a look at the tests!
Something that still doesn't work as I hoped is the following: When Currency from my example is a related field in another table, let's say Fund, a query for funds does not call get_queryset in the CurrencyType class.
This issue should actually be fixed as part of my PR. I'll add a test to confirm.
@jkimbo great!!!!
@jkimbo thanks for releasing this, but it still doesn't cover my case.
I had a look at the test_get_queryset_foreign_key test, but I was hoping for the reverse, actually querying for articles
instead of reporters
and calling get_queryset
in the ReporterModel
. If some reporter would be excluded in that call, the articles of that reporter should not be returned.
I'm not sure if this is actually possible, but it would be a very useful feature IMHO. It provides a very easy way (located at a single spot, inside the ObjectType definition) to avoid leaking out data in a complex query.
A test for this would look like:
def test_get_queryset_filter(self):
class Reporter(DjangoObjectType):
class Meta:
model = ReporterModel
fields = ("first_name", "email")
@classmethod
def get_queryset(cls, queryset, info):
# Only get reporters with an email address
return queryset.filter(email__isnull=False)
class Query(ObjectType):
reporters = DjangoListField(Reporter)
schema = Schema(query=Query)
query = """
query {
articles {
headline
}
}
"""
r1 = ReporterModel.objects.create(first_name="Tara", email="tara@cnn.com")
r2 = ReporterModel.objects.create(first_name="Debra")
ArticleModel.objects.create(
headline="Amazing news",
reporter=r1,
pub_date=datetime.date.today(),
pub_date_time=datetime.datetime.now(),
editor=r1,
)
ArticleModel.objects.create(
headline="More news",
reporter=r2,
pub_date=datetime.date.today(),
pub_date_time=datetime.datetime.now(),
editor=r2,
)
result = schema.execute(query)
assert not result.errors
assert result.data == {"articles": [{"headline": "Amazing News"},]}
@pors I don't think your example makes sense since you're not interacting with the reporter model at all in that query. I would not expect the get_queryset
method on the ReporterType to be called if I'm just querying articles.
@jkimbo yeah you are right, what if we would do:
query = """
query {
articles {
headline
reporter {
first_name
}
}
}
"""
It still won't call get_queryset
in the ReporterModel
.
@jkimbo please let me know if my requirement makes no sense in this context, then I'll close the issue.
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Why not define get_queryset
at the articles level and have it do: queryset.filter(reporter__email__isnull=False)
?
Why not define
get_queryset
at the articles level and have it do:queryset.filter(reporter__email__isnull=False)
?
It is much cleaner to keep restrictions (filters) connected to the actual model. In my code, I use get_queryset
in the User model, which is used in many queries. I would then have to repeat the same filter in each model instead of just the User model.
Here is another issue that explains the case: https://github.com/graphql-python/graphene-django/issues/1526
The documentation describes the use of a default QuerySet on ObjectType level.
The unit test for this functionality makes use of DjangoConnectionField.
Using
graphene.List
instead ignores the existence of the default QuerySet.The
get_queryset
method is never called, and the filter is not applied.Please refer to graphql-python/graphene#698 for some history on this.