Skip to content

fix various SerializerMutation regressions #261

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 1, 2017
Merged

fix various SerializerMutation regressions #261

merged 2 commits into from
Sep 1, 2017

Conversation

q3aiml
Copy link
Contributor

@q3aiml q3aiml commented Aug 31, 2017 •
edited
Loading

Following the 2.x work SerializerMutation is no longer working for me. I think I found 3 issues:

  • Support for input fields was removed in 72529b7 causing SerializerMutations to accept only clientMutationId as input
  • _meta.serializer_class is no longer set causing mutate_and_get_payload to throw a NoneType not callable error
  • On success SerializerMutation.errors is graphene.List rather than None, leading to an error because graphene.List is not a valid value for a list

Thanks for looking

Fixes #259

@q3aiml
fix SerializerMutation regression for 2.x
cc58b91 
72529b7 seems to break SerializerMutation by commenting out support for
input fields. As a result input only ever had a clientMutationId field.
@q3aiml q3aiml mentioned this pull request Aug 31, 2017
Closed
@q3aiml
ensure SerializerMutation.errors is None on success in 2.x
c130490 
Upon success the result was correct but also included:

"errors": [
  {
    "message": "User Error: expected iterable, but did not find one
for field <SerializerMutation_Subclass>Payload.errors."
  }
]

This seemed to be due to Payload.errors defaulting to graphene.List
rather than unset or None. Unsure what exactly changed with 2.x to break
this, so I welcome a better fix, but explicitly setting errors to None
also seems easy enough.
@coveralls
Copy link

coveralls commented Aug 31, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.9%) to 93.168% when pulling c130490 on q3aiml:fix-serializermutation-regression into 9cac83b on graphql-python:master.

@q3aiml
Copy link
Contributor Author

q3aiml commented Aug 31, 2017

Travis failures are from master being broken which should be fixed by #260

@syrusakbary
Copy link
Member

Thanks for the fixes! Merging :)

@syrusakbary syrusakbary merged commit 728bcac into graphql-python:master Sep 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@q3aiml @coveralls @syrusakbary