POST route recognised as GET
romankovt opened this issue · 2 comments
Hello!
I'm following the README guide from the main branch (which points to v2.0.0.alpha6 now). The Testing section shows an example of how it does not recognize a POST route when the router has a route defined as GET.
require "hanami/router"
router = Hanami::Router.new do
get "/books/:id", to: "books#show", as: :book
end
route = router.recognize("/books/23", method: :post)
route.verb # "POST"
route.routable? # => false
But in fact, what I get is:
require "hanami/router"
router = Hanami::Router.new do
get "/books/:id", to: "books#show", as: :book
end
route = router.recognize("/books/23", method: :post)
route.verb # => "GET"
route.routable? # => "true"
Upon further investigation, it seems like the other examples when you recognize non-String routes also don't work
require "hanami/router"
router = Hanami::Router.new do
get "/books/:id", to: "books#show", as: :book
end
route = router.recognize(:book, id: 23, method: :POST)
route.verb # => GET
route.routable? # => true
The cause of the problem is that #env_for takes 2 hashes as arguments, but when you use router.recognize(:book, id: 23, method: :post)
it takes id and method as the first hash argument, but the intended behavior is for them to be 2 separate arguments.
It can be fixed by passing the first argument as empty hash:
require "hanami/router"
router = Hanami::Router.new do
get "/books/:id", to: "books#show", as: :book
end
route = router.recognize(:book, { id: 23 }, { method: :post })
route.verb # => POST
route.routable? # => false
route = router.recognize("/books/32", {}, { method: :post })
route.verb # => POST
route.routable? # => false
So, the docs should be updated, or if it was not intended to be this way it is worth changing the #env_for behavior leaving it with only 2 arguments def env_for(env, params = {})
and parsing and separating all the possible options keys from the params hash here, but as a side effect one wouldn't be able to have reserved keywords as HTTP param names:)
@romankovt Good catch! Thanks for reporting.