Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Refactor recall parameter normalization [#5021 state:resolved]
Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information
pixeltrix authored and josevalim committed Jul 3, 2010
1 parent 8cc7463 commit 54250a5
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 18 deletions.
22 changes: 6 additions & 16 deletions actionpack/lib/action_dispatch/routing/route_set.rb
Expand Up @@ -318,7 +318,6 @@ 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
Expand All @@ -335,7 +334,11 @@ def current_controller

def use_recall_for(key)
if @recall[key] && (!@options.key?(key) || @options[key] == @recall[key])
@options[key] = @recall.delete(key)
if named_route_exists?
@options[key] = @recall.delete(key) if segment_keys.include?(key)
else
@options[key] = @recall.delete(key)
end
end
end

Expand All @@ -359,15 +362,6 @@ 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
Expand Down Expand Up @@ -440,12 +434,8 @@ 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 : []
set.named_routes[named_route].segment_keys
end
end

Expand Down
29 changes: 27 additions & 2 deletions actionpack/test/template/url_helper_test.rb
Expand Up @@ -406,6 +406,14 @@ def sort_query_string_params(uri)
class UrlHelperControllerTest < ActionController::TestCase
class UrlHelperController < ActionController::Base
test_routes do |map|
match 'url_helper_controller_test/url_helper/show/:id',
:to => 'url_helper_controller_test/url_helper#show',
:as => :show

match 'url_helper_controller_test/url_helper/profile/:name',
:to => 'url_helper_controller_test/url_helper#show',
:as => :profile

match 'url_helper_controller_test/url_helper/show_named_route',
:to => 'url_helper_controller_test/url_helper#show_named_route',
:as => :show_named_route
Expand All @@ -418,6 +426,14 @@ class UrlHelperController < ActionController::Base
:as => :normalize_recall_params
end

def show
if params[:name]
render :inline => 'ok'
else
redirect_to profile_path(params[:id])
end
end

def show_url_for
render :inline => "<%= url_for :controller => 'url_helper_controller_test/url_helper', :action => 'show_url_for' %>"
end
Expand Down Expand Up @@ -484,15 +500,24 @@ def default_url_options(options = nil)
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
def test_recall_params_should_be_normalized
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
def test_recall_params_should_not_be_changed
get :recall_params_not_changed
assert_equal '/url_helper_controller_test/url_helper/show_url_for', @response.body
end

def test_recall_params_should_normalize_id
get :show, :id => '123'
assert_equal 302, @response.status
assert_equal 'http://test.host/url_helper_controller_test/url_helper/profile/123', @response.location

get :show, :name => '123'
assert_equal 'ok', @response.body
end
end

class TasksController < ActionController::Base
Expand Down

0 comments on commit 54250a5

Please sign in to comment.