Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix shorthand routes where controller and action are in the scope

Merge `:action` from routing scope and assign endpoint if both `:controller`
and `:action` are present. The endpoint assignment only occurs if there is
no `:to` present in the options hash so should only affect routes using the
shorthand syntax (i.e. endpoint is inferred from the the path).

Fixes #9856

Backport of 37b4276
  • Loading branch information...
commit 29d5c2c7939f4e7d5fdb304e328626351c8ada68 1 parent abe6ce1
@pixeltrix pixeltrix authored
View
9 actionpack/CHANGELOG.md
@@ -1,5 +1,14 @@
## unreleased ##
+* Merge `:action` from routing scope and assign endpoint if both `:controller`
+ and `:action` are present. The endpoint assignment only occurs if there is
+ no `:to` present in the options hash so should only affect routes using the
+ shorthand syntax (i.e. endpoint is inferred from the the path).
+
+ Fixes #9856
+
+ *Yves Senn*, *Andrew White*
+
* Use a case insensitive URI Regexp for #asset_path.
This fix a problem where the same asset path using different case are generating
View
12 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -11,8 +11,8 @@ module Routing
class Mapper
URL_OPTIONS = [:protocol, :subdomain, :domain, :host, :port]
SCOPE_OPTIONS = [:path, :shallow_path, :as, :shallow_prefix, :module,
- :controller, :path_names, :constraints, :defaults,
- :shallow, :blocks, :options]
+ :controller, :action, :path_names, :constraints,
+ :shallow, :blocks, :defaults, :options]
class Constraints #:nodoc:
def self.new(app, constraints, request = Rack::Request)
@@ -869,6 +869,10 @@ def merge_controller_scope(parent, child) #:nodoc:
child
end
+ def merge_action_scope(parent, child) #:nodoc:
+ child
+ end
+
def merge_path_names_scope(parent, child) #:nodoc:
merge_options_scope(parent, child)
end
@@ -1378,6 +1382,10 @@ def match(path, *rest)
raise ArgumentError, "Unknown scope #{on.inspect} given to :on"
end
+ if @scope[:controller] && @scope[:action]
+ options[:to] ||= "#{@scope[:controller]}##{@scope[:action]}"
+ end
+
paths.each do |_path|
route_options = options.dup
route_options[:path] ||= _path if _path.is_a?(String)
View
13 actionpack/test/dispatch/routing_test.rb
@@ -1253,6 +1253,19 @@ def test_match_shorthand_inside_nested_namespaces_and_scopes_with_controller
assert_equal 'api/v3/products#list', @response.body
end
+ def test_controller_option_with_nesting_and_leading_slash
+ draw do
+ scope '/job', controller: 'job' do
+ scope ':id', action: 'manage_applicant' do
+ get "/active"
+ end
+ end
+ end
+
+ get '/job/5/active'
+ assert_equal 'job#manage_applicant', @response.body
+ end
+
def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper
draw do
resources :replies do
Please sign in to comment.
Something went wrong with that request. Please try again.