Repository: graphql-python/graphene-django

Issue:
Validate Meta.fields and Meta.exclude on DjangoObjectType
tl;dr: DjangoObjectType ignores all unknown values in `Meta.fields`. It should compare the fields list with the available Model's fields instead.

---

I'm in the process of rewriting DRF-based backend to graphene-django, and I was surprised when my graphene-django generated schema was silently missing the fields I specified in `fields`.

(I'm copy-pasting `fields` from DRF serializers to DjangoObjectType's Meta class).

Turns out some of these fields were implemented as properties or methods on models, and I'm ok with writing custom resolvers for those (otherwise there's no way to detect types, at least in the absence of type hints), but I didn't expect DjangoObjectType to quietly accept unknown values.

I believe the reason for this is that `graphene_django.types.construct_fields` iterates over model's fields, but it could/should iterate over `only_fields` too.

Implementing the same check for `exclude` also seems like a good idea to me (otherwise you could make a typo in `exclude`, but never notice it until it's too late).

Discussion:

Patch:
diff --git a/graphene_django/types.py b/graphene_django/types.py
index ec426f1..4824c45 100644
--- a/graphene_django/types.py
+++ b/graphene_django/types.py
@@ -33,6 +33,24 @@ def construct_fields(
 ):
     _model_fields = get_model_fields(model)
 
+    # Validate the given fields against the model's fields.
+    model_field_names = set(field[0] for field in _model_fields)
+    for fields_list in (only_fields, exclude_fields):
+        if not fields_list:
+            continue
+        for name in fields_list:
+            if name in model_field_names:
+                continue
+
+            if hasattr(model, name):
+                raise Exception(
+                    '"{}" exists on model {} but it\'s not a field.'.format(name, model)
+                )
+            else:
+                raise Exception(
+                    'Field "{}" doesn\'t exist on model {}.'.format(name, model)
+                )
+
     fields = OrderedDict()
     for name, field in _model_fields:
         is_not_in_only = only_fields and name not in only_fields


Test patch:
diff --git a/graphene_django/tests/models.py b/graphene_django/tests/models.py
index 14a8367..44a5d8a 100644
--- a/graphene_django/tests/models.py
+++ b/graphene_django/tests/models.py
@@ -64,6 +64,9 @@ class Reporter(models.Model):
         if self.reporter_type == 2:  # quick and dirty way without enums
             self.__class__ = CNNReporter
 
+    def some_method(self):
+        return 123
+
 
 class CNNReporterManager(models.Manager):
     def get_queryset(self):
diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py
index 5e9d1c2..cb31a9c 100644
--- a/graphene_django/tests/test_types.py
+++ b/graphene_django/tests/test_types.py
@@ -315,7 +315,31 @@ def test_django_objecttype_fields_exclude_type_checking():
         class Reporter2(DjangoObjectType):
             class Meta:
                 model = ReporterModel
-                fields = "foo"
+                exclude = "foo"
+
+
+@with_local_registry
+def test_django_objecttype_fields_exclude_exist_on_model():
+    with pytest.raises(Exception, match=r"Field .* doesn't exist"):
+
+        class Reporter(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                fields = ["first_name", "foo", "email"]
+
+    with pytest.raises(Exception, match=r"Field .* doesn't exist"):
+
+        class Reporter2(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                exclude = ["first_name", "foo", "email"]
+
+    with pytest.raises(Exception, match=r".* exists on model .* but it's not a field"):
+
+        class Reporter3(DjangoObjectType):
+            class Meta:
+                model = ReporterModel
+                fields = ["first_name", "some_method", "email"]
 
 
 class TestDjangoObjectType: