diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index a25383f..c32f46c 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -319,7 +319,7 @@ def test_django_objecttype_fields_exclude_type_checking(): @with_local_registry -def test_django_objecttype_fields_exclude_exist_on_model(): +def test_django_objecttype_fields_exist_on_model(): with pytest.warns(UserWarning, match=r"Field name .* doesn't exist"): class Reporter(DjangoObjectType): @@ -350,6 +350,42 @@ def test_django_objecttype_fields_exclude_exist_on_model(): assert len(record) == 0 +@with_local_registry +def test_django_objecttype_exclude_fields_exist_on_model(): + with pytest.warns( + UserWarning, + match=r"Django model .* does not have a field or attribute named .*", + ): + + class Reporter(DjangoObjectType): + class Meta: + model = ReporterModel + exclude = ["foo"] + + # Don't warn if selecting a custom field + with pytest.warns( + UserWarning, + match=r"Excluding the custom field .* on DjangoObjectType .* has no effect.", + ): + + class Reporter3(DjangoObjectType): + custom_field = String() + + class Meta: + model = ReporterModel + exclude = ["custom_field"] + + # Don't warn on exclude fields + with pytest.warns(None) as record: + + class Reporter4(DjangoObjectType): + class Meta: + model = ReporterModel + exclude = ["email", "first_name"] + + assert len(record) == 0 + + class TestDjangoObjectType: @pytest.fixture def PetModel(self): diff --git a/graphene_django/types.py b/graphene_django/types.py index 129dbe1..922790c 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -65,32 +65,58 @@ def construct_fields( def validate_fields(type_, model, fields, only_fields, exclude_fields): # Validate the given fields against the model's fields and custom fields all_field_names = set(fields.keys()) - for fields_list in (only_fields, exclude_fields): - if not fields_list: + for name in only_fields or (): + if name in all_field_names: continue - for name in fields_list: - if name in all_field_names: - continue - if hasattr(model, name): - warnings.warn( - ( - 'Field name "{field_name}" matches an attribute on Django model "{app_label}.{object_name}" ' - "but it's not a model field so Graphene cannot determine what type it should be. " - 'Either define the type of the field on DjangoObjectType "{type_}" or remove it from the "fields" list.' - ).format( - field_name=name, - app_label=model._meta.app_label, - object_name=model._meta.object_name, - type_=type_, - ) + if hasattr(model, name): + warnings.warn( + ( + 'Field name "{field_name}" matches an attribute on Django model "{app_label}.{object_name}" ' + "but it's not a model field so Graphene cannot determine what type it should be. " + 'Either define the type of the field on DjangoObjectType "{type_}" or remove it from the "fields" list.' + ).format( + field_name=name, + app_label=model._meta.app_label, + object_name=model._meta.object_name, + type_=type_, ) + ) - else: + else: + warnings.warn( + ( + 'Field name "{field_name}" doesn\'t exist on Django model "{app_label}.{object_name}". ' + 'Consider removing the field from the "fields" list of DjangoObjectType "{type_}" because it has no effect.' + ).format( + field_name=name, + app_label=model._meta.app_label, + object_name=model._meta.object_name, + type_=type_, + ) + ) + + # Validate exclude fields + for name in exclude_fields or (): + if name in all_field_names: + # Field is a custom field + warnings.warn( + ( + 'Excluding the custom field "{field_name} on DjangoObjectType "{type_}" has no effect. ' + 'Either remove the custom field or remove the field from the "exclude" list.' + ).format( + field_name=name, + app_label=model._meta.app_label, + object_name=model._meta.object_name, + type_=type_, + ) + ) + else: + if not hasattr(model, name): warnings.warn( ( - 'Field name "{field_name}" doesn\'t exist on Django model "{app_label}.{object_name}". ' - 'Consider removing the field from the "fields" list of DjangoObjectType "{type_}" because it has no effect.' + 'Django model "{app_label}.{object_name}" does not have a field or attribute named "{field_name}". ' + 'Consider removing the field from the "exclude" list of DjangoObjectType "{type_}" because it has no effect' ).format( field_name=name, app_label=model._meta.app_label,