Inconsistent behavior for ID field doing update in DjangoModelFormMutation

[ace-han]

ID field in DjangoModelFormMutation displaying Global ID, however doing updates form id requires raw DB pk field.

Say I have below scripts

# cookbook/ingredients/models.py
from django.db import models
class Category(models.Model):
    name = models.CharField(max_length=100)

    def __str__(self):
        return self.name

# cookbook/ingredients/forms.py
from django.forms.models import ALL_FIELDS, ModelForm
from cookbook.ingredients.models import Category

class CategoryForm(ModelForm):
    class Meta:
        model = Category
        fields = ALL_FIELDS

# cookbook/ingredients/schema.py
from graphene import Field, Node, ObjectType
from graphene_django.filter import DjangoFilterConnectionField
from graphene_django.forms.mutation import DjangoModelFormMutation
from graphene_django.types import DjangoObjectType

from cookbook.ingredients.forms import CategoryForm
from cookbook.ingredients.models import Category

class CategoryNode(DjangoObjectType):
    class Meta:
        model = Category
        interfaces = (Node,)
        filter_fields = ["name", "ingredients"]

class Queries(ObjectType):
    category = Node.Field(CategoryNode)
    all_categories = DjangoFilterConnectionField(CategoryNode)

class CategoryMutation(DjangoModelFormMutation):
    category = Field(CategoryNode)

    class Meta:
        form_class = CategoryForm

class Mutations(ObjectType):
    category_form_create = CategoryMutation.Field()
    category_form_update = CategoryMutation.Field()

while I do query allCategories like this

query {
  allCategories {
    edges {
      node {
        id
        name
      }
    }
  }
}

result as below

{
  "data": {
    "allCategories": {
      "edges": [
        {
          "node": {
            "id": "Q2F0ZWdvcnlOb2RlOjE=", // global id
            "name": "Dairy"
          }
        },
        {
          "node": {
            "id": "Q2F0ZWdvcnlOb2RlOjI=", // global id
            "name": "Meat"
          }
        }
      ]
    }
  }
}

above are expected

Now I would like to do some update

# query part
mutation xxx($formInput: CategoryMutationInput!) {
  categoryFormUpdate (input: $formInput){
    category {
      id
      name
    }
  }
}

// variables part
{
  "formInput": {
    "id": "Q2F0ZWdvcnlOb2RlOjE=", // <- using global id
    "name": "Diary 1"
  }
}

I got errors sth like Field 'id' expected a number but got 'Q2F0ZWdvcnlOb2RlOjE='.
so I decode this Q2F0ZWdvcnlOb2RlOjE= and get this CategoryNode:1 string

and then I change the id to raw DB pk in variables part

// variables part
{
  "formInput": {
    "id": 1, // <- using raw DB pk
    "name": "Diary 1"
  }
}

I got the expected result

{
  "data": {
    "categoryFormUpdate": {
      "category": {
        "id": "Q2F0ZWdvcnlOb2RlOjU=", // <- global id
        "name": "Diary 1"
      }
    }
  }
}

Is this behavior expected?
I think this is confusing and inconsistent (should be always global id, I think). Shouldn't it be mentioned in the doc? Or Is it a bug?

[ace-han]

So I did a little bit code digging and found that graphene_django.rest_framework.mutation.SerializerMutation has the same issue

and I think using global id for update in DjangoModelFormMutation and SerializerMutation is better.

I found the relevant code as below
https://github.com/graphql-python/graphene-django/blob/8ec456285bb84fad6e1ee6644543140335f613af/graphene_django/forms/mutation.py#L56-L67

How about introducing graphene.relay.node.Node.from_global_id

from graphene.relay.node import Node
# ...
    @classmethod
    def get_form_kwargs(cls, root, info, **input):
        kwargs = {"data": input}

        pk = input.pop("id", None)
        if pk:
            _, raw_pk = Node.from_global_id(pk)  # parsing global id to (type, id)
            instance = cls._meta.model._default_manager.get(pk=raw_pk)
            kwargs["instance"] = instance

        return kwargs

Sth like that also applies to SerializerMutation, is that acceptable?
I could make a PR then

[ace-han]

Found some issue similar to this issue #728

[zbyte64]

Also related to PR #546 which was closed due to inactivity

[L0rdCr1s]

So I did a little bit code digging and found that graphene_django.rest_framework.mutation.SerializerMutation has the same issue

and I think using global id for update in DjangoModelFormMutation and SerializerMutation is better.

I found the relevant code as below https://github.com/graphql-python/graphene-django/blob/8ec456285bb84fad6e1ee6644543140335f613af/graphene_django/forms/mutation.py#L56-L67

How about introducing graphene.relay.node.Node.from_global_id

from graphene.relay.node import Node
# ...
    @classmethod
    def get_form_kwargs(cls, root, info, **input):
        kwargs = {"data": input}

        pk = input.pop("id", None)
        if pk:
            _, raw_pk = Node.from_global_id(pk)  # parsing global id to (type, id)
            instance = cls._meta.model._default_manager.get(pk=raw_pk)
            kwargs["instance"] = instance

        return kwargs

Sth like that also applies to SerializerMutation, is that acceptable? I could make a PR then

@ace-han did you make a PR, because i'm facing similar issue unless i convert global_id into raw id on the get_form method

if not how did you solve your problem

[ace-han]

@L0rdCr1s I did not get it work. please refer to this comment and be patient 😃

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

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

[krzysieqq]

I hade the same issue here. My workaround was added to DjangoModelFormMutation

    @classmethod
    def mutate_and_get_payload(cls, root, info, **input):
        model = cls._meta.form_class._meta.model
        for field, value in input.items():
            if not hasattr(model, field):
                continue
            model_field = model._meta.get_field(field)
            if isinstance(model_field, (AutoField, ForeignKey)):
                try:
                    _, input[field] = from_global_id(value)
                except UnicodeDecodeError:
                    pass
            if isinstance(model_field, ManyToManyField):
                values = []
                for v in value:
                    try:
                        _, obj = from_global_id(v)
                        values.append(obj)
                    except UnicodeDecodeError:
                        pass
                input[field] = values
        return super().mutate_and_get_payload(root, info, **input)

But I think it's not the best solution.

[apoorvyadav1111]

Is there any solution for this other than the work around, maybe like a default class method that decouples the type and id from the global id as well as returns the object to create/ mutation the one we require in an efficient way (say when we have around 10 million records)

I am sorry if this sounds too naive but I am new to graphene and docs are'nt that helping, I have to salvage issues on github to look for solution xD.

[pfcodes]

Anybody come up with a solution to this? Thinking I might just manually convert from the Global ID to the pk in perform_mutate