Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #9039 from senny/warn_on_controller_option_with_ru…

…by_constant_syntax

ruby constant syntax is not supported as routing `:controller` option.
  • Loading branch information...
commit 329c82380ee886714ab4e4620e7a5b507716a794 2 parents 99bb2fd + 69f28a7
@rafaelfranca rafaelfranca authored
View
13 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.
View
6 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?
View
49 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
View
13 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:
Please sign in to comment.
Something went wrong with that request. Please try again.