diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index fec35a36bd02b273a6e5336f9b221b05b18f3cad..c1181466f95a48433f77840d706fc4b0aac8bd1c 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,18 @@ ## Rails 4.0.0 (unreleased) ## +* We don't support the `:controller` option for route definitions + with the ruby constant notation. This will now result in an + `ArgumentError`. + + Example: + # This raises an ArgumentError: + resources :posts, :controller => "Admin::Posts" + + # Use directory notation instead: + resources :posts, :controller => "admin/posts" + + *Yves Senn* + * `assert_template` can be used to verify the locals of partials, which live inside a directory. Fixes #8516. diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 82ef1d03334fb35fa679725c3965d0da5490aa73..34f5f80d4d15e5f97a392a9b71caac729a94bf65 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -246,6 +246,12 @@ def default_controller_and_action(to_shorthand=nil) raise ArgumentError, "missing :action" end + if controller.is_a?(String) && controller !~ /\A[a-z_\/]+\z/ + message = "'#{controller}' is not a supported controller name. This can lead to potential routing problems." + message << " See http://guides.rubyonrails.org/routing.html#specifying-a-controller-to-use" + raise ArgumentError, message + end + hash = {} hash[:controller] = controller unless controller.blank? hash[:action] = action unless action.blank? diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 9f31ce8127cd332cc56aed61b8b575dc23b112eb..fb1b8526d01c11fe4e0f8ee0ec5c53438027fb94 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -2833,21 +2833,52 @@ def index end end - DefaultScopeRoutes = ActionDispatch::Routing::RouteSet.new - DefaultScopeRoutes.draw do - namespace :admin do - resources :storage_files, :controller => "StorageFiles" - end + def draw(&block) + @app = ActionDispatch::Routing::RouteSet.new + @app.draw(&block) end - def app - DefaultScopeRoutes - end + def test_valid_controller_options_inside_namespace + draw do + namespace :admin do + resources :storage_files, :controller => "storage_files" + end + end - def test_controller_options get '/admin/storage_files' assert_equal "admin/storage_files#index", @response.body end + + def test_resources_with_valid_namespaced_controller_option + draw do + resources :storage_files, :controller => 'admin/storage_files' + end + + get 'storage_files' + assert_equal "admin/storage_files#index", @response.body + end + + def test_warn_with_ruby_constant_syntax_controller_option + e = assert_raise(ArgumentError) do + draw do + namespace :admin do + resources :storage_files, :controller => "StorageFiles" + end + end + end + + assert_match "'admin/StorageFiles' is not a supported controller name", e.message + end + + def test_warn_with_ruby_constant_syntax_namespaced_controller_option + e = assert_raise(ArgumentError) do + draw do + resources :storage_files, :controller => 'Admin::StorageFiles' + end + end + + assert_match "'Admin::StorageFiles' is not a supported controller name", e.message + end end class TestDefaultScope < ActionDispatch::IntegrationTest diff --git a/guides/source/routing.md b/guides/source/routing.md index 14f23d4020dc92729f797704579c720cfad91f3b..46141696535e484e8eb24dd7023b84e8bab47334 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -832,6 +832,19 @@ will recognize incoming paths beginning with `/photos` but route to the `Images` NOTE: Use `photos_path`, `new_photo_path`, etc. to generate paths for this resource. +For namespaced controllers you can use the directory notation. For example: + +```ruby +resources :user_permissions, controller: 'admin/user_permissions' +``` + +This will route to the `Admin::UserPermissions` controller. + +NOTE: Only the directory notation is supported. specifying the +controller with ruby constant notation (eg. `:controller => +'Admin::UserPermissions'`) can lead to routing problems and results in +a warning. + ### Specifying Constraints You can use the `:constraints` option to specify a required format on the implicit `id`. For example: