Skip to content

Only warn if a field doesn't exist on the Django model #862

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

Merged
merged 2 commits into from
Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions graphene_django/tests/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,26 +320,34 @@ class Meta:

@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 .* matches an attribute on Django model .* but it's not a model field",
) as record:

class Reporter2(DjangoObjectType):
class Meta:
model = ReporterModel
exclude = ["first_name", "foo", "email"]
fields = ["first_name", "some_method", "email"]

with pytest.raises(Exception, match=r".* exists on model .* but it's not a field"):
# Don't warn if selecting a custom field
with pytest.warns(None) as record:

class Reporter3(DjangoObjectType):
custom_field = String()

class Meta:
model = ReporterModel
fields = ["first_name", "some_method", "email"]
fields = ["first_name", "custom_field", "email"]

assert len(record) == 0


class TestDjangoObjectType:
Expand Down
59 changes: 41 additions & 18 deletions graphene_django/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -80,6 +62,44 @@ def construct_fields(
return 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:
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_,
)
)

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_,
)
)


class DjangoObjectTypeOptions(ObjectTypeOptions):
model = None # type: Model
registry = None # type: Registry
Expand Down Expand Up @@ -211,6 +231,9 @@ def __init_subclass_with_meta__(
_meta=_meta, interfaces=interfaces, **options
)

# Validate fields
validate_fields(cls, model, _meta.fields, fields, exclude)

if not skip_registry:
registry.register(cls)

Expand Down