From 871e7b2a520cf60640252e5ca15ec6903836d02e Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Mon, 21 Oct 2019 12:21:30 -0700 Subject: [PATCH 1/4] django-filter: resolve field along with lookup expression to properly resolve field --- graphene_django/filter/tests/test_fields.py | 13 ++--- graphene_django/filter/utils.py | 53 ++++++++++++++------- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 1ffa0f452..462fcf80e 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -877,7 +877,7 @@ def get_filters(cls): filters = super(FilterSet, cls).get_filters() filters.update( { - "viewer__email__in": django_filters.CharFilter( + "reporter__email__in": django_filters.CharFilter( method="filter_email_in", field_name="reporter__email__in" ) } @@ -897,16 +897,11 @@ class Meta: interfaces = (Node,) class NewArticleFilterNode(DjangoObjectType): - viewer = Field(NewReporterNode) - class Meta: model = Article interfaces = (Node,) filterset_class = NewArticleFilter - def resolve_viewer(self, info): - return self.reporter - class Query(ObjectType): all_articles = DjangoFilterConnectionField(NewArticleFilterNode) @@ -939,11 +934,11 @@ class Query(ObjectType): query = ( """ query NodeFilteringQuery { - allArticles(viewer_Email_In: "%s") { + allArticles(reporter_Email_In: "%s") { edges { node { headline - viewer { + reporter { email } } @@ -960,7 +955,7 @@ class Query(ObjectType): { "node": { "headline": article_1.headline, - "viewer": {"email": reporter_1.email}, + "reporter": {"email": reporter_1.email}, } } ] diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index abb03a998..2ec6b40ed 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -1,8 +1,36 @@ import six +from django.db.models.constants import LOOKUP_SEP +from django.core.exceptions import FieldDoesNotExist +from django.db.models.fields.related import ForeignObjectRel, RelatedField from .filterset import custom_filterset_factory, setup_filterset +def get_field_parts_with_expression(model, query_expr): + """ + Traverses the model with a given query expression, + returns the found fields along the path and the remaining expression + """ + parts = query_expr.split(LOOKUP_SEP) + opts = model._meta + fields = [] + + # walk relationships + for i, name in enumerate(parts): + try: + field = opts.get_field(name) + except FieldDoesNotExist: + return fields, LOOKUP_SEP.join(parts[i:]) + + fields.append(field) + if isinstance(field, RelatedField): + opts = field.remote_field.model._meta + elif isinstance(field, ForeignObjectRel): + opts = field.related_model._meta + + return fields, "exact" + + def get_filtering_args_from_filterset(filterset_class, type): """ Inspect a FilterSet and produce the arguments to pass to a Graphene Field. These arguments will be available to @@ -18,22 +46,15 @@ def get_filtering_args_from_filterset(filterset_class, type): if name in filterset_class.declared_filters: form_field = filter_field.field else: - try: - field_name, filter_type = name.rsplit("__", 1) - except ValueError: - field_name = name - filter_type = None - - # If the filter type is `isnull` then use the filter provided by - # DjangoFilter (a BooleanFilter). - # Otherwise try and get a filter based on the actual model field - if filter_type != "isnull" and hasattr(model, field_name): - model_field = model._meta.get_field(field_name) - - if hasattr(model_field, "formfield"): - form_field = model_field.formfield( - required=filter_field.extra.get("required", False) - ) + fields, lookup_expr = get_field_parts_with_expression(model, name) + assert fields, str((model, name, filterset_class)) + model_field = fields[-1] + filter_type = lookup_expr + + if filter_type != "isnull" and hasattr(model_field, "formfield"): + form_field = model_field.formfield( + required=filter_field.extra.get("required", False) + ) # Fallback to field defined on filter if we can't get it from the # model field From 15b661ae262682f722e5efc5937adea3e1c736a6 Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Mon, 21 Oct 2019 12:55:44 -0700 Subject: [PATCH 2/4] bring back django-filter with method test --- graphene_django/filter/tests/test_fields.py | 13 +++++++++---- graphene_django/filter/utils.py | 14 ++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 462fcf80e..1ffa0f452 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -877,7 +877,7 @@ def get_filters(cls): filters = super(FilterSet, cls).get_filters() filters.update( { - "reporter__email__in": django_filters.CharFilter( + "viewer__email__in": django_filters.CharFilter( method="filter_email_in", field_name="reporter__email__in" ) } @@ -897,11 +897,16 @@ class Meta: interfaces = (Node,) class NewArticleFilterNode(DjangoObjectType): + viewer = Field(NewReporterNode) + class Meta: model = Article interfaces = (Node,) filterset_class = NewArticleFilter + def resolve_viewer(self, info): + return self.reporter + class Query(ObjectType): all_articles = DjangoFilterConnectionField(NewArticleFilterNode) @@ -934,11 +939,11 @@ class Query(ObjectType): query = ( """ query NodeFilteringQuery { - allArticles(reporter_Email_In: "%s") { + allArticles(viewer_Email_In: "%s") { edges { node { headline - reporter { + viewer { email } } @@ -955,7 +960,7 @@ class Query(ObjectType): { "node": { "headline": article_1.headline, - "reporter": {"email": reporter_1.email}, + "viewer": {"email": reporter_1.email}, } } ] diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index 2ec6b40ed..411a3d0ae 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -6,12 +6,13 @@ from .filterset import custom_filterset_factory, setup_filterset -def get_field_parts_with_expression(model, query_expr): +def get_field_parts_with_expression(model, field_name, lookup_expr): """ Traverses the model with a given query expression, returns the found fields along the path and the remaining expression """ - parts = query_expr.split(LOOKUP_SEP) + parts = field_name.split(LOOKUP_SEP) # + lookup_expr.split(LOOKUP_SEP) + lparts = lookup_expr.split(LOOKUP_SEP) opts = model._meta fields = [] @@ -20,7 +21,7 @@ def get_field_parts_with_expression(model, query_expr): try: field = opts.get_field(name) except FieldDoesNotExist: - return fields, LOOKUP_SEP.join(parts[i:]) + return fields, LOOKUP_SEP.join(parts[i:] + lparts) fields.append(field) if isinstance(field, RelatedField): @@ -28,7 +29,7 @@ def get_field_parts_with_expression(model, query_expr): elif isinstance(field, ForeignObjectRel): opts = field.related_model._meta - return fields, "exact" + return fields, lookup_expr def get_filtering_args_from_filterset(filterset_class, type): @@ -46,8 +47,9 @@ def get_filtering_args_from_filterset(filterset_class, type): if name in filterset_class.declared_filters: form_field = filter_field.field else: - fields, lookup_expr = get_field_parts_with_expression(model, name) - assert fields, str((model, name, filterset_class)) + fields, lookup_expr = get_field_parts_with_expression( + model, filter_field.field_name, filter_field.lookup_expr + ) model_field = fields[-1] filter_type = lookup_expr From cae2ba88864f22d6a142c924b00a0c41fb2f230a Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Mon, 21 Oct 2019 12:57:52 -0700 Subject: [PATCH 3/4] remove dangling comment --- graphene_django/filter/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index 411a3d0ae..19fd46b1a 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -11,7 +11,7 @@ def get_field_parts_with_expression(model, field_name, lookup_expr): Traverses the model with a given query expression, returns the found fields along the path and the remaining expression """ - parts = field_name.split(LOOKUP_SEP) # + lookup_expr.split(LOOKUP_SEP) + parts = field_name.split(LOOKUP_SEP) lparts = lookup_expr.split(LOOKUP_SEP) opts = model._meta fields = [] From 98d717e30ea13b6ebf07f3afe4a1652ef1f71ac9 Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Mon, 21 Oct 2019 14:50:42 -0700 Subject: [PATCH 4/4] refactor based on better knowledge of django-filters --- graphene_django/filter/utils.py | 38 +++------------------------------ 1 file changed, 3 insertions(+), 35 deletions(-) diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index 19fd46b1a..c5f18e29c 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -1,37 +1,9 @@ import six -from django.db.models.constants import LOOKUP_SEP -from django.core.exceptions import FieldDoesNotExist -from django.db.models.fields.related import ForeignObjectRel, RelatedField +from django_filters.utils import get_model_field from .filterset import custom_filterset_factory, setup_filterset -def get_field_parts_with_expression(model, field_name, lookup_expr): - """ - Traverses the model with a given query expression, - returns the found fields along the path and the remaining expression - """ - parts = field_name.split(LOOKUP_SEP) - lparts = lookup_expr.split(LOOKUP_SEP) - opts = model._meta - fields = [] - - # walk relationships - for i, name in enumerate(parts): - try: - field = opts.get_field(name) - except FieldDoesNotExist: - return fields, LOOKUP_SEP.join(parts[i:] + lparts) - - fields.append(field) - if isinstance(field, RelatedField): - opts = field.remote_field.model._meta - elif isinstance(field, ForeignObjectRel): - opts = field.related_model._meta - - return fields, lookup_expr - - def get_filtering_args_from_filterset(filterset_class, type): """ Inspect a FilterSet and produce the arguments to pass to a Graphene Field. These arguments will be available to @@ -47,12 +19,8 @@ def get_filtering_args_from_filterset(filterset_class, type): if name in filterset_class.declared_filters: form_field = filter_field.field else: - fields, lookup_expr = get_field_parts_with_expression( - model, filter_field.field_name, filter_field.lookup_expr - ) - model_field = fields[-1] - filter_type = lookup_expr - + model_field = get_model_field(model, filter_field.field_name) + filter_type = filter_field.lookup_expr if filter_type != "isnull" and hasattr(model_field, "formfield"): form_field = model_field.formfield( required=filter_field.extra.get("required", False)