From f5549113979d9c8a8e639d8c5d6a4a8c118f338d Mon Sep 17 00:00:00 2001 From: Nishchit Date: Sat, 7 Nov 2020 05:34:45 +0530 Subject: [PATCH 01/22] Section added `GraphQL testing clients` (#919) --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 8605065..d813508 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,11 @@ To learn more check out the following [examples](examples/): * **Relay Schema**: [Starwars Relay example](examples/starwars) +## GraphQL testing clients + - [Firecamp](https://firecamp.io/graphql) + - [GraphiQL](https://github.com/graphql/graphiql) + + ## Contributing See [CONTRIBUTING.md](CONTRIBUTING.md) From 0888c748fdc02c5446e15c538b1b1294d2fbb2ea Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Sun, 8 Nov 2020 10:44:37 +0500 Subject: [PATCH 02/22] Change build badge from travis to github actions (#1058) --- README.md | 6 +++--- README.rst | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index d813508..73ce5bf 100644 --- a/README.md +++ b/README.md @@ -3,13 +3,13 @@ A [Django](https://www.djangoproject.com/) integration for [Graphene](http://graphene-python.org/). -[![travis][travis-image]][travis-url] +[![build][build-image]][build-url] [![pypi][pypi-image]][pypi-url] [![Anaconda-Server Badge][conda-image]][conda-url] [![coveralls][coveralls-image]][coveralls-url] -[travis-image]: https://travis-ci.org/graphql-python/graphene-django.svg?branch=master&style=flat -[travis-url]: https://travis-ci.org/graphql-python/graphene-django +[build-image]: https://github.com/graphql-python/graphene-django/workflows/Tests/badge.svg +[build-url]: https://github.com/graphql-python/graphene-django/actions [pypi-image]: https://img.shields.io/pypi/v/graphene-django.svg?style=flat [pypi-url]: https://pypi.org/project/graphene-django/ [coveralls-image]: https://coveralls.io/repos/github/graphql-python/graphene-django/badge.svg?branch=master diff --git a/README.rst b/README.rst index 44feaee..dc7275c 100644 --- a/README.rst +++ b/README.rst @@ -114,8 +114,8 @@ Contributing See `CONTRIBUTING.md `__. .. |Graphene Logo| image:: http://graphene-python.org/favicon.png -.. |Build Status| image:: https://travis-ci.org/graphql-python/graphene-django.svg?branch=master - :target: https://travis-ci.org/graphql-python/graphene-django +.. |Build Status| image:: https://github.com/graphql-python/graphene-django/workflows/Tests/badge.svg + :target: https://github.com/graphql-python/graphene-django/actions .. |PyPI version| image:: https://badge.fury.io/py/graphene-django.svg :target: https://badge.fury.io/py/graphene-django .. |Coverage Status| image:: https://coveralls.io/repos/graphql-python/graphene-django/badge.svg?branch=master&service=github From eb7a0265d8360d539385f9ce152ec95d74cc5f1a Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Mon, 9 Nov 2020 22:06:53 +0500 Subject: [PATCH 03/22] Use explicit classmethod in simple mutation example (#1059) rel #1038 --- docs/mutations.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/mutations.rst b/docs/mutations.rst index aef32eb..9b6f179 100644 --- a/docs/mutations.rst +++ b/docs/mutations.rst @@ -36,7 +36,8 @@ Simple example # The class attributes define the response of the mutation question = graphene.Field(QuestionType) - def mutate(self, info, text, id): + @classmethod + def mutate(cls, root, info, text, id): question = Question.objects.get(pk=id) question.text = text question.save() From 4b7119d691611ee1e2fe14e1adbfc9d06c7e3ef9 Mon Sep 17 00:00:00 2001 From: Tonye Jack Date: Fri, 27 Nov 2020 18:52:42 -0500 Subject: [PATCH 04/22] Add a default msg to show the response content. (#1064) * Add a default msg to show the response content. This seems like an issue with using assertResponseNoErrors and assertResponseHasErrors Which doesn't include any errors specific to the response and currently just shows. ```python self.assertNotIn("errors", list(content.keys())) AssertionError: 'errors' unexpectedly found in ['errors', 'data'] ``` * Update testing.py --- graphene_django/utils/testing.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py index 7c9b152..871c440 100644 --- a/graphene_django/utils/testing.py +++ b/graphene_django/utils/testing.py @@ -109,9 +109,9 @@ class GraphQLTestCase(TestCase): the call was fine. :resp HttpResponse: Response """ - self.assertEqual(resp.status_code, 200) content = json.loads(resp.content) - self.assertNotIn("errors", list(content.keys()), msg) + self.assertEqual(resp.status_code, 200, msg or content) + self.assertNotIn("errors", list(content.keys()), msg or content) def assertResponseHasErrors(self, resp, msg=None): """ @@ -119,4 +119,4 @@ class GraphQLTestCase(TestCase): :resp HttpResponse: Response """ content = json.loads(resp.content) - self.assertIn("errors", list(content.keys()), msg) + self.assertIn("errors", list(content.keys()), msg or content) From 4c0c821b7462580c6edc0ab7e8281705e2b3630b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Clgen=20Sar=C4=B1kavak?= Date: Sat, 28 Nov 2020 21:30:18 +0300 Subject: [PATCH 05/22] Register MultipleChoiceField as list type (#1033) In general I welcome reviews even from non-maintainers to build confidence. I haven't seen any objections and this has sat with approval for a week so I am going to go ahead and merge. --- graphene_django/forms/converter.py | 5 +++++ graphene_django/forms/tests/test_converter.py | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/graphene_django/forms/converter.py b/graphene_django/forms/converter.py index 8916456..5d17680 100644 --- a/graphene_django/forms/converter.py +++ b/graphene_django/forms/converter.py @@ -63,6 +63,11 @@ def convert_form_field_to_list(field): return List(ID, required=field.required) +@convert_form_field.register(forms.MultipleChoiceField) +def convert_form_field_to_string_list(field): + return List(String, required=field.required) + + @convert_form_field.register(forms.DateField) def convert_form_field_to_date(field): return Date(description=field.help_text, required=field.required) diff --git a/graphene_django/forms/tests/test_converter.py b/graphene_django/forms/tests/test_converter.py index 955b952..ccf630f 100644 --- a/graphene_django/forms/tests/test_converter.py +++ b/graphene_django/forms/tests/test_converter.py @@ -101,7 +101,14 @@ def test_should_decimal_convert_float(): assert_conversion(forms.DecimalField, Float) -def test_should_multiple_choice_convert_connectionorlist(): +def test_should_multiple_choice_convert_list(): + field = forms.MultipleChoiceField() + graphene_type = convert_form_field(field) + assert isinstance(graphene_type, List) + assert graphene_type.of_type == String + + +def test_should_model_multiple_choice_convert_connectionorlist(): field = forms.ModelMultipleChoiceField(queryset=None) graphene_type = convert_form_field(field) assert isinstance(graphene_type, List) From 454b74052e1d3813e1e0c7f3e08c8532e68986e5 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Wed, 23 Dec 2020 05:04:45 +0100 Subject: [PATCH 06/22] Fix backward Relay pagination (#1046) * Fix backward Relay pagination * linting Co-authored-by: Thomas Leonard --- graphene_django/fields.py | 15 ++--- graphene_django/tests/test_query.py | 97 +++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 78efceb..f7d9a19 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -147,14 +147,11 @@ class DjangoConnectionField(ConnectionField): if isinstance(iterable, QuerySet): list_length = iterable.count() - list_slice_length = ( - min(max_limit, list_length) if max_limit is not None else list_length - ) else: list_length = len(iterable) - list_slice_length = ( - min(max_limit, list_length) if max_limit is not None else list_length - ) + list_slice_length = ( + min(max_limit, list_length) if max_limit is not None else list_length + ) # If after is higher than list_length, connection_from_list_slice # would try to do a negative slicing which makes django throw an @@ -162,7 +159,11 @@ class DjangoConnectionField(ConnectionField): after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length) if max_limit is not None and "first" not in args: - args["first"] = max_limit + if "last" in args: + args["first"] = list_length + list_slice_length = list_length + else: + args["first"] = max_limit connection = connection_from_list_slice( iterable[after:], diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 6add0b8..a2d8373 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1213,6 +1213,103 @@ def test_should_have_next_page(graphene_settings): } +class TestBackwardPagination: + def setup_schema(self, graphene_settings, max_limit): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit + reporters = [Reporter(**kwargs) for kwargs in REPORTERS] + Reporter.objects.bulk_create(reporters) + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + return schema + + def do_queries(self, schema): + # Simply last 3 + query_last = """ + query { + allReporters(last: 3) { + edges { + node { + firstName + } + } + } + } + """ + + result = schema.execute(query_last) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 3 + assert [ + e["node"]["firstName"] for e in result.data["allReporters"]["edges"] + ] == ["First 3", "First 4", "First 5"] + + # Use a combination of first and last + query_first_and_last = """ + query { + allReporters(first: 4, last: 3) { + edges { + node { + firstName + } + } + } + } + """ + + result = schema.execute(query_first_and_last) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 3 + assert [ + e["node"]["firstName"] for e in result.data["allReporters"]["edges"] + ] == ["First 1", "First 2", "First 3"] + + # Use a combination of first and last and after + query_first_last_and_after = """ + query queryAfter($after: String) { + allReporters(first: 4, last: 3, after: $after) { + edges { + node { + firstName + } + } + } + } + """ + + after = base64.b64encode(b"arrayconnection:0").decode() + result = schema.execute( + query_first_last_and_after, variable_values=dict(after=after) + ) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 3 + assert [ + e["node"]["firstName"] for e in result.data["allReporters"]["edges"] + ] == ["First 2", "First 3", "First 4"] + + def test_should_query(self, graphene_settings): + """ + Backward pagination should work as expected + """ + schema = self.setup_schema(graphene_settings, max_limit=100) + self.do_queries(schema) + + def test_should_query_with_low_max_limit(self, graphene_settings): + """ + When doing backward pagination (using last) in combination with a max limit higher than the number of objects + we should really retrieve the last ones. + """ + schema = self.setup_schema(graphene_settings, max_limit=4) + self.do_queries(schema) + + def test_should_preserve_prefetch_related(django_assert_num_queries): class ReporterType(DjangoObjectType): class Meta: From 7b356950677d07dba41c94969b787b32223c6e3b Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Tue, 22 Dec 2020 20:10:28 -0800 Subject: [PATCH 07/22] Fix 1061: DjangoListField should not cache queries (#1063) * fix( DjangoListField ): test that default functionality should resolve/call queryset at view time, first attempt at solution * fix( DjangoListField ): DjangoListField defines get_manager just like DjangoConnectionField for a better variable name default_manager instead of default_queryset * fix: apply specific black formatting --- graphene_django/fields.py | 13 +++++------ graphene_django/tests/test_fields.py | 33 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index f7d9a19..fdf95aa 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -43,16 +43,16 @@ class DjangoListField(Field): def model(self): return self._underlying_type._meta.model - def get_default_queryset(self): - return self.model._default_manager.get_queryset() + def get_manager(self): + return self.model._default_manager @staticmethod def list_resolver( - django_object_type, resolver, default_queryset, root, info, **args + django_object_type, resolver, default_manager, root, info, **args ): queryset = maybe_queryset(resolver(root, info, **args)) if queryset is None: - queryset = default_queryset + queryset = maybe_queryset(default_manager) if isinstance(queryset, QuerySet): # Pass queryset to the DjangoObjectType get_queryset method @@ -66,10 +66,7 @@ class DjangoListField(Field): _type = _type.of_type django_object_type = _type.of_type.of_type return partial( - self.list_resolver, - django_object_type, - parent_resolver, - self.get_default_queryset(), + self.list_resolver, django_object_type, parent_resolver, self.get_manager(), ) diff --git a/graphene_django/tests/test_fields.py b/graphene_django/tests/test_fields.py index 57f913e..f68470e 100644 --- a/graphene_django/tests/test_fields.py +++ b/graphene_django/tests/test_fields.py @@ -75,6 +75,39 @@ class TestDjangoListField: "reporters": [{"firstName": "Tara"}, {"firstName": "Debra"}] } + def test_list_field_queryset_is_not_cached(self): + class Reporter(DjangoObjectType): + class Meta: + model = ReporterModel + fields = ("first_name",) + + class Query(ObjectType): + reporters = DjangoListField(Reporter) + + schema = Schema(query=Query) + + query = """ + query { + reporters { + firstName + } + } + """ + + result = schema.execute(query) + assert not result.errors + assert result.data == {"reporters": []} + + ReporterModel.objects.create(first_name="Tara", last_name="West") + ReporterModel.objects.create(first_name="Debra", last_name="Payne") + + result = schema.execute(query) + + assert not result.errors + assert result.data == { + "reporters": [{"firstName": "Tara"}, {"firstName": "Debra"}] + } + def test_override_resolver(self): class Reporter(DjangoObjectType): class Meta: From 99512c53a100e8fcc515f62caa4311956ca6847f Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Wed, 23 Dec 2020 05:10:39 +0100 Subject: [PATCH 08/22] fix: in and range filters on DjangoFilterConnectionField (#1070) Co-authored-by: Thomas Leonard --- graphene_django/filter/fields.py | 7 +- .../filter/tests/test_in_filter.py | 139 ++++++++++++++++++ graphene_django/filter/utils.py | 67 +++++++-- 3 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 graphene_django/filter/tests/test_in_filter.py diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index 3a98e8d..2ee374c 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -21,6 +21,7 @@ class DjangoFilterConnectionField(DjangoConnectionField): self._fields = fields self._provided_filterset_class = filterset_class self._filterset_class = None + self._filtering_args = None self._extra_filter_meta = extra_filter_meta self._base_args = None super(DjangoFilterConnectionField, self).__init__(type, *args, **kwargs) @@ -50,7 +51,11 @@ class DjangoFilterConnectionField(DjangoConnectionField): @property def filtering_args(self): - return get_filtering_args_from_filterset(self.filterset_class, self.node_type) + if not self._filtering_args: + self._filtering_args = get_filtering_args_from_filterset( + self.filterset_class, self.node_type + ) + return self._filtering_args @classmethod def resolve_queryset( diff --git a/graphene_django/filter/tests/test_in_filter.py b/graphene_django/filter/tests/test_in_filter.py new file mode 100644 index 0000000..3d4034e --- /dev/null +++ b/graphene_django/filter/tests/test_in_filter.py @@ -0,0 +1,139 @@ +import pytest + +from graphene import ObjectType, Schema +from graphene.relay import Node +from graphene_django import DjangoObjectType +from graphene_django.tests.models import Pet +from graphene_django.utils import DJANGO_FILTER_INSTALLED + +pytestmark = [] + +if DJANGO_FILTER_INSTALLED: + from graphene_django.filter import DjangoFilterConnectionField +else: + pytestmark.append( + pytest.mark.skipif( + True, reason="django_filters not installed or not compatible" + ) + ) + + +class PetNode(DjangoObjectType): + class Meta: + model = Pet + interfaces = (Node,) + filter_fields = { + "name": ["exact", "in"], + "age": ["exact", "in", "range"], + } + + +class Query(ObjectType): + pets = DjangoFilterConnectionField(PetNode) + + +def test_string_in_filter(): + """ + Test in filter on a string field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=3) + Pet.objects.create(name="Jojo, the rabbit", age=3) + + schema = Schema(query=Query) + + query = """ + query { + pets (name_In: ["Brutus", "Jojo, the rabbit"]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Brutus"}}, + {"node": {"name": "Jojo, the rabbit"}}, + ] + + +def test_int_in_filter(): + """ + Test in filter on an integer field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=3) + Pet.objects.create(name="Jojo, the rabbit", age=3) + + schema = Schema(query=Query) + + query = """ + query { + pets (age_In: [3]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Mimi"}}, + {"node": {"name": "Jojo, the rabbit"}}, + ] + + query = """ + query { + pets (age_In: [3, 12]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Brutus"}}, + {"node": {"name": "Mimi"}}, + {"node": {"name": "Jojo, the rabbit"}}, + ] + + +def test_int_range_filter(): + """ + Test in filter on an integer field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=8) + Pet.objects.create(name="Jojo, the rabbit", age=3) + Pet.objects.create(name="Picotin", age=5) + + schema = Schema(query=Query) + + query = """ + query { + pets (age_Range: [4, 9]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Mimi"}}, + {"node": {"name": "Picotin"}}, + ] diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index c5f18e2..becd5f5 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -1,6 +1,10 @@ import six +from graphene import List + from django_filters.utils import get_model_field +from django_filters.filters import Filter, BaseCSVFilter + from .filterset import custom_filterset_factory, setup_filterset @@ -17,8 +21,11 @@ def get_filtering_args_from_filterset(filterset_class, type): form_field = None if name in filterset_class.declared_filters: + # Get the filter field from the explicitly declared filter form_field = filter_field.field + field = convert_form_field(form_field) else: + # Get the filter field with no explicit type declaration 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"): @@ -26,12 +33,19 @@ def get_filtering_args_from_filterset(filterset_class, type): required=filter_field.extra.get("required", False) ) - # Fallback to field defined on filter if we can't get it from the - # model field - if not form_field: - form_field = filter_field.field + # Fallback to field defined on filter if we can't get it from the + # model field + if not form_field: + form_field = filter_field.field - field_type = convert_form_field(form_field).Argument() + field = convert_form_field(form_field) + + if filter_type in ["in", "range"]: + # Replace CSV filters (`in`, `range`) argument type to be a list of the same type as the field. + # See comments in `replace_csv_filters` method for more details. + field = List(field.get_type()) + + field_type = field.Argument() field_type.description = filter_field.label args[name] = field_type @@ -39,9 +53,42 @@ def get_filtering_args_from_filterset(filterset_class, type): def get_filterset_class(filterset_class, **meta): - """Get the class to be used as the FilterSet""" + """ + Get the class to be used as the FilterSet. + """ if filterset_class: - # If were given a FilterSet class, then set it up and - # return it - return setup_filterset(filterset_class) - return custom_filterset_factory(**meta) + # If were given a FilterSet class, then set it up. + graphene_filterset_class = setup_filterset(filterset_class) + else: + # Otherwise create one. + graphene_filterset_class = custom_filterset_factory(**meta) + + replace_csv_filters(graphene_filterset_class) + return graphene_filterset_class + + +def replace_csv_filters(filterset_class): + """ + Replace the "in" and "range" filters (that are not explicitly declared) to not be BaseCSVFilter (BaseInFilter, BaseRangeFilter) objects anymore + but regular Filter objects that simply use the input value as filter argument on the queryset. + + This is because those BaseCSVFilter are expecting a string as input with comma separated value but with GraphQl we + can actually have a list as input and have a proper type verification of each value in the list. + + See issue https://github.com/graphql-python/graphene-django/issues/1068. + """ + for name, filter_field in six.iteritems(filterset_class.base_filters): + filter_type = filter_field.lookup_expr + if ( + filter_type in ["in", "range"] + and name not in filterset_class.declared_filters + ): + assert isinstance(filter_field, BaseCSVFilter) + filterset_class.base_filters[name] = Filter( + field_name=filter_field.field_name, + lookup_expr=filter_field.lookup_expr, + label=filter_field.label, + method=filter_field.method, + exclude=filter_field.exclude, + **filter_field.extra + ) From cc3bd05472588b64b50f2db321ebb541b145a36f Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Wed, 23 Dec 2020 09:12:22 +0500 Subject: [PATCH 09/22] Replace Unidecode package with text_unidecode package #1014 (#1060) Closes #1014 --- graphene_django/utils/str_converters.py | 2 +- setup.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/graphene_django/utils/str_converters.py b/graphene_django/utils/str_converters.py index f41e87a..77a0f37 100644 --- a/graphene_django/utils/str_converters.py +++ b/graphene_django/utils/str_converters.py @@ -1,5 +1,5 @@ import re -from unidecode import unidecode +from text_unidecode import unidecode def to_const(string): diff --git a/setup.py b/setup.py index e33cfca..e6615b8 100644 --- a/setup.py +++ b/setup.py @@ -66,7 +66,7 @@ setup( "Django>=1.11", "singledispatch>=3.4.0.3", "promise>=2.1", - "unidecode>=1.1.1,<2", + "text-unidecode", ], setup_requires=["pytest-runner"], tests_require=tests_require, From 0e123438530a89fb0ce6b774542c9934e6a7c5f6 Mon Sep 17 00:00:00 2001 From: Leonardo Arroyo Date: Wed, 23 Dec 2020 01:13:34 -0300 Subject: [PATCH 10/22] Fix issue #1055 (#1056) * Fix issue #1055 * Fix if to elif * Use self.stdout.write instead of print when printing graphql schema Co-authored-by: leonardo arroyo <[contato@leonardoarroyo.com](mailto:contato@leonardoarroyo.com)> --- graphene_django/management/commands/graphql_schema.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/graphene_django/management/commands/graphql_schema.py b/graphene_django/management/commands/graphql_schema.py index dcef73c..bd1c8e6 100644 --- a/graphene_django/management/commands/graphql_schema.py +++ b/graphene_django/management/commands/graphql_schema.py @@ -60,8 +60,10 @@ class Command(CommandArguments): def get_schema(self, schema, out, indent): schema_dict = {"data": schema.introspect()} - if out == "-": + if out == "-" or out == "-.json": self.stdout.write(json.dumps(schema_dict, indent=indent, sort_keys=True)) + elif out == "-.graphql": + self.stdout.write(print_schema(schema)) else: # Determine format _, file_extension = os.path.splitext(out) From a51c2bffd9a59c6fa3700c0117b65a2242219a16 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Wed, 23 Dec 2020 09:15:38 +0500 Subject: [PATCH 11/22] Allow to use camel case in order by field (#1054) Fixes #1008 --- graphene_django/filter/fields.py | 14 ++++- graphene_django/filter/tests/filters.py | 2 +- graphene_django/filter/tests/test_fields.py | 67 +++++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index 2ee374c..7d8d2d8 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -3,6 +3,7 @@ from functools import partial from django.core.exceptions import ValidationError from graphene.types.argument import to_arguments +from graphene.utils.str_converters import to_snake_case from ..fields import DjangoConnectionField from .utils import get_filtering_args_from_filterset, get_filterset_class @@ -61,12 +62,21 @@ class DjangoFilterConnectionField(DjangoConnectionField): def resolve_queryset( cls, connection, iterable, info, args, filtering_args, filterset_class ): + def filter_kwargs(): + kwargs = {} + for k, v in args.items(): + if k in filtering_args: + if k == "order_by": + v = to_snake_case(v) + kwargs[k] = v + return kwargs + qs = super(DjangoFilterConnectionField, cls).resolve_queryset( connection, iterable, info, args ) - filter_kwargs = {k: v for k, v in args.items() if k in filtering_args} + filterset = filterset_class( - data=filter_kwargs, queryset=qs, request=info.context + data=filter_kwargs(), queryset=qs, request=info.context ) if filterset.form.is_valid(): return filterset.qs diff --git a/graphene_django/filter/tests/filters.py b/graphene_django/filter/tests/filters.py index 359d2ba..43b6a87 100644 --- a/graphene_django/filter/tests/filters.py +++ b/graphene_django/filter/tests/filters.py @@ -21,7 +21,7 @@ class ReporterFilter(django_filters.FilterSet): model = Reporter fields = ["first_name", "last_name", "email", "pets"] - order_by = OrderingFilter(fields=("pub_date",)) + order_by = OrderingFilter(fields=("first_name",)) class PetFilter(django_filters.FilterSet): diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 88749aa..18e7f0c 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -713,6 +713,73 @@ def test_should_query_filter_node_limit(): assert result.data == expected +def test_order_by(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(ObjectType): + all_reporters = DjangoFilterConnectionField( + ReporterType, filterset_class=ReporterFilter + ) + + Reporter.objects.create(first_name="b") + Reporter.objects.create(first_name="a") + + schema = Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(orderBy: "-firstName") { + edges { + node { + firstName + } + } + } + } + """ + expected = { + "allReporters": { + "edges": [{"node": {"firstName": "b"}}, {"node": {"firstName": "a"}}] + } + } + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + query = """ + query NodeFilteringQuery { + allReporters(orderBy: "-first_name") { + edges { + node { + firstName + } + } + } + } + """ + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + query = """ + query NodeFilteringQuery { + allReporters(orderBy: "-firtsnaMe") { + edges { + node { + firstName + } + } + } + } + """ + result = schema.execute(query) + assert result.errors + + def test_order_by_is_perserved(): class ReporterType(DjangoObjectType): class Meta: From 8f63199a63e14b0d0ad471a9beaa79875c3258f5 Mon Sep 17 00:00:00 2001 From: Ignacio Orlandini Date: Wed, 23 Dec 2020 01:18:14 -0300 Subject: [PATCH 12/22] Handle database transactions (#1039) * Handle Django database atomic requests * Create and handle database atomic mutations * Make code compatible with Python 2.7 * Code style * Define set_rollback instead of using the one in rest_framework.views because of backward compatibility * Implement mock.patch.dict --- docs/mutations.rst | 118 ++++++++ graphene_django/constants.py | 1 + graphene_django/forms/mutation.py | 13 + graphene_django/forms/tests/test_mutation.py | 188 ++++++++----- graphene_django/settings.py | 1 + graphene_django/tests/forms.py | 16 ++ graphene_django/tests/mutations.py | 18 ++ graphene_django/tests/schema_view.py | 4 + graphene_django/tests/test_views.py | 270 +++++++++++++++++++ graphene_django/tests/types.py | 9 + graphene_django/utils/utils.py | 8 +- graphene_django/views.py | 37 ++- 12 files changed, 613 insertions(+), 70 deletions(-) create mode 100644 graphene_django/constants.py create mode 100644 graphene_django/tests/forms.py create mode 100644 graphene_django/tests/mutations.py create mode 100644 graphene_django/tests/types.py diff --git a/docs/mutations.rst b/docs/mutations.rst index 9b6f179..3853381 100644 --- a/docs/mutations.rst +++ b/docs/mutations.rst @@ -230,3 +230,121 @@ This argument is also sent back to the client with the mutation result (you do not have to do anything). For services that manage a pool of many GraphQL requests in bulk, the ``clientIDMutation`` allows you to match up a specific mutation with the response. + + + +Django Database Transactions +---------------------------- + +Django gives you a few ways to control how database transactions are managed. + +Tying transactions to HTTP requests +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A common way to handle transactions in Django is to wrap each request in a transaction. +Set ``ATOMIC_REQUESTS`` settings to ``True`` in the configuration of each database for +which you want to enable this behavior. + +It works like this. Before calling ``GraphQLView`` Django starts a transaction. If the +response is produced without problems, Django commits the transaction. If the view, a +``DjangoFormMutation`` or a ``DjangoModelFormMutation`` produces an exception, Django +rolls back the transaction. + +.. warning:: + + While the simplicity of this transaction model is appealing, it also makes it + inefficient when traffic increases. Opening a transaction for every request has some + overhead. The impact on performance depends on the query patterns of your application + and on how well your database handles locking. + +Check the next section for a better solution. + +Tying transactions to mutations +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +A mutation can contain multiple fields, just like a query. There's one important +distinction between queries and mutations, other than the name: + +.. + + `While query fields are executed in parallel, mutation fields run in series, one + after the other.` + +This means that if we send two ``incrementCredits`` mutations in one request, the first +is guaranteed to finish before the second begins, ensuring that we don't end up with a +race condition with ourselves. + +On the other hand, if the first ``incrementCredits`` runs successfully but the second +one does not, the operation cannot be retried as it is. That's why is a good idea to +run all mutation fields in a transaction, to guarantee all occur or nothing occurs. + +To enable this behavior for all databases set the graphene ``ATOMIC_MUTATIONS`` settings +to ``True`` in your settings file: + +.. code:: python + + GRAPHENE = { + # ... + "ATOMIC_MUTATIONS": True, + } + +On the contrary, if you want to enable this behavior for a specific database, set +``ATOMIC_MUTATIONS`` to ``True`` in your database settings: + +.. code:: python + + DATABASES = { + "default": { + # ... + "ATOMIC_MUTATIONS": True, + }, + # ... + } + +Now, given the following example mutation: + +.. code:: + + mutation IncreaseCreditsTwice { + + increaseCredits1: increaseCredits(input: { amount: 10 }) { + balance + errors { + field + messages + } + } + + increaseCredits2: increaseCredits(input: { amount: -1 }) { + balance + errors { + field + messages + } + } + + } + +The server is going to return something like: + +.. code:: json + + { + "data": { + "increaseCredits1": { + "balance": 10.0, + "errors": [] + }, + "increaseCredits2": { + "balance": null, + "errors": [ + { + "field": "amount", + "message": "Amount should be a positive number" + } + ] + }, + } + } + +But the balance will remain the same. diff --git a/graphene_django/constants.py b/graphene_django/constants.py new file mode 100644 index 0000000..e4d7ea9 --- /dev/null +++ b/graphene_django/constants.py @@ -0,0 +1 @@ +MUTATION_ERRORS_FLAG = "graphene_mutation_has_errors" diff --git a/graphene_django/forms/mutation.py b/graphene_django/forms/mutation.py index 692f8d5..cc7d647 100644 --- a/graphene_django/forms/mutation.py +++ b/graphene_django/forms/mutation.py @@ -11,8 +11,13 @@ from graphene.types.mutation import MutationOptions # InputObjectType, # ) from graphene.types.utils import yank_fields_from_attrs +from graphene_django.constants import MUTATION_ERRORS_FLAG from graphene_django.registry import get_global_registry + +from django.core.exceptions import ValidationError +from django.db import connection + from ..types import ErrorType from .converter import convert_form_field @@ -46,6 +51,7 @@ class BaseDjangoFormMutation(ClientIDMutation): return cls.perform_mutate(form, info) else: errors = ErrorType.from_errors(form.errors) + _set_errors_flag_to_context(info) return cls(errors=errors, **form.data) @@ -170,6 +176,7 @@ class DjangoModelFormMutation(BaseDjangoFormMutation): return cls.perform_mutate(form, info) else: errors = ErrorType.from_errors(form.errors) + _set_errors_flag_to_context(info) return cls(errors=errors) @@ -178,3 +185,9 @@ class DjangoModelFormMutation(BaseDjangoFormMutation): obj = form.save() kwargs = {cls._meta.return_field_name: obj} return cls(errors=[], **kwargs) + + +def _set_errors_flag_to_context(info): + # This is not ideal but necessary to keep the response errors empty + if info and info.context: + setattr(info.context, MUTATION_ERRORS_FLAG, True) diff --git a/graphene_django/forms/tests/test_mutation.py b/graphene_django/forms/tests/test_mutation.py index a455a0a..ed92863 100644 --- a/graphene_django/forms/tests/test_mutation.py +++ b/graphene_django/forms/tests/test_mutation.py @@ -5,21 +5,13 @@ from py.test import raises from graphene import Field, ObjectType, Schema, String from graphene_django import DjangoObjectType +from graphene_django.tests.forms import PetForm from graphene_django.tests.models import Pet +from graphene_django.tests.mutations import PetMutation from ..mutation import DjangoFormMutation, DjangoModelFormMutation -@pytest.fixture() -def pet_type(): - class PetType(DjangoObjectType): - class Meta: - model = Pet - fields = "__all__" - - return PetType - - class MyForm(forms.Form): text = forms.CharField() @@ -33,18 +25,6 @@ class MyForm(forms.Form): pass -class PetForm(forms.ModelForm): - class Meta: - model = Pet - fields = "__all__" - - def clean_age(self): - age = self.cleaned_data["age"] - if age >= 99: - raise ValidationError("Too old") - return age - - def test_needs_form_class(): with raises(Exception) as exc: @@ -70,11 +50,18 @@ def test_has_input_fields(): assert "text" in MyMutation.Input._meta.fields -def test_mutation_error_camelcased(pet_type, graphene_settings): +def test_mutation_error_camelcased(graphene_settings): class ExtraPetForm(PetForm): test_field = forms.CharField(required=True) + class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" + class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + class Meta: form_class = ExtraPetForm @@ -146,21 +133,13 @@ def test_form_valid_input(): assert result.data["myMutation"]["text"] == "VALID_INPUT" -def test_default_meta_fields(pet_type): - class PetMutation(DjangoModelFormMutation): - class Meta: - form_class = PetForm - +def test_default_meta_fields(): assert PetMutation._meta.model is Pet assert PetMutation._meta.return_field_name == "pet" assert "pet" in PetMutation._meta.fields -def test_default_input_meta_fields(pet_type): - class PetMutation(DjangoModelFormMutation): - class Meta: - form_class = PetForm - +def test_default_input_meta_fields(): assert PetMutation._meta.model is Pet assert PetMutation._meta.return_field_name == "pet" assert "name" in PetMutation.Input._meta.fields @@ -168,8 +147,15 @@ def test_default_input_meta_fields(pet_type): assert "id" in PetMutation.Input._meta.fields -def test_exclude_fields_input_meta_fields(pet_type): +def test_exclude_fields_input_meta_fields(): + class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" + class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + class Meta: form_class = PetForm exclude_fields = ["id"] @@ -182,8 +168,15 @@ def test_exclude_fields_input_meta_fields(pet_type): assert "id" not in PetMutation.Input._meta.fields -def test_custom_return_field_name(pet_type): +def test_custom_return_field_name(): + class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" + class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + class Meta: form_class = PetForm model = Pet @@ -194,13 +187,7 @@ def test_custom_return_field_name(pet_type): assert "animal" in PetMutation._meta.fields -def test_model_form_mutation_mutate_existing(pet_type): - class PetMutation(DjangoModelFormMutation): - pet = Field(pet_type) - - class Meta: - form_class = PetForm - +def test_model_form_mutation_mutate_existing(): class Mutation(ObjectType): pet_mutation = PetMutation.Field() @@ -229,13 +216,7 @@ def test_model_form_mutation_mutate_existing(pet_type): assert pet.name == "Mia" -def test_model_form_mutation_creates_new(pet_type): - class PetMutation(DjangoModelFormMutation): - pet = Field(pet_type) - - class Meta: - form_class = PetForm - +def test_model_form_mutation_creates_new(): class Mutation(ObjectType): pet_mutation = PetMutation.Field() @@ -265,13 +246,7 @@ def test_model_form_mutation_creates_new(pet_type): assert pet.age == 10 -def test_model_form_mutation_invalid_input(pet_type): - class PetMutation(DjangoModelFormMutation): - pet = Field(pet_type) - - class Meta: - form_class = PetForm - +def test_model_form_mutation_invalid_input(): class Mutation(ObjectType): pet_mutation = PetMutation.Field() @@ -301,11 +276,7 @@ def test_model_form_mutation_invalid_input(pet_type): assert Pet.objects.count() == 0 -def test_model_form_mutation_mutate_invalid_form(pet_type): - class PetMutation(DjangoModelFormMutation): - class Meta: - form_class = PetForm - +def test_model_form_mutation_mutate_invalid_form(): result = PetMutation.mutate_and_get_payload(None, None) # A pet was not created @@ -317,3 +288,98 @@ def test_model_form_mutation_mutate_invalid_form(pet_type): assert result.errors[1].messages == ["This field is required."] assert "age" in fields_w_error assert "name" in fields_w_error + + +def test_model_form_mutation_multiple_creation_valid(): + class Mutation(ObjectType): + pet_mutation = PetMutation.Field() + + schema = Schema(query=MockQuery, mutation=Mutation) + + result = schema.execute( + """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 10 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + ) + assert result.errors is None + assert result.data["petMutation1"]["pet"] == {"name": "Mia", "age": 10} + assert result.data["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 2 + + pet1 = Pet.objects.first() + assert pet1.name == "Mia" + assert pet1.age == 10 + + pet2 = Pet.objects.last() + assert pet2.name == "Enzo" + assert pet2.age == 0 + + +def test_model_form_mutation_multiple_creation_invalid(): + class Mutation(ObjectType): + pet_mutation = PetMutation.Field() + + schema = Schema(query=MockQuery, mutation=Mutation) + + result = schema.execute( + """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 99 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + ) + assert result.errors is None + + assert result.data["petMutation1"]["pet"] is None + assert result.data["petMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert result.data["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 diff --git a/graphene_django/settings.py b/graphene_django/settings.py index 71b791c..8a990dc 100644 --- a/graphene_django/settings.py +++ b/graphene_django/settings.py @@ -45,6 +45,7 @@ DEFAULTS = { # This sets headerEditorEnabled GraphiQL option, for details go to # https://github.com/graphql/graphiql/tree/main/packages/graphiql#options "GRAPHIQL_HEADER_EDITOR_ENABLED": True, + "ATOMIC_MUTATIONS": False, } if settings.DEBUG: diff --git a/graphene_django/tests/forms.py b/graphene_django/tests/forms.py new file mode 100644 index 0000000..95e3534 --- /dev/null +++ b/graphene_django/tests/forms.py @@ -0,0 +1,16 @@ +from django import forms +from django.core.exceptions import ValidationError + +from .models import Pet + + +class PetForm(forms.ModelForm): + class Meta: + model = Pet + fields = "__all__" + + def clean_age(self): + age = self.cleaned_data["age"] + if age >= 99: + raise ValidationError("Too old") + return age diff --git a/graphene_django/tests/mutations.py b/graphene_django/tests/mutations.py new file mode 100644 index 0000000..3aa8bfc --- /dev/null +++ b/graphene_django/tests/mutations.py @@ -0,0 +1,18 @@ +from graphene import Field + +from graphene_django.forms.mutation import DjangoFormMutation, DjangoModelFormMutation + +from .forms import PetForm +from .types import PetType + + +class PetFormMutation(DjangoFormMutation): + class Meta: + form_class = PetForm + + +class PetMutation(DjangoModelFormMutation): + pet = Field(PetType) + + class Meta: + form_class = PetForm diff --git a/graphene_django/tests/schema_view.py b/graphene_django/tests/schema_view.py index 9b3bd1e..8ed2ecf 100644 --- a/graphene_django/tests/schema_view.py +++ b/graphene_django/tests/schema_view.py @@ -1,6 +1,8 @@ import graphene from graphene import ObjectType, Schema +from .mutations import PetFormMutation, PetMutation + class QueryRoot(ObjectType): @@ -19,6 +21,8 @@ class QueryRoot(ObjectType): class MutationRoot(ObjectType): + pet_form_mutation = PetFormMutation.Field() + pet_mutation = PetMutation.Field() write_test = graphene.Field(QueryRoot) def resolve_write_test(self, info): diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index db6cc4e..5d85905 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -2,6 +2,14 @@ import json import pytest +from mock import patch + +from django.db import connection + +from graphene_django.settings import graphene_settings + +from .models import Pet + try: from urllib import urlencode except ImportError: @@ -558,3 +566,265 @@ def test_passes_request_into_context_request(client): assert response.status_code == 200 assert response_json(response) == {"data": {"request": "testing"}} + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": True} +) +def test_form_mutation_multiple_creation_invalid_atomic_request(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": True, "ATOMIC_REQUESTS": False} +) +def test_form_mutation_multiple_creation_invalid_atomic_mutation_1(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", True) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_form_mutation_multiple_creation_invalid_atomic_mutation_2(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_form_mutation_multiple_creation_invalid_non_atomic(client): + query = """ + mutation PetMutations { + petFormMutation1: petFormMutation(input: { name: "Mia", age: 99 }) { + errors { + field + messages + } + } + petFormMutation2: petFormMutation(input: { name: "Enzo", age: 0 }) { + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petFormMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petFormMutation2"]["errors"] == [] + + assert Pet.objects.count() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": True} +) +def test_model_form_mutation_multiple_creation_invalid_atomic_request(client): + query = """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 99 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petMutation1"]["pet"] is None + assert content["data"]["petMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 0 + + +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_model_form_mutation_multiple_creation_invalid_non_atomic(client): + query = """ + mutation PetMutations { + petMutation1: petMutation(input: { name: "Mia", age: 99 }) { + pet { + name + age + } + errors { + field + messages + } + } + petMutation2: petMutation(input: { name: "Enzo", age: 0 }) { + pet { + name + age + } + errors { + field + messages + } + } + } + """ + + response = client.post(url_string(query=query)) + content = response_json(response) + + assert "errors" not in content + + assert content["data"]["petMutation1"]["pet"] is None + assert content["data"]["petMutation1"]["errors"] == [ + {"field": "age", "messages": ["Too old"]} + ] + + assert content["data"]["petMutation2"]["pet"] == {"name": "Enzo", "age": 0} + + assert Pet.objects.count() == 1 + + pet = Pet.objects.get() + assert pet.name == "Enzo" + assert pet.age == 0 + + +@patch("graphene_django.utils.utils.transaction.set_rollback") +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": True} +) +def test_query_errors_atomic_request(set_rollback_mock, client): + client.get(url_string(query="force error")) + set_rollback_mock.assert_called_once_with(True) + + +@patch("graphene_django.utils.utils.transaction.set_rollback") +@patch("graphene_django.settings.graphene_settings.ATOMIC_MUTATIONS", False) +@patch.dict( + connection.settings_dict, {"ATOMIC_MUTATIONS": False, "ATOMIC_REQUESTS": False} +) +def test_query_errors_non_atomic(set_rollback_mock, client): + client.get(url_string(query="force error")) + set_rollback_mock.assert_not_called() diff --git a/graphene_django/tests/types.py b/graphene_django/tests/types.py new file mode 100644 index 0000000..56fe51d --- /dev/null +++ b/graphene_django/tests/types.py @@ -0,0 +1,9 @@ +from graphene_django.types import DjangoObjectType + +from .models import Pet + + +class PetType(DjangoObjectType): + class Meta: + model = Pet + fields = "__all__" diff --git a/graphene_django/utils/utils.py b/graphene_django/utils/utils.py index c1d3572..b1c9a7d 100644 --- a/graphene_django/utils/utils.py +++ b/graphene_django/utils/utils.py @@ -1,7 +1,7 @@ import inspect import six -from django.db import models +from django.db import connection, models, transaction from django.db.models.manager import Manager from django.utils.encoding import force_text from django.utils.functional import Promise @@ -100,3 +100,9 @@ def import_single_dispatch(): ) return singledispatch + + +def set_rollback(): + atomic_requests = connection.settings_dict.get("ATOMIC_REQUESTS", False) + if atomic_requests and connection.in_atomic_block: + transaction.set_rollback(True) diff --git a/graphene_django/views.py b/graphene_django/views.py index 5ee0297..e81f760 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -3,6 +3,7 @@ import json import re import six +from django.db import connection, transaction from django.http import HttpResponse, HttpResponseNotAllowed from django.http.response import HttpResponseBadRequest from django.shortcuts import render @@ -17,6 +18,9 @@ from graphql.execution import ExecutionResult from graphql.type.schema import GraphQLSchema from graphql.execution.middleware import MiddlewareManager +from graphene_django.constants import MUTATION_ERRORS_FLAG +from graphene_django.utils.utils import set_rollback + from .settings import graphene_settings @@ -203,11 +207,15 @@ class GraphQLView(View): request, data, query, variables, operation_name, show_graphiql ) + if getattr(request, MUTATION_ERRORS_FLAG, False) is True: + set_rollback() + status_code = 200 if execution_result: response = {} if execution_result.errors: + set_rollback() response["errors"] = [ self.format_error(e) for e in execution_result.errors ] @@ -312,14 +320,27 @@ class GraphQLView(View): # executor is not a valid argument in all backends extra_options["executor"] = self.executor - return document.execute( - root_value=self.get_root_value(request), - variable_values=variables, - operation_name=operation_name, - context_value=self.get_context(request), - middleware=self.get_middleware(request), - **extra_options - ) + options = { + "root_value": self.get_root_value(request), + "variable_values": variables, + "operation_name": operation_name, + "context_value": self.get_context(request), + "middleware": self.get_middleware(request), + } + options.update(extra_options) + + operation_type = document.get_operation_type(operation_name) + if operation_type == "mutation" and ( + graphene_settings.ATOMIC_MUTATIONS is True + or connection.settings_dict.get("ATOMIC_MUTATIONS", False) is True + ): + with transaction.atomic(): + result = document.execute(**options) + if getattr(request, MUTATION_ERRORS_FLAG, False) is True: + transaction.set_rollback(True) + return result + + return document.execute(**options) except Exception as e: return ExecutionResult(errors=[e], invalid=True) From 558288afcefe49c0183e0ab16bb2c6ead4d8041d Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Tue, 22 Dec 2020 20:23:41 -0800 Subject: [PATCH 13/22] v2.14.0 --- graphene_django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/__init__.py b/graphene_django/__init__.py index 03ccbeb..792f7be 100644 --- a/graphene_django/__init__.py +++ b/graphene_django/__init__.py @@ -1,7 +1,7 @@ from .fields import DjangoConnectionField, DjangoListField from .types import DjangoObjectType -__version__ = "2.13.0" +__version__ = "2.14.0" __all__ = [ "__version__", From dab6080fcf36b521107d19c5b55a35b6487da63c Mon Sep 17 00:00:00 2001 From: Rustam Ganeyev Date: Tue, 29 Dec 2020 19:30:10 +0000 Subject: [PATCH 14/22] Fixed typo in documentation (#1078) Added missing kwargs to documentation --- docs/queries.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/queries.rst b/docs/queries.rst index 02a2bf2..d2da781 100644 --- a/docs/queries.rst +++ b/docs/queries.rst @@ -287,7 +287,7 @@ Where "foo" is the name of the field declared in the ``Query`` object. class Query(graphene.ObjectType): foo = graphene.List(QuestionType) - def resolve_foo(root, info): + def resolve_foo(root, info, **kwargs): id = kwargs.get("id") return Question.objects.get(id) From e559a42374529378dbdcb6bcb4f3ff704312a66b Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 30 Dec 2020 06:30:30 +1100 Subject: [PATCH 15/22] docs: fix simple typo, outputing -> outputting (#1077) There is a small typo in docs/debug.rst. Should read `outputting` rather than `outputing`. --- docs/debug.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/debug.rst b/docs/debug.rst index d1cbb21..2286519 100644 --- a/docs/debug.rst +++ b/docs/debug.rst @@ -3,7 +3,7 @@ Django Debug Middleware You can debug your GraphQL queries in a similar way to `django-debug-toolbar `__, -but outputing in the results in GraphQL response as fields, instead of +but outputting in the results in GraphQL response as fields, instead of the graphical HTML interface. For that, you will need to add the plugin in your graphene schema. @@ -43,7 +43,7 @@ And in your ``settings.py``: Querying -------- -You can query it for outputing all the sql transactions that happened in +You can query it for outputting all the sql transactions that happened in the GraphQL request, like: .. code:: From 2d0b9ddd426810678e2dbaa28be9dbe55f919bf2 Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Wed, 30 Dec 2020 08:25:41 -0800 Subject: [PATCH 16/22] improvement: convert decimal field to graphene decimal (#1083) --- graphene_django/converter.py | 5 +++++ graphene_django/tests/test_converter.py | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 0de6964..63cc35d 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -18,6 +18,7 @@ from graphene import ( DateTime, Date, Time, + Decimal, ) from graphene.types.json import JSONString from graphene.utils.str_converters import to_camel_case @@ -160,6 +161,10 @@ def convert_field_to_boolean(field, registry=None): @convert_django_field.register(models.DecimalField) +def convert_field_to_decimal(field, registry=None): + return Decimal(description=field.help_text, required=not field.null) + + @convert_django_field.register(models.FloatField) @convert_django_field.register(models.DurationField) def convert_field_to_float(field, registry=None): diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py index 7d8e669..287ec82 100644 --- a/graphene_django/tests/test_converter.py +++ b/graphene_django/tests/test_converter.py @@ -242,6 +242,10 @@ def test_should_float_convert_float(): assert_conversion(models.FloatField, graphene.Float) +def test_should_float_convert_decimal(): + assert_conversion(models.DecimalField, graphene.Decimal) + + def test_should_manytomany_convert_connectionorlist(): registry = Registry() dynamic_field = convert_django_field(Reporter._meta.local_many_to_many[0], registry) From 8c4851609355831af66a71ce178bffc98356e269 Mon Sep 17 00:00:00 2001 From: Thiago Bellini Ribeiro Date: Thu, 31 Dec 2020 02:03:57 -0300 Subject: [PATCH 17/22] Also convert BaseCSVFilter for custom fields (#1081) --- .../filter/tests/test_in_filter.py | 50 ++++++++++++++++++- graphene_django/filter/utils.py | 16 +++--- graphene_django/tests/models.py | 4 ++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/graphene_django/filter/tests/test_in_filter.py b/graphene_django/filter/tests/test_in_filter.py index 3d4034e..7bbee65 100644 --- a/graphene_django/filter/tests/test_in_filter.py +++ b/graphene_django/filter/tests/test_in_filter.py @@ -1,9 +1,11 @@ import pytest +from django_filters import FilterSet +from django_filters import rest_framework as filters from graphene import ObjectType, Schema from graphene.relay import Node from graphene_django import DjangoObjectType -from graphene_django.tests.models import Pet +from graphene_django.tests.models import Pet, Person from graphene_django.utils import DJANGO_FILTER_INSTALLED pytestmark = [] @@ -28,8 +30,27 @@ class PetNode(DjangoObjectType): } +class PersonFilterSet(FilterSet): + class Meta: + model = Person + fields = {} + + names = filters.BaseInFilter(method="filter_names") + + def filter_names(self, qs, name, value): + return qs.filter(name__in=value) + + +class PersonNode(DjangoObjectType): + class Meta: + model = Person + interfaces = (Node,) + filterset_class = PersonFilterSet + + class Query(ObjectType): pets = DjangoFilterConnectionField(PetNode) + people = DjangoFilterConnectionField(PersonNode) def test_string_in_filter(): @@ -61,6 +82,33 @@ def test_string_in_filter(): ] +def test_string_in_filter_with_filterset_class(): + """Test in filter on a string field with a custom filterset class.""" + Person.objects.create(name="John") + Person.objects.create(name="Michael") + Person.objects.create(name="Angela") + + schema = Schema(query=Query) + + query = """ + query { + people (names: ["John", "Michael"]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["people"]["edges"] == [ + {"node": {"name": "John"}}, + {"node": {"name": "Michael"}}, + ] + + def test_int_in_filter(): """ Test in filter on an integer field. diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index becd5f5..71c5b49 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -19,6 +19,7 @@ def get_filtering_args_from_filterset(filterset_class, type): model = filterset_class._meta.model for name, filter_field in six.iteritems(filterset_class.base_filters): form_field = None + filter_type = filter_field.lookup_expr if name in filterset_class.declared_filters: # Get the filter field from the explicitly declared filter @@ -27,7 +28,6 @@ def get_filtering_args_from_filterset(filterset_class, type): else: # Get the filter field with no explicit type declaration 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) @@ -40,10 +40,11 @@ def get_filtering_args_from_filterset(filterset_class, type): field = convert_form_field(form_field) - if filter_type in ["in", "range"]: - # Replace CSV filters (`in`, `range`) argument type to be a list of the same type as the field. - # See comments in `replace_csv_filters` method for more details. - field = List(field.get_type()) + if filter_type in ["in", "range"]: + # Replace CSV filters (`in`, `range`) argument type to be a list of + # the same type as the field. See comments in + # `replace_csv_filters` method for more details. + field = List(field.get_type()) field_type = field.Argument() field_type.description = filter_field.label @@ -79,10 +80,7 @@ def replace_csv_filters(filterset_class): """ for name, filter_field in six.iteritems(filterset_class.base_filters): filter_type = filter_field.lookup_expr - if ( - filter_type in ["in", "range"] - and name not in filterset_class.declared_filters - ): + if filter_type in ["in", "range"]: assert isinstance(filter_field, BaseCSVFilter) filterset_class.base_filters[name] = Filter( field_name=filter_field.field_name, diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py index 44a5d8a..20f509c 100644 --- a/graphene_django/tests/models.py +++ b/graphene_django/tests/models.py @@ -6,6 +6,10 @@ from django.utils.translation import ugettext_lazy as _ CHOICES = ((1, "this"), (2, _("that"))) +class Person(models.Model): + name = models.CharField(max_length=30) + + class Pet(models.Model): name = models.CharField(max_length=30) age = models.PositiveIntegerField() From 40e525293626f760a95ed0f2cc37f78048502728 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Clgen=20Sar=C4=B1kavak?= Date: Thu, 31 Dec 2020 08:12:24 +0300 Subject: [PATCH 18/22] Use the Django TestCase's Client (#1084) * Use the Django Client test utility instance that Django provides with its TestCase class. This allows GraphQL tests to make use of the stateful client methods like login() * Add missing test case initializer call * Don't break backward compability * Add test for pending deprecation warning on GraphQLTestCase._client Co-authored-by: Tom Nightingale --- graphene_django/tests/test_utils.py | 2 ++ graphene_django/utils/testing.py | 20 ++++++++++------- graphene_django/utils/tests/test_testing.py | 24 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 graphene_django/utils/tests/test_testing.py diff --git a/graphene_django/tests/test_utils.py b/graphene_django/tests/test_utils.py index f5a8b05..e7aa027 100644 --- a/graphene_django/tests/test_utils.py +++ b/graphene_django/tests/test_utils.py @@ -51,7 +51,9 @@ def test_graphql_test_case_op_name(post_mock): pass tc = TestClass() + tc._pre_setup() tc.setUpClass() + tc.query("query { }", op_name="QueryName") body = json.loads(post_mock.call_args.args[1]) # `operationName` field from https://graphql.org/learn/serving-over-http/#post-request diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py index 871c440..b758ac8 100644 --- a/graphene_django/utils/testing.py +++ b/graphene_django/utils/testing.py @@ -1,6 +1,7 @@ import json +import warnings -from django.test import TestCase, Client +from django.test import Client, TestCase DEFAULT_GRAPHQL_URL = "/graphql/" @@ -68,12 +69,6 @@ class GraphQLTestCase(TestCase): # URL to graphql endpoint GRAPHQL_URL = DEFAULT_GRAPHQL_URL - @classmethod - def setUpClass(cls): - super(GraphQLTestCase, cls).setUpClass() - - cls._client = Client() - def query(self, query, op_name=None, input_data=None, variables=None, headers=None): """ Args: @@ -99,10 +94,19 @@ class GraphQLTestCase(TestCase): input_data=input_data, variables=variables, headers=headers, - client=self._client, + client=self.client, graphql_url=self.GRAPHQL_URL, ) + @property + def _client(self): + warnings.warn( + "Using `_client` is deprecated in favour of `client`.", + PendingDeprecationWarning, + stacklevel=2, + ) + return self.client + def assertResponseNoErrors(self, resp, msg=None): """ Assert that the call went through correctly. 200 means the syntax is ok, if there are no `errors`, diff --git a/graphene_django/utils/tests/test_testing.py b/graphene_django/utils/tests/test_testing.py new file mode 100644 index 0000000..df78321 --- /dev/null +++ b/graphene_django/utils/tests/test_testing.py @@ -0,0 +1,24 @@ +import pytest + +from .. import GraphQLTestCase +from ...tests.test_types import with_local_registry + + +@with_local_registry +def test_graphql_test_case_deprecated_client(): + """ + Test that `GraphQLTestCase._client`'s should raise pending deprecation warning. + """ + + class TestClass(GraphQLTestCase): + GRAPHQL_SCHEMA = True + + def runTest(self): + pass + + tc = TestClass() + tc._pre_setup() + tc.setUpClass() + + with pytest.warns(PendingDeprecationWarning): + tc._client From caf954861025b9f3d9d3f9c204a7cbbc87352265 Mon Sep 17 00:00:00 2001 From: Jason Kraus Date: Wed, 30 Dec 2020 22:46:51 -0800 Subject: [PATCH 19/22] v2.15.0 --- graphene_django/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/__init__.py b/graphene_django/__init__.py index 792f7be..7472a06 100644 --- a/graphene_django/__init__.py +++ b/graphene_django/__init__.py @@ -1,7 +1,7 @@ from .fields import DjangoConnectionField, DjangoListField from .types import DjangoObjectType -__version__ = "2.14.0" +__version__ = "2.15.0" __all__ = [ "__version__", From 1281c1338df27df529ff7f891d2f23f2993a3178 Mon Sep 17 00:00:00 2001 From: Rustam Ganeyev Date: Sat, 2 Jan 2021 04:36:50 +0000 Subject: [PATCH 20/22] Introduced optional_fields to SerializationMutation (#1080) * added optional_field to SerializationMutation to forcefully mark some fields as optional * added tests --- graphene_django/rest_framework/mutation.py | 10 ++++++++- .../rest_framework/serializer_converter.py | 9 ++++++-- .../rest_framework/tests/test_mutation.py | 21 ++++++++++++++++++- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/graphene_django/rest_framework/mutation.py b/graphene_django/rest_framework/mutation.py index 000b21e..9e2ae12 100644 --- a/graphene_django/rest_framework/mutation.py +++ b/graphene_django/rest_framework/mutation.py @@ -18,6 +18,7 @@ class SerializerMutationOptions(MutationOptions): model_class = None model_operations = ["create", "update"] serializer_class = None + optional_fields = () def fields_for_serializer( @@ -27,6 +28,7 @@ def fields_for_serializer( is_input=False, convert_choices_to_enum=True, lookup_field=None, + optional_fields=(), ): fields = OrderedDict() for name, field in serializer.fields.items(): @@ -44,9 +46,13 @@ def fields_for_serializer( if is_not_in_only or is_excluded: continue + is_optional = name in optional_fields fields[name] = convert_serializer_field( - field, is_input=is_input, convert_choices_to_enum=convert_choices_to_enum + field, + is_input=is_input, + convert_choices_to_enum=convert_choices_to_enum, + force_optional=is_optional, ) return fields @@ -70,6 +76,7 @@ class SerializerMutation(ClientIDMutation): exclude_fields=(), convert_choices_to_enum=True, _meta=None, + optional_fields=(), **options ): @@ -95,6 +102,7 @@ class SerializerMutation(ClientIDMutation): is_input=True, convert_choices_to_enum=convert_choices_to_enum, lookup_field=lookup_field, + optional_fields=optional_fields, ) output_fields = fields_for_serializer( serializer, diff --git a/graphene_django/rest_framework/serializer_converter.py b/graphene_django/rest_framework/serializer_converter.py index 82a113a..18f34b8 100644 --- a/graphene_django/rest_framework/serializer_converter.py +++ b/graphene_django/rest_framework/serializer_converter.py @@ -19,7 +19,9 @@ def get_graphene_type_from_serializer_field(field): ) -def convert_serializer_field(field, is_input=True, convert_choices_to_enum=True): +def convert_serializer_field( + field, is_input=True, convert_choices_to_enum=True, force_optional=False +): """ Converts a django rest frameworks field to a graphql field and marks the field as required if we are creating an input type @@ -32,7 +34,10 @@ def convert_serializer_field(field, is_input=True, convert_choices_to_enum=True) graphql_type = get_graphene_type_from_serializer_field(field) args = [] - kwargs = {"description": field.help_text, "required": is_input and field.required} + kwargs = { + "description": field.help_text, + "required": is_input and field.required and not force_optional, + } # if it is a tuple or a list it means that we are returning # the graphql type and the child type diff --git a/graphene_django/rest_framework/tests/test_mutation.py b/graphene_django/rest_framework/tests/test_mutation.py index ffbc4b5..5c2518d 100644 --- a/graphene_django/rest_framework/tests/test_mutation.py +++ b/graphene_django/rest_framework/tests/test_mutation.py @@ -3,7 +3,7 @@ import datetime from py.test import raises from rest_framework import serializers -from graphene import Field, ResolveInfo +from graphene import Field, ResolveInfo, NonNull, String from graphene.types.inputobjecttype import InputObjectType from ...types import DjangoObjectType @@ -98,6 +98,25 @@ def test_exclude_fields(): assert "created" not in MyMutation.Input._meta.fields +def test_model_serializer_required_fields(): + class MyMutation(SerializerMutation): + class Meta: + serializer_class = MyModelSerializer + + assert "cool_name" in MyMutation.Input._meta.fields + assert MyMutation.Input._meta.fields["cool_name"].type == NonNull(String) + + +def test_model_serializer_optional_fields(): + class MyMutation(SerializerMutation): + class Meta: + serializer_class = MyModelSerializer + optional_fields = ("cool_name",) + + assert "cool_name" in MyMutation.Input._meta.fields + assert MyMutation.Input._meta.fields["cool_name"].type == String + + def test_write_only_field(): class WriteOnlyFieldModelSerializer(serializers.ModelSerializer): password = serializers.CharField(write_only=True) From aff56b882b55bc20a3896b98aa77868a0e01b6a5 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Sun, 10 Jan 2021 04:15:56 +0100 Subject: [PATCH 21/22] Validate in and range filter inputs (#1092) Co-authored-by: Thomas Leonard --- graphene_django/filter/__init__.py | 2 +- graphene_django/filter/filters.py | 75 ++++++++++++ graphene_django/filter/filterset.py | 25 +--- .../filter/tests/test_in_filter.py | 12 +- .../filter/tests/test_range_filter.py | 115 ++++++++++++++++++ graphene_django/filter/utils.py | 16 ++- 6 files changed, 211 insertions(+), 34 deletions(-) create mode 100644 graphene_django/filter/filters.py create mode 100644 graphene_django/filter/tests/test_range_filter.py diff --git a/graphene_django/filter/__init__.py b/graphene_django/filter/__init__.py index daafe56..5de36ad 100644 --- a/graphene_django/filter/__init__.py +++ b/graphene_django/filter/__init__.py @@ -9,7 +9,7 @@ if not DJANGO_FILTER_INSTALLED: ) else: from .fields import DjangoFilterConnectionField - from .filterset import GlobalIDFilter, GlobalIDMultipleChoiceFilter + from .filters import GlobalIDFilter, GlobalIDMultipleChoiceFilter __all__ = [ "DjangoFilterConnectionField", diff --git a/graphene_django/filter/filters.py b/graphene_django/filter/filters.py new file mode 100644 index 0000000..44832b5 --- /dev/null +++ b/graphene_django/filter/filters.py @@ -0,0 +1,75 @@ +from django.core.exceptions import ValidationError +from django.forms import Field + +from django_filters import Filter, MultipleChoiceFilter + +from graphql_relay.node.node import from_global_id + +from ..forms import GlobalIDFormField, GlobalIDMultipleChoiceField + + +class GlobalIDFilter(Filter): + """ + Filter for Relay global ID. + """ + + field_class = GlobalIDFormField + + def filter(self, qs, value): + """ Convert the filter value to a primary key before filtering """ + _id = None + if value is not None: + _, _id = from_global_id(value) + return super(GlobalIDFilter, self).filter(qs, _id) + + +class GlobalIDMultipleChoiceFilter(MultipleChoiceFilter): + field_class = GlobalIDMultipleChoiceField + + def filter(self, qs, value): + gids = [from_global_id(v)[1] for v in value] + return super(GlobalIDMultipleChoiceFilter, self).filter(qs, gids) + + +class InFilter(Filter): + """ + Filter for a list of value using the `__in` Django filter. + """ + + def filter(self, qs, value): + """ + Override the default filter class to check first weather the list is + empty or not. + This needs to be done as in this case we expect to get an empty output + (if not an exclude filter) but django_filter consider an empty list + to be an empty input value (see `EMPTY_VALUES`) meaning that + the filter does not need to be applied (hence returning the original + queryset). + """ + if value is not None and len(value) == 0: + if self.exclude: + return qs + else: + return qs.none() + else: + return super(InFilter, self).filter(qs, value) + + +def validate_range(value): + """ + Validator for range filter input: the list of value must be of length 2. + Note that validators are only run if the value is not empty. + """ + if len(value) != 2: + raise ValidationError( + "Invalid range specified: it needs to contain 2 values.", code="invalid" + ) + + +class RangeField(Field): + default_validators = [validate_range] + empty_values = [None] + + +class RangeFilter(Filter): + field_class = RangeField diff --git a/graphene_django/filter/filterset.py b/graphene_django/filter/filterset.py index 7676ea8..0fd0a82 100644 --- a/graphene_django/filter/filterset.py +++ b/graphene_django/filter/filterset.py @@ -1,32 +1,11 @@ import itertools from django.db import models -from django_filters import Filter, MultipleChoiceFilter, VERSION +from django_filters import VERSION from django_filters.filterset import BaseFilterSet, FilterSet from django_filters.filterset import FILTER_FOR_DBFIELD_DEFAULTS -from graphql_relay.node.node import from_global_id - -from ..forms import GlobalIDFormField, GlobalIDMultipleChoiceField - - -class GlobalIDFilter(Filter): - field_class = GlobalIDFormField - - def filter(self, qs, value): - """ Convert the filter value to a primary key before filtering """ - _id = None - if value is not None: - _, _id = from_global_id(value) - return super(GlobalIDFilter, self).filter(qs, _id) - - -class GlobalIDMultipleChoiceFilter(MultipleChoiceFilter): - field_class = GlobalIDMultipleChoiceField - - def filter(self, qs, value): - gids = [from_global_id(v)[1] for v in value] - return super(GlobalIDMultipleChoiceFilter, self).filter(qs, gids) +from .filters import GlobalIDFilter, GlobalIDMultipleChoiceFilter GRAPHENE_FILTER_SET_OVERRIDES = { diff --git a/graphene_django/filter/tests/test_in_filter.py b/graphene_django/filter/tests/test_in_filter.py index 7bbee65..9e9c323 100644 --- a/graphene_django/filter/tests/test_in_filter.py +++ b/graphene_django/filter/tests/test_in_filter.py @@ -157,20 +157,19 @@ def test_int_in_filter(): ] -def test_int_range_filter(): +def test_in_filter_with_empty_list(): """ - Test in filter on an integer field. + Check that using a in filter with an empty list provided as input returns no objects. """ Pet.objects.create(name="Brutus", age=12) Pet.objects.create(name="Mimi", age=8) - Pet.objects.create(name="Jojo, the rabbit", age=3) Pet.objects.create(name="Picotin", age=5) schema = Schema(query=Query) query = """ query { - pets (age_Range: [4, 9]) { + pets (name_In: []) { edges { node { name @@ -181,7 +180,4 @@ def test_int_range_filter(): """ result = schema.execute(query) assert not result.errors - assert result.data["pets"]["edges"] == [ - {"node": {"name": "Mimi"}}, - {"node": {"name": "Picotin"}}, - ] + assert len(result.data["pets"]["edges"]) == 0 diff --git a/graphene_django/filter/tests/test_range_filter.py b/graphene_django/filter/tests/test_range_filter.py new file mode 100644 index 0000000..4d8db4f --- /dev/null +++ b/graphene_django/filter/tests/test_range_filter.py @@ -0,0 +1,115 @@ +import ast +import json +import pytest + +from django_filters import FilterSet +from django_filters import rest_framework as filters +from graphene import ObjectType, Schema +from graphene.relay import Node +from graphene_django import DjangoObjectType +from graphene_django.tests.models import Pet +from graphene_django.utils import DJANGO_FILTER_INSTALLED + +pytestmark = [] + +if DJANGO_FILTER_INSTALLED: + from graphene_django.filter import DjangoFilterConnectionField +else: + pytestmark.append( + pytest.mark.skipif( + True, reason="django_filters not installed or not compatible" + ) + ) + + +class PetNode(DjangoObjectType): + class Meta: + model = Pet + interfaces = (Node,) + filter_fields = { + "name": ["exact", "in"], + "age": ["exact", "in", "range"], + } + + +class Query(ObjectType): + pets = DjangoFilterConnectionField(PetNode) + + +def test_int_range_filter(): + """ + Test range filter on an integer field. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=8) + Pet.objects.create(name="Jojo, the rabbit", age=3) + Pet.objects.create(name="Picotin", age=5) + + schema = Schema(query=Query) + + query = """ + query { + pets (age_Range: [4, 9]) { + edges { + node { + name + } + } + } + } + """ + result = schema.execute(query) + assert not result.errors + assert result.data["pets"]["edges"] == [ + {"node": {"name": "Mimi"}}, + {"node": {"name": "Picotin"}}, + ] + + +def test_range_filter_with_invalid_input(): + """ + Test range filter used with invalid inputs raise an error. + """ + Pet.objects.create(name="Brutus", age=12) + Pet.objects.create(name="Mimi", age=8) + Pet.objects.create(name="Jojo, the rabbit", age=3) + Pet.objects.create(name="Picotin", age=5) + + schema = Schema(query=Query) + + query = """ + query ($rangeValue: [Int]) { + pets (age_Range: $rangeValue) { + edges { + node { + name + } + } + } + } + """ + expected_error = json.dumps( + { + "age__range": [ + { + "message": "Invalid range specified: it needs to contain 2 values.", + "code": "invalid", + } + ] + } + ) + + # Empty list + result = schema.execute(query, variables={"rangeValue": []}) + assert len(result.errors) == 1 + assert ast.literal_eval(result.errors[0].message)[0] == expected_error + + # Only one item in the list + result = schema.execute(query, variables={"rangeValue": [1]}) + assert len(result.errors) == 1 + assert ast.literal_eval(result.errors[0].message)[0] == expected_error + + # More than 2 items in the list + result = schema.execute(query, variables={"rangeValue": [1, 2, 3]}) + assert len(result.errors) == 1 + assert ast.literal_eval(result.errors[0].message)[0] == expected_error diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index 71c5b49..dce08c7 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -6,6 +6,7 @@ from django_filters.utils import get_model_field from django_filters.filters import Filter, BaseCSVFilter from .filterset import custom_filterset_factory, setup_filterset +from .filters import InFilter, RangeFilter def get_filtering_args_from_filterset(filterset_class, type): @@ -80,9 +81,20 @@ def replace_csv_filters(filterset_class): """ for name, filter_field in six.iteritems(filterset_class.base_filters): filter_type = filter_field.lookup_expr - if filter_type in ["in", "range"]: + if filter_type == "in": assert isinstance(filter_field, BaseCSVFilter) - filterset_class.base_filters[name] = Filter( + filterset_class.base_filters[name] = InFilter( + field_name=filter_field.field_name, + lookup_expr=filter_field.lookup_expr, + label=filter_field.label, + method=filter_field.method, + exclude=filter_field.exclude, + **filter_field.extra + ) + + if filter_type == "range": + assert isinstance(filter_field, BaseCSVFilter) + filterset_class.base_filters[name] = RangeFilter( field_name=filter_field.field_name, lookup_expr=filter_field.lookup_expr, label=filter_field.label, From e24675e5b7ebba7528760638aa819b23ae20bf43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Clgen=20Sar=C4=B1kavak?= Date: Sun, 10 Jan 2021 06:16:18 +0300 Subject: [PATCH 22/22] Fix backward compability on GraphQLTestCase._client setter (#1093) --- graphene_django/utils/testing.py | 13 +++++++++++ graphene_django/utils/tests/test_testing.py | 25 +++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/graphene_django/utils/testing.py b/graphene_django/utils/testing.py index b758ac8..afe83c2 100644 --- a/graphene_django/utils/testing.py +++ b/graphene_django/utils/testing.py @@ -99,6 +99,10 @@ class GraphQLTestCase(TestCase): ) @property + def _client(self): + pass + + @_client.getter def _client(self): warnings.warn( "Using `_client` is deprecated in favour of `client`.", @@ -107,6 +111,15 @@ class GraphQLTestCase(TestCase): ) return self.client + @_client.setter + def _client(self, client): + warnings.warn( + "Using `_client` is deprecated in favour of `client`.", + PendingDeprecationWarning, + stacklevel=2, + ) + self.client = client + def assertResponseNoErrors(self, resp, msg=None): """ Assert that the call went through correctly. 200 means the syntax is ok, if there are no `errors`, diff --git a/graphene_django/utils/tests/test_testing.py b/graphene_django/utils/tests/test_testing.py index df78321..2ef78f9 100644 --- a/graphene_django/utils/tests/test_testing.py +++ b/graphene_django/utils/tests/test_testing.py @@ -2,12 +2,13 @@ import pytest from .. import GraphQLTestCase from ...tests.test_types import with_local_registry +from django.test import Client @with_local_registry -def test_graphql_test_case_deprecated_client(): +def test_graphql_test_case_deprecated_client_getter(): """ - Test that `GraphQLTestCase._client`'s should raise pending deprecation warning. + `GraphQLTestCase._client`' getter should raise pending deprecation warning. """ class TestClass(GraphQLTestCase): @@ -22,3 +23,23 @@ def test_graphql_test_case_deprecated_client(): with pytest.warns(PendingDeprecationWarning): tc._client + + +@with_local_registry +def test_graphql_test_case_deprecated_client_setter(): + """ + `GraphQLTestCase._client`' setter should raise pending deprecation warning. + """ + + class TestClass(GraphQLTestCase): + GRAPHQL_SCHEMA = True + + def runTest(self): + pass + + tc = TestClass() + tc._pre_setup() + tc.setUpClass() + + with pytest.warns(PendingDeprecationWarning): + tc._client = Client()