-
Notifications
You must be signed in to change notification settings - Fork 768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix: call resolver
function in DjangoConnectionField
as documented
#1529
base: main
Are you sure you want to change the base?
Changes from 3 commits
0a3c2f7
886ce5e
c127aec
9ecf322
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,19 +190,13 @@ def convert_field_to_string(field, registry=None): | |
) | ||
|
||
|
||
@convert_django_field.register(models.BigAutoField) | ||
@convert_django_field.register(models.AutoField) | ||
@convert_django_field.register(models.BigAutoField) | ||
@convert_django_field.register(models.SmallAutoField) | ||
def convert_field_to_id(field, registry=None): | ||
return ID(description=get_django_field_description(field), required=not field.null) | ||
|
||
|
||
Comment on lines
193
to
199
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if hasattr(models, "SmallAutoField"): | ||
|
||
@convert_django_field.register(models.SmallAutoField) | ||
def convert_field_small_to_id(field, registry=None): | ||
return convert_field_to_id(field, registry) | ||
|
||
|
||
Comment on lines
-199
to
-205
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this could be considered a backwards-incompatible change as it removes the (undocumented) So if you want, I can turn this into two PR: one only including c127aec (the bugfix) and the remaining commits that fix the warnings. |
||
@convert_django_field.register(models.UUIDField) | ||
def convert_field_to_uuid(field, registry=None): | ||
return UUID( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
from django import forms | ||
from django import VERSION as DJANGO_VERSION, forms | ||
from pytest import raises | ||
|
||
from graphene import ( | ||
|
@@ -19,12 +19,16 @@ | |
from ..converter import convert_form_field | ||
|
||
|
||
def assert_conversion(django_field, graphene_field, *args): | ||
field = django_field(*args, help_text="Custom Help Text") | ||
def assert_conversion(django_field, graphene_field, *args, **kwargs): | ||
# Arrange | ||
help_text = kwargs.setdefault("help_text", "Custom Help Text") | ||
field = django_field(*args, **kwargs) | ||
# Act | ||
graphene_type = convert_form_field(field) | ||
# Assert | ||
assert isinstance(graphene_type, graphene_field) | ||
field = graphene_type.Field() | ||
assert field.description == "Custom Help Text" | ||
assert field.description == help_text | ||
return field | ||
|
||
|
||
|
@@ -59,7 +63,12 @@ def test_should_slug_convert_string(): | |
|
||
|
||
def test_should_url_convert_string(): | ||
assert_conversion(forms.URLField, String) | ||
kwargs = {} | ||
if DJANGO_VERSION >= (5, 0): | ||
# silence RemovedInDjango60Warning | ||
kwargs["assume_scheme"] = "https" | ||
|
||
assert_conversion(forms.URLField, String, **kwargs) | ||
|
||
|
||
def test_should_choice_convert_string(): | ||
|
@@ -75,8 +84,7 @@ def test_should_regex_convert_string(): | |
|
||
|
||
def test_should_uuid_convert_string(): | ||
if hasattr(forms, "UUIDField"): | ||
assert_conversion(forms.UUIDField, UUID) | ||
assert_conversion(forms.UUIDField, UUID) | ||
|
||
Comment on lines
86
to
+87
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
def test_should_integer_convert_int(): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,8 +96,7 @@ def test_should_regex_convert_string(): | |
|
||
|
||
def test_should_uuid_convert_string(): | ||
if hasattr(serializers, "UUIDField"): | ||
assert_conversion(serializers.UUIDField, graphene.String) | ||
assert_conversion(serializers.UUIDField, graphene.String) | ||
|
||
Comment on lines
98
to
100
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DRF added support for |
||
|
||
def test_should_model_convert_field(): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,9 +53,8 @@ def assert_conversion(django_field, graphene_field, *args, **kwargs): | |
|
||
|
||
def test_should_unknown_django_field_raise_exception(): | ||
with raises(Exception) as excinfo: | ||
with raises(Exception, match="Don't know how to convert the Django field"): | ||
convert_django_field(None) | ||
assert "Don't know how to convert the Django field" in str(excinfo.value) | ||
|
||
|
||
def test_should_date_time_convert_string(): | ||
|
@@ -115,8 +114,7 @@ def test_should_big_auto_convert_id(): | |
|
||
|
||
def test_should_small_auto_convert_id(): | ||
if hasattr(models, "SmallAutoField"): | ||
assert_conversion(models.SmallAutoField, graphene.ID, primary_key=True) | ||
assert_conversion(models.SmallAutoField, graphene.ID, primary_key=True) | ||
|
||
|
||
def test_should_uuid_convert_id(): | ||
|
@@ -166,14 +164,14 @@ def test_field_with_choices_convert_enum(): | |
help_text="Language", choices=(("es", "Spanish"), ("en", "English")) | ||
) | ||
|
||
class TranslatedModel(models.Model): | ||
class ChoicesModel(models.Model): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes a Django warning about the
|
||
language = field | ||
|
||
class Meta: | ||
app_label = "test" | ||
|
||
graphene_type = convert_django_field_with_choices(field).type.of_type | ||
assert graphene_type._meta.name == "TestTranslatedModelLanguageChoices" | ||
assert graphene_type._meta.name == "TestChoicesModelLanguageChoices" | ||
assert graphene_type._meta.enum.__members__["ES"].value == "es" | ||
assert graphene_type._meta.enum.__members__["ES"].description == "Spanish" | ||
assert graphene_type._meta.enum.__members__["EN"].value == "en" | ||
|
@@ -186,14 +184,14 @@ def get_choices(): | |
|
||
field = models.CharField(help_text="Language", choices=get_choices) | ||
|
||
class TranslatedModel(models.Model): | ||
class CallableChoicesModel(models.Model): | ||
language = field | ||
|
||
class Meta: | ||
app_label = "test" | ||
|
||
graphene_type = convert_django_field_with_choices(field).type.of_type | ||
assert graphene_type._meta.name == "TestTranslatedModelLanguageChoices" | ||
assert graphene_type._meta.name == "TestCallableChoicesModelLanguageChoices" | ||
assert graphene_type._meta.enum.__members__["ES"].value == "es" | ||
assert graphene_type._meta.enum.__members__["ES"].description == "Spanish" | ||
assert graphene_type._meta.enum.__members__["EN"].value == "en" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolve_only_args
is deprecated