From 19fcc3cdf2b0852a1f017922263005fd4ffbfaad Mon Sep 17 00:00:00 2001 From: Patrick Arminio Date: Sun, 28 Oct 2018 20:25:26 +0000 Subject: [PATCH] Update form mutation errors --- graphene_django/forms/mutation.py | 35 ++- graphene_django/forms/tests/test_mutation.py | 222 ++++++++++-------- graphene_django/rest_framework/mutation.py | 16 +- .../rest_framework/tests/test_mutation.py | 6 +- graphene_django/utils.py | 24 +- 5 files changed, 176 insertions(+), 127 deletions(-) diff --git a/graphene_django/forms/mutation.py b/graphene_django/forms/mutation.py index 63ea089..0c1c89b 100644 --- a/graphene_django/forms/mutation.py +++ b/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) diff --git a/graphene_django/forms/tests/test_mutation.py b/graphene_django/forms/tests/test_mutation.py index 20401ab..14d03c4 100644 --- a/graphene_django/forms/tests/test_mutation.py +++ b/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? diff --git a/graphene_django/rest_framework/mutation.py b/graphene_django/rest_framework/mutation.py index 1869d0d..02af0dc 100644 --- a/graphene_django/rest_framework/mutation.py +++ b/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) diff --git a/graphene_django/rest_framework/tests/test_mutation.py b/graphene_django/rest_framework/tests/test_mutation.py index 10d5828..773f195 100644 --- a/graphene_django/rest_framework/tests/test_mutation.py +++ b/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(): diff --git a/graphene_django/utils.py b/graphene_django/utils.py index 560f604..fb9b1b0 100644 --- a/graphene_django/utils.py +++ b/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), + ) \ No newline at end of file