Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use DjangoObjectType (or something like it) as an Input class? #121

Closed
nuschk opened this issue Feb 27, 2017 · 10 comments

Comments

@nuschk
Copy link

commented Feb 27, 2017

We're using Graphene with Relay for some boring CRUD style mutations. Now, since we have some rather large (i.e., many fields) models, it's a bit tedious and also error-prone, to repeat typing in the model's fields again for each mutation. Is it possible to use DjangoObjectType for this (from my 1st try it seems that, no)?

I'm mainly interested in the Meta class' model and only_fields options.

And guys, thanks for the hard work on such a cool tool.

@nuschk

This comment has been minimized.

Copy link
Author

commented Feb 27, 2017

Here's something I put together, which seem to work, but is certainly work in progress:

import collections
from graphene_django import converter, utils

from graphene.types.utils import merge
from graphene.utils.is_base_type import is_base_type


def convert_fields(model, only_fields):
    model_fields = utils.get_model_fields(model=model)
    fields = collections.OrderedDict()

    for field in model_fields:
        name = field.name

        # Must be in only_fields
        if name not in only_fields:
            continue

        converted = converter.convert_django_field(field, None)
        if not converted:
            continue

        fields[name] = converted

    return fields


class DjangoModelInputMeta(type):

    @staticmethod
    def __new__(cls, name, bases, attrs):
        # We'll get called also for non-user classes like DjangoModelInput. Only
        # kick in when called for a sub-class.
        if not is_base_type(bases, DjangoModelInputMeta):
            return type.__new__(cls, name, bases, attrs)

        # Pop Meta info. Must be removed from class, otherwise graphene will
        # complain.
        meta = attrs.pop('Meta')
        fields = convert_fields(model=meta.model, only_fields=meta.only_fields)
        attrs = merge(attrs, fields)

        return type.__new__(cls, name, bases, attrs)


class DjangoModelInput(metaclass=DjangoModelInputMeta):
    """
    Derive a mutation's Input class from this and define a meta class with 
    `model` and `only_fields` members. This will populate the input class
    with the converted django members.
    """
    pass
@ZhangBohan

This comment has been minimized.

Copy link

commented Mar 14, 2017

@nuschk great job, can you make a merge request for this code?

@nuschk

This comment has been minimized.

Copy link
Author

commented Mar 14, 2017

I could, but I'm still not sure whether it really fits in (I didn't really get the architecture of the whole library). Also, there are a few caveats still (no support for enumerations, and only_fields is used a bit differently.

I'd appreciate a word from one of the maintainers first, if that's possible.

@ZhangBohan

This comment has been minimized.

Copy link

commented Mar 14, 2017

@nuschk ok, but it seems the maintainers is not focus on this product. 😭

@nuschk

This comment has been minimized.

Copy link
Author

commented Mar 14, 2017

Yeah, but then how would I be able to merge the PR? :-)

I'll do the PR as soon as there's hope of merging, but before that, I don't really see the reason to put in the effort (adding tests, docs etc.). For the time being, this will live in our code base.

@johandc

This comment has been minimized.

Copy link

commented May 4, 2017

Thank you for posting your solution.

There is a similar request over on the main graphene repository here:
graphql-python/graphene#143

@charlesverdad

This comment has been minimized.

Copy link

commented Sep 18, 2018

I was able to dynamically generate an Input class from Model using a Django Rest Framework serializer and the convert_serializer_to_input_type tool in graphene_django.rest_framework.serializer_converter.

Sample Code:

from rest_framework import serializers
from graphene_django.rest_framework.serializer_converter import convert_serializer_to_input_type

class PersonSerializer(serializers.ModelSerializer):
    class Meta:
        model = Person
        exclude = ('profile',)

class PersonArgsInput(convert_serializer_to_input_type(PersonSerializer)):
    create = graphene.Boolean(required=True)

In the above code I [optionally] created a PersonArgsInput, which inherits from the generated Input class, so I can add additional fields. I can then use the the new class in my mutations however I want. like so:

class PersonsMutation(graphene.Mutation):
    person_set = graphene.List(PersonType)
    class Arguments:
        person_set = graphene.List(PersonArgsInput)
    def mutate():
    ...
@zbyte64

This comment has been minimized.

Copy link
Collaborator

commented Sep 21, 2018

@charlesverdad is there a reason you didn't use graphene_django.rest_framework.mutation. SerializerMutation instead? This is what I got:

class UseMutation(SerializerMutation):
    class Meta:
        serializer_class = UseSerializer

I believe it signals a create vs update based on the presence of the id field in the input. Doesn't seem to offer a delete method.

Edit: Personally I'd rather not use rest framework, things get confusing when you have custom types (eg GeoJSON). I've found directly defining fields in graphene to be much clearer.

@charlesverdad

This comment has been minimized.

Copy link

commented Sep 21, 2018

@zbyte64 I considered that as well but for my use case, I wanted a graphene.List of the object as an input to be able to modify many objects in a single query.

I agree that the solution is dirty. In fact, I have a problem where the converter marks some input fields as required if the corresponding model's fields are non-nullable. If a lot of fields are required then there won't be any benefit to the partial update feature since I'd have to query the required fields and send them to the mutation along with the updated fields.

@stale

This comment has been minimized.

Copy link

commented Jun 11, 2019

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.

@stale stale bot added the wontfix label Jun 11, 2019

@stale stale bot closed this Jun 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.