Navigation Menu

Skip to content

Commit

Permalink
Improve rake routes output for redirects - closes #6369.
Browse files Browse the repository at this point in the history
  • Loading branch information
pixeltrix committed May 19, 2012
1 parent 98657ad commit ec77498
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 17 deletions.
34 changes: 22 additions & 12 deletions actionpack/lib/action_dispatch/routing/redirection.rb
Expand Up @@ -37,10 +37,25 @@ def path(params, request)
end

def inspect
"Redirect (#{status})"
"redirect(#{status})"
end
end

class PathRedirect < Redirect
def path(params, request)
(params.empty? || !block.match(/%\{\w*\}/)) ? block : (block % escape(params))
end

def inspect
"redirect(#{status}, #{block})"
end

private
def escape(params)
Hash[params.map{ |k,v| [k, Rack::Utils.escape(v)] }]
end
end

class OptionRedirect < Redirect # :nodoc:
alias :options :block

Expand All @@ -60,6 +75,10 @@ def path(params, request)
ActionDispatch::Http::URL.url_for url_options
end

def inspect
"redirect(#{status}, #{options.map{ |k,v| "#{k}: #{v}" }.join(', ')})"
end

private
def escape_path(params)
Hash[params.map{ |k,v| [k, URI.parser.escape(v)] }]
Expand Down Expand Up @@ -106,24 +125,15 @@ module Redirection
def redirect(*args, &block)
options = args.extract_options!
status = options.delete(:status) || 301
path = args.shift

return OptionRedirect.new(status, options) if options.any?

path = args.shift

block = lambda { |params, request|
(params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % escape(params))
} if String === path
return PathRedirect.new(status, path) if String === path

block = path if path.respond_to? :call
raise ArgumentError, "redirection argument not supported" unless block
Redirect.new status, block
end

private
def escape(params)
Hash[params.map{ |k,v| [k, Rack::Utils.escape(v)] }]
end
end
end
end
2 changes: 1 addition & 1 deletion railties/lib/rails/application/route_inspector.rb
Expand Up @@ -16,7 +16,7 @@ def rack_app(app = self.app)
class_name = app.class.name.to_s
if class_name == "ActionDispatch::Routing::Mapper::Constraints"
rack_app(app.app)
elsif class_name == "ActionDispatch::Routing::Redirect" || class_name !~ /^ActionDispatch::Routing/
elsif ActionDispatch::Routing::Redirect === app || class_name !~ /^ActionDispatch::Routing/
app
end
end
Expand Down
11 changes: 7 additions & 4 deletions railties/test/application/route_inspect_test.rb
Expand Up @@ -155,11 +155,14 @@ def test_rake_routes_dont_show_app_mounted_in_assets_prefix

def test_redirect
output = draw do
match '/foo' => redirect("/bar")
match '/foo2' => redirect("/bar", status: 307)
get "/foo" => redirect("/foo/bar"), :constraints => { :subdomain => "admin" }
get "/bar" => redirect(path: "/foo/bar", status: 307)
get "/foobar" => redirect{ "/foo/bar" }
end
assert_equal " foo /foo(.:format) Redirect (301)", output[0]
assert_equal "foo2 /foo2(.:format) Redirect (307)", output[1]

assert_equal " foo GET /foo(.:format) redirect(301, /foo/bar) {:subdomain=>\"admin\"}", output[0]
assert_equal " bar GET /bar(.:format) redirect(307, path: /foo/bar)", output[1]
assert_equal "foobar GET /foobar(.:format) redirect(301)", output[2]
end
end
end

2 comments on commit ec77498

@vijaydev
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a changelog entry for this ?

@pixeltrix
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vijaydev added here: 71a83a9

Please sign in to comment.