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

Omitted Optional Parameters fails with error. #67

Closed
GeertWille opened this issue Feb 15, 2018 · 12 comments
Closed

Omitted Optional Parameters fails with error. #67

GeertWille opened this issue Feb 15, 2018 · 12 comments

Comments

@GeertWille
Copy link

Hello,

I have a case where I want to combine multiple parameters together to do a specific query. Some of them are optional but as soon as I trigger the query I always get a response that I have missing params to run the query. Is it possible that there is no support yet for optional params?

@fbartho
Copy link
Collaborator

fbartho commented Feb 16, 2018

@GeertWille If you can submit a test for us to explain what you mean, I’d love to take a look!

@HeyHugo
Copy link
Contributor

HeyHugo commented Feb 16, 2018

I think I've encountered thing as well. Heres a small example:

gql`
  query FilterItems(
    $priceMin: Int
    $priceMax: Int
  ) {
    filterItems(
      priceMin: $priceMin
      priceMax: $priceMax
    )
      @rest(
        type: "Item"
        path: "filter?priceMax=:priceMax&priceMin=:priceMin"
      ) {
      id
      name
      price
    }
  }
`

So here I may or may not have priceMax and priceMin, but if I don't have them I will get the:
Missing params to run query error...

Maybe there is an easy way to dynamically build up the query string somehow?

@fbartho
Copy link
Collaborator

fbartho commented Feb 16, 2018

does that still happen if you pass null for that parameter to the variables hash?

@HeyHugo
Copy link
Contributor

HeyHugo commented Feb 16, 2018

Yes confirmed now that it still happens with null values

@fbartho
Copy link
Collaborator

fbartho commented Feb 16, 2018

@HeyHugo What do you expect the path: to be turned into?
filter?priceMax=undefined&priceMin=5

If that's not what you expect, I don't think we can make the path omit undefined chunks and it would be clunky to provide a function to map the path.

@fbartho fbartho changed the title Optional parameters doesn't work? Omitted Optional Parameters fails with error. Feb 16, 2018
@HeyHugo
Copy link
Contributor

HeyHugo commented Feb 16, 2018

Hmm maybe this is crazy bad idea :)

..but what if instead of having the path inside the rest graphql directive the API was to provide a function somwhere taking the variables (and props?) as arguments and returning the path.

@fbartho
Copy link
Collaborator

fbartho commented Feb 16, 2018

I thought about that! -- We even have an example of the bodyBuilder as a function -- the problem is that a fundamental feature of apollo-link-rest is that you can @rest(path: "filter…") on any level of any GraphQL query. The result is that you'd have to provide your pathMapper-function as a variable that you pass in to all the queries that use @rest(path: "filter…") -- that would be really clunky. :(

@fbartho
Copy link
Collaborator

fbartho commented Feb 16, 2018

We could maybe have a registry of dynamic path-mappers at the RestLink-level, and have your @rest directive state the path-mapper-identifier to use, and pass the current variables in to the function, maybe?

@HeyHugo
Copy link
Contributor

HeyHugo commented Feb 16, 2018

Cool, yes that would cover my use case at least 👍

@fbartho
Copy link
Collaborator

fbartho commented Feb 16, 2018

@HeyHugo -- I recommend opening a new GitHub issue to track that feature idea. If you have time & the inclination to help improve apollo-link-rest if you wanted to implement this, I could help you get it reviewed. -- Like I said, most of what you'll need to do is like the bodyBuilder

@HeyHugo
Copy link
Contributor

HeyHugo commented Feb 16, 2018

Ok, I'll make an attempt at this.

Would be my first TypeScript code ever, so I'm expecting some bumps 😬

HeyHugo added a commit to HeyHugo/apollo-link-rest that referenced this issue Feb 17, 2018
pathBuilder is a user provided function via a graphql variable to build up the path string. A use case it enables is having one or more optional query string parameters e.g:
"/items/filter?minPrice=5&maxPrice=20"
Here minPrice and maxPrice could be omitted

See proposal in apollographql#69 and the problem it solves in apollographql#67
HeyHugo added a commit to HeyHugo/apollo-link-rest that referenced this issue Feb 18, 2018
pathBuilder is a user provided function via a graphql variable to build up the path string. A use case it enables is having one or more optional query string parameters e.g:
"/items/filter?minPrice=5&maxPrice=20"
Here minPrice and maxPrice could be omitted

See proposal in apollographql#69 and the problem it solves in apollographql#67
HeyHugo added a commit to HeyHugo/apollo-link-rest that referenced this issue Feb 18, 2018
pathBuilder is a user provided function via a graphql variable to build up the path string. A use case it enables is having one or more optional query string parameters e.g:
"/items/filter?minPrice=5&maxPrice=20"
Here minPrice and maxPrice could be omitted

See proposal in apollographql#69 and the problem it solves in apollographql#67
HeyHugo added a commit to HeyHugo/apollo-link-rest that referenced this issue Feb 18, 2018
pathBuilder is a user provided function via a graphql variable to build up the path string. A use case it enables is having one or more optional query string parameters e.g:
"/items/filter?minPrice=5&maxPrice=20"
Here minPrice and maxPrice could be omitted

See proposal in apollographql#69 and the problem it solves in apollographql#67
@GeertWille
Copy link
Author

GeertWille commented Feb 21, 2018

looks good to me! Thanks for the very fast response and PR 👍

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

No branches or pull requests

3 participants