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

Saving null value for nullable fields #280

Closed
prokofiev opened this issue Sep 12, 2016 · 28 comments
Closed

Saving null value for nullable fields #280

prokofiev opened this issue Sep 12, 2016 · 28 comments

Comments

@prokofiev
Copy link

I had a problem saving the null values in the mutations. Is it possible to implement functionality to get_argument_values function could distinguish passed null value and not passed value?
I think it can be implemented similar to the executor.ececute_fields using Undefined.

@syrusakbary
Copy link
Member

syrusakbary commented Sep 15, 2016

Hi @prokofiev!

You can actually use the default_value in the Argument and set another default than None.

So currently you can do:

class Undefined(object): pass

class Query(graphene.ObjectType):
    field = graphene.String(
        input=graphene.String(default_value=Undefined)
    )

It might be interesting to set Undefined as default_value in a GraphQLArgument in graphql-core instead of None, but probably this is something that we should investigate a little bit further :)

@prokofiev
Copy link
Author

prokofiev commented Sep 15, 2016

Hi @syrusakbary !
Thanks for answer, but default_value will not be able to solve my problem.
I try to describe the problem in more detail:

class Foo(models.Model):
    ''' Example model with nullable field '''
    not_null = models.CharField(max_length=10)
    nullable = models.CharField(max_length=10, null=True)

Next my use cases:

  1. Insert with default null value
mutation InsertFoo{
    saveFoo (input:{
        clientMutationId:"1"
        notNull:"value"
    }) {
      ...
    }
}
  1. Update nullable field (set "value"), not_null ignored
mutation UpdateFoo{
    saveFoo (input:{
        clientMutationId:"1"
        id:"id"
        nullable: "value"
    }) {
      ...
    }
}
  1. Update nullable field (set null), not_null ignored
mutation UpdateFoo{
    saveFoo (input:{
        clientMutationId:"1"
        id:"id"
        nullable: null
    }) {
      ...
    }
}

How can I use "default_value" to set the nullable field to null (case 3)?

@syrusakbary
Copy link
Member

Hey @prokofiev, after thinking about this I realized this could be approached in a easier way. As input is a dict, you could check if not_null in exists in input.

Something like

@classmethod
def mutate_and_get_payload(cls, input, context, info):
    not_null = 'UNDEFINED'
    if 'not_null' in input:
        # ....
        not_null = input['not_null']
   # ....

Does that make sense?

@prokofiev
Copy link
Author

prokofiev commented Sep 23, 2016

Hi @syrusakbary !
It does not make sense because the fields with null values are removed from the argument list and not in the input dict.
Problem in graphql-core in this and this functions.
Function value_from_ast return None, and then function get_argument_values check if value is not None.

@syrusakbary
Copy link
Member

After checking, I realized that this query:

mutation UpdateFoo{
    saveFoo (input:{
        clientMutationId:"1"
        id:"id"
        nullable: null
    }) {
      ...
    }
}

Is actually not valid as null is not a valid value in GraphQL.
https://github.com/graphql-python/graphql-core/blob/master/graphql/language/tests/test_parser.py#L80-L84

For this reason I guess that some client's check when the value is null and then skip it (Relay?).

As an extra exercise, I checked that arguments and input fields that are not provided don't fill the dictionary with None values:

def test_does_not_include_arguments_that_were_not_set():
    InputObject = GraphQLInputObjectType(
        name='InputObject',
        fields={
            'value': GraphQLInputObjectField(GraphQLString, default_value=None),
            'other': GraphQLInputObjectField(GraphQLString, default_value=None)
        }
    )
    def resolver(data, args, *_):
        print args
        return 'other' in args['c']

    schema = GraphQLSchema(GraphQLObjectType(
        'Type',
        {
            'field': GraphQLField(
                GraphQLInt,
                resolver=resolver,
                args={
                    'a': GraphQLArgument(GraphQLBoolean),
                    'b': GraphQLArgument(GraphQLBoolean),
                    'c': GraphQLArgument(InputObject),
                    'd': GraphQLArgument(GraphQLInt),
                    'e': GraphQLArgument(GraphQLInt),
                }
            )
        }
    ))

    ast = parse('{ field(c: {value: "3"}) }')
    result = execute(schema, ast)
    assert result.data == {
        'field': False
    }

If you can provide more insights that I might be missing please let me know.

@prokofiev
Copy link
Author

null value may be passed like this:

mutation UpdateFoo($var1:String!){
    saveFoo (input:{
        clientMutationId:"1"
        id:"id"
        nullable: $var1
    }) {
      ...
    }
}

variables:

{"var1": null}

@nuschk
Copy link

nuschk commented Mar 27, 2017

Hey @prokofiev , did you find a workaround for this?

I'm struggling with exactly the same problem and am very interested in your approach. The least hack I could come up with, is to always require all arguments and then use 'input.get('myVar', None)' to retrieve them in 'mutate_and_get_payload()'.

Even setting the field to be required and having a default value will not make it appear in the input dict...

@prokofiev
Copy link
Author

Hi @nuschk.
I found solution for my case. It's monkey patch but it works...
You can see my patch here
Call apply from any place when initialize your app.

@prokofiev
Copy link
Author

If somebody knows how to fix it more correctly, please let me know.

@nuschk
Copy link

nuschk commented Mar 29, 2017

@prokofiev Hehe, thanks for the snippet. Will look into it.

@BossGrand
Copy link
Member

@syrusakbary have you seen this pull request?
graphql/graphql-spec#83

graphql now supports nullable types. Can we get some support for this in graphene?

@NiekHoekstra
Copy link

Any progress on this?

@Eraldo
Copy link

Eraldo commented Sep 20, 2017

@NiekHoekstra Does a PR exist yet for this? If not are you willing to take the patch and turn it into a PR? :)

@patrickkempff
Copy link

We are having the same problem!

@BossGrand
Copy link
Member

This issue is solved in graphene 2.0 and should be closed

@arif-hanif
Copy link

Does someone have an example for 2.0?

@rdemetrescu
Copy link

@BossGrand I am still having this problem with null values.
If my javascript client sends a mutation with those variables:
{"id": 4, "titulo": "TITULO", "resenha": "RESENHA", "autorId": 1}

my server receives this as **args (at mutate function):
{'autor_id': 1, 'id': 4, 'resenha': 'RESENHA', 'titulo': 'TITULO'}

However, if the client sends this:

{"id": 4, "titulo": "TITULO", "resenha": null, "autorId": null}

My python **args param receives only 2 keys/values:

{'id': 4, 'titulo': 'TITULO'}

(where I would expect receiving {'id': 4, 'titulo': 'TITULO', 'resenha': None, 'autorId': None})

My packages versions:

➜ pip freeze | grep -i graph
Flask-GraphQL==1.4.1
graphene==2.0
graphene-sqlalchemy==2.0.0
graphql-core==2.0
graphql-relay==0.4.5

Should this issue be reopened ?

Thanks in advance

@arif-hanif
Copy link

@rdemetrescu i get the same thing... so not sure if this is really fixed.

@rdemetrescu
Copy link

@arif-hanif, just noticed that mutations using relay work perfectly.

@jinty
Copy link

jinty commented Nov 22, 2017

I have seen this query:

                mutation myMutation {
                       editIt(Input: {id: "AFJS", title: null}) {
                           It {id}
                       }
                  }

fail with:

 'Syntax Error GraphQL request (3:112) Unexpected Name "null"

However when I pass title as a "variable" it does work.

@jkimbo
Copy link
Member

jkimbo commented Feb 25, 2018

graphql-core is responsible for the query parsing so that is where adding null to the language will need to be added. Looks like there is already a pull request open to implement it: graphql-python/graphql-core#119

@rdemetrescu I can't reproduce your issue. If you are still having problems can you open a new issue with everything needed to reproduce it?

Closing this for now

@jkimbo jkimbo closed this as completed Feb 25, 2018
@brabeji
Copy link

brabeji commented Jul 25, 2020

@jkimbo Hi, this is still unsolved and the link to the graphql-core pull request is dead. Can you please advise where this is/was tracked? Thanks!

@jkimbo
Copy link
Member

jkimbo commented Jul 25, 2020

@brabeji we will shortly be releasing graphene v3 that is based on graphql-core v3 which is a complete rewrite and has support for null values. You can try out the beta release now. Release notes are here: https://github.com/graphql-python/graphene/wiki/v3-release-notes

@laurent-brisbois
Copy link

@jkimbo Hello !
Any luck to see the graphene v3 official release soon to solve this ?

@JBrVJxsc
Copy link

Need this change as well. It does not make sense to remove null values in the parameters.

@arghodayah
Copy link

need this as well

@laurent-brisbois
Copy link

laurent-brisbois commented Jan 18, 2021

Is there any breaking change in V3 ? Maybe we can use the latest beta while waiting the official release ?
It is quite annoying at the moment since we cannot unset a variable by sending 'null' from the frontend to the mutation.

Edit:
Tried quickly and it seems django-graphql-jwt which I use for authentication isn't compatible...
Tried with :

django==3.1.5
psycopg2==2.8.6
graphene-django==3.0.0b7
django-graphql-jwt==0.3.1
PyJWT>=1.5.0,<2
pyyaml==5.3.1
gunicorn==20.0.4

instead of :

django==3.1.5
psycopg2==2.8.6
graphene-django==2.15.0
django-graphql-jwt==0.3.1
PyJWT>=1.5.0,<2
pyyaml==5.3.1
gunicorn==20.0.4

@arghodayah
Copy link

Is there any breaking change in V3 ? Maybe we can use the latest beta while waiting the official release ?
It is quite annoying at the moment since we cannot unset a variable by sending 'null' from the frontend to the mutation.

Edit:
Tried quickly and it seems django-graphql-jwt which I use for authentication isn't compatible...
Tried with :

django==3.1.5
psycopg2==2.8.6
graphene-django==3.0.0b7
django-graphql-jwt==0.3.1
PyJWT>=1.5.0,<2
pyyaml==5.3.1
gunicorn==20.0.4

instead of :

django==3.1.5
psycopg2==2.8.6
graphene-django==2.15.0
django-graphql-jwt==0.3.1
PyJWT>=1.5.0,<2
pyyaml==5.3.1
gunicorn==20.0.4

Had same issue, we need to wait for django-graphql-jwt update

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