This project is archived and is in readonly mode.
Fix assert_redirected_to for nested controllers
Hey Koz,
This is a forward port of the fix you applied for me on on the 2.0-stable branch. I applied it to current HEAD and tests are passing. Have you made any more progress on your refactoring of assert_redirected_to? Let me know and I'll help you QA it.
-
assert_redirected_to_fix.diff
4.29 KB
-
Repository- State changed from new to resolved
(from [55e5219f9912860abdb28636749c0cb2d8b6a809]) Fix assert_redirected_to for nested controllers and named routes
[#308 state:resolved]
Signed-off-by: Michael Koziarski
-
Repository
(from [025515b234d8380f195e3de3818d076302605b12]) Fix assert_redirected_to for nested controllers and named routes
[#308 state:resolved]
Signed-off-by: Michael Koziarski
-
Andrew Vit- Tag set to actionpack, patch
This patch looks like it breaks existing documented functionality of assert_redirected_to. It's now too fussy about getting exact parameters in order to match a route. The method should just match the route segments that are provided, and ignore other params that are omitted.
It looks like this patch makes some basic tests impossible:
def test_create_should_redirect_to_edit post :create, valid_form_input assert_redirected_to :action => 'edit' end ## Result: # No route matches {:action=>"edit"}The route requires an id in order to match, but we can't guess the id of the new record...
-
Andrew Vit
It looks like this patch modified existing tests to specify the controller wherever it was omitted. For the record, I did try adding the controller in my own tests, but that doesn't fix the test failures where other params are needed.
-
Gabe da Silveira
Looking at this right now. Hit me on #rails-contrib if you want to talk about this.
-
Andrew Vit
Gabe, it doesn't look like your patch attached here is the culprit, because the whole thing has since been rewritten... See ticket #c3aaba01
A comment on the rescue block in the previous code said:
rescue ActionController::RoutingError # routing failed us, so match the strings only.But the new code that Koz wrote normalizes into strings straightaway, instead of diffing the requested route segments like before.
I'm out of my league with this. Not sure if this should be a new ticket or if it's still related to this fix... I've attached a test which shows the problem I described above.
-
test_assert_redirected_to.diff
964 Bytes
-
-
Michael KoziarskiHey there,
definitely open a new ticket with some examples that failed.
The test cases I changed in the previous ticket were passing not only due to the partial matching, but due to regular route expiry rules. i.e. url_for :action=>'index' implicitly defaults to this controller.
I'm not sure that the previous behaviour makes sense to me, but if enough people feel strongly about it we can check it out.
So yeah, another ticket with a failing example which doesn't involve components and we can take a look :)
-
Andrew Vit
The follow-up ticket is #584 Route matching in assert_redirected_to. The test I attached to that ticket is the same as here, I stayed out of the nested modules ("components?").
Thanks!