Skip to content

Django choice fields cannot be blank, even if field is not required -- graphene-django 2.1.0 / graphql-core 2.1 regression #474

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

Closed
picturedots opened this issue Jul 23, 2018 · 22 comments

Comments

@picturedots
Copy link

This seems to be a regression in the upgrade of graphql-core to 2.1 together withj graphene-django to 2.1.0.

Prior to the upgrade, I could create an optional choice field in Django, and could query the value if the value of the choice field was blank.

For example, assume that I have a Django model MyModel with an optional choice field my_choices_field, and a query handler that would accept a query like

{
  myModel(id: 1) {
    id
    myChoicesField
  }
}

With the updated versions, if the value of my_choices_field is blank, this query throws a handled exception like:

 Traceback (most recent call last):
   File "/usr/local/lib/python3.7/site-packages/promise/promise.py", line 65, in try_catch
     return (handler(*args, **kwargs), None)
   File "/usr/local/lib/python3.7/site-packages/graphql/execution/executor.py", line 527, in <lambda>
     exe_context, return_type, field_asts, info, path, resolved
   File "/usr/local/lib/python3.7/site-packages/graphql/execution/executor.py", line 556, in complete_value
     return complete_leaf_value(return_type, path, result)
   File "/usr/local/lib/python3.7/site-packages/graphql/execution/executor.py", line 622, in complete_leaf_value
     path=path,
 graphql.error.base.GraphQLError: Expected a value of type "MyModelMyChoicesField" but received:

And the output of the query includes errors when the value of the field is blank:

{
  "errors": [
    {
      "message": "Expected a value of type \"MyModelMyChoicesField\" but received: ",
      "path": [
        "myModel",
        "myChoicesField"
      ]
    }
  ],
  "data": {
    "myModel": {
      "id": "1",
      "myChoicesField": null
    }
  }
}

I imagine the issue is related to the work in graphql-core to related to enum fields

Improved GraphQLEnumType serialization allowing python Enum values 0cef6fb 24687b6

@atomboulian
Copy link

I am experiencing the same thing - only started noticing it Monday morning when we got in - Friday it was working perfectly. Thank you for opening this @picturedots.

@picturedots
Copy link
Author

It seems like the real issue is with graphql-core 2.1 -- I've tried to track down the issue a bit and added an issue there.

@atomboulian
Copy link

Thank you @picturedots. I'll chime in.

@sciyoshi
Copy link
Contributor

I wasn't able to reproduce this, here's a test case:

import graphene
from graphene_django import DjangoObjectType
from django.db import models

class Car(models.Model):
    class Meta:
        app_label = 'app'

    choice = models.IntegerField(choices=[(1, 'Blue'), (2, 'Green')], null=True)

class CarNode(DjangoObjectType, model=Car):
    pass

class Query(graphene.ObjectType):
    car = graphene.Field(CarNode)

    def resolve_car(self, info):
        return Car(choice=None)
    
schema = graphene.Schema(query=Query)
result = schema.execute('{ car { choice } }')

print(result.data, result.errors)

@atomboulian
Copy link

atomboulian commented Jul 31, 2018

@sciyoshi This is happening for CharField's, not IntegerField's (as far as I can tell, since I don't have IntegerField's in my application). The major difference I am noticing is that for CharFields, they must be blank=True instead of null=True.

@picturedots is your myChoicesField a CharField or some other field?

@picturedots
Copy link
Author

picturedots commented Aug 1, 2018

@atomboulian Yes I can confirm that it's a CharField, For example, I have a field defined like

PERIODIC_INTERVAL_CHOICES = (('Weekly', 'Weekly'),
     ('Bi-Weekly', 'Bi-Weekly'),
      ('Monthly', 'Monthly'),
      ('Quarterly', 'Quarterly'),
      ('Semi-Annually', 'Semi-Annually'),
      'Annually', 'Annually'))

payment_frequency = models.CharField(
    blank=True,
    null=True,
    choices=PERIODIC_INTERVAL_CHOICES,
    max_length=13)

I get the error whenever fetching "payment_frequency" and there is a blank or null value for that object.

I also get the error if null=True is removed, which is actually the correct way to represent an empty value in this case.

@atomboulian
Copy link

Yep, exactly.

@ikedumancas
Copy link

ikedumancas commented Aug 16, 2018

@sciyoshi I'm having this issue as well.

The code below will give me an error saying Cannot return null for non-nullable field ModelObjectType.frequency.

FREQUENCIES = (
    ('TWO_WEEKS', 'Every Two Week'),
    ('FOUR_WEEKS', 'Every Four Weeks')
)

frequency = models.CharField(choices=FREQUENCIES, blank=True)

If I add a null=True to the choice field, it will return a null value instead of an empty string.

If I add an empty string to the choices like this:

FREQUENCIES = (
    ('', '----'),
    ('TWO_WEEKS', 'Every Two Week'),
    ('FOUR_WEEKS', 'Every Four Weeks')
)

frequency will return A_

@atomboulian
Copy link

@ikedumancas Yep not good to put null=True on CharFields in Django - https://docs.djangoproject.com/en/2.1/ref/models/fields/#null

We should be able to use blank=True though, and it's not working.

I was curious about trying the empty string case as you did. You're literally getting "A_" in your responses that have an empty string though? Can you post your response? That is so weird!

Thanks for posting!

@ikedumancas
Copy link

@atomboulian Yeah. It's weird. It has to do with this code in converter.py

@picturedots
Copy link
Author

I've experienced the issue with blank=True as well. In order to not confuse issues I've created a separate (but related) issue. #503 .

@eamigo86
Copy link

Hi @picturedots, I have tried your solution but still the error reported.

@picturedots
Copy link
Author

The same error as reported is also an issue with graphene-django 2.2.0

@bk-equityzen
Copy link

bk-equityzen commented Dec 12, 2018

I believe the root issue here is that graphene-django uses the Enum type to represents django fields with choices. The python Enum class doesn't make sense to have the name be an empty string. Possibly writing a custom Enum class could solve this.

MyEnum = Enum('MyEnum', {'':'---'}) # The Enum lib errors when trying to initialize an empty string as the name. 

My workaround solution for this is to comment out the code that generates the enums and just use raw String types for choice CharFields:

def convert_django_field_with_choices(field, registry=None):
    if registry is not None:
        converted = registry.get_converted_field(field)
        if converted:
            return converted
    converted = convert_django_field(field, registry)
    if registry is not None:
        registry.register_converted_field(field, converted)
    return converted

@picturedots
Copy link
Author

@bk-equityzen I am also thinking about creating a workaround that will create a value in the enumeration that represents blank, but would then be converted into an actual blank value after being processed by graphene (possibly in the the model's save or clean method.) . Will post here if I get something working.

@bk-equityzen
Copy link

@bk-equityzen I am also thinking about creating a workaround that will create a value in the enumeration that represents blank, but would then be converted into an actual blank value after being processed by graphene (possibly in the the model's save or clean method.) . Will post here if I get something working.

I was looking into the same but it more correctly might be somewhere in the grapql-core code to change A_ into a blank string for queries. Putting it in your application model code seems a bit hacky and will cause problems in other places like the front end probably.

@mvanlonden
Copy link
Member

I'm curious what the thread thinks about the solution proposed above. This would add an EMPTY enum to the choices if blank=True

@picturedots
Copy link
Author

Since this bug is still outstanding and is blocking my upgrade to newer versions of graaphene, I am considering converting all of the affected fields in my project to use graphene string types over the API -- the frontend would need to be passed the set of strings formerly coming from the enumeration.

@mvanlonden
Copy link
Member

#598 (comment)

I think a person should be able to define the label for the blank option. For django forms the pattern is that you explicitly define a blank choice or pass empty_label: https://stackoverflow.com/questions/14541074/empty-label-choicefield-django

@dreamawakening
Copy link

dreamawakening commented Apr 18, 2019

I figured out a workaround. In this example gender is an Enum:

class UserNode(DjangoObjectType):
    gender = graphene.Field(graphene.String, source='gender')

    class Meta:
        model = User
        only_fields = ('id', 'gender')

Does anyone know when this bug will be fixed?

@dan-klasson
Copy link
Contributor

Filtering doesn't work on native enum types either.

@cansin
Copy link

cansin commented Jun 16, 2020

I am on the same boat. Currently, I am using the solution @dspacejs suggested. But it'd be nice to not need to explicitly state CharFields that are blank=True and have choices.

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

No branches or pull requests

10 participants