graphql icon indicating copy to clipboard operation
graphql copied to clipboard

Ability to provide default filtering arguments

Open darrellwarde opened this issue 4 years ago • 4 comments

First, thank you for investigating $options.sort not being properly resolved in cypher for custom query types. This is more important than the rest.

Wrt default values in where and options, I need to provide where, since I always have mandatory arguments, like login, so I do not want to get rid of where, but to specify default values for arguments inside where The same applies to default sort and limit in options

In the deprecated js library, arguments were supplied inline, and I was able to declare the default values like this:

downline(login: String!,  typeLevel: String="All")

and the value 'All' would be passed, although typeLevel is not specified in the method call.

 {
        downline(login: "c4") {
            id
            name
            level
            typeLevel
        }
    }

I have asked on discord and you suggested (I am not quoting you :)) to use ClientWhere = { typeLevels = ["All"] } syntax.

I think it could be a really useful feature.

Do you have any example of a custom resolver that does anything similar to what you had in mind?

Originally posted by @magaton in https://github.com/neo4j/graphql/issues/520#issuecomment-944611789

darrellwarde avatar Oct 18 '21 12:10 darrellwarde

Also from that issue was my insight into the problem:

Right, regarding the default values, this isn't necessarily a bug, it's actually doing exactly what I would expect it to do.

When you provide the custom query with the default values as above, you're basically saying "if I don't provide a where value, I want it to be this", and not "I want this default value to be added to the where values that I pass in". This is confirmed when you run a query with no arguments:

query {
  downline {
    id
    name
    login
    typeLevel
  }
}

To make this work, we would need to define a way (probably a directive), to pass in default values and output them to the input types themselves... But then this would affect everywhere they are used, including the generated queries and mutations. So this is actually a much trickier problem than it first looks.

Honestly, I would suggest using a custom resolver using the OGM instead of the @cypher directive for this - you can achieve this level of flexibility through JavaScript resolvers then.

darrellwarde avatar Oct 18 '21 12:10 darrellwarde

This is technically a feature request and not a bug, because the default value is being passed in as expected - it is for where and not the nested fields. The new feature would be a way to provide these arguments, and I'm not sure how to go about it and also whether it's worth doing.

darrellwarde avatar Oct 18 '21 12:10 darrellwarde

Let me try to clarify In my upline and downline query, the arguments are ClientWhere and ClientOptions. Let's make a default ClientOptions

upline(where: ClientWhere, options: ClientOptions = {sort: [{level: DESC}], limit: 1}): 

in custom cypher make reference to $options.sort and $options.limit

and in the request don't specify options at all.

The following is observed:

$options.limit is correctly resolved, but options.sort is not!

So, I don't think that the whole default object (ClientOptions) is passed through. If I understand your comment correctly, this should work!

Wrt ClientWhere, there are various fields, some of them could be specified as default (like typeLevel='All', but some certainly not, like login. I am not able to see the use case where the whole ClientWhere is specified as default without the ability to override some of the fields.

Now, about possible implementation, sorry but as you could already guess, I am not a JS developer, I have to use JS library because neo4j-graphql-java is abandoned and there I could contribute with a PR.

Anyhow, I would imagine that the library takes the specified field from ClientWhere and sets the not specified fields from the default statement as you suggested before.

In example for:

upline(where: ClientWhere = {typeLevels: 'All'}, options: ClientOptions = {sort: [{level: DESC}], limit: 1}): 

and the request:

upline(login: "c4") {
  name
}

The library would expand where by passing the enriched $where object so that in the underlying cypher query I could refer to$where.login and $where.typeLevels.

Does that make sense to you?

Thanks

magaton avatar Oct 18 '21 13:10 magaton

Right! Let's try this again! 😅

Let me try to clarify In my upline and downline query, the arguments are ClientWhere and ClientOptions. Let's make a default ClientOptions

upline(where: ClientWhere, options: ClientOptions = {sort: [{level: DESC}], limit: 1}): 

in custom cypher make reference to $options.sort and $options.limit

and in the request don't specify options at all.

The following is observed:

$options.limit is correctly resolved, but options.sort is not!

So, I don't think that the whole default object (ClientOptions) is passed through. If I understand your comment correctly, this should work!

Right, sounds like this is a suspected bug, different from the feature request to automatically inject the Cypher (#512). I'll add a new bug report for this and this alone.

Wrt ClientWhere, there are various fields, some of them could be specified as default (like typeLevel='All', but some certainly not, like login. I am not able to see the use case where the whole ClientWhere is specified as default without the ability to override some of the fields.

Now, about possible implementation, sorry but as you could already guess, I am not a JS developer, I have to use JS library because neo4j-graphql-java is abandoned and there I could contribute with a PR.

Anyhow, I would imagine that the library takes the specified field from ClientWhere and sets the not specified fields from the default statement as you suggested before.

In example for:

upline(where: ClientWhere = {typeLevels: 'All'}, options: ClientOptions = {sort: [{level: DESC}], limit: 1}): 

and the request:

upline(login: "c4") {
  name
}

The library would expand where by passing the enriched $where object so that in the underlying cypher query I could refer to$where.login and $where.typeLevels.

Does that make sense to you?

Thanks

It makes perfect sense, and I agree it should be a feature, but doing this in this format is not on us (it is not currently GraphQL spec compliant) - see this RFC in the graphql-spec repo: https://github.com/graphql/graphql-spec/pull/793

I imagine this will be addressed in GraphQL 16.x, so it might just be a case of holding out for a while.

darrellwarde avatar Oct 18 '21 15:10 darrellwarde