Add convert_choices_to_enum option on DjangoObjectType Meta class

[jkimbo]

This PR adds a new option to the DjangoObjectType Meta class: convert_choices_to_enum. This option (which defaults to True) can be set to False to disable all automatic choices to Enum conversions on that ObjectType. Or set to a list of field names to be automatically converted to Enums.

The aim of this new feature is provide a quick way to disable the automatic Enum conversion for fields that have choices. There are a lot of issues with the automatic conversion currently and hopefully this feature can alleviate some of those issues.

[patrick91]

Looks good to me :)

[phalt]

This is a good feature!

[zbyte64]

I like this approach. Perhaps a similar tact could be used for specifying explicit enums

[dopeboy]

Well documented and tested - thanks @jkimbo. LGTM.

[rhigueraa]

Hello! is it possible that this same thing happens when using an Enum (TemplateType) as an argument in a query and the same Enum as a parameter in a mutation ?

Example:

class MicrotemplateQuery(graphene.ObjectType):
    microtemplates_by_template_type = DjangoFilterConnectionField(
        MicrotemplateNode,
        args={
            'template_type': graphene.Argument(
                graphene.Enum.from_enum(TemplateType),
                required=True
            )
        }
    )

and

class CreateTemplate(graphene.Mutation):

    class Input:

        template_type = graphene.Argument(
            graphene.Enum.from_enum(TemplateType),
            required=True,
            default_value=TemplateType.site.value
        )

gets:
AssertionError: Found different types with the same name in the schema: TemplateType, TemplateType.

A provisional fix was to define the argument in a global variable and pass that to the query and mutation.

[jkimbo]

@rhigueraa I'm assuming that TemplateType is the same enum? Your solution with defining the result as a variable and then passing it to the query and mutation doesn't seem unreasonable. The from_enum method creates a graphene enum from the input and so it results in a new type with the same name which is why it clashes.

I guess it could have some kind of cache built in so that the same class would result in the same enum but I'm not sure it's a good idea to implement caching behaviour unless it's absolutely necessary because it can lead to unexpected outcomes.

[glassipbel]

@jkimbo Hi there.
The solution above is unreasonable, because is just a quick fix and it wont scale in a long term solution.

Regarding the cache solution i don't really know how the library is built, so i don't know if that could be a good solution but the way it is right now is not allowing us to implement a proper solution while using enums.

So it would be nice if you could take a look around that cache solution.

[jkimbo]

@kbelter can you explain more why "it wont scale in a long term solution"? Converting an enum to a graphene enum once and then exposing that result so that it can be reused in multiple places seems like a fine solution to me.

[glassipbel]

@jkimbo
There are a couple of reasons of why this approach is not good:

1. You need to declare if that parameter is required when you initialize that property, so the problem there is that if in some queries that parameter is required and in some other it's not, therefore you have a problem because that value will change over time.

2. You are instantiating an instance at that level and because it is outside the class whenever you import that file that's when that initialization it's happening, so you may never use it and that instance is never getting deallocated. (If we could instantiate the instance properly and just use it in the 2 methods then the instance would get killed after the method finishes.)

Also is not really snazzy to approach the feature this way and in the future other developers may have the same problem, so it's not really an intuitive solution.

I know this is an open source library and i really appreciate the time and effort that all of you guys are putting into this project, this is just a constructive comment.

[glassipbel]

@jkimbo have you thought about what i told you in the above comment?

[jkimbo]

You have some good points @kbelter that I hadn’t considered. Consider my mind changed! Could you create a PR that implements this and I’ll happily review and get it merged? It will have to be on the graphene repo since the enum conversion lives there.

[gabelimon]

@jkimbo Is a global flag currently being worked on? We got blocked upgrading from 2.5->2.6 because we've already built a more complex enum recognition system and never want to go through this codepath. It would be nice if we didn't have to rewrite my whole codebase to account for this. I can put in a PR if necessary.