Browse files

Merge pull request #7668 from Draiken/fix_issue_6497

Removing to_shorthand to fix #6497
Conflicts:
	actionpack/CHANGELOG.md
  • Loading branch information...
1 parent a82f1e3 commit 61d5d2d8a97fd289b81991cd79dca3112e7ca135 @rafaelfranca rafaelfranca committed Sep 19, 2012
View
10 actionpack/CHANGELOG.md
@@ -1,5 +1,15 @@
## Rails 3.2.9 (unreleased) ##
+* Fixed a bug with shorthand routes scoped with the `:module` option not
+ adding the module to the controller as described in issue #6497.
+ This should now work properly:
+
+ scope :module => "engine" do
+ get "api/version" # routes to engine/api#version
+ end
+
+ *Luiz Felipe Garcia Pereira*
+
* Respect `config.digest = false` for `asset_path`
Previously, the `asset_path` internals only respected the `:digest`
View
2 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -166,7 +166,7 @@ def default_controller_and_action(to_shorthand=nil)
controller ||= default_controller
action ||= default_action
- unless controller.is_a?(Regexp) || to_shorthand
+ unless controller.is_a?(Regexp)
controller = [@scope[:module], controller].compact.join("/").presence
end
View
7 actionpack/test/dispatch/routing_test.rb
@@ -362,6 +362,7 @@ def self.call(params, request)
resources :errors, :shallow => true do
resources :notices
end
+ get 'api/version'
end
scope :path => 'api' do
@@ -1376,6 +1377,12 @@ def test_match_shorthand_inside_namespace
end
end
+ def test_match_shorthand_with_module
+ assert_equal '/api/version', api_version_path
+ get '/api/version'
+ assert_equal 'api/api#version', @response.body
+ end
+
def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper
with_test_routes do
assert_equal '/replies', replies_path

2 comments on commit 61d5d2d

@pixeltrix
Ruby on Rails member

I think this needs to be reverted as it may break quite a few apps. The shorthand syntax has historically ignored everything in the scope so changing it in a patch release is probably too risky. We definitely need to sort out the shorthand syntax for 4.0 though - I'll drop into Campfire later to discuss what needs fixing.

@spastorino
Ruby on Rails member

@pixeltrix yes I was waiting the release because of this guy issue. @rafaelfranca was trying to fix it in his branch https://github.com/rafaelfranca/rails/tree/route_fix Please let's discuss in CF so we can release the final version properly ❤️

Please sign in to comment.