Update form mutation errors

graphene_django/forms/mutation.py

@@ -13,6 +13,7 @@ from graphene.types.mutation import MutationOptions
 from graphene.types.utils import yank_fields_from_attrs
 from graphene_django.registry import get_global_registry
 
+from ..utils import create_errors_type
 from .converter import convert_form_field
 from .types import ErrorType
 
@@ -45,10 +46,8 @@ class BaseDjangoFormMutation(ClientIDMutation):
         if form.is_valid():
             return cls.perform_mutate(form, info)
         else:
-            errors = [
-                ErrorType(field=key, messages=value)
-                for key, value in form.errors.items()
-            ]
+            # TODO: double check non field errors name
+            errors = cls.Errors(**form.errors)
 
             return cls(errors=errors)
 
@@ -99,8 +98,6 @@ class DjangoFormMutation(BaseDjangoFormMutation):
     class Meta:
         abstract = True
 
-    errors = graphene.List(ErrorType)
-
     @classmethod
     def __init_subclass_with_meta__(
         cls, form_class=None, only_fields=(), exclude_fields=(), **options
@@ -112,12 +109,21 @@ class DjangoFormMutation(BaseDjangoFormMutation):
         form = form_class()
         input_fields = fields_for_form(form, only_fields, exclude_fields)
         output_fields = fields_for_form(form, only_fields, exclude_fields)
+        input_fields = yank_fields_from_attrs(input_fields, _as=InputField)
+
+        base_name = cls.__name__
+
+        cls.Errors = create_errors_type(
+            "{}Errors".format(base_name),
+            input_fields
+        )
+
+        output_fields['errors'] = graphene.Field(cls.Errors, required=True)
 
         _meta = DjangoFormMutationOptions(cls)
         _meta.form_class = form_class
         _meta.fields = yank_fields_from_attrs(output_fields, _as=Field)
 
-        input_fields = yank_fields_from_attrs(input_fields, _as=InputField)
         super(DjangoFormMutation, cls).__init_subclass_with_meta__(
             _meta=_meta, input_fields=input_fields, **options
         )
@@ -137,8 +143,6 @@ class DjangoModelFormMutation(BaseDjangoFormMutation):
     class Meta:
         abstract = True
 
-    errors = graphene.List(ErrorType)
-
     @classmethod
     def __init_subclass_with_meta__(
         cls,
@@ -173,6 +177,16 @@ class DjangoModelFormMutation(BaseDjangoFormMutation):
 
         output_fields = OrderedDict()
         output_fields[return_field_name] = graphene.Field(model_type)
+        input_fields = yank_fields_from_attrs(input_fields, _as=InputField)
+
+        base_name = cls.__name__
+
+        cls.Errors = create_errors_type(
+            "{}Errors".format(base_name),
+            input_fields
+        )
+
+        output_fields['errors'] = graphene.Field(cls.Errors, required=True)
 
         _meta = DjangoModelDjangoFormMutationOptions(cls)
         _meta.form_class = form_class
@@ -180,7 +194,6 @@ class DjangoModelFormMutation(BaseDjangoFormMutation):
         _meta.return_field_name = return_field_name
         _meta.fields = yank_fields_from_attrs(output_fields, _as=Field)
 
-        input_fields = yank_fields_from_attrs(input_fields, _as=InputField)
         super(DjangoModelFormMutation, cls).__init_subclass_with_meta__(
             _meta=_meta, input_fields=input_fields, **options
         )
@@ -189,4 +202,4 @@ class DjangoModelFormMutation(BaseDjangoFormMutation):
     def perform_mutate(cls, form, info):
         obj = form.save()
         kwargs = {cls._meta.return_field_name: obj}
-        return cls(errors=[], **kwargs)
+        return cls(errors={}, **kwargs)

graphene_django/forms/tests/test_mutation.py

@@ -1,6 +1,9 @@
+import pytest
+
 from django import forms
 from django.test import TestCase
-from py.test import raises
+
+from graphene import NonNull, List
 
 from graphene_django.tests.models import Pet, Film, FilmDetails
 from ..mutation import DjangoFormMutation, DjangoModelFormMutation
@@ -17,8 +20,7 @@ class PetForm(forms.ModelForm):
 
 
 def test_needs_form_class():
-    with raises(Exception) as exc:
-
+    with pytest.raises(Exception) as exc:
         class MyMutation(DjangoFormMutation):
             pass
 
@@ -41,101 +43,127 @@ def test_has_input_fields():
     assert "text" in MyMutation.Input._meta.fields
 
 
-class ModelFormMutationTests(TestCase):
-    def test_default_meta_fields(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
+def test_default_meta_fields():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
 
-        self.assertEqual(PetMutation._meta.model, Pet)
-        self.assertEqual(PetMutation._meta.return_field_name, "pet")
-        self.assertIn("pet", PetMutation._meta.fields)
+    assert PetMutation._meta.model == Pet
+    assert PetMutation._meta.return_field_name == "pet"
 
-    def test_default_input_meta_fields(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-
-        self.assertEqual(PetMutation._meta.model, Pet)
-        self.assertEqual(PetMutation._meta.return_field_name, "pet")
-        self.assertIn("name", PetMutation.Input._meta.fields)
-        self.assertIn("client_mutation_id", PetMutation.Input._meta.fields)
-        self.assertIn("id", PetMutation.Input._meta.fields)
-
-    def test_exclude_fields_input_meta_fields(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-                exclude_fields = ['id']
-
-        self.assertEqual(PetMutation._meta.model, Pet)
-        self.assertEqual(PetMutation._meta.return_field_name, "pet")
-        self.assertIn("name", PetMutation.Input._meta.fields)
-        self.assertIn("age", PetMutation.Input._meta.fields)
-        self.assertIn("client_mutation_id", PetMutation.Input._meta.fields)
-        self.assertNotIn("id", PetMutation.Input._meta.fields)
-
-    def test_return_field_name_is_camelcased(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-                model = FilmDetails
-
-        self.assertEqual(PetMutation._meta.model, FilmDetails)
-        self.assertEqual(PetMutation._meta.return_field_name, "filmDetails")
-
-    def test_custom_return_field_name(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-                model = Film
-                return_field_name = "animal"
-
-        self.assertEqual(PetMutation._meta.model, Film)
-        self.assertEqual(PetMutation._meta.return_field_name, "animal")
-        self.assertIn("animal", PetMutation._meta.fields)
-
-    def test_model_form_mutation_mutate(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-
-        pet = Pet.objects.create(name="Axel", age=10)
-
-        result = PetMutation.mutate_and_get_payload(None, None, id=pet.pk, name="Mia", age=10)
-
-        self.assertEqual(Pet.objects.count(), 1)
-        pet.refresh_from_db()
-        self.assertEqual(pet.name, "Mia")
-        self.assertEqual(result.errors, [])
-
-    def test_model_form_mutation_updates_existing_(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-
-        result = PetMutation.mutate_and_get_payload(None, None, name="Mia", age=10)
-
-        self.assertEqual(Pet.objects.count(), 1)
-        pet = Pet.objects.get()
-        self.assertEqual(pet.name, "Mia")
-        self.assertEqual(pet.age, 10)
-        self.assertEqual(result.errors, [])
-
-    def test_model_form_mutation_mutate_invalid_form(self):
-        class PetMutation(DjangoModelFormMutation):
-            class Meta:
-                form_class = PetForm
-
-        result = PetMutation.mutate_and_get_payload(None, None)
-
-        # A pet was not created
-        self.assertEqual(Pet.objects.count(), 0)
+    assert "pet" in PetMutation._meta.fields
 
 
-        fields_w_error = [e.field for e in result.errors]
-        self.assertEqual(len(result.errors), 2)
-        self.assertIn("name", fields_w_error)
-        self.assertEqual(result.errors[0].messages, ["This field is required."])
-        self.assertIn("age", fields_w_error)
-        self.assertEqual(result.errors[1].messages, ["This field is required."])
+def test_default_input_meta_fields():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+
+    assert PetMutation._meta.model == Pet
+    assert PetMutation._meta.return_field_name == "pet"
+
+    assert "name" in PetMutation.Input._meta.fields
+    assert "client_mutation_id" in PetMutation.Input._meta.fields
+    assert "id" in PetMutation.Input._meta.fields
+
+
+def test_exclude_fields_input_meta_fields():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+            exclude_fields = ['id']
+
+    assert PetMutation._meta.model == Pet
+    assert PetMutation._meta.return_field_name == "pet"
+    assert "name" in PetMutation.Input._meta.fields
+    assert "age" in PetMutation.Input._meta.fields
+    assert "client_mutation_id" in PetMutation.Input._meta.fields
+    assert "id" not in PetMutation.Input._meta.fields
+
+
+def test_return_field_name_is_camelcased():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+            model = FilmDetails
+
+    assert PetMutation._meta.model == FilmDetails
+    assert PetMutation._meta.return_field_name == "filmDetails"
+
+
+def test_custom_return_field_name():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+            model = Film
+            return_field_name = "animal"
+
+    assert PetMutation._meta.model == Film
+    assert PetMutation._meta.return_field_name == "animal"
+    assert "animal" in PetMutation._meta.fields
+
+
+@pytest.mark.django_db
+def test_model_form_mutation_mutate():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+
+    pet = Pet.objects.create(name="Axel", age=10)
+
+    result = PetMutation.mutate_and_get_payload(None, None, id=pet.pk, name="Mia", age=10)
+
+    assert Pet.objects.count() == 1
+    pet.refresh_from_db()
+    assert pet.name == "Mia"
+    assert result.errors == {}
+
+
+@pytest.mark.django_db
+def test_model_form_mutation_updates_existing():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+
+    result = PetMutation.mutate_and_get_payload(None, None, name="Mia", age=10)
+
+    assert Pet.objects.count() == 1
+    pet = Pet.objects.get()
+    assert pet.name == "Mia"
+    assert pet.age == 10
+    assert result.errors == {}
+
+
+@pytest.mark.django_db
+def test_model_form_mutation_mutate_invalid_form():
+    class PetMutation(DjangoModelFormMutation):
+        class Meta:
+            form_class = PetForm
+
+    result = PetMutation.mutate_and_get_payload(None, None)
+
+    # A pet was not created
+    assert Pet.objects.count() == 0
+
+    assert result.errors != {}
+
+    assert result.errors.name == ["This field is required."]
+    assert result.errors.age == ["This field is required."]
+
+
+def test_errors_field():
+    class MyMutation(DjangoFormMutation):
+        class Meta:
+            form_class = MyForm
+
+    errors_field = MyMutation._meta.fields['errors']
+
+    assert MyMutation.Errors
+
+    assert type(errors_field.type) == NonNull
+
+    errors_field = errors_field.type.of_type
+
+    assert type(errors_field.text.type) == List
+    assert type(errors_field.text.type.of_type) == NonNull
+    # TODO: how to test that the nonnull type is a string?

graphene_django/rest_framework/mutation.py

@@ -8,6 +8,7 @@ from graphene.types.mutation import MutationOptions
 from graphene.relay.mutation import ClientIDMutation
 from graphene.types.objecttype import yank_fields_from_attrs
 
+from ..utils import create_errors_type
 from .serializer_converter import convert_serializer_field
 from .types import ErrorType
 
@@ -73,22 +74,11 @@ class SerializerMutation(ClientIDMutation):
             serializer, only_fields, exclude_fields, is_input=False
         )
 
-        error_fields = {
-            key: graphene.List(graphene.NonNull(graphene.String))
-            for key in input_fields.keys()
-        }
-
-        # TODO: from settings
-        error_fields['non_field_errors'] = graphene.List(
-            graphene.NonNull(graphene.String)
-        )
-
         base_name = cls.__name__
 
-        cls.Errors = type(
+        cls.Errors = create_errors_type(
             "{}Errors".format(base_name),
-            (graphene.ObjectType, ),
-            yank_fields_from_attrs(error_fields, _as=Field),
+            input_fields
         )
 
         output_fields['errors'] = graphene.Field(cls.Errors, required=True)

graphene_django/rest_framework/tests/test_mutation.py

@@ -159,14 +159,14 @@ def test_mutate_and_get_payload_error():
 
     # missing required fields
     result = MyMutation.mutate_and_get_payload(None, mock_info(), **{})
-    assert result.errors.text[0] == "This field is required."
-    assert result.errors.model[0] == "This field is required."
+    assert result.errors.text == ["This field is required."]
+    assert result.errors.model == ["This field is required."]
 
 
 def test_model_mutate_and_get_payload_error():
     # missing required fields
     result = MyModelMutation.mutate_and_get_payload(None, mock_info(), **{})
-    assert result.errors.cool_name[0] == "This field is required."
+    assert result.errors.cool_name == ["This field is required."]
 
 
 def test_invalid_serializer_operations():

graphene_django/utils.py

@@ -1,12 +1,12 @@
 import inspect
 
+import graphene
+from graphene.types.objecttype import yank_fields_from_attrs
+
 from django.db import models
 from django.db.models.manager import Manager
 
 
-# from graphene.utils import LazyList
-
-
 class LazyList(object):
     pass
 
@@ -81,3 +81,21 @@ def import_single_dispatch():
         )
 
     return singledispatch
+
+
+def create_errors_type(name, input_fields):
+    error_fields = {
+        key: graphene.List(graphene.NonNull(graphene.String))
+        for key in input_fields.keys()
+    }
+
+    # TODO: from settings
+    error_fields['non_field_errors'] = graphene.List(
+        graphene.NonNull(graphene.String)
+    )
+
+    return type(
+        name,
+        (graphene.ObjectType, ),
+        yank_fields_from_attrs(error_fields, _as=graphene.Field),
+    )