Skip to content

Commit

Permalink
Apply camel case converter to field names in DRF errors (#514)
Browse files Browse the repository at this point in the history
* Apply camel case converter to field names in DRF errors

* Implement recursive error camelize, add setting.
  • Loading branch information
kalekseev authored and jkimbo committed Jun 25, 2019
1 parent 692540c commit e2e496f
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 23 deletions.
7 changes: 2 additions & 5 deletions graphene_django/forms/mutation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
from graphene.types.utils import yank_fields_from_attrs
from graphene_django.registry import get_global_registry

from .converter import convert_form_field
from ..types import ErrorType
from .converter import convert_form_field


def fields_for_form(form, only_fields, exclude_fields):
Expand Down Expand Up @@ -45,10 +45,7 @@ def mutate_and_get_payload(cls, root, info, **input):
if form.is_valid():
return cls.perform_mutate(form, info)
else:
errors = [
ErrorType(field=key, messages=value)
for key, value in form.errors.items()
]
errors = ErrorType.from_errors(form.errors)

return cls(errors=errors)

Expand Down
20 changes: 19 additions & 1 deletion graphene_django/forms/tests/test_mutation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
from django.test import TestCase
from py.test import raises

from graphene_django.tests.models import Pet, Film, FilmDetails
from graphene_django.tests.models import Film, FilmDetails, Pet

from ...settings import graphene_settings
from ..mutation import DjangoFormMutation, DjangoModelFormMutation


Expand Down Expand Up @@ -41,6 +43,22 @@ class Meta:
assert "text" in MyMutation.Input._meta.fields


def test_mutation_error_camelcased():
class ExtraPetForm(PetForm):
test_field = forms.CharField(required=True)

class PetMutation(DjangoModelFormMutation):
class Meta:
form_class = ExtraPetForm

result = PetMutation.mutate_and_get_payload(None, None)
assert {f.field for f in result.errors} == {"name", "age", "test_field"}
graphene_settings.DJANGO_GRAPHENE_CAMELCASE_ERRORS = True
result = PetMutation.mutate_and_get_payload(None, None)
assert {f.field for f in result.errors} == {"name", "age", "testField"}
graphene_settings.DJANGO_GRAPHENE_CAMELCASE_ERRORS = False


class ModelFormMutationTests(TestCase):
def test_default_meta_fields(self):
class PetMutation(DjangoModelFormMutation):
Expand Down
9 changes: 3 additions & 6 deletions graphene_django/rest_framework/mutation.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
from django.shortcuts import get_object_or_404

import graphene
from graphene.relay.mutation import ClientIDMutation
from graphene.types import Field, InputField
from graphene.types.mutation import MutationOptions
from graphene.relay.mutation import ClientIDMutation
from graphene.types.objecttype import yank_fields_from_attrs

from .serializer_converter import convert_serializer_field
from ..types import ErrorType
from .serializer_converter import convert_serializer_field


class SerializerMutationOptions(MutationOptions):
Expand Down Expand Up @@ -127,10 +127,7 @@ def mutate_and_get_payload(cls, root, info, **input):
if serializer.is_valid():
return cls.perform_mutate(serializer, info)
else:
errors = [
ErrorType(field=key, messages=value)
for key, value in serializer.errors.items()
]
errors = ErrorType.from_errors(serializer.errors)

return cls(errors=errors)

Expand Down
14 changes: 11 additions & 3 deletions graphene_django/rest_framework/tests/test_mutation.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import datetime

from py.test import mark, raises
from rest_framework import serializers

from graphene import Field, ResolveInfo
from graphene.types.inputobjecttype import InputObjectType
from py.test import raises
from py.test import mark
from rest_framework import serializers

from ...settings import graphene_settings
from ...types import DjangoObjectType
from ..models import MyFakeModel, MyFakeModelWithPassword
from ..mutation import SerializerMutation
Expand Down Expand Up @@ -213,6 +214,13 @@ def test_model_mutate_and_get_payload_error():
assert len(result.errors) > 0


def test_mutation_error_camelcased():
graphene_settings.DJANGO_GRAPHENE_CAMELCASE_ERRORS = True
result = MyModelMutation.mutate_and_get_payload(None, mock_info(), **{})
assert result.errors[0].field == "coolName"
graphene_settings.DJANGO_GRAPHENE_CAMELCASE_ERRORS = False


def test_invalid_serializer_operations():
with raises(Exception) as exc:

Expand Down
1 change: 1 addition & 0 deletions graphene_django/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"RELAY_CONNECTION_ENFORCE_FIRST_OR_LAST": False,
# Max items returned in ConnectionFields / FilterConnectionFields
"RELAY_CONNECTION_MAX_LIMIT": 100,
"DJANGO_GRAPHENE_CAMELCASE_ERRORS": False,
}

if settings.DEBUG:
Expand Down
22 changes: 21 additions & 1 deletion graphene_django/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from ..utils import get_model_fields
from django.utils.translation import gettext_lazy

from ..utils import camelize, get_model_fields
from .models import Film, Reporter


Expand All @@ -10,3 +12,21 @@ def test_get_model_fields_no_duplication():
film_fields = get_model_fields(Film)
film_name_set = set([field[0] for field in film_fields])
assert len(film_fields) == len(film_name_set)


def test_camelize():
assert camelize({}) == {}
assert camelize("value_a") == "value_a"
assert camelize({"value_a": "value_b"}) == {"valueA": "value_b"}
assert camelize({"value_a": ["value_b"]}) == {"valueA": ["value_b"]}
assert camelize({"value_a": ["value_b"]}) == {"valueA": ["value_b"]}
assert camelize({"nested_field": {"value_a": ["error"], "value_b": ["error"]}}) == {
"nestedField": {"valueA": ["error"], "valueB": ["error"]}
}
assert camelize({"value_a": gettext_lazy("value_b")}) == {"valueA": "value_b"}
assert camelize({"value_a": [gettext_lazy("value_b")]}) == {"valueA": ["value_b"]}
assert camelize(gettext_lazy("value_a")) == "value_a"
assert camelize({gettext_lazy("value_a"): gettext_lazy("value_b")}) == {
"valueA": "value_b"
}
assert camelize({0: {"field_a": ["errors"]}}) == {0: {"fieldA": ["errors"]}}
21 changes: 18 additions & 3 deletions graphene_django/types.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import six
from collections import OrderedDict

import six
from django.db.models import Model
from django.utils.functional import SimpleLazyObject

import graphene
from graphene import Field
from graphene.relay import Connection, Node
Expand All @@ -11,8 +12,13 @@

from .converter import convert_django_field_with_choices
from .registry import Registry, get_global_registry
from .utils import DJANGO_FILTER_INSTALLED, get_model_fields, is_valid_django_model

from .settings import graphene_settings
from .utils import (
DJANGO_FILTER_INSTALLED,
camelize,
get_model_fields,
is_valid_django_model,
)

if six.PY3:
from typing import Type
Expand Down Expand Up @@ -182,3 +188,12 @@ def get_node(cls, info, id):
class ErrorType(ObjectType):
field = graphene.String(required=True)
messages = graphene.List(graphene.NonNull(graphene.String), required=True)

@classmethod
def from_errors(cls, errors):
data = (
camelize(errors)
if graphene_settings.DJANGO_GRAPHENE_CAMELCASE_ERRORS
else errors
)
return [ErrorType(field=key, messages=value) for key, value in data.items()]
10 changes: 6 additions & 4 deletions graphene_django/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
from .testing import GraphQLTestCase
from .utils import (
DJANGO_FILTER_INSTALLED,
get_reverse_fields,
maybe_queryset,
camelize,
get_model_fields,
is_valid_django_model,
get_reverse_fields,
import_single_dispatch,
is_valid_django_model,
maybe_queryset,
)
from .testing import GraphQLTestCase

__all__ = [
"DJANGO_FILTER_INSTALLED",
"get_reverse_fields",
"maybe_queryset",
"get_model_fields",
"camelize",
"is_valid_django_model",
"import_single_dispatch",
"GraphQLTestCase",
Expand Down
26 changes: 26 additions & 0 deletions graphene_django/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

from django.db import models
from django.db.models.manager import Manager
from django.utils import six
from django.utils.encoding import force_text
from django.utils.functional import Promise

from graphene.utils.str_converters import to_camel_case

try:
import django_filters # noqa
Expand All @@ -12,6 +16,28 @@
DJANGO_FILTER_INSTALLED = False


def isiterable(value):
try:
iter(value)
except TypeError:
return False
return True


def _camelize_django_str(s):
if isinstance(s, Promise):
s = force_text(s)
return to_camel_case(s) if isinstance(s, six.string_types) else s


def camelize(data):
if isinstance(data, dict):
return {_camelize_django_str(k): camelize(v) for k, v in data.items()}
if isiterable(data) and not isinstance(data, (six.string_types, Promise)):
return [camelize(d) for d in data]
return data


def get_reverse_fields(model, local_field_names):
for name, attr in model.__dict__.items():
# Don't duplicate any local fields
Expand Down

0 comments on commit e2e496f

Please sign in to comment.