Permalink
Browse files

determine the match shorthand target early.

Backport #9361.
Closes #7554.

This patch determines the `controller#action` directly
in the `match` method when the shorthand syntax is used.
this prevents problems with namespaces and scopes.
  • Loading branch information...
1 parent fdcd7c0 commit a72dab0b6a16ef9e83e66c665b0f2b4364d90fb6 @senny senny committed Feb 21, 2013
View
@@ -1,5 +1,20 @@
## unreleased ##
+* Determine the controller#action from only the matched path when using the
+ shorthand syntax. Previously the complete path was used, which led
+ to problems with nesting (scopes and namespaces).
+ Fixes #7554.
+ Backport #9361.
+
+ Example:
+
+ # this will route to questions#new
+ scope ':locale' do
+ get 'questions/new'
+ end
+
+ *Yves Senn*
+
* Fix `assert_template` with `render :stream => true`.
Fix #1743.
Backport #5288.
@@ -51,7 +51,6 @@ def constraint_args(constraint, request)
class Mapping #:nodoc:
IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix]
ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
- SHORTHAND_REGEX = %r{/[\w/]+$}
WILDCARD_PATH = %r{\*([^/\)]+)\)?$}
def initialize(set, scope, path, options)
@@ -70,12 +69,7 @@ def to_route
def normalize_options!
path_without_format = @path.sub(/\(\.:format\)$/, '')
- if using_match_shorthand?(path_without_format, @options)
- to_shorthand = @options[:to].blank?
- @options[:to] ||= path_without_format.gsub(/\(.*\)/, "")[1..-1].sub(%r{/([^/]*)$}, '#\1')
- end
-
- @options.merge!(default_controller_and_action(to_shorthand))
+ @options.merge!(default_controller_and_action)
requirements.each do |name, requirement|
# segment_keys.include?(k.to_s) || k == :controller
@@ -153,7 +147,7 @@ def defaults
end
end
- def default_controller_and_action(to_shorthand=nil)
+ def default_controller_and_action
if to.respond_to?(:call)
{ }
else
@@ -166,7 +160,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
@@ -1261,6 +1255,11 @@ def match(path, *rest)
paths = [path] + rest
end
+ path_without_format = path.to_s.sub(/\(\.:format\)$/, '')
+ if using_match_shorthand?(path_without_format, options)
+ options[:to] ||= path_without_format.gsub(%r{^/}, "").sub(%r{/([^/]*)$}, '#\1')
+ end
+
options[:anchor] = true unless options.key?(:anchor)
if options[:on] && !VALID_ON_OPTIONS.include?(options[:on])
@@ -1271,6 +1270,10 @@ def match(path, *rest)
self
end
+ def using_match_shorthand?(path, options)
+ path && (options[:to] || options[:action]).nil? && path =~ %r{/[\w/]+$}
+ end
+
def decomposed_match(path, options) # :nodoc:
if on = options.delete(:on)
send(on) { decomposed_match(path, options) }
@@ -517,6 +517,18 @@ def self.call(params, request)
end
get 'search' => 'search'
+
+ scope ':locale' do
+ match 'questions/new', via: [:get]
+ end
+
+ namespace :api do
+ namespace :v3 do
+ scope ':locale' do
+ get "products/list"
+ end
+ end
+ end
end
end
@@ -1417,6 +1429,21 @@ def test_match_shorthand_inside_namespace_with_controller
end
end
+ def test_match_shorthand_inside_scope_with_variables_with_controller
+ with_test_routes do
+ get '/de/questions/new'
+ assert_equal 'questions#new', @response.body
+ assert_equal 'de', @request.params[:locale]
+ end
+ end
+
+ def test_match_shorthand_inside_nested_namespaces_and_scopes_with_controller
+ with_test_routes do
+ get '/api/v3/en/products/list'
+ assert_equal 'api/v3/products#list', @response.body
+ end
+ end
+
def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper
with_test_routes do
assert_equal '/replies', replies_path

0 comments on commit a72dab0

Please sign in to comment.