From 36ac5626e9e29d3fa2415caf154eb952fe12d9d6 Mon Sep 17 00:00:00 2001 From: Andrew Bettke Date: Wed, 27 Mar 2019 17:09:25 +1300 Subject: [PATCH 1/5] Adds enhanced support for proxy models. --- graphene_django/tests/models.py | 7 +++ graphene_django/tests/test_query.py | 93 ++++++++--------------------- graphene_django/types.py | 6 +- 3 files changed, 37 insertions(+), 69 deletions(-) diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py index 4fe546d..b4eb3ce 100644 --- a/graphene_django/tests/models.py +++ b/graphene_django/tests/models.py @@ -65,6 +65,11 @@ class Reporter(models.Model): self.__class__ = CNNReporter +class CNNReporterManager(models.Manager): + def get_queryset(self): + return super(CNNReporterManager, self).get_queryset().filter(reporter_type=2) + + class CNNReporter(Reporter): """ This class is a proxy model for Reporter, used for testing @@ -74,6 +79,8 @@ class CNNReporter(Reporter): class Meta: proxy = True + objects = CNNReporterManager() + class Article(models.Model): headline = models.CharField(max_length=100) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 1716034..82d7d75 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1,3 +1,4 @@ +import base64 import datetime import pytest @@ -895,8 +896,7 @@ def test_should_handle_inherited_choices(): def test_proxy_model_support(): """ - This test asserts that we can query for all Reporters, - even if some are of a proxy model type at runtime. + This test asserts that we can query for all Reporters and proxied Reporters. """ class ReporterType(DjangoObjectType): @@ -905,11 +905,17 @@ def test_proxy_model_support(): interfaces = (Node,) use_connection = True - reporter_1 = Reporter.objects.create( + class CNNReporterType(DjangoObjectType): + class Meta: + model = CNNReporter + interfaces = (Node,) + use_connection = True + + reporter = Reporter.objects.create( first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 ) - reporter_2 = CNNReporter.objects.create( + cnn_reporter = CNNReporter.objects.create( first_name="Some", last_name="Guy", email="someguy@cnn.com", @@ -919,6 +925,7 @@ def test_proxy_model_support(): class Query(graphene.ObjectType): all_reporters = DjangoConnectionField(ReporterType) + cnn_reporters = DjangoConnectionField(CNNReporterType) schema = graphene.Schema(query=Query) query = """ @@ -930,14 +937,26 @@ def test_proxy_model_support(): } } } + cnnReporters { + edges { + node { + id + } + } + } } """ expected = { "allReporters": { "edges": [ - {"node": {"id": "UmVwb3J0ZXJUeXBlOjE="}}, - {"node": {"id": "UmVwb3J0ZXJUeXBlOjI="}}, + {"node": {"id": base64.b64encode("ReporterType:{}".format(reporter.id))}}, + {"node": {"id": base64.b64encode("ReporterType:{}".format(cnn_reporter.id))}}, + ] + }, + "cnnReporters": { + "edges": [ + {"node": {"id": base64.b64encode("CNNReporterType:{}".format(cnn_reporter.id))}} ] } } @@ -945,65 +964,3 @@ def test_proxy_model_support(): result = schema.execute(query) assert not result.errors assert result.data == expected - - -def test_proxy_model_fails(): - """ - This test asserts that if you try to query for a proxy model, - that query will fail with: - GraphQLError('Expected value of type "CNNReporterType" but got: - CNNReporter.',) - - This is because a proxy model has the identical model definition - to its superclass, and defines its behavior at runtime, rather than - at the database level. Currently, filtering objects of the proxy models' - type isn't supported. It would require a field on the model that would - represent the type, and it doesn't seem like there is a clear way to - enforce this pattern across all projects - """ - - class CNNReporterType(DjangoObjectType): - class Meta: - model = CNNReporter - interfaces = (Node,) - use_connection = True - - reporter_1 = Reporter.objects.create( - first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 - ) - - reporter_2 = CNNReporter.objects.create( - first_name="Some", - last_name="Guy", - email="someguy@cnn.com", - a_choice=1, - reporter_type=2, # set this guy to be CNN - ) - - class Query(graphene.ObjectType): - all_reporters = DjangoConnectionField(CNNReporterType) - - schema = graphene.Schema(query=Query) - query = """ - query ProxyModelQuery { - allReporters { - edges { - node { - id - } - } - } - } - """ - - expected = { - "allReporters": { - "edges": [ - {"node": {"id": "UmVwb3J0ZXJUeXBlOjE="}}, - {"node": {"id": "UmVwb3J0ZXJUeXBlOjI="}}, - ] - } - } - - result = schema.execute(query) - assert result.errors diff --git a/graphene_django/types.py b/graphene_django/types.py index 4441a9a..6c386e0 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -130,7 +130,11 @@ class DjangoObjectType(ObjectType): if not is_valid_django_model(type(root)): raise Exception(('Received incompatible instance "{}".').format(root)) - model = root._meta.model._meta.concrete_model + if cls._meta.model._meta.proxy: + model = root._meta.model + else: + model = root._meta.model._meta.concrete_model + return model == cls._meta.model @classmethod From 980142dfcfb5411b39bbc95e1777475f81402558 Mon Sep 17 00:00:00 2001 From: Andrew Bettke Date: Wed, 27 Mar 2019 17:24:13 +1300 Subject: [PATCH 2/5] Fix linting. --- graphene_django/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/types.py b/graphene_django/types.py index 6c386e0..2a402d7 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -134,7 +134,7 @@ class DjangoObjectType(ObjectType): model = root._meta.model else: model = root._meta.model._meta.concrete_model - + return model == cls._meta.model @classmethod From 83a2ad34cdb07de23038feefbe1167e45bbc8536 Mon Sep 17 00:00:00 2001 From: Andrew Bettke Date: Wed, 27 Mar 2019 17:28:56 +1300 Subject: [PATCH 3/5] Encode strings before passing to b64encode. --- graphene_django/tests/test_query.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 82d7d75..c99c8e1 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -950,13 +950,13 @@ def test_proxy_model_support(): expected = { "allReporters": { "edges": [ - {"node": {"id": base64.b64encode("ReporterType:{}".format(reporter.id))}}, - {"node": {"id": base64.b64encode("ReporterType:{}".format(cnn_reporter.id))}}, + {"node": {"id": base64.b64encode("ReporterType:{}".format(reporter.id).encode())}}, + {"node": {"id": base64.b64encode("ReporterType:{}".format(cnn_reporter.id).encode())}}, ] }, "cnnReporters": { "edges": [ - {"node": {"id": base64.b64encode("CNNReporterType:{}".format(cnn_reporter.id))}} + {"node": {"id": base64.b64encode("CNNReporterType:{}".format(cnn_reporter.id).encode())}} ] } } From a461e80ee461b7409f3728f747823dc75d56ce0e Mon Sep 17 00:00:00 2001 From: Andrew Bettke Date: Wed, 27 Mar 2019 17:56:06 +1300 Subject: [PATCH 4/5] Correctly encode / decode for python3+. --- graphene_django/tests/test_query.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index c99c8e1..5c38ce5 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -947,16 +947,19 @@ def test_proxy_model_support(): } """ + def str_to_node_id(val): + return base64.b64encode(val.encode()).decode() + expected = { "allReporters": { "edges": [ - {"node": {"id": base64.b64encode("ReporterType:{}".format(reporter.id).encode())}}, - {"node": {"id": base64.b64encode("ReporterType:{}".format(cnn_reporter.id).encode())}}, + {"node": {"id": str_to_node_id("ReporterType:{}".format(reporter.id))}}, + {"node": {"id": str_to_node_id("ReporterType:{}".format(cnn_reporter.id))}}, ] }, "cnnReporters": { "edges": [ - {"node": {"id": base64.b64encode("CNNReporterType:{}".format(cnn_reporter.id).encode())}} + {"node": {"id": str_to_node_id("CNNReporterType:{}".format(cnn_reporter.id))}} ] } } From 959e98eeb0c87295e2535b04f870ace2028a394b Mon Sep 17 00:00:00 2001 From: Andrew Bettke Date: Thu, 28 Mar 2019 09:56:10 +1300 Subject: [PATCH 5/5] Refactor to use formal to_global_id. --- graphene_django/tests/test_query.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 5c38ce5..e74b8d6 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -8,6 +8,7 @@ from py.test import raises from django.db.models import Q +from graphql_relay import to_global_id import graphene from graphene.relay import Node @@ -947,19 +948,16 @@ def test_proxy_model_support(): } """ - def str_to_node_id(val): - return base64.b64encode(val.encode()).decode() - expected = { "allReporters": { "edges": [ - {"node": {"id": str_to_node_id("ReporterType:{}".format(reporter.id))}}, - {"node": {"id": str_to_node_id("ReporterType:{}".format(cnn_reporter.id))}}, + {"node": {"id": to_global_id("ReporterType", reporter.id)}}, + {"node": {"id": to_global_id("ReporterType", cnn_reporter.id)}}, ] }, "cnnReporters": { "edges": [ - {"node": {"id": str_to_node_id("CNNReporterType:{}".format(cnn_reporter.id))}} + {"node": {"id": to_global_id("CNNReporterType", cnn_reporter.id)}} ] } }