From 47650e760a4712dcdd38153b5a42cbf828b433a1 Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Wed, 29 Jan 2020 10:50:50 +0000 Subject: [PATCH] Only warn if a field doesn't exist on the Django model Also don't warn if the field name matches a custom field. --- graphene_django/tests/test_types.py | 21 +++++++++++-- graphene_django/types.py | 46 ++++++++++++++++++----------- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/graphene_django/tests/test_types.py b/graphene_django/tests/test_types.py index cb31a9c..dcfabc4 100644 --- a/graphene_django/tests/test_types.py +++ b/graphene_django/tests/test_types.py @@ -320,27 +320,42 @@ def test_django_objecttype_fields_exclude_type_checking(): @with_local_registry def test_django_objecttype_fields_exclude_exist_on_model(): - with pytest.raises(Exception, match=r"Field .* doesn't exist"): + with pytest.warns(UserWarning, match=r"Field name .* 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"): + with pytest.warns(UserWarning, match=r"Field name .* 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"): + with pytest.warns( + UserWarning, + match=r"Field name .* exists on Django model .* but it's not a model field", + ): class Reporter3(DjangoObjectType): class Meta: model = ReporterModel fields = ["first_name", "some_method", "email"] + # Don't warn if selecting a custom field + with pytest.warns(None) as record: + + class Reporter4(DjangoObjectType): + custom_field = String() + + class Meta: + model = ReporterModel + fields = ["first_name", "custom_field", "email"] + + assert len(record) == 0 + class TestDjangoObjectType: @pytest.fixture diff --git a/graphene_django/types.py b/graphene_django/types.py index 4824c45..1d2af71 100644 --- a/graphene_django/types.py +++ b/graphene_django/types.py @@ -33,24 +33,6 @@ 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 @@ -80,6 +62,31 @@ def construct_fields( return fields +def validate_fields(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: + continue + for name in fields_list: + if name in all_field_names: + continue + + if hasattr(model, name): + warnings.warn( + 'Field name "{}" exists on Django model {} but it\'s not a model field.'.format( + name, model + ) + ) + + else: + warnings.warn( + 'Field name "{}" doesn\'t exist on Django model {}.'.format( + name, model + ) + ) + + class DjangoObjectTypeOptions(ObjectTypeOptions): model = None # type: Model registry = None # type: Registry @@ -211,6 +218,9 @@ class DjangoObjectType(ObjectType): _meta=_meta, interfaces=interfaces, **options ) + # Validate fields + validate_fields(model, _meta.fields, fields, exclude) + if not skip_registry: registry.register(cls)