diff options
author | Andrew White <andrew.white@unboxedconsulting.com> | 2016-01-18 16:57:56 +0000 |
---|---|---|
committer | Andrew White <andrew.white@unboxedconsulting.com> | 2016-02-16 09:52:26 +0000 |
commit | 8ca8a2d773b942c4ea76baabe2df502a339d05b1 (patch) | |
tree | ecb74115dce7798f682d0984ef23ca55a1b47f5a /actionpack/test/dispatch/routing_test.rb | |
parent | 3dd9fe5db44777eaf3765875fe259d53e72a0448 (diff) | |
download | rails-8ca8a2d773b942c4ea76baabe2df502a339d05b1.tar.gz rails-8ca8a2d773b942c4ea76baabe2df502a339d05b1.tar.bz2 rails-8ca8a2d773b942c4ea76baabe2df502a339d05b1.zip |
Refactor handling of :action default in routing
The longstanding convention in Rails is that if the :action parameter
is missing or nil then it defaults to 'index'. Up until Rails 5.0.0.beta1
this was handled slightly differently than other routing defaults by
deleting it from the route options and adding it to the recall parameters.
With the recent focus of removing unnecessary duplications this has
exposed a problem in this strategy - we are now mutating the request's
path parameters and causing problems for later url generation. This will
typically affect url_for rather a named url helper since the latter
explicitly pass :controller, :action, etc.
The fix is to add a default for :action in the route class if the path
contains an :action segment and no default is passed. This change also
revealed an issue with the parameterized part expiry in that it doesn't
follow a right to left order - as soon as a dynamic segment is required
then all other segments become required.
Fixes #23019.
Diffstat (limited to 'actionpack/test/dispatch/routing_test.rb')
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 62 |
1 files changed, 51 insertions, 11 deletions
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 5ead9357ae..02cd640fa5 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3964,16 +3964,6 @@ class TestUnicodePaths < ActionDispatch::IntegrationTest end class TestMultipleNestedController < ActionDispatch::IntegrationTest - module ::Foo - module Bar - class BazController < ActionController::Base - def index - render :inline => "<%= url_for :controller => '/pooh', :action => 'index' %>" - end - end - end - end - Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| app.draw do namespace :foo do @@ -3985,7 +3975,18 @@ class TestMultipleNestedController < ActionDispatch::IntegrationTest end end - include Routes.url_helpers + module ::Foo + module Bar + class BazController < ActionController::Base + include Routes.url_helpers + + def index + render :inline => "<%= url_for :controller => '/pooh', :action => 'index' %>" + end + end + end + end + APP = build_app Routes def app; APP end @@ -4717,3 +4718,42 @@ class TestPartialDynamicPathSegments < ActionDispatch::IntegrationTest assert_equal(params, request.path_parameters) end end + +class TestPathParameters < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| + app.draw do + scope module: 'test_path_parameters' do + scope ':locale', locale: /en|ar/ do + root to: 'home#index' + get '/about', to: 'pages#about' + end + end + + get ':controller(/:action/(:id))' + end + end + + class HomeController < ActionController::Base + include Routes.url_helpers + + def index + render inline: "<%= root_path %>" + end + end + + class PagesController < ActionController::Base + include Routes.url_helpers + + def about + render inline: "<%= root_path(locale: :ar) %> | <%= url_for(locale: :ar) %>" + end + end + + APP = build_app Routes + def app; APP end + + def test_path_parameters_are_not_mutated + get '/en/about' + assert_equal "/ar | /ar/about", @response.body + end +end |