From 5f1731dca31bc1b8df766a73011be02744f59c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikolai=20R=C3=B8ed=20Kristiansen?= Date: Mon, 15 Aug 2022 11:41:39 +0200 Subject: [PATCH 01/12] Fix: Use .formatted instead of format_error (#1327) & Fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 👽 Use .formatted instead of format_error * ✅ Fix test with newer graphene null default values (graphql-python/graphene@03277a5) no more trailing newlines --- .../tests/test_array_field_exact_filter.py | 5 +---- .../filter/tests/test_enum_filtering.py | 5 +---- graphene_django/filter/tests/test_fields.py | 10 ++++------ .../filter/tests/test_typed_filter.py | 2 +- graphene_django/tests/test_command.py | 3 +-- graphene_django/tests/test_types.py | 20 +++++++------------ graphene_django/tests/test_views.py | 5 ----- graphene_django/views.py | 3 +-- 8 files changed, 16 insertions(+), 37 deletions(-) diff --git a/graphene_django/filter/tests/test_array_field_exact_filter.py b/graphene_django/filter/tests/test_array_field_exact_filter.py index cd72868..10e32ef 100644 --- a/graphene_django/filter/tests/test_array_field_exact_filter.py +++ b/graphene_django/filter/tests/test_array_field_exact_filter.py @@ -120,10 +120,7 @@ def test_array_field_filter_schema_type(Query): "randomField": "[Boolean!]", } filters_str = ", ".join( - [ - f"{filter_field}: {gql_type} = null" - for filter_field, gql_type in filters.items() - ] + [f"{filter_field}: {gql_type}" for filter_field, gql_type in filters.items()] ) assert ( f"type Query {{\n events({filters_str}): EventTypeConnection\n}}" in schema_str diff --git a/graphene_django/filter/tests/test_enum_filtering.py b/graphene_django/filter/tests/test_enum_filtering.py index 09c69b3..4fe7ddd 100644 --- a/graphene_django/filter/tests/test_enum_filtering.py +++ b/graphene_django/filter/tests/test_enum_filtering.py @@ -152,9 +152,6 @@ def test_filter_enum_field_schema_type(schema): "reporter_AChoice_In": "[TestsReporterAChoiceChoices]", } filters_str = ", ".join( - [ - f"{filter_field}: {gql_type} = null" - for filter_field, gql_type in filters.items() - ] + [f"{filter_field}: {gql_type}" for filter_field, gql_type in filters.items()] ) assert f" allArticles({filters_str}): ArticleTypeConnection\n" in schema_str diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 7d440f4..370c894 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -1008,7 +1008,7 @@ def test_integer_field_filter_type(): assert str(schema) == dedent( """\ type Query { - pets(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null): PetTypeConnection + pets(offset: Int, before: String, after: String, first: Int, last: Int, age: Int): PetTypeConnection } type PetTypeConnection { @@ -1056,8 +1056,7 @@ def test_integer_field_filter_type(): interface Node { \"""The ID of the object\""" id: ID! - } - """ + }""" ) @@ -1077,7 +1076,7 @@ def test_other_filter_types(): assert str(schema) == dedent( """\ type Query { - pets(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null, age: Int = null, age_Isnull: Boolean = null, age_Lt: Int = null): PetTypeConnection + pets(offset: Int, before: String, after: String, first: Int, last: Int, age: Int, age_Isnull: Boolean, age_Lt: Int): PetTypeConnection } type PetTypeConnection { @@ -1125,8 +1124,7 @@ def test_other_filter_types(): interface Node { \"""The ID of the object\""" id: ID! - } - """ + }""" ) diff --git a/graphene_django/filter/tests/test_typed_filter.py b/graphene_django/filter/tests/test_typed_filter.py index b903b59..cc0bafe 100644 --- a/graphene_django/filter/tests/test_typed_filter.py +++ b/graphene_django/filter/tests/test_typed_filter.py @@ -98,7 +98,7 @@ def test_typed_filter_schema(schema): ) for filter_field, gql_type in filters.items(): - assert "{}: {} = null".format(filter_field, gql_type) in all_articles_filters + assert "{}: {}".format(filter_field, gql_type) in all_articles_filters def test_typed_filters_work(schema): diff --git a/graphene_django/tests/test_command.py b/graphene_django/tests/test_command.py index 70116b8..11a15bc 100644 --- a/graphene_django/tests/test_command.py +++ b/graphene_django/tests/test_command.py @@ -53,6 +53,5 @@ def test_generate_graphql_file_on_call_graphql_schema(): """\ type Query { hi: String - } - """ + }""" ) diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index bde72c7..4885917 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -183,7 +183,7 @@ def test_schema_representation(): pets: [Reporter!]! aChoice: TestsReporterAChoiceChoices reporterType: TestsReporterReporterTypeChoices - articles(offset: Int = null, before: String = null, after: String = null, first: Int = null, last: Int = null): ArticleConnection! + articles(offset: Int, before: String, after: String, first: Int, last: Int): ArticleConnection! } \"""An enumeration.\""" @@ -244,8 +244,7 @@ def test_schema_representation(): \"""The ID of the object\""" id: ID! ): Node - } - """ + }""" ) assert str(schema) == expected @@ -525,8 +524,7 @@ class TestDjangoObjectType: id: ID! kind: String! cuteness: Int! - } - """ + }""" ) def test_django_objecttype_convert_choices_enum_list(self, PetModel): @@ -560,8 +558,7 @@ class TestDjangoObjectType: \"""Dog\""" DOG - } - """ + }""" ) def test_django_objecttype_convert_choices_enum_empty_list(self, PetModel): @@ -586,8 +583,7 @@ class TestDjangoObjectType: id: ID! kind: String! cuteness: Int! - } - """ + }""" ) def test_django_objecttype_convert_choices_enum_naming_collisions( @@ -621,8 +617,7 @@ class TestDjangoObjectType: \"""Dog\""" DOG - } - """ + }""" ) def test_django_objecttype_choices_custom_enum_name( @@ -660,8 +655,7 @@ class TestDjangoObjectType: \"""Dog\""" DOG - } - """ + }""" ) diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index 945fa87..c2f18c3 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -109,12 +109,10 @@ def test_reports_validation_errors(client): { "message": "Cannot query field 'unknownOne' on type 'QueryRoot'.", "locations": [{"line": 1, "column": 9}], - "path": None, }, { "message": "Cannot query field 'unknownTwo' on type 'QueryRoot'.", "locations": [{"line": 1, "column": 21}], - "path": None, }, ] } @@ -135,8 +133,6 @@ def test_errors_when_missing_operation_name(client): "errors": [ { "message": "Must provide operation name if query contains multiple operations.", - "locations": None, - "path": None, } ] } @@ -477,7 +473,6 @@ def test_handles_syntax_errors_caught_by_graphql(client): { "locations": [{"column": 1, "line": 1}], "message": "Syntax Error: Unexpected Name 'syntaxerror'.", - "path": None, } ] } diff --git a/graphene_django/views.py b/graphene_django/views.py index c23b020..f533f70 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -11,7 +11,6 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.generic import View from graphql import OperationType, get_operation_ast, parse, validate from graphql.error import GraphQLError -from graphql.error import format_error as format_graphql_error from graphql.execution import ExecutionResult from graphene import Schema @@ -387,7 +386,7 @@ class GraphQLView(View): @staticmethod def format_error(error): if isinstance(error, GraphQLError): - return format_graphql_error(error) + return error.formatted return {"message": str(error)} From 2aeb86ba3bed7dc821cc8508e1dca8297aa7dea4 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Tue, 6 Sep 2022 14:00:13 +0200 Subject: [PATCH 02/12] fix: backward pagination indexing error when using bigger last argument than total number of elements (#1344) Co-authored-by: Thomas Leonard --- graphene_django/fields.py | 24 ++++----- graphene_django/tests/test_query.py | 75 +++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 11 deletions(-) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index eb932c1..c881456 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -146,36 +146,38 @@ class DjangoConnectionField(ConnectionField): iterable = maybe_queryset(iterable) if isinstance(iterable, QuerySet): - list_length = iterable.count() + array_length = iterable.count() else: - list_length = len(iterable) - list_slice_length = ( - min(max_limit, list_length) if max_limit is not None else list_length + array_length = len(iterable) + array_slice_length = ( + min(max_limit, array_length) if max_limit is not None else array_length ) # If after is higher than list_length, connection_from_list_slice # would try to do a negative slicing which makes django throw an # AssertionError - after = min(get_offset_with_default(args.get("after"), -1) + 1, list_length) + slice_start = min( + get_offset_with_default(args.get("after"), -1) + 1, array_length + ) if max_limit is not None and args.get("first", None) is None: if args.get("last", None) is not None: - after = list_length - args["last"] + slice_start = max(array_length - args["last"], 0) else: args["first"] = max_limit connection = connection_from_array_slice( - iterable[after:], + iterable[slice_start:], args, - slice_start=after, - array_length=list_length, - array_slice_length=list_slice_length, + slice_start=slice_start, + array_length=array_length, + array_slice_length=array_slice_length, connection_type=partial(connection_adapter, connection), edge_type=connection.Edge, page_info_type=page_info_adapter, ) connection.iterable = iterable - connection.length = list_length + connection.length = array_length return connection @classmethod diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index aabe19c..5cbf90e 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1593,3 +1593,78 @@ def test_connection_should_allow_offset_filtering_with_after(): "allReporters": {"edges": [{"node": {"firstName": "Jane", "lastName": "Roe"}},]} } assert result.data == expected + + +def test_connection_should_succeed_if_last_higher_than_number_of_objects(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + fields = "__all__" + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query ReporterPromiseConnectionQuery ($last: Int) { + allReporters(last: $last) { + edges { + node { + firstName + lastName + } + } + } + } + """ + + result = schema.execute(query, variable_values=dict(last=2)) + assert not result.errors + expected = {"allReporters": {"edges": []}} + assert result.data == expected + + Reporter.objects.create(first_name="John", last_name="Doe") + Reporter.objects.create(first_name="Some", last_name="Guy") + Reporter.objects.create(first_name="Jane", last_name="Roe") + Reporter.objects.create(first_name="Some", last_name="Lady") + + result = schema.execute(query, variable_values=dict(last=2)) + assert not result.errors + expected = { + "allReporters": { + "edges": [ + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] + } + } + assert result.data == expected + + result = schema.execute(query, variable_values=dict(last=4)) + assert not result.errors + expected = { + "allReporters": { + "edges": [ + {"node": {"firstName": "John", "lastName": "Doe"}}, + {"node": {"firstName": "Some", "lastName": "Guy"}}, + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] + } + } + assert result.data == expected + + result = schema.execute(query, variable_values=dict(last=20)) + assert not result.errors + expected = { + "allReporters": { + "edges": [ + {"node": {"firstName": "John", "lastName": "Doe"}}, + {"node": {"firstName": "Some", "lastName": "Guy"}}, + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] + } + } + assert result.data == expected From 8ae576394ec9922cc1ab962697c2239b6a9a3325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nikolai=20R=C3=B8ed=20Kristiansen?= Date: Mon, 19 Sep 2022 14:31:04 +0200 Subject: [PATCH 03/12] =?UTF-8?q?=F0=9F=92=A5=20Stop=20supporting=20EOL=20?= =?UTF-8?q?djangos=20and=20pythons=20(#1337)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 💥 Stop supporting EOL djangos and pythons * 👷 Run only supported version in test workflow --- .github/workflows/tests.yml | 10 +++------- setup.py | 9 ++++----- tox.ini | 14 ++++---------- 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index c63742a..c2cdc99 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,15 +8,11 @@ jobs: strategy: max-parallel: 4 matrix: - django: ["2.2", "3.0", "3.1", "3.2"] - python-version: ["3.6", "3.7", "3.8", "3.9"] + django: ["3.2", "4.0", "4.1"] + python-version: ["3.8", "3.9", "3.10"] include: - django: "3.2" - python-version: "3.10" - - django: "4.0" - python-version: "3.10" - - django: "main" - python-version: "3.10" + python-version: "3.7" steps: - uses: actions/checkout@v2 - name: Set up Python ${{ matrix.python-version }} diff --git a/setup.py b/setup.py index 1762760..306ec33 100644 --- a/setup.py +++ b/setup.py @@ -46,16 +46,15 @@ setup( "Intended Audience :: Developers", "Topic :: Software Development :: Libraries", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", "Programming Language :: Python :: Implementation :: PyPy", "Framework :: Django", - "Framework :: Django :: 2.2", - "Framework :: Django :: 3.0", - "Framework :: Django :: 3.1", "Framework :: Django :: 3.2", + "Framework :: Django :: 4.0", + "Framework :: Django :: 4.1", ], keywords="api graphql protocol rest relay graphene", packages=find_packages(exclude=["tests", "examples", "examples.*"]), @@ -63,7 +62,7 @@ setup( "graphene>=3.0,<4", "graphql-core>=3.1.0,<4", "graphql-relay>=3.1.1,<4", - "Django>=2.2", + "Django>=3.2", "promise>=2.1", "text-unidecode", ], diff --git a/tox.ini b/tox.ini index d65839a..11b4893 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,11 @@ [tox] envlist = - py{36,37,38,39}-django{22,30,31}, - py{36,37,38,39,310}-django32, - py{38,39,310}-django{40,main}, + py{37,38,39,310}-django32, + py{38,39,310}-django{40,41,main}, black,flake8 [gh-actions] python = - 3.6: py36 3.7: py37 3.8: py38 3.9: py39 @@ -15,11 +13,9 @@ python = [gh-actions:env] DJANGO = - 2.2: django22 - 3.0: django30 - 3.1: django31 3.2: django32 4.0: django40 + 4.1: django41 main: djangomain [testenv] @@ -30,11 +26,9 @@ setenv = deps = -e.[test] psycopg2-binary - django22: Django>=2.2,<3.0 - django30: Django>=3.0,<3.1 - django31: Django>=3.1,<3.2 django32: Django>=3.2,<4.0 django40: Django>=4.0,<4.1 + django41: Django>=4.1,<4.2 djangomain: https://github.com/django/django/archive/main.zip commands = {posargs:py.test --cov=graphene_django graphene_django examples} From 42a40b4df0f9f90e918c2b11985079f52f8de73b Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Thu, 22 Sep 2022 10:26:21 +0100 Subject: [PATCH 04/12] chore: update dev dependencies (#1345) Co-authored-by: Thomas Leonard --- Makefile | 17 ++++++++--------- graphene_django/fields.py | 5 ++++- .../filter/filters/global_id_filter.py | 2 +- graphene_django/filter/filterset.py | 10 ++++------ graphene_django/filter/tests/conftest.py | 8 ++++---- .../filter/tests/test_enum_filtering.py | 14 ++++++++++---- graphene_django/filter/tests/test_fields.py | 16 +++++++++++++--- .../filter/tests/test_typed_filter.py | 12 +++--------- graphene_django/filter/utils.py | 4 +++- graphene_django/tests/test_query.py | 16 +++++++++++++--- graphene_django/types.py | 2 +- .../utils/tests/test_str_converters.py | 2 +- setup.py | 14 +++++++------- 13 files changed, 72 insertions(+), 50 deletions(-) diff --git a/Makefile b/Makefile index b850ae8..d8ceaef 100644 --- a/Makefile +++ b/Makefile @@ -1,22 +1,21 @@ +.PHONY: help +help: + @echo "Please use \`make ' where is one of" + @grep -E '^\.PHONY: [a-zA-Z_-]+ .*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = "(: |##)"}; {printf "\033[36m%-30s\033[0m %s\n", $$2, $$3}' + .PHONY: dev-setup ## Install development dependencies dev-setup: pip install -e ".[dev]" -.PHONY: install-dev -install-dev: dev-setup # Alias install-dev -> dev-setup - -.PHONY: tests +.PHONY: tests ## Run unit tests tests: py.test graphene_django --cov=graphene_django -vv -.PHONY: test -test: tests # Alias test -> tests - -.PHONY: format +.PHONY: format ## Format code format: black --exclude "/migrations/" graphene_django examples setup.py -.PHONY: lint +.PHONY: lint ## Lint code lint: flake8 graphene_django examples diff --git a/graphene_django/fields.py b/graphene_django/fields.py index c881456..f26f851 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -69,7 +69,10 @@ class DjangoListField(Field): _type = _type.of_type django_object_type = _type.of_type.of_type return partial( - self.list_resolver, django_object_type, resolver, self.get_manager(), + self.list_resolver, + django_object_type, + resolver, + self.get_manager(), ) diff --git a/graphene_django/filter/filters/global_id_filter.py b/graphene_django/filter/filters/global_id_filter.py index a612a8a..da16585 100644 --- a/graphene_django/filter/filters/global_id_filter.py +++ b/graphene_django/filter/filters/global_id_filter.py @@ -13,7 +13,7 @@ class GlobalIDFilter(Filter): field_class = GlobalIDFormField def filter(self, qs, value): - """ Convert the filter value to a primary key before filtering """ + """Convert the filter value to a primary key before filtering""" _id = None if value is not None: _, _id = from_global_id(value) diff --git a/graphene_django/filter/filterset.py b/graphene_django/filter/filterset.py index b3333bf..57c35af 100644 --- a/graphene_django/filter/filterset.py +++ b/graphene_django/filter/filterset.py @@ -18,8 +18,8 @@ GRAPHENE_FILTER_SET_OVERRIDES = { class GrapheneFilterSetMixin(BaseFilterSet): - """ A django_filters.filterset.BaseFilterSet with default filter overrides - to handle global IDs """ + """A django_filters.filterset.BaseFilterSet with default filter overrides + to handle global IDs""" FILTER_DEFAULTS = dict( itertools.chain( @@ -29,8 +29,7 @@ class GrapheneFilterSetMixin(BaseFilterSet): def setup_filterset(filterset_class): - """ Wrap a provided filterset in Graphene-specific functionality - """ + """Wrap a provided filterset in Graphene-specific functionality""" return type( "Graphene{}".format(filterset_class.__name__), (filterset_class, GrapheneFilterSetMixin), @@ -39,8 +38,7 @@ def setup_filterset(filterset_class): def custom_filterset_factory(model, filterset_base_class=FilterSet, **meta): - """ Create a filterset for the given model using the provided meta data - """ + """Create a filterset for the given model using the provided meta data""" meta.update({"model": model}) meta_class = type(str("Meta"), (object,), meta) filterset = type( diff --git a/graphene_django/filter/tests/conftest.py b/graphene_django/filter/tests/conftest.py index 57924af..e2bba68 100644 --- a/graphene_django/filter/tests/conftest.py +++ b/graphene_django/filter/tests/conftest.py @@ -89,10 +89,10 @@ def Query(EventType): def resolve_events(self, info, **kwargs): events = [ - Event(name="Live Show", tags=["concert", "music", "rock"],), - Event(name="Musical", tags=["movie", "music"],), - Event(name="Ballet", tags=["concert", "dance"],), - Event(name="Speech", tags=[],), + Event(name="Live Show", tags=["concert", "music", "rock"]), + Event(name="Musical", tags=["movie", "music"]), + Event(name="Ballet", tags=["concert", "dance"]), + Event(name="Speech", tags=[]), ] STORE["events"] = events diff --git a/graphene_django/filter/tests/test_enum_filtering.py b/graphene_django/filter/tests/test_enum_filtering.py index 4fe7ddd..a284d08 100644 --- a/graphene_django/filter/tests/test_enum_filtering.py +++ b/graphene_django/filter/tests/test_enum_filtering.py @@ -54,13 +54,13 @@ def reporter_article_data(): first_name="Jane", last_name="Doe", email="janedoe@example.com", a_choice=2 ) Article.objects.create( - headline="Article Node 1", reporter=john, editor=john, lang="es", + headline="Article Node 1", reporter=john, editor=john, lang="es" ) Article.objects.create( - headline="Article Node 2", reporter=john, editor=john, lang="en", + headline="Article Node 2", reporter=john, editor=john, lang="en" ) Article.objects.create( - headline="Article Node 3", reporter=jane, editor=jane, lang="en", + headline="Article Node 3", reporter=jane, editor=jane, lang="en" ) @@ -80,7 +80,13 @@ def test_filter_enum_on_connection(schema, reporter_article_data): } """ - expected = {"allArticles": {"edges": [{"node": {"headline": "Article Node 1"}},]}} + expected = { + "allArticles": { + "edges": [ + {"node": {"headline": "Article Node 1"}}, + ] + } + } result = schema.execute(query) assert not result.errors diff --git a/graphene_django/filter/tests/test_fields.py b/graphene_django/filter/tests/test_fields.py index 370c894..fe4ae87 100644 --- a/graphene_django/filter/tests/test_fields.py +++ b/graphene_django/filter/tests/test_fields.py @@ -1224,7 +1224,7 @@ def test_filter_filterset_based_on_mixin(): } } - result = schema.execute(query, variable_values={"email": reporter_1.email},) + result = schema.execute(query, variable_values={"email": reporter_1.email}) assert not result.errors assert result.data == expected @@ -1265,13 +1265,23 @@ def test_filter_string_contains(): result = schema.execute(query, variables={"filter": "Ja"}) assert not result.errors assert result.data == { - "people": {"edges": [{"node": {"name": "Jack"}}, {"node": {"name": "Jane"}},]} + "people": { + "edges": [ + {"node": {"name": "Jack"}}, + {"node": {"name": "Jane"}}, + ] + } } result = schema.execute(query, variables={"filter": "o"}) assert not result.errors assert result.data == { - "people": {"edges": [{"node": {"name": "Joe"}}, {"node": {"name": "Bob"}},]} + "people": { + "edges": [ + {"node": {"name": "Joe"}}, + {"node": {"name": "Bob"}}, + ] + } } diff --git a/graphene_django/filter/tests/test_typed_filter.py b/graphene_django/filter/tests/test_typed_filter.py index cc0bafe..a7edc56 100644 --- a/graphene_django/filter/tests/test_typed_filter.py +++ b/graphene_django/filter/tests/test_typed_filter.py @@ -103,15 +103,9 @@ def test_typed_filter_schema(schema): def test_typed_filters_work(schema): reporter = Reporter.objects.create(first_name="John", last_name="Doe", email="") - Article.objects.create( - headline="A", reporter=reporter, editor=reporter, lang="es", - ) - Article.objects.create( - headline="B", reporter=reporter, editor=reporter, lang="es", - ) - Article.objects.create( - headline="C", reporter=reporter, editor=reporter, lang="en", - ) + Article.objects.create(headline="A", reporter=reporter, editor=reporter, lang="es") + Article.objects.create(headline="B", reporter=reporter, editor=reporter, lang="es") + Article.objects.create(headline="C", reporter=reporter, editor=reporter, lang="en") query = "query { articles (lang_In: [ES]) { edges { node { headline } } } }" diff --git a/graphene_django/filter/utils.py b/graphene_django/filter/utils.py index cd05a87..ebd2a00 100644 --- a/graphene_django/filter/utils.py +++ b/graphene_django/filter/utils.py @@ -94,7 +94,9 @@ def get_filtering_args_from_filterset(filterset_class, type): field_type = graphene.List(field_type) args[name] = graphene.Argument( - field_type, description=filter_field.label, required=required, + field_type, + description=filter_field.label, + required=required, ) return args diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 5cbf90e..f1815d7 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1480,7 +1480,11 @@ def test_connection_should_enable_offset_filtering(): result = schema.execute(query) assert not result.errors expected = { - "allReporters": {"edges": [{"node": {"firstName": "Some", "lastName": "Guy"}},]} + "allReporters": { + "edges": [ + {"node": {"firstName": "Some", "lastName": "Guy"}}, + ] + } } assert result.data == expected @@ -1521,7 +1525,9 @@ def test_connection_should_enable_offset_filtering_higher_than_max_limit( assert not result.errors expected = { "allReporters": { - "edges": [{"node": {"firstName": "Some", "lastName": "Lady"}},] + "edges": [ + {"node": {"firstName": "Some", "lastName": "Lady"}}, + ] } } assert result.data == expected @@ -1590,7 +1596,11 @@ def test_connection_should_allow_offset_filtering_with_after(): result = schema.execute(query, variable_values=dict(after=after)) assert not result.errors expected = { - "allReporters": {"edges": [{"node": {"firstName": "Jane", "lastName": "Roe"}},]} + "allReporters": { + "edges": [ + {"node": {"firstName": "Jane", "lastName": "Roe"}}, + ] + } } assert result.data == expected diff --git a/graphene_django/types.py b/graphene_django/types.py index d272412..c256f1d 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -216,7 +216,7 @@ class DjangoObjectType(ObjectType): "Creating a DjangoObjectType without either the `fields` " "or the `exclude` option is deprecated. Add an explicit `fields " "= '__all__'` option on DjangoObjectType {class_name} to use all " - "fields".format(class_name=cls.__name__,), + "fields".format(class_name=cls.__name__), DeprecationWarning, stacklevel=2, ) diff --git a/graphene_django/utils/tests/test_str_converters.py b/graphene_django/utils/tests/test_str_converters.py index 6460c4e..d3d33c2 100644 --- a/graphene_django/utils/tests/test_str_converters.py +++ b/graphene_django/utils/tests/test_str_converters.py @@ -6,4 +6,4 @@ def test_to_const(): def test_to_const_unicode(): - assert to_const(u"Skoða þetta unicode stöff") == "SKODA_THETTA_UNICODE_STOFF" + assert to_const("Skoða þetta unicode stöff") == "SKODA_THETTA_UNICODE_STOFF" diff --git a/setup.py b/setup.py index 306ec33..d9aefef 100644 --- a/setup.py +++ b/setup.py @@ -14,22 +14,22 @@ rest_framework_require = ["djangorestframework>=3.6.3"] tests_require = [ - "pytest>=3.6.3", + "pytest>=7.1.3", "pytest-cov", "pytest-random-order", "coveralls", "mock", "pytz", - "django-filter>=2", - "pytest-django>=3.3.2", + "django-filter>=22.1", + "pytest-django>=4.5.2", ] + rest_framework_require dev_requires = [ - "black==19.10b0", - "flake8==3.7.9", - "flake8-black==0.1.1", - "flake8-bugbear==20.1.4", + "black==22.8.0", + "flake8==5.0.4", + "flake8-black==0.3.3", + "flake8-bugbear==22.9.11", ] + tests_require setup( From 3473fe025e1efb5de97040087dbe6edf050ef373 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Thu, 22 Sep 2022 16:01:28 +0100 Subject: [PATCH 05/12] fix: backward pagination (#1346) Co-authored-by: Thomas Leonard Co-authored-by: Laurent --- graphene_django/fields.py | 22 ++++++------ graphene_django/tests/test_query.py | 53 ++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/graphene_django/fields.py b/graphene_django/fields.py index f26f851..3c48595 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -152,22 +152,24 @@ class DjangoConnectionField(ConnectionField): array_length = iterable.count() else: array_length = len(iterable) - array_slice_length = ( - min(max_limit, array_length) if max_limit is not None else array_length - ) - # If after is higher than list_length, connection_from_list_slice + # If after is higher than array_length, connection_from_array_slice # would try to do a negative slicing which makes django throw an # AssertionError slice_start = min( - get_offset_with_default(args.get("after"), -1) + 1, array_length + get_offset_with_default(args.get("after"), -1) + 1, + array_length, ) + array_slice_length = array_length - slice_start - if max_limit is not None and args.get("first", None) is None: - if args.get("last", None) is not None: - slice_start = max(array_length - args["last"], 0) - else: - args["first"] = max_limit + # Impose the maximum limit via the `first` field if neither first or last are already provided + # (note that if any of them is provided they must be under max_limit otherwise an error is raised). + if ( + max_limit is not None + and args.get("first", None) is None + and args.get("last", None) is None + ): + args["first"] = max_limit connection = connection_from_array_slice( iterable[slice_start:], diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index f1815d7..207c211 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1243,6 +1243,7 @@ def test_should_have_next_page(graphene_settings): } +@pytest.mark.parametrize("max_limit", [100, 4]) class TestBackwardPagination: def setup_schema(self, graphene_settings, max_limit): graphene_settings.RELAY_CONNECTION_MAX_LIMIT = max_limit @@ -1261,8 +1262,8 @@ class TestBackwardPagination: schema = graphene.Schema(query=Query) return schema - def do_queries(self, schema): - # Simply last 3 + def test_query_last(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) query_last = """ query { allReporters(last: 3) { @@ -1282,7 +1283,8 @@ class TestBackwardPagination: e["node"]["firstName"] for e in result.data["allReporters"]["edges"] ] == ["First 3", "First 4", "First 5"] - # Use a combination of first and last + def test_query_first_and_last(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) query_first_and_last = """ query { allReporters(first: 4, last: 3) { @@ -1302,7 +1304,8 @@ class TestBackwardPagination: 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 + def test_query_first_last_and_after(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) query_first_last_and_after = """ query queryAfter($after: String) { allReporters(first: 4, last: 3, after: $after) { @@ -1317,7 +1320,8 @@ class TestBackwardPagination: after = base64.b64encode(b"arrayconnection:0").decode() result = schema.execute( - query_first_last_and_after, variable_values=dict(after=after) + query_first_last_and_after, + variable_values=dict(after=after), ) assert not result.errors assert len(result.data["allReporters"]["edges"]) == 3 @@ -1325,20 +1329,35 @@ class TestBackwardPagination: e["node"]["firstName"] for e in result.data["allReporters"]["edges"] ] == ["First 2", "First 3", "First 4"] - def test_should_query(self, graphene_settings): + def test_query_last_and_before(self, graphene_settings, max_limit): + schema = self.setup_schema(graphene_settings, max_limit=max_limit) + query_first_last_and_after = """ + query queryAfter($before: String) { + allReporters(last: 1, before: $before) { + edges { + node { + firstName + } + } + } + } """ - 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) + result = schema.execute( + query_first_last_and_after, + ) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 1 + assert result.data["allReporters"]["edges"][0]["node"]["firstName"] == "First 5" + + before = base64.b64encode(b"arrayconnection:5").decode() + result = schema.execute( + query_first_last_and_after, + variable_values=dict(before=before), + ) + assert not result.errors + assert len(result.data["allReporters"]["edges"]) == 1 + assert result.data["allReporters"]["edges"][0]["node"]["firstName"] == "First 4" def test_should_preserve_prefetch_related(django_assert_num_queries): From 37848fa2dfab6feaa2d91d8e41adf7a619b4eb67 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Thu, 22 Sep 2022 19:09:11 +0100 Subject: [PATCH 06/12] fix: convert Django BigIntegerField to BigInt GraphQL type (#1318) Co-authored-by: Thomas Leonard --- graphene_django/converter.py | 7 ++++++- graphene_django/tests/test_converter.py | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/graphene_django/converter.py b/graphene_django/converter.py index c243e82..90b1466 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -24,6 +24,7 @@ from graphene import ( Decimal, ) from graphene.types.json import JSONString +from graphene.types.scalars import BigInt from graphene.utils.str_converters import to_camel_case from graphql import GraphQLError, assert_valid_name from graphql.pyutils import register_description @@ -186,10 +187,14 @@ def convert_field_to_uuid(field, registry=None): ) +@convert_django_field.register(models.BigIntegerField) +def convert_big_int_field(field, registry=None): + return BigInt(description=field.help_text, required=not field.null) + + @convert_django_field.register(models.PositiveIntegerField) @convert_django_field.register(models.PositiveSmallIntegerField) @convert_django_field.register(models.SmallIntegerField) -@convert_django_field.register(models.BigIntegerField) @convert_django_field.register(models.IntegerField) def convert_field_to_int(field, registry=None): return Int(description=get_django_field_description(field), required=not field.null) diff --git a/graphene_django/tests/test_converter.py b/graphene_django/tests/test_converter.py index afd744f..9158b12 100644 --- a/graphene_django/tests/test_converter.py +++ b/graphene_django/tests/test_converter.py @@ -10,6 +10,7 @@ from graphene import NonNull from graphene.relay import ConnectionField, Node from graphene.types.datetime import Date, DateTime, Time from graphene.types.json import JSONString +from graphene.types.scalars import BigInt from ..compat import ( ArrayField, @@ -140,8 +141,8 @@ def test_should_small_integer_convert_int(): assert_conversion(models.SmallIntegerField, graphene.Int) -def test_should_big_integer_convert_int(): - assert_conversion(models.BigIntegerField, graphene.Int) +def test_should_big_integer_convert_big_int(): + assert_conversion(models.BigIntegerField, BigInt) def test_should_integer_convert_int(): From a53ded611bbe1519742a5254e4cbfa12af0cc2d9 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Thu, 22 Sep 2022 19:09:29 +0100 Subject: [PATCH 07/12] feat: update name of DjangoFilterConnectionField type input to be consistent with graphene (Issue #1316) (#1317) Co-authored-by: Thomas Leonard --- graphene_django/filter/fields.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphene_django/filter/fields.py b/graphene_django/filter/fields.py index c6dd50e..eeb197e 100644 --- a/graphene_django/filter/fields.py +++ b/graphene_django/filter/fields.py @@ -30,7 +30,7 @@ def convert_enum(data): class DjangoFilterConnectionField(DjangoConnectionField): def __init__( self, - type, + type_, fields=None, order_by=None, extra_filter_meta=None, @@ -44,7 +44,7 @@ class DjangoFilterConnectionField(DjangoConnectionField): self._filtering_args = None self._extra_filter_meta = extra_filter_meta self._base_args = None - super(DjangoFilterConnectionField, self).__init__(type, *args, **kwargs) + super(DjangoFilterConnectionField, self).__init__(type_, *args, **kwargs) @property def args(self): From 4f315c365d36c54b0803a9a029d534ae3bf03fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yi=C4=9Fit=20Y=2E=20Er?= Date: Thu, 22 Sep 2022 21:10:52 +0300 Subject: [PATCH 08/12] minor fix on schema.py part (#1306) The documentation already suggests importing ObjectType from graphene, graphene.ObjectType is not necessary while defining the Query class. --- docs/tutorial-relay.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tutorial-relay.rst b/docs/tutorial-relay.rst index 3de9022..a27e255 100644 --- a/docs/tutorial-relay.rst +++ b/docs/tutorial-relay.rst @@ -151,7 +151,7 @@ Create ``cookbook/ingredients/schema.py`` and type the following: interfaces = (relay.Node, ) - class Query(graphene.ObjectType): + class Query(ObjectType): category = relay.Node.Field(CategoryNode) all_categories = DjangoFilterConnectionField(CategoryNode) From 56892d7f4b2c4d237f726092a5ec5987631569e6 Mon Sep 17 00:00:00 2001 From: andrei-datcu Date: Thu, 22 Sep 2022 12:13:30 -0600 Subject: [PATCH 09/12] Cast translated description for DecimalField (#1255) * Cast translated description for DecimalField https://github.com/graphql-python/graphene-django/pull/976 casts all the description fields to strings to prevent schema printing from failing whenever the description is a lazy translated string. The `DecimalField` however got in after the v3 merge and it currently misses the cast. * Fix row size --- graphene_django/converter.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 90b1466..2c4fa19 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -210,7 +210,9 @@ 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) + return Decimal( + description=get_django_field_description(field), required=not field.null + ) @convert_django_field.register(models.FloatField) From b2f83eb277ddaca49ad3320038e6baf381890a3e Mon Sep 17 00:00:00 2001 From: Firas K <3097061+firaskafri@users.noreply.github.com> Date: Fri, 23 Sep 2022 11:38:11 +0300 Subject: [PATCH 10/12] Bump version to 3.0.0b8 (#1348) --- 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 999f3de..93a697a 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__ = "3.0.0b7" +__version__ = "3.0.0b8" __all__ = [ "__version__", From 5d81ba04f9cc4643f537ba56aac224f5862f0d38 Mon Sep 17 00:00:00 2001 From: Thomas Leonard <64223923+tcleonard@users.noreply.github.com> Date: Fri, 23 Sep 2022 09:45:02 +0100 Subject: [PATCH 11/12] fix: unit test for graphene pr#1412 (#1315) * Issue #1111: foreign key should also call get_queryset method * fix: test for graphene PR https://github.com/graphql-python/graphene/pull/1412 Co-authored-by: Thomas Leonard --- graphene_django/converter.py | 19 +- graphene_django/tests/models.py | 3 + graphene_django/tests/test_get_queryset.py | 355 +++++++++++++++++++++ graphene_django/tests/test_query.py | 70 +++- setup.py | 3 +- 5 files changed, 445 insertions(+), 5 deletions(-) create mode 100644 graphene_django/tests/test_get_queryset.py diff --git a/graphene_django/converter.py b/graphene_django/converter.py index 2c4fa19..338ab6d 100644 --- a/graphene_django/converter.py +++ b/graphene_django/converter.py @@ -308,7 +308,24 @@ def convert_field_to_djangomodel(field, registry=None): if not _type: return - return Field( + class CustomField(Field): + def wrap_resolve(self, parent_resolver): + """ + Implements a custom resolver which go through the `get_node` method to insure that + it goes through the `get_queryset` method of the DjangoObjectType. + """ + resolver = super().wrap_resolve(parent_resolver) + + def custom_resolver(root, info, **args): + fk_obj = resolver(root, info, **args) + if fk_obj is None: + return None + else: + return _type.get_node(info, fk_obj.pk) + + return custom_resolver + + return CustomField( _type, description=get_django_field_description(field), required=not field.null, diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py index 7b76cd3..c26a6d8 100644 --- a/graphene_django/tests/models.py +++ b/graphene_django/tests/models.py @@ -13,6 +13,9 @@ class Person(models.Model): class Pet(models.Model): name = models.CharField(max_length=30) age = models.PositiveIntegerField() + owner = models.ForeignKey( + "Person", on_delete=models.CASCADE, null=True, blank=True, related_name="pets" + ) class FilmDetails(models.Model): diff --git a/graphene_django/tests/test_get_queryset.py b/graphene_django/tests/test_get_queryset.py new file mode 100644 index 0000000..b2647c3 --- /dev/null +++ b/graphene_django/tests/test_get_queryset.py @@ -0,0 +1,355 @@ +import pytest + +import graphene +from graphene.relay import Node + +from graphql_relay import to_global_id + +from ..fields import DjangoConnectionField +from ..types import DjangoObjectType + +from .models import Article, Reporter + + +class TestShouldCallGetQuerySetOnForeignKey: + """ + Check that the get_queryset method is called in both forward and reversed direction + of a foreignkey on types. + (see issue #1111) + """ + + @pytest.fixture(autouse=True) + def setup_schema(self): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + + @classmethod + def get_queryset(cls, queryset, info): + if info.context and info.context.get("admin"): + return queryset + raise Exception("Not authorized to access reporters.") + + class ArticleType(DjangoObjectType): + class Meta: + model = Article + + @classmethod + def get_queryset(cls, queryset, info): + return queryset.exclude(headline__startswith="Draft") + + class Query(graphene.ObjectType): + reporter = graphene.Field(ReporterType, id=graphene.ID(required=True)) + article = graphene.Field(ArticleType, id=graphene.ID(required=True)) + + def resolve_reporter(self, info, id): + return ( + ReporterType.get_queryset(Reporter.objects, info) + .filter(id=id) + .last() + ) + + def resolve_article(self, info, id): + return ( + ArticleType.get_queryset(Article.objects, info).filter(id=id).last() + ) + + self.schema = graphene.Schema(query=Query) + + self.reporter = Reporter.objects.create(first_name="Jane", last_name="Doe") + + self.articles = [ + Article.objects.create( + headline="A fantastic article", + reporter=self.reporter, + editor=self.reporter, + ), + Article.objects.create( + headline="Draft: My next best seller", + reporter=self.reporter, + editor=self.reporter, + ), + ] + + def test_get_queryset_called_on_field(self): + # If a user tries to access an article it is fine as long as it's not a draft one + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + } + } + """ + # Non-draft + result = self.schema.execute(query, variables={"id": self.articles[0].id}) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + } + # Draft + result = self.schema.execute(query, variables={"id": self.articles[1].id}) + assert not result.errors + assert result.data["article"] is None + + # If a non admin user tries to access a reporter they should get our authorization error + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute(query, variables={"id": self.reporter.id}) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute( + query, variables={"id": self.reporter.id}, context_value={"admin": True}, + ) + assert not result.errors + assert result.data == {"reporter": {"firstName": "Jane"}} + + def test_get_queryset_called_on_foreignkey(self): + # If a user tries to access a reporter through an article they should get our authorization error + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute(query, variables={"id": self.articles[0].id}) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters through an article + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute( + query, variables={"id": self.articles[0].id}, context_value={"admin": True}, + ) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + "reporter": {"firstName": "Jane"}, + } + + # An admin user should not be able to access draft article through a reporter + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + articles { + headline + } + } + } + """ + + result = self.schema.execute( + query, variables={"id": self.reporter.id}, context_value={"admin": True}, + ) + assert not result.errors + assert result.data["reporter"] == { + "firstName": "Jane", + "articles": [{"headline": "A fantastic article"}], + } + + +class TestShouldCallGetQuerySetOnForeignKeyNode: + """ + Check that the get_queryset method is called in both forward and reversed direction + of a foreignkey on types using a node interface. + (see issue #1111) + """ + + @pytest.fixture(autouse=True) + def setup_schema(self): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + @classmethod + def get_queryset(cls, queryset, info): + if info.context and info.context.get("admin"): + return queryset + raise Exception("Not authorized to access reporters.") + + class ArticleType(DjangoObjectType): + class Meta: + model = Article + interfaces = (Node,) + + @classmethod + def get_queryset(cls, queryset, info): + return queryset.exclude(headline__startswith="Draft") + + class Query(graphene.ObjectType): + reporter = Node.Field(ReporterType) + article = Node.Field(ArticleType) + + self.schema = graphene.Schema(query=Query) + + self.reporter = Reporter.objects.create(first_name="Jane", last_name="Doe") + + self.articles = [ + Article.objects.create( + headline="A fantastic article", + reporter=self.reporter, + editor=self.reporter, + ), + Article.objects.create( + headline="Draft: My next best seller", + reporter=self.reporter, + editor=self.reporter, + ), + ] + + def test_get_queryset_called_on_node(self): + # If a user tries to access an article it is fine as long as it's not a draft one + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + } + } + """ + # Non-draft + result = self.schema.execute( + query, variables={"id": to_global_id("ArticleType", self.articles[0].id)} + ) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + } + # Draft + result = self.schema.execute( + query, variables={"id": to_global_id("ArticleType", self.articles[1].id)} + ) + assert not result.errors + assert result.data["article"] is None + + # If a non admin user tries to access a reporter they should get our authorization error + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute( + query, variables={"id": to_global_id("ReporterType", self.reporter.id)} + ) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + } + } + """ + + result = self.schema.execute( + query, + variables={"id": to_global_id("ReporterType", self.reporter.id)}, + context_value={"admin": True}, + ) + assert not result.errors + assert result.data == {"reporter": {"firstName": "Jane"}} + + def test_get_queryset_called_on_foreignkey(self): + # If a user tries to access a reporter through an article they should get our authorization error + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute( + query, variables={"id": to_global_id("ArticleType", self.articles[0].id)} + ) + assert len(result.errors) == 1 + assert result.errors[0].message == "Not authorized to access reporters." + + # An admin user should be able to get reporters through an article + query = """ + query getArticle($id: ID!) { + article(id: $id) { + headline + reporter { + firstName + } + } + } + """ + + result = self.schema.execute( + query, + variables={"id": to_global_id("ArticleType", self.articles[0].id)}, + context_value={"admin": True}, + ) + assert not result.errors + assert result.data["article"] == { + "headline": "A fantastic article", + "reporter": {"firstName": "Jane"}, + } + + # An admin user should not be able to access draft article through a reporter + query = """ + query getReporter($id: ID!) { + reporter(id: $id) { + firstName + articles { + edges { + node { + headline + } + } + } + } + } + """ + + result = self.schema.execute( + query, + variables={"id": to_global_id("ReporterType", self.reporter.id)}, + context_value={"admin": True}, + ) + assert not result.errors + assert result.data["reporter"] == { + "firstName": "Jane", + "articles": {"edges": [{"node": {"headline": "A fantastic article"}}]}, + } diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 207c211..e6ae64f 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -15,7 +15,7 @@ from ..compat import IntegerRangeField, MissingType from ..fields import DjangoConnectionField from ..types import DjangoObjectType from ..utils import DJANGO_FILTER_INSTALLED -from .models import Article, CNNReporter, Film, FilmDetails, Reporter +from .models import Article, CNNReporter, Film, FilmDetails, Person, Pet, Reporter def test_should_query_only_fields(): @@ -251,8 +251,8 @@ def test_should_node(): def test_should_query_onetoone_fields(): - film = Film(id=1) - film_details = FilmDetails(id=1, film=film) + film = Film.objects.create(id=1) + film_details = FilmDetails.objects.create(id=1, film=film) class FilmNode(DjangoObjectType): class Meta: @@ -1697,3 +1697,67 @@ def test_connection_should_succeed_if_last_higher_than_number_of_objects(): } } assert result.data == expected + + +def test_should_query_nullable_foreign_key(): + class PetType(DjangoObjectType): + class Meta: + model = Pet + + class PersonType(DjangoObjectType): + class Meta: + model = Person + + class Query(graphene.ObjectType): + pet = graphene.Field(PetType, name=graphene.String(required=True)) + person = graphene.Field(PersonType, name=graphene.String(required=True)) + + def resolve_pet(self, info, name): + return Pet.objects.filter(name=name).first() + + def resolve_person(self, info, name): + return Person.objects.filter(name=name).first() + + schema = graphene.Schema(query=Query) + + person = Person.objects.create(name="Jane") + pets = [ + Pet.objects.create(name="Stray dog", age=1), + Pet.objects.create(name="Jane's dog", owner=person, age=1), + ] + + query_pet = """ + query getPet($name: String!) { + pet(name: $name) { + owner { + name + } + } + } + """ + result = schema.execute(query_pet, variables={"name": "Stray dog"}) + assert not result.errors + assert result.data["pet"] == { + "owner": None, + } + + result = schema.execute(query_pet, variables={"name": "Jane's dog"}) + assert not result.errors + assert result.data["pet"] == { + "owner": {"name": "Jane"}, + } + + query_owner = """ + query getOwner($name: String!) { + person(name: $name) { + pets { + name + } + } + } + """ + result = schema.execute(query_owner, variables={"name": "Jane"}) + assert not result.errors + assert result.data["person"] == { + "pets": [{"name": "Jane's dog"}], + } diff --git a/setup.py b/setup.py index d9aefef..3a46d24 100644 --- a/setup.py +++ b/setup.py @@ -59,7 +59,8 @@ setup( keywords="api graphql protocol rest relay graphene", packages=find_packages(exclude=["tests", "examples", "examples.*"]), install_requires=[ - "graphene>=3.0,<4", + # "graphene>=3.0,<4", + "graphene @ git+https://github.com/loft-orbital/graphene.git@loft-v3-1.0#egg=graphene", "graphql-core>=3.1.0,<4", "graphql-relay>=3.1.1,<4", "Django>=3.2", From 0f40da7b317e474bbb0d777e4e96599f3b989903 Mon Sep 17 00:00:00 2001 From: Semyon Pupkov Date: Fri, 23 Sep 2022 13:47:10 +0500 Subject: [PATCH 12/12] Make errors in form mutation non nullable (#1286) --- graphene_django/forms/mutation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphene_django/forms/mutation.py b/graphene_django/forms/mutation.py index 5a3d8e7..13e9863 100644 --- a/graphene_django/forms/mutation.py +++ b/graphene_django/forms/mutation.py @@ -117,7 +117,7 @@ class DjangoModelFormMutation(BaseDjangoFormMutation): class Meta: abstract = True - errors = graphene.List(ErrorType) + errors = graphene.List(graphene.NonNull(ErrorType), required=True) @classmethod def __init_subclass_with_meta__(