graphql-python / graphql-python/graphene-django
SerializerMutation return types are incompatible with the relay updater api
- Dominant language
- Python
- Stars
- 4.4k
- Forks
- 760
- PR merge metrics
- No merged PRs in 30d
Description
**Is your feature request related to a problem? Please describe.**
In relay, mutations use a store updater that has the ability to update automatically the store, without the need to write an updater function, if the mutation object return type is provided in a shape undersood by relay. The current implementation of the SerializerMutation is incompatible with this api.
In particular, reading the latest [useMutation](https://relay.dev/docs/api-reference/use-mutation/) relay hook docs ([additional explanation here](https://relay.dev/docs/tutorial/mutations-updates/#how-relay-automatically-processes-mutation-responses)), I saw ` If updater is not provided, by default, Relay will know to automatically update the fields on the records referenced in the mutation response;`. The mutation default return type automatically generated (MyMutationPayload)is not an object recognized by relay for an automatic update of the store.
**Describe the solution you'd like**
Provided that the `SerializerMutation` already implements the `clientMutationId` api,
the cleanest would be simply to change the in `SerializerMutation` the mutation return type by doing the following : first providing a registry to the `__init_subclass_with__meta__` and reading from it in the `_meta` initialization, like this is done in the `DjangoObjectType`. The registry, is then used to fetch the Node corresponding to the `model_class` also available in the _meta defintion to create the return type. Something like :
```python
if not _meta:
_meta = RelaySerializerMutationOptions(cls)
_meta.lookup_field = lookup_field
_meta.model_operations = model_operations
_meta.serializer_class = serializer_class
_meta.model_class = model_class
if registry:
_meta.registry = registry
node_type = registry.get_type_for_model(model_class)
_meta.node_type = node_type
new_output_fields = OrderedDict()
new_output_fields["instance"] = graphene.Field(
node_type, description="The created/updated instance."
)
_meta.fields = yank_fields_from_attrs(new_output_fields, _as=Field)
```
Also, this would require the following change in `perform_mutate` method.
```python
@classmethod
def perform_mutate(cls, serializer, info):
obj = serializer.save()
kwargs = {}
for f, field in serializer.fields.items():
if not field.write_only:
if isinstance(field, serializers.SerializerMethodField):
kwargs[f] = field.to_representation(obj)
else:
kwargs[f] = field.get_attribute(obj)
if cls._meta.registry:
node_type = cls._meta.node_type
kwargs["id"] = to_global_id(node_type.__name__, obj.pk)
instance = node_type(**kwargs)
# without it it breaks (NodeType has no attribute pk. ).
instance.pk = obj.pk
return cls(errors=None, instance=instance)
return cls(errors=None, **kwargs)
```
The code above is working.
This solution however, implies a breaking change since we would not be spreading anymore the model instance alongside the clientMutationId, but in a separate relay node (`instance` in the snippet above).
In the two snippets, the `if registry:` tests are for rapid prototyping only, I would imagine an implementation where this is the default behaviour.
**Describe alternatives you've considered**
Other solutions would be less elegant, one would be to create a children class of SerializerMutation that implements this behaviour, this would not have the downside of being a breaking change rather than an opt in. However, it does also require passing the register to the SerializerMutation, which also needs to be changed to receive it as a param, but does not uses it itself.
Another one I considered early was to simply change the ID returned in the MutationPayload to a globalId. I managed to do the change, and I got an interesting result with useMutation. Instead of the store of relay not updating, it would update to a blank item - even if the graphql mutation payload received contained the data. This means that the automatic store updater mentioned in the introduction does indeed require a globalId as mentionned in the introduction, but also the __typename to be matching.
**Additional context**
Thank you for reading this ! I would enjoy reading your thoughts if you have any. Implementing this changewould ensure greater compatibility with the relay api with the downside of being a breaking change. I would be happy to provide a PR for consideration if the change appears relevant.
Contributor guide
Assessment
This issue has not been assessed yet.