Skip to content

Commit

Permalink
Fix shorthand routes where controller and action are in the scope
Browse files Browse the repository at this point in the history
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
pixeltrix committed Jun 25, 2013
1 parent 540f15e commit 51be99c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 2 deletions.
9 changes: 9 additions & 0 deletions 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*

* Always escape the result of `link_to_unless` method.

Before:
Expand Down
12 changes: 10 additions & 2 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -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)
Expand Down Expand Up @@ -874,6 +874,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
Expand Down Expand Up @@ -1383,6 +1387,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)
Expand Down
13 changes: 13 additions & 0 deletions actionpack/test/dispatch/routing_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit 51be99c

Please sign in to comment.