Update mutation.py

[protasovse]

I do not add data to the error class, but add data to payload.
Without sending form data, this request does not work correctly if there are errors in the form.

mutation SignIn {
  signin(input: {name: "Sergey Protasov", email: "protasovse@gmail.com"}) {
    name
    email
    errors {
      field
      messages
    }
  }
}

Here is the answer:

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field SignIn2Payload.name.",
      "locations": [
        {
          "line": 3,
          "column": 5
        }
      ],
      "path": [
        "signin",
        "name"
      ]
    }
  ],
  "data": {
    "signin": null
  }
}

...and we cannot see form errors from the response of this request...
From the Documentation Explorer we see that the request should look like this:

email: String!
name: String!
errors: [ErrorType]
clientMutationId: String

And this is not at all achievable without overriding the method:

@classmethod
def perform_mutate(cls, form, info):
    form.save()
    return cls(errors=[])

[zbyte64]

Please add a description, I'll try to help you get the ball rolling:

This appears to add the following behavior to the Django forms mutation: pass the form data to the error class.

What we need to move forward is a unit test demonstrating the use case. We want to make sure the error class can handle the extra data and we would want verify the resulting graphql payload (I am not sure how that extra form data appears to the graphql client for instance).

We also need to state the rationale, which I think is to simplify client-side validation by including the data that failed validation in the response. But it might be good to expand on how exactly this will make your life easier.

[protasovse]

Description added

[zbyte64]

I think I understand now. This seems valid to me, going forward I think we should copy the following test to reflect this desired behavior:

graphene-django/graphene_django/forms/tests/test_mutation.py

Line 145 in e51e602

def test_model_form_mutation_mutate_invalid_form(self):

It would supply age but not name and verify age is sent back as part of the response.

[jkimbo]

Added tests @zbyte64

[jarcoal]

Am I crazy or does this PR completely break DjangoModelFormMutation? Just following the example here: https://docs.graphene-python.org/projects/django/en/latest/mutations/#djangomodelformmutation

I think the issue is DjangoModelFormMutation wraps the fields in an envelope (e.g. return_field_name), so trying to spread form.data into the mutation type fails.

graphene-django/graphene_django/forms/mutation.py

Line 50 in 0da06d4

return cls(errors=errors, **form.data)

It's late here so forgive me if I'm overlooking something.

[svengt]

Same here, tried to use DjangoModelFormMutation. When any field does not successfully validate the mutation runs into an exception. Had to role back to version 2.8.1

[jkimbo]

Damn that might well be true. Apologies but we don't have good test coverage in that part of the library so we might have accidentally broken something. I'll try and find some time to fix it.

[jkimbo]

Created a PR to fix it here: #915

[jkimbo]

@jarcoal @svengt I've just released v2.9.1 that should fix the issue: https://github.com/graphql-python/graphene-django/releases/tag/v2.9.1

[jarcoal]

Thanks @jkimbo! It's working for me.