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

Choice fields automatically converted to enums #67

Closed
kaglowka opened this issue Nov 24, 2016 · 22 comments
Closed

Choice fields automatically converted to enums #67

kaglowka opened this issue Nov 24, 2016 · 22 comments

Comments

@kaglowka
Copy link

class User(DjangoObjectType):
    class Meta:
        model = get_user_model()
        only_fields = ['signup_site_version']

class Query(graphene.ObjectType):
    user = graphene.Field(User)

    def resolve_user(self, args, context, info):
        return user_from_info(info)  # return django user from context

User model:

@python_2_unicode_compatible
class User(AbstractUser):
    (...)
    signup_site_version_choices = [(1, u'site 1'), (2, u'site 2')]
    signup_site_version = models.IntegerField(choices=signup_site_version_choices)

Query

query { 
  user {
    signupSiteVersion
  }
}

Result

{
  "data": {
    "user": {
      "signupSiteVersion": "A_1"
    }
  }
}

A copy-paste from where the weird value is born:

def convert_choice_name(name):
    name = to_const(force_text(name))
    try:
        assert_valid_name(name)
    except AssertionError:
        name = "A_%s" % name
    return name

We've had similar problems in the old graphene (< 1.0): graphql-python/graphene#134
The output is "A_1", because the field converter assumes django choices are tuples (id, name) where name is a constant name (not containing spaces etc.) or is convertible to a valid constant name. For some reason it is not successful while doing that for "site 1", but that is not an issue. The problem is it assumes we would like to do that.
In our django application (I suppose it is the norm) we use the second tuple element as a human-readable label. It means that in most cases replacing space with an underscore will do the work, but
in the other names are still bound to be long and there is no character we can safely assume the name won't contain (including hyphens, all native character varieties etc.).
The Enum fields are certainly nice, as their values are schema-validated, to say the least, but I doubt automatic conversion should be the only possible behaviour.

@mjtamlyn
Copy link

My personal opinion is that it should be easy to make choices an Enum, but that it shouldn't be the default. The requirements of django choices are much, much more permissive than those of an Enum (in either Python or GraphQL)

@khanetor
Copy link

I also ran into this but I thought it were intensional.

@pity7736
Copy link

Hi,

How I can overwrite the "A_1" output?

Thanks

@BrianChapman
Copy link

Choices are also converted to uppercase, which makes it difficult to manage during mutations, especially if the choices were mixed upper and lower case. There should be an option to just leave your choices alone.

@denis-ryzhkov
Copy link

Maybe someone adds GRAPHENE_CONVERT_CHOICES_TO_ENUMS = False setting to graphene_django once.
Until then, you can opt out with monkey-patch somewhere before schema is imported, for example in settings file.
Patch: https://gist.github.com/denis-ryzhkov/fcb944f6ebfad4775efb74848703f0d7

@mvanlonden
Copy link
Member

What would be the desired behavior for enums? I think it makes sense to handle them automatically unless explicitly defined on in the schema but maybe we can introduce some config to handle the different use cases.

@zbyte64
Copy link
Collaborator

zbyte64 commented Mar 22, 2019

Way I see it there are 3 options:

  • Create one converter to rule them all. This is what we're doing right now, I don't think it will ever work.
  • Option to disable enum globally. More like a stopgap solution, should be avoided
  • Option to explicitly set the enum value. This is probably the right way to go. Check out Support formatting of enum choices #209

There are quite a few issues related to this behavior:

@mvanlonden
Copy link
Member

mvanlonden commented Apr 27, 2019

I like the simplicity of #209 but don't like that it relies on another dependency and requires changes in the model layer.

After reading the various proposals, I think a hybrid approach might work.

  • Improve the default enum conversion to cover most use cases. Remove any custom formatting so the query-mutation round trip is easily accomplished. Custom enum mapping can be defined in a field's resolver (should add an example to the docs).
  • Provide a global setting GRAPHENE_DISABLE_ENUM_CONVERSION, False by default

@phalt phalt moved this from Potential candidates to Accepted in Improvements Apr 29, 2019
@denis-ryzhkov
Copy link

Please add one more step to the hybrid approach above:

  • Provide a setting GRAPHENE_DISABLE_ENUM_CONVERSION being False by default for backward compatibility. When set to True, it will save a user from a lot of boilerplate code and effort of setting Meta.disable_enum_conversion lists and keeping them updated

@mvanlonden
Copy link
Member

@denis-ryzhkov updated the original post—removed the list approach and replaced with the global setting. Custom resolvers can be used instead of Meta config list.

@erik-megarad
Copy link

Any updates on this? Using the monkeypatch for now, but that's really sketchy.

@phalt
Copy link
Contributor

phalt commented May 7, 2019

@subwindow we've made no development process on this but it looks like we've agreed on the approach we will take, as @mvanlonden said:

  • Improve the default enum conversion to cover most use cases. Remove any custom formatting so the query-mutation round trip is easily accomplished. Custom enum mapping can be defined in a field's resolver (should add an example to the docs).
  • Provide a global setting GRAPHENE_DISABLE_ENUM_CONVERSION, False by default

We are currently focused on setting up governance and organising the support around the graphene project as a whole, when that is more stable this should be one of the first features we tackle. I want it too, so I can't wait to get started on it!

@phalt phalt moved this from Accepted to In progress in Improvements May 7, 2019
@phalt phalt moved this from In progress to In review in Improvements May 8, 2019
@Destroy666x
Copy link

Destroy666x commented May 22, 2019

There's also an issue with enum fields containing non-English letters and throwing an error. I guess a way to just disable the conversion overall would be fine, there are many edge cases like that that this conversion doesn't cover

BTW, the integration with Django filter lib is weird too, ChoiceFilter is used by default which shows all results for non-existent choices, had to also switch to CharFilter

@zbyte64
Copy link
Collaborator

zbyte64 commented May 31, 2019

#654 Is a small improvement for generating Enums with integer fields.

Looks like we need tests with non-english choices.

@jkimbo
Copy link
Member

jkimbo commented Jul 13, 2019

@kaglowka v2.4.0 contains a new feature which allows you to turn off automatic enum creation on a case by case basis: http://docs.graphene-python.org/projects/django/en/latest/queries/#choices-to-enum-conversion

@jkimbo jkimbo closed this as completed Jul 13, 2019
@jkimbo jkimbo moved this from In review to Done in Improvements Jul 13, 2019
@o3o3o
Copy link

o3o3o commented Jul 25, 2019

If I want to use the converted choices Enum as the filter argument.
How I can easily define it in the query?
Here is a way to do it:

class Label(BaseModel):
    TYPES = [(x, x) for x in ("AAA", "BBB")]
    name = models.CharField(max_length=256)
    type = models.CharField(max_length=8, null=True, choices=TYPES)

class LabelQL(DjangoObjectType):
    class Meta:
        model = Label
        only_fields = ("name", "type")
        name = "Label"

LabelType = LabelQL._meta.fields["type"].type # That is tricky

class Query(graphene.ObjectType):
     labels = graphene.List(LabelQL, types=graphene.List(LabelType))

     def resolve_labels(root, info, types=None):

         qs = Label.objects.all()
         if types and len(types) > 0:
             qs = qs.filter(type__in=types)
         return qs

I have to get the converted type from LabelQL._meta.fields["type"].type

I there any natural way to use LabelType in the query?

@levrik
Copy link

levrik commented Aug 6, 2019

@jkimbo What about users using Flask? How can we turn this feature off?

@jkimbo
Copy link
Member

jkimbo commented Aug 7, 2019

@levrik you will have to raise that issue with the graphene-flask repo

@Suor
Copy link

Suor commented Jul 6, 2020

Why is this closed? I still have this issue.

@jkimbo
Copy link
Member

jkimbo commented Jul 6, 2020

@Suor because Graphene Django now lets you turn off the automatic enum conversion: http://docs.graphene-python.org/projects/django/en/latest/queries/#choices-to-enum-conversion

@japrogramer
Copy link

japrogramer commented Aug 18, 2020

@o3o3o

    I there any natural way to use LabelType in the query?

I found that this works

ServiceStage = Service._meta.fields["stage"].type
...
Query:
   orf_service_by_organization_id = graphene.List(Service,
       stage=graphene.Argument(ServiceStage, required=False, default_value='A'),
       id = graphene.UUID(required=False))

@japrogramer
Copy link

japrogramer commented Aug 18, 2020

Hello I have managed to make the Choices field use an Enum,
Question:
How do I query the available choices ?

In the past I have done

# Here the self is an empty form instance
class UserProfileFormChoicesType(graphene.ObjectType):
    clinical_expertise = generic.GenericScalar()
    psychosocial_expertise = generic.GenericScalar()

    def resolve_clinical_expertise(self, info):
        return [i for i in self.fields['clinical_expertise'].choices]

    def resolve_psychosocial_expertise(self, info):
        return [i for i in self.fields['psychosocial_expertise'].choices]

# The form for the choices
class UserProfileSettingsForm(forms.ModelForm):


    class Meta:
        model = User
        fields = [ ... ]

I know that i can introspect the whole schema with

query IntrospectionQuery {
  __schema {
    queryType {
      name
    }
    mutationType {
      name
    }
    subscriptionType {
      name
    }
    types {
      ...FullType
    }
    directives {
      name
      description
      locations
      args {
        ...InputValue
      }
    }
  }
}

fragment FullType on __Type {
  kind
  name
  description
  fields(includeDeprecated: true) {
    name
    description
    args {
      ...InputValue
    }
    type {
      ...TypeRef
    }
    isDeprecated
    deprecationReason
  }
  inputFields {
    ...InputValue
  }
  interfaces {
    ...TypeRef
  }
  enumValues(includeDeprecated: true) {
    name
    description
    isDeprecated
    deprecationReason
  }
  possibleTypes {
    ...TypeRef
  }
}

fragment InputValue on __InputValue {
  name
  description
  type {
    ...TypeRef
  }
  defaultValue
}

fragment TypeRef on __Type {
  kind
  name
  ofType {
    kind
    name
    ofType {
      kind
      name
      ofType {
        kind
        name
        ofType {
          kind
          name
          ofType {
            kind
            name
            ofType {
              kind
              name
              ofType {
                kind
                name
              }
            }
          }
        }
      }
    }
  }
}

but how do I introspect the specific type generated for the Enum choices ?
Is using a form still the best choice ?

Update found solution

graphene.Field(generic.GenericScalar)
# resolve to this
Service._meta.get_field('stage').choices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Improvements
  
Done
Development

No branches or pull requests