soveran / cuba

Rum based microframework for web development.
http://cuba.is
MIT License
1.44k stars 249 forks source link

Optional query parameters #95

Closed pjmartorell closed 5 years ago

pjmartorell commented 5 years ago

Hi, I have a question regarding optional query parameters. I have the following routes defined:

Cuba.define do
  on get do
    on 'movies' do
      on param("genre"), param("offset"), param("limit") do |genre, offset, limit|
        ...
      end
    end
  end
end

But this route only matches when exactly all query params are passed and I would like this route to match even if none are passed or just some. I mean, make these parameters are completely optional. Is there any way to achieve that in a simple way instead of using req.env["rack.request.query_hash"] directly?

Thanks

soveran commented 5 years ago

Maybe you can try something like this?

on param("genre") || param("offset") || param("limit") do |genre, offset, limit|
  ...
end

Another idea would be to use default values (the second parameter to the param matcher), so you can do something like this:

on param("genre", "all") || param("offset", "0") do |genre, offset|
  ...
end

Or a combination of those ideas. Let me know if it works for your use case.

pjmartorell commented 5 years ago

Thanks! I don't know I didn't came up with this solution, both suggestions were very helpful :)

pjmartorell commented 5 years ago

Oops, I noticed that because of the OR clause, whenparam("genre") is present, the rest (offset and limit) are not being evaluated resulting in nil values instead of the passed value or the default value.

soveran commented 5 years ago

My solutions were terrible! But at least the second one can be fixed:

on param("genre", "all"), param("offset", "0") do |genre, offset|
  ...
end

In the original, I intended to use , when passing default values, but I left the ||. The first example should never work.

pjmartorell commented 5 years ago

Thanks for the fast reply! If I'm right, this makes the offset param mandatory, but I would like it to be optional. Anyway I'm considering moving the logic to set default values to another place rather than handling it in the matchers.

pjmartorell commented 5 years ago

Sorry, you were right, it matches the route and sets the default value if it's the query param is not passed. Thanks!