From fede977f8af70788b4ee6af8e82592ea0a559de0 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Thu, 11 Apr 2024 21:53:08 +0800 Subject: [PATCH 1/2] Revert "feat!: check django model has a default ordering when used in a relay connection (#1495)" This reverts commit 96c09ac439985d9548678a08221e86056ec1e703. --- examples/starwars/models.py | 3 -- graphene_django/fields.py | 8 +----- graphene_django/filter/tests/conftest.py | 3 -- graphene_django/tests/models.py | 12 -------- graphene_django/tests/test_fields.py | 36 ++---------------------- graphene_django/tests/test_types.py | 23 ++++++++------- 6 files changed, 16 insertions(+), 69 deletions(-) diff --git a/examples/starwars/models.py b/examples/starwars/models.py index c49206a4f..fb76b0395 100644 --- a/examples/starwars/models.py +++ b/examples/starwars/models.py @@ -24,9 +24,6 @@ def __str__(self): class Ship(models.Model): - class Meta: - ordering = ["pk"] - name = models.CharField(max_length=50) faction = models.ForeignKey(Faction, on_delete=models.CASCADE, related_name="ships") diff --git a/graphene_django/fields.py b/graphene_django/fields.py index a1b9a2ccb..1bbe1f3d2 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -101,19 +101,13 @@ def type(self): non_null = True assert issubclass( _type, DjangoObjectType - ), "DjangoConnectionField only accepts DjangoObjectType types as underlying type" + ), "DjangoConnectionField only accepts DjangoObjectType types" assert _type._meta.connection, "The type {} doesn't have a connection".format( _type.__name__ ) connection_type = _type._meta.connection if non_null: return NonNull(connection_type) - # Since Relay Connections require to have a predictible ordering for pagination, - # check on init that the Django model provided has a default ordering declared. - model = connection_type._meta.node._meta.model - assert ( - len(getattr(model._meta, "ordering", [])) > 0 - ), f"Django model {model._meta.app_label}.{model.__name__} has to have a default ordering to be used in a Connection." return connection_type @property diff --git a/graphene_django/filter/tests/conftest.py b/graphene_django/filter/tests/conftest.py index 8824042e9..a4097b183 100644 --- a/graphene_django/filter/tests/conftest.py +++ b/graphene_django/filter/tests/conftest.py @@ -26,9 +26,6 @@ class Event(models.Model): - class Meta: - ordering = ["pk"] - name = models.CharField(max_length=50) tags = ArrayField(models.CharField(max_length=50)) tag_ids = ArrayField(models.IntegerField()) diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py index 821b3701f..ece1bb6db 100644 --- a/graphene_django/tests/models.py +++ b/graphene_django/tests/models.py @@ -5,9 +5,6 @@ class Person(models.Model): - class Meta: - ordering = ["pk"] - name = models.CharField(max_length=30) parent = models.ForeignKey( "self", on_delete=models.CASCADE, null=True, blank=True, related_name="children" @@ -15,9 +12,6 @@ class Meta: class Pet(models.Model): - class Meta: - ordering = ["pk"] - name = models.CharField(max_length=30) age = models.PositiveIntegerField() owner = models.ForeignKey( @@ -37,9 +31,6 @@ class FilmDetails(models.Model): class Film(models.Model): - class Meta: - ordering = ["pk"] - genre = models.CharField( max_length=2, help_text="Genre", @@ -55,9 +46,6 @@ def get_queryset(self): class Reporter(models.Model): - class Meta: - ordering = ["pk"] - first_name = models.CharField(max_length=30) last_name = models.CharField(max_length=30) email = models.EmailField() diff --git a/graphene_django/tests/test_fields.py b/graphene_django/tests/test_fields.py index caaa6ddf7..0f5b79a0b 100644 --- a/graphene_django/tests/test_fields.py +++ b/graphene_django/tests/test_fields.py @@ -2,12 +2,11 @@ import re import pytest -from django.db.models import Count, Model, Prefetch +from django.db.models import Count, Prefetch from graphene import List, NonNull, ObjectType, Schema, String -from graphene.relay import Node -from ..fields import DjangoConnectionField, DjangoListField +from ..fields import DjangoListField from ..types import DjangoObjectType from .models import ( Article as ArticleModel, @@ -717,34 +716,3 @@ def resolve_articles(root, info): r'SELECT .* FROM "tests_film" INNER JOIN "tests_film_reporters" .* LEFT OUTER JOIN "tests_filmdetails"', captured.captured_queries[1]["sql"], ) - - -class TestDjangoConnectionField: - def test_model_ordering_assertion(self): - class Chaos(Model): - class Meta: - app_label = "test" - - class ChaosType(DjangoObjectType): - class Meta: - model = Chaos - interfaces = (Node,) - - class Query(ObjectType): - chaos = DjangoConnectionField(ChaosType) - - with pytest.raises( - TypeError, - match=r"Django model test\.Chaos has to have a default ordering to be used in a Connection\.", - ): - Schema(query=Query) - - def test_only_django_object_types(self): - class Query(ObjectType): - something = DjangoConnectionField(String) - - with pytest.raises( - TypeError, - match="DjangoConnectionField only accepts DjangoObjectType types as underlying type", - ): - Schema(query=Query) diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index 72514d23b..5c36bb92e 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -1,4 +1,3 @@ -import warnings from collections import OrderedDict, defaultdict from textwrap import dedent from unittest.mock import patch @@ -400,7 +399,7 @@ class Meta: with pytest.warns( UserWarning, match=r"Field name .* matches an attribute on Django model .* but it's not a model field", - ): + ) as record: class Reporter2(DjangoObjectType): class Meta: @@ -408,8 +407,7 @@ class Meta: fields = ["first_name", "some_method", "email"] # Don't warn if selecting a custom field - with warnings.catch_warnings(): - warnings.simplefilter("error") + with pytest.warns(None) as record: class Reporter3(DjangoObjectType): custom_field = String() @@ -418,6 +416,8 @@ class Meta: model = ReporterModel fields = ["first_name", "custom_field", "email"] + assert len(record) == 0 + @with_local_registry def test_django_objecttype_exclude_fields_exist_on_model(): @@ -445,14 +445,15 @@ class Meta: exclude = ["custom_field"] # Don't warn on exclude fields - with warnings.catch_warnings(): - warnings.simplefilter("error") + with pytest.warns(None) as record: class Reporter4(DjangoObjectType): class Meta: model = ReporterModel exclude = ["email", "first_name"] + assert len(record) == 0 + @with_local_registry def test_django_objecttype_neither_fields_nor_exclude(): @@ -466,22 +467,24 @@ class Reporter(DjangoObjectType): class Meta: model = ReporterModel - with warnings.catch_warnings(): - warnings.simplefilter("error") + with pytest.warns(None) as record: class Reporter2(DjangoObjectType): class Meta: model = ReporterModel fields = ["email"] - with warnings.catch_warnings(): - warnings.simplefilter("error") + assert len(record) == 0 + + with pytest.warns(None) as record: class Reporter3(DjangoObjectType): class Meta: model = ReporterModel exclude = ["email"] + assert len(record) == 0 + def custom_enum_name(field): return f"CustomEnum{field.name.title()}" From d03822ed83c9843fde41d72fc4a41ef14cce4f5f Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 16 Apr 2024 12:00:48 +0800 Subject: [PATCH 2/2] Fix assert no warning for pytest>=8 --- graphene_django/tests/test_types.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index 5c36bb92e..72514d23b 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -1,3 +1,4 @@ +import warnings from collections import OrderedDict, defaultdict from textwrap import dedent from unittest.mock import patch @@ -399,7 +400,7 @@ class Meta: with pytest.warns( UserWarning, match=r"Field name .* matches an attribute on Django model .* but it's not a model field", - ) as record: + ): class Reporter2(DjangoObjectType): class Meta: @@ -407,7 +408,8 @@ class Meta: fields = ["first_name", "some_method", "email"] # Don't warn if selecting a custom field - with pytest.warns(None) as record: + with warnings.catch_warnings(): + warnings.simplefilter("error") class Reporter3(DjangoObjectType): custom_field = String() @@ -416,8 +418,6 @@ class Meta: model = ReporterModel fields = ["first_name", "custom_field", "email"] - assert len(record) == 0 - @with_local_registry def test_django_objecttype_exclude_fields_exist_on_model(): @@ -445,15 +445,14 @@ class Meta: exclude = ["custom_field"] # Don't warn on exclude fields - with pytest.warns(None) as record: + with warnings.catch_warnings(): + warnings.simplefilter("error") class Reporter4(DjangoObjectType): class Meta: model = ReporterModel exclude = ["email", "first_name"] - assert len(record) == 0 - @with_local_registry def test_django_objecttype_neither_fields_nor_exclude(): @@ -467,24 +466,22 @@ class Reporter(DjangoObjectType): class Meta: model = ReporterModel - with pytest.warns(None) as record: + with warnings.catch_warnings(): + warnings.simplefilter("error") class Reporter2(DjangoObjectType): class Meta: model = ReporterModel fields = ["email"] - assert len(record) == 0 - - with pytest.warns(None) as record: + with warnings.catch_warnings(): + warnings.simplefilter("error") class Reporter3(DjangoObjectType): class Meta: model = ReporterModel exclude = ["email"] - assert len(record) == 0 - def custom_enum_name(field): return f"CustomEnum{field.name.title()}"