SerializerMutation return types are incompatible with the relay updater api
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Idoneità per principianti
- 35/100
- Tipo di issue
- Funzionalità
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Ferma
- Ambito
- api, backend, backend-api-design
Direzione di ricerca
Inizia leggendo le implementazioni di SerializerMutation.init_subclass_with__meta e perform_mutate, quindi confronta la relativa gestione dei metadati e del registro con DjangoObjectType. Esamina i requisiti della risposta della mutation Relay descritti nell'issue e decidi se la forma di restituzione basata sul nodo debba essere breaking o opt-in. Il lavoro è completato quando un'implementazione concordata include la copertura del payload della mutation generata e degli aggiornamenti dello store Relay.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
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 relay hook docs (additional explanation here), 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 :
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.
@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.
- Lingua principale
- Python
- Stelle
- 4.4k
- Fork
- 761
- Metriche di merge delle PR
- Nessuna PR unita negli ultimi 30g
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Nessun modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di graphql-python/graphene-django
-
Difficoltà 5/5 Più di una settimana Idoneità per principianti 25/100
graphql-python/graphene-django#1561 · 1 commento ·
-
🐛bug
Difficoltà 4/5 3-5 giorni Idoneità per principianti 45/100
graphql-python/graphene-django#1558 · 1 commento ·
-
🐛bug
Difficoltà 4/5 3-5 giorni Idoneità per principianti 35/100
graphql-python/graphene-django#1551 · 1 commento ·
-
🐛bug
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 38/100
graphql-python/graphene-django#1547 · 1 commento ·
-
🐛bug
Difficoltà 3/5 1-2 giorni Idoneità per principianti 35/100
graphql-python/graphene-django#1542 · 3 commenti ·
Tutte le issue di graphql-python/graphene-django
Issue simili
-
Difficoltà 1/5 1-3 ore Idoneità per principianti 66/100
AlexsLemonade/scpca-docs#518 ·
-
bug
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 95/100
rocketride-org/rocketride-server#2705 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
documentation
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
Metabuilder-Labs/tokenjam#786 ·
I maintainer di solito rispondono entro 1 giorno
-
prototype(verifier_token): map disallowed warning to contraindicated before composite precedence checkForse già presa @drk-ram l’ha presa oggi. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 88/100
agentrust-io/trace-spec#582 ·
I maintainer di solito rispondono entro 1 giorno
-
area:cli priority:low refactor size:small
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 88/100
RonaldHensbergen/composable-data-stack#906 ·
I maintainer di solito rispondono entro 1 giorno