Skip to content

Check exclude fields correctly #873

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 1 commit into from
Feb 17, 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
38 changes: 37 additions & 1 deletion graphene_django/tests/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ class Meta:


@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):
Expand Down Expand Up @@ -350,6 +350,42 @@ class Meta:
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):
Expand Down
66 changes: 46 additions & 20 deletions graphene_django/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down