Skip to content
Browse files

remove the :path feature to redirects, since it cannot work

  • Loading branch information...
1 parent a8a4264 commit 0809c675ef5831852b7c1aa8497402b2beff5185 @tenderlove tenderlove committed Oct 31, 2011
Showing with 7 additions and 48 deletions.
  1. +7 −15 actionpack/lib/action_dispatch/routing/redirection.rb
  2. +0 −33 actionpack/test/dispatch/routing_test.rb
View
22 actionpack/lib/action_dispatch/routing/redirection.rb
@@ -68,23 +68,15 @@ def redirect(*args, &block)
def options_proc(options)
proc do |params, request|
- path = if options[:path].nil?
- request.path
- elsif params.empty? || !options[:path].match(/%\{\w*\}/)
- options.delete(:path)
- else
- (options.delete(:path) % params)
- end
-
- default_options = {
+ url_options = {
:protocol => request.protocol,
- :host => request.host,
- :port => request.optional_port,
- :path => path,
- :params => request.query_parameters
- }
+ :host => request.host,
+ :port => request.optional_port,
+ :path => request.path,
+ :params => request.query_parameters
+ }.merge options
- ActionDispatch::Http::URL.url_for(options.reverse_merge(default_options))
+ ActionDispatch::Http::URL.url_for url_options
end
end
View
33 actionpack/test/dispatch/routing_test.rb
@@ -62,13 +62,8 @@ def self.call(params, request)
match 'secure', :to => redirect("/secure/login")
match 'mobile', :to => redirect(:subdomain => 'mobile')
- match 'documentation', :to => redirect(:domain => 'example-documentation.com', :path => '')
- match 'new_documentation', :to => redirect(:path => '/documentation/new')
match 'super_new_documentation', :to => redirect(:host => 'super-docs.com')
- match 'stores/:name', :to => redirect(:subdomain => 'stores', :path => '/%{name}')
- match 'stores/:name(*rest)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{rest}')
-
match 'youtube_favorites/:youtube_id/:name', :to => redirect(YoutubeFavoritesRedirector)
constraints(lambda { |req| true }) do
@@ -711,41 +706,13 @@ def test_redirect_hash_with_subdomain
end
end
- def test_redirect_hash_with_domain_and_path
- with_test_routes do
- get '/documentation'
- verify_redirect 'http://www.example-documentation.com'
- end
- end
-
- def test_redirect_hash_with_path
- with_test_routes do
- get '/new_documentation'
- verify_redirect 'http://www.example.com/documentation/new'
- end
- end
-
def test_redirect_hash_with_host
with_test_routes do
get '/super_new_documentation?section=top'
verify_redirect 'http://super-docs.com/super_new_documentation?section=top'
end
end
- def test_redirect_hash_path_substitution
- with_test_routes do
- get '/stores/iernest'
- verify_redirect 'http://stores.example.com/iernest'
- end
- end
-
- def test_redirect_hash_path_substitution_with_catch_all
- with_test_routes do
- get '/stores/iernest/products'
- verify_redirect 'http://stores.example.com/iernest/products'
- end
- end
-
def test_redirect_class
with_test_routes do
get '/youtube_favorites/oHg5SJYRHA0/rick-rolld'

4 comments on commit 0809c67

@pixeltrix
Ruby on Rails member

Why can't it work?

@pixeltrix
Ruby on Rails member

And if it can't work with :path why can it work with a string passed to redirect?

@tenderlove
Ruby on Rails member

Hrm. I don't remember. Maybe try reverting this commit? IIRC it was to do with automatically adding :format to the path, and losing that data when doing the redirect.

@pixeltrix
Ruby on Rails member

Yes, I can see how an optional format couldn't be interpolated into the destination path but that's true for a string redirect, e.g:

get '/products(.:format)' => redirect(:path => '/foo/products.%{format}')
get '/products(.:format)' => redirect('/foo/products.%{format}')

However I think that limitation shouldn't necessitate it's removal - I'll add it back in.

Please sign in to comment.
Something went wrong with that request. Please try again.