one to one ModelSerializer mutations

[srtucker22]

Hey there,

First of all, this project is fantastic!

I've been attempting to slowly apply graphene-django to some of the models/endpoints we use at work, and struggled with writing mutations over ModelSerializers.

If you consider the Star Wars example, where every Character has a Ship, it would be pretty normal to write a CharacterSerializer like this:

class CharacterSerializer(serializers.ModelSerializer):
    class Meta:
        model = Character
        fields = "__all__"

This will include a field ship of class PrimaryKeyRelatedField

To use this serializer to power a GQL mutation, you'd like to write something really clean like this:

class CreateCharacter(SerializerMutation):
    class Meta:
        serializer_class = CharacterSerializer

Currently, this mutation will run, but the response won't match the GQL we expect:

# what we would expect
mutation createCharacter(name: String, ship: ID) {
    id
    name
    ship {
        id
        name
    }
    errors {
        messages	
    }
}

# what it creates
mutation createCharacter(name: String, ship: ID) {
    id
    name
    ship # a String :/
    errors {
        messages	
    }
}

You could roll your own mutation like in the star wars example:

graphene-django/examples/starwars/schema.py

Line 41 in 52d14f3

class IntroduceShip(relay.ClientIDMutation):

but it would be great to support DRF ModelSerializer out of the box (we use DRF all over our codebase, often with complex validation)

This is a first crack at solving this problem. More work would be needed to support M2M relationships, but I wanted to get this in front of y'all first and get some feedback first.

Let me know what you think!

[coveralls]

Coverage increased (+0.07%) to 94.737% when pulling 0b811a0 on srtucker22:model-serializer-mutations into 52d14f3 on graphql-python:master.

[danpalmer]

I'm afraid I don't have any experience with Django Rest Framework, so I'm going to opt-out of reviewing this one.

[hcharley]

Why start accepting **kwargs in these functions?

[srtucker22]

Hey @Charlex, thanks for reviewing!

IIUC, for any convert_x_serializer_to_y, we run these converters on both inputs and resolvers. However, we need to treat inputs differently than resolvers for certain fields like PrimaryKeyRelatedField. In a mutation, an InputType for a PrimaryKeyRelatedField should be an ID (String), but should resolve the entire Field object.

In convert_serializer_field, we immediately check for the graphene type from the serializer field, but we need to know whether or not the field is an input for PrimaryKeyRelatedField. So now convert_serializer_key_to_field needs a second argument about whether we are converting for an input or a resolver.

We could conditionally pass extra args into get_graphene_type_from_serializer_field e.g.:

if isinstance(field, serializers.PrimaryKeyRelatedField):
  get_graphene_type_from_serializer_field(field, is_input=is_input)
else:
  get_graphene_type_from_serializer_field(field)

but, I thought it would make sense to let the other convert_x_serializer_to_y converters flexibly accept kwargs:

get_graphene_type_from_serializer_field(
  field,
  is_input=is_input,
  other_arg=something_else
)

...

convert_x_serializer_to_y(field, **kwargs):
   # conversion logic here

If we're cool with this approach, I'd like to also apply this logic to 1:N mutations -- PrimaryKeyRelatedField(many=True)

Happy to adjust the code if there's a better approach!

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.