Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Normalize recall params when the route is not a standard route otherw…

…ise :controller and :action may appear in the generated url [#4326 state:resolved]

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
commit 91b52c795f62aa15505f2f098bc86d6f6db75105 1 parent abd568b
Andrew White pixeltrix authored josevalim committed
29 actionpack/lib/action_dispatch/routing/route_set.rb
View
@@ -185,7 +185,7 @@ def #{selector}(*args)
end
end
- attr_accessor :routes, :named_routes
+ attr_accessor :set, :routes, :named_routes
attr_accessor :disable_clear_and_finalize, :resources_path_names
attr_accessor :default_url_options, :request_class
@@ -296,6 +296,7 @@ def initialize(options, recall, set, extras = false)
@extras = extras
normalize_options!
+ normalize_recall!
normalize_controller_action_id!
use_relative_controller!
controller.sub!(%r{^/}, '') if controller
@@ -336,6 +337,15 @@ def normalize_options!
end
end
+ def normalize_recall!
+ # If the target route is not a standard route then remove controller and action
+ # from the options otherwise they will appear in the url parameters
+ if block_or_proc_route_target?
+ recall.delete(:controller) unless segment_keys.include?(:controller)
+ recall.delete(:action) unless segment_keys.include?(:action)
+ end
+ end
+
# This pulls :controller, :action, and :id out of the recall.
# The recall key is only used if there is no key in the options
# or if the key in the options is identical. If any of
@@ -371,7 +381,7 @@ def handle_nil_action!
def generate
error = ActionController::RoutingError.new("No route matches #{options.inspect}")
- path, params = @set.generate(:path_info, named_route, options, recall, opts)
+ path, params = @set.set.generate(:path_info, named_route, options, recall, opts)
raise error unless path
@@ -402,6 +412,19 @@ def different_controller?
return false unless current_controller
controller.to_param != current_controller.to_param
end
+
+ private
+ def named_route_exists?
+ named_route && set.named_routes[named_route]
+ end
+
+ def block_or_proc_route_target?
+ named_route_exists? && !set.named_routes[named_route].app.is_a?(Dispatcher)
+ end
+
+ def segment_keys
+ named_route_exists? ? set.named_routes[named_route].segment_keys : []
+ end
end
# Generate the path indicated by the arguments, and return an array of
@@ -415,7 +438,7 @@ def generate_extras(options, recall={})
end
def generate(options, recall = {}, extras = false)
- Generator.new(options, recall, @set, extras).generate
+ Generator.new(options, recall, self, extras).generate
end
RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash]
22 actionpack/test/template/url_helper_test.rb
View
@@ -429,6 +429,10 @@ class UrlHelperController < ActionController::Base
map.connect ":controller/:action/:id"
# match "/:controller(/:action(/:id))"
+
+ match 'url_helper_controller_test/url_helper/normalize_recall_params',
+ :to => UrlHelperController.action(:normalize_recall),
+ :as => :normalize_recall_params
end
def show_url_for
@@ -447,6 +451,14 @@ def nil_url_for
render :inline => '<%= url_for(nil) %>'
end
+ def normalize_recall_params
+ render :inline => '<%= normalize_recall_params_path %>'
+ end
+
+ def recall_params_not_changed
+ render :inline => '<%= url_for(:action => :show_url_for) %>'
+ end
+
def rescue_action(e) raise e end
end
@@ -488,6 +500,16 @@ def default_url_options(options = nil)
get :show_named_route, :kind => 'url'
assert_equal 'http://testtwo.host/url_helper_controller_test/url_helper/show_named_route', @response.body
end
+
+ def test_recall_params_should_be_normalized_when_using_block_route
+ get :normalize_recall_params
+ assert_equal '/url_helper_controller_test/url_helper/normalize_recall_params', @response.body
+ end
+
+ def test_recall_params_should_not_be_changed_when_using_normal_route
+ get :recall_params_not_changed
+ assert_equal '/url_helper_controller_test/url_helper/show_url_for', @response.body
+ end
end
class TasksController < ActionController::Base
Please sign in to comment.
Something went wrong with that request. Please try again.