Graphene-django: replace relay global id with object id

[ramyak-mehra]

So basically I am trying to query through the objects using relay. when I fetch the id it returns the relay global id.

{
  allShops(first: 1){
    edges{
      node{
        id
        name
      }
    }
  }
}

output

{
  "data": {
    "allShops": {
      "edges": [
        {
          "node": {
            "id": "U2hvcE5vZGU6NTA4NTRhYTUtNzJmZC00NzNmLThhNTEtNmQxNWJjYWUwNjE5",
            "name": "pops"
          }
        }
      ]
    }
  }
}

it works just fine , even filtering and stuff. But the problem occurs in mutation i cant pass the relay id while updating or referencing an entry in django. I want the object id there.

A workaround I tried
Use this function

id = from_global_id(id)

its in graphql_relay.node.node
but the problem is it increases a lot of my code and everywhere I want i have to use this and I have a lot of code to manage already.

*Is there any way to globally replace relay global id with model id,(i am using Django's UUID field)
keeping two ids is redundant code

[ulgens]

Hey @ramyak-mehra

Instead of getting rid of Relay global id, btw i think its benetifts are open to discussion but it's a bit overkill, i'd recommend a fix on mutation class.

This is the original code on BaseDjangoFormMutation, base of DjangoModelFormMutation

@classmethod
def get_form_kwargs(cls, root, info, **input):
    kwargs = {"data": input}

    pk = input.pop("id", None)
    if pk:
        instance = cls._meta.model._default_manager.get(pk=pk)
        kwargs["instance"] = instance

    return kwargs

This code tries to fetch your object with Django PK, which i don't see any logic in it. Maybe this base class has other usages that i don't know, but i started to believe it needs to be changed.

def get_form_kwargs(cls, root, info, **input):
    kwargs = {"data": input}

    global_id = input.pop("id", None)
    if global_id:
        node_type, pk = from_global_id(global_id)
        instance = cls._meta.model._default_manager.get(pk=pk)
        kwargs["instance"] = instance

    return kwargs

I use this code on all mutations. It converts relay global id to Django's PK first, then tries to fetch to object.

@jkimbo ping.

[ramyak-mehra]

DjangoModelFormMutation

so basically if I am understanding correctly you are trying to say i should edit the source files of the graphene?
sorry if I get it wrong m still new to this technology
and also would it affect any other logic ?

[ulgens]

Just override it in the mutation class you wrote.

[ramyak-mehra]

this will just be used in the mutation, that purpose of getting it was solved bt the function

id = from_global_id(id)

what about when I query data and try to get details.
this is my schema

class ShopTypeFilter(django_filters.FilterSet):
    class Meta:
        model = ShopType
        fields = '__all__'
        filter_overrides = {
            models.ImageField :{
                'filter_class' : django_filters.CharFilter,
                'extra' : lambda f:{
                    'lookup_expr': 'icontains'
                }
            }
        }
class ShopTypeNode(DjangoObjectType):
    class Meta:
        model = ShopType
        interfaces = (graphene.relay.Node , )
class RelayQuery(graphene.ObjectType):
    node = node = graphene.relay.Node.Field()
    all_shops =  DjangoFilterConnectionField(ShopNode , filterset_class=ShopFilter)
    shop = graphene.relay.Node.Field(ShopNode)

i need to return the pk here also,
i heard something about creating a customnode but its confusing and there is no proper documentation for it

[jkimbo]

I think there 2 issues here which are causing confusion:

1. DjangoFilterConnectionField returns Relay IDs by default

The documentation doesn't do a good job of differentiating what is a relay only concept and what is not the code often defaults to assumption that you are using relay which is often not the case. The connection pattern (edges, nodes etc) is a relay concept that has been adopted by other graphql client libraries because it allows efficient filtering and pagination of long lists. The Relay Global ID is specific to Relay only and (as far as I know) hasn't been adopted by any other graphql client library and is often very confusing to people who are not using Relay.

The fact that Graphene-Django's API defaults to relay in some places and not others (this issue being a good example) is not great and we should move to default to non relay specific APIs with relay being support being an option.

In this particular use case the support is there to not return Relay IDs from DjangoFilterConnectionField. You can do it by changing the ShopTypeNode type to not implement the relay.Node interface but instead mark it as a plain connection:

class ShopTypeNode(DjangoObjectType):
    class Meta:
        model = ShopType
        use_connection = True

This option is not documented and IMO is not very intuitive. I find it confusing that you would "enable the connection" on the ObjectType and I think it makes more sense for you to define a wrapper Connection type like you would do it in graphene: https://docs.graphene-python.org/en/latest/relay/connection/ . For example:

class ShopTypeNode(DjangoObjectType):
    class Meta:
        model = ShopType

class ShopTypeConnection(relay.Connection):
    class Meta:
        node = ShopTypeNode

class Query(ObjectType):
    all_shops = DjangoFilterConnectionField(ShopTypeConnection, filterset_class=ShopFilter)

It's more verbose but I think it makes a lot more sense. Feedback welcome.

Additionally I think we should try and add filter support to the plain DjangoListField because I don't think you should have to use the connection pattern if you don't want to.

2. Support for Relay IDs in DjangoModelFormMutation

Again, because the Graphene-Django API is inconsistent it's quite easy to assume that the mutation classes would accept Relay IDs because that is what is being returned by the connection fields. However that is not the case as @ulgens points out. In my opinion it would be better to make relay specific versions of the mutation classes (or at least document how to accept Relay IDs) rather than try and figure out if the ID is a Relay ID or not. Not sure exactly what the API should look like. @ulgens any ideas?

[ramyak-mehra]

@jkimbo the first solution DjangoConnectionField
DjangoConnectionField only accepts DjangoObjectType types

class UpdateShop(graphene.relay.ClientIDMutation):
    shop = graphene.Field(ShopNode)
    
    class Input:
        id = graphene.String()
        name = graphene.String()
        about = graphene.String()
        owner = graphene.String()
        phone_no = graphene.String()
        time_open = graphene.types.datetime.Time()
        delivery = graphene.Boolean()
        in_campus = graphene.Boolean()
        stars = graphene.Float()
        time_close = graphene.types.datetime.Time()
        paytm_id = graphene.String()
        location = graphene.String()
    @classmethod
    def mutate_and_get_payload(cls,root, info, **input):
        id = input.get('id')
        id = from_global_id(id)
        id = id[1]
        name = input.get('name')
        about = input.get('about')
        delivery = input.get('delivery')
        in_campus = input.get('in_campus')
        type = input.get('type')
        stars = input.get('stars')
        owner = input.get('owner')
        phone_no = input.get('phone_no')
        time_open = input.get('time_open')
        time_close = input.get('time_close')
        paytm_id = input.get('paytm_id')
        location = input.get('location')
        shop = Shop.objects.get(pk=id)
        if name:
            shop.name = name
        if about:
            shop.about = about
        if delivery:
            shop.delivery = delivery
        if in_campus:
            shop.in_campus = in_campus
        if stars:
            shop.stars = stars
        if type:
            shop.type = type
        if owner:
            shop.owner = owner
        if phone_no:
            shop.phone_no = phone_no
        if time_open:
            shop.time_open = time_open
        if time_close:
            shop.time_close = time_close
        if paytm_id:
            shop.paytm_id = paytm_id
        if location:
            shop.location = location
        
        shop.save()
        return UpdateShop(shop=shop)

This is my current code for updating a item. I know it's not the perfect but see at the starting of the mutate_get_ payload method.
This works, but I am not happy with the implementation

[ulgens]

@jkimbo What about creating a new issue to discuss relay - non-relay stuff? I think it's doable and i have some ideas, but not sure when / where to implement them. v3 is a good candidate, but it's too late for that version imo, maybe with 3.1.

[jottenlips]

We had to subclass SerializerMutation to use the client mutation id. It would be nice if that was an option on serializer mutation. I would gladly share our implementation if anyone is interested.

[ramyak-mehra]

Used from_global_id only it added one step but I don't thik so it would be that bad

[ramyak-mehra]

@jottenlips would love to see your implementation also

[jottenlips]

This is our implementation. credit to @kevinharvey

from collections import OrderedDict

import graphene
from graphene_django.types import ErrorType
from graphene.types.objecttype import yank_fields_from_attrs
from graphene.types import Field, InputField
from graphene_django.registry import get_global_registry
from graphene_django.rest_framework.mutation import SerializerMutation, SerializerMutationOptions
from graphene_django.rest_framework.serializer_converter import (
    get_graphene_type_from_serializer_field,
    convert_serializer_to_input_type
)
from graphql_relay.node.node import from_global_id
from rest_framework import serializers
from django.shortcuts import get_object_or_404


def fohboh_convert_serializer_field(field, is_input=True, convert_choices_to_enum=True):
    """
    Converts a django rest frameworks field to a graphql field
    and marks the field as required if we are creating an input type
    and the field itself is required
    """
    if isinstance(field, serializers.ChoiceField) and not convert_choices_to_enum:
        graphql_type = graphene.String
    elif field.label == 'ID' and is_input:
        graphql_type = graphene.ID
    elif isinstance(field, serializers.PrimaryKeyRelatedField):
        graphql_type = graphene.ID
    else:
        graphql_type = get_graphene_type_from_serializer_field(field)

    args = []
    kwargs = {"description": field.help_text,
              "required": is_input and field.required}

    # if it is a tuple or a list it means that we are returning
    # the graphql type and the child type
    if isinstance(graphql_type, (list, tuple)):
        kwargs["of_type"] = graphql_type[1]
        graphql_type = graphql_type[0]

    if isinstance(field, serializers.ModelSerializer):
        if is_input:
            graphql_type = convert_serializer_to_input_type(field.__class__)
        else:
            global_registry = get_global_registry()
            field_model = field.Meta.model
            args = [global_registry.get_type_for_model(field_model)]
    elif isinstance(field, serializers.ListSerializer):
        field = field.child
        if is_input:
            kwargs["of_type"] = convert_serializer_to_input_type(
                field.__class__)
        else:
            del kwargs["of_type"]
            global_registry = get_global_registry()
            field_model = field.Meta.model
            args = [global_registry.get_type_for_model(field_model)]

    return graphql_type(*args, **kwargs)


def fohboh_fields_for_serializer(
    serializer,
    only_fields,
    exclude_fields,
    is_input=False,
    convert_choices_to_enum=True,
):
    fields = OrderedDict()
    for name, field in serializer.fields.items():
        is_not_in_only = only_fields and name not in only_fields
        is_excluded = any(
            [
                name in exclude_fields,
                field.write_only
                and not is_input,  # don't show write_only fields in Query
                field.read_only and is_input  # don't show read_only fields in Input
                # ALLOW FOR ID-BASED PARTIAL UPDATE
                and name not in ['id', 'pk'],
            ]
        )

        if is_not_in_only or is_excluded:
            continue

        fields[name] = fohboh_convert_serializer_field(
            field, is_input=is_input, convert_choices_to_enum=convert_choices_to_enum
        )
    return fields


class FohbohSerializerMutation(SerializerMutation):
    """
    Copied almost entirely from graphen_django.rest_framework.mutations, with the fix described
    in https://github.com/graphql-python/graphene-django/issues/906
    """
    class Meta:
        abstract = True

    errors = graphene.List(
        ErrorType, description="May contain more than one error for same field."
    )
    global_id_fields = []

    @classmethod
    def __init_subclass_with_meta__(
        cls,
        lookup_field=None,
        serializer_class=None,
        model_class=None,
        model_operations=("create", "update"),
        only_fields=(),
        exclude_fields=(),
        convert_choices_to_enum=True,
        **options
    ):

        if not serializer_class:
            raise Exception(
                "serializer_class is required for the SerializerMutation")

        if "update" not in model_operations and "create" not in model_operations:
            raise Exception(
                'model_operations must contain "create" and/or "update"')

        serializer = serializer_class()
        if model_class is None:
            serializer_meta = getattr(serializer_class, "Meta", None)
            if serializer_meta:
                model_class = getattr(serializer_meta, "model", None)

        if lookup_field is None and model_class:
            lookup_field = model_class._meta.pk.name

        input_fields = fohboh_fields_for_serializer(
            serializer,
            only_fields,
            exclude_fields,
            is_input=True,
            convert_choices_to_enum=convert_choices_to_enum,
        )
        output_fields = fohboh_fields_for_serializer(
            serializer,
            only_fields,
            exclude_fields,
            is_input=False,
            convert_choices_to_enum=convert_choices_to_enum,
        )

        _meta = SerializerMutationOptions(cls)
        _meta.lookup_field = lookup_field
        _meta.model_operations = model_operations
        _meta.serializer_class = serializer_class
        _meta.model_class = model_class
        _meta.fields = yank_fields_from_attrs(output_fields, _as=Field)

        input_fields = yank_fields_from_attrs(input_fields, _as=InputField)
        super(SerializerMutation, cls).__init_subclass_with_meta__(
            _meta=_meta, input_fields=input_fields, **options
        )

    @classmethod
    def get_serializer_kwargs(cls, root, info, **input):
        """
        Customized to allow update using the object's global ID
        """
        lookup_field = cls._meta.lookup_field
        model_class = cls._meta.model_class

        for field_name, field_input in input.items():
            if field_name in cls.global_id_fields:
                _, row_id = from_global_id(field_input)
                input[field_name] = int(row_id)

        if model_class:
            if "update" in cls._meta.model_operations and lookup_field in input:
                lookup_kwargs = {
                    'klass': model_class,
                    lookup_field: input[lookup_field]
                }
                if lookup_field in ['id', 'pk'] and type(input[lookup_field]) == type(''):
                    _, object_pk = from_global_id(input[lookup_field])
                    lookup_kwargs[lookup_field] = object_pk

                instance = get_object_or_404(**lookup_kwargs)
                partial = True
            elif "create" in cls._meta.model_operations:
                instance = None
                partial = False
            else:
                raise Exception(
                    'Invalid update operation. Input parameter "{}" required.'.format(
                        lookup_field
                    )
                )

            return {
                "instance": instance,
                "data": input,
                "context": {"request": info.context},
                "partial": partial,
            }

        return {"data": input, "context": {"request": info.context}}

    @classmethod
    def mutate_and_get_payload(cls, root, info, **input):
        kwargs = cls.get_serializer_kwargs(root, info, **input)
        serializer = cls._meta.serializer_class(**kwargs)
        if serializer.is_valid():
            return cls.perform_mutate(serializer, info)
        else:
            errors = ErrorType.from_errors(serializer.errors)
            return cls(errors=errors)

    @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)
        kwargs[cls.output_field] = obj

        return cls(errors=None, **kwargs)

[zbyte64]

Would it make sense to allow global ids to be optional? Could we define the behavior as a DjangoObjectType meta var?

[ramyak-mehra]

Would it make sense to allow global ids to be optional? Could we define the behavior as a DjangoObjectType meta var?

I guess you could make a custom node and - have your configuration there

[tcleonard]

I have actually just submitted 2 PRs (for v2 and v3) for that on graphene: https://github.com/graphql-python/graphene/issues/1276