-
Notifications
You must be signed in to change notification settings - Fork 31
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
route_param regression from 0.86 -> 0.87 #84
Comments
I'll take a look ASAP. Thanks for reporting. |
Maybe I misunderstand how
|
To me, its not clear how route_params can be mixed with query params... but saying that the above code works as described in 0.86 and doesnt in 0.87. But strangely, it routes correctly in 0.87 but doesnt populate the $_[0] with a hashref of parameters. If i have misinterpreted the documentation and somehow relied upon non-intentional behaviour i am happy to change my code. But for now i have Raisin pinned to 0.86 in my cpanfile |
Part of the problem is that nested This was fixed in 0.87. I suspect this is what you need for 0.87:
And as a nice benefit you don't have to repeat the But per the raisin docs, preceding the |
Ok this seems to work but has the annoying side effect that previously if the route_param value type didnt match the type it would return 400. Where as now it returns a 404. I am guessing this is due to failing to match causing it to not route past the handler code. |
That indeed is not fixed yet. It's on my roadmap. |
Just out of curiosity, because I did not find it anywhere, and before digging into the code, I would ask: How do you access the |
that would be something like: get sub {
my $params = shift;
$params->{postcode};
$params->{locale};
$params->{...};
} Where Line 12 in 5f77df3
The problem here must be that nested path arguments are not stored properly and/or got overwritten somewhere inside that function: Line 102 in 5f77df3
This would be the patch which introduced the issue: 06d50a9 |
my bad, I messed up the |
This works in 0.86 but not in 0.87
The api is as follows
/etd///
or
/etd///?material=&material=
Locale, PostCode, ad MaterialCode are custom constraints.
What doesnt work in 0.87 is the route_param version. The &guess_etd is called BUT the $_[0] is empty rather than containing the params as it does with the material in query query version
the fix_material sub massages the material= so that its always an arrayref even if there is just one value (which is an annoying bug of itself, as a single value will give a scalar and the type will fail)
The text was updated successfully, but these errors were encountered: