Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

rake routes for a redirect is unhelpful #6369

Closed
Jaym3s opened this Issue May 17, 2012 · 10 comments

Comments

Projects
None yet
6 participants

Jaym3s commented May 17, 2012

Given a route in routes.rb:

get '/e/:id' => redirect('/foo'), as: 'share_event'

When I run rake routes
Then I expect to see:

share_event GET  /e/:id(.:format)    redirect('/foo') # or something similar

But I currently see:

share_event GET  /e/:id(.:format)    :controller#:action

@ghost ghost assigned pixeltrix May 17, 2012

Owner

pixeltrix commented May 17, 2012

This isn't as straightforward as it sounds because the string is wrapped in a Proc that does the interpolation and the only way to get at the value is by doing eval "path", route.block.binding. The options form of redirect is a little easier since that stores the options in the block variable so we can just access it directly using route.block. I'll put it on my todo list.

Contributor

markmcspadden commented May 18, 2012

@pixeltrix - This might be a start...but you are right...it's not straightforward.

https://github.com/markmcspadden/rails/compare/issue_6369

@drogus drogus closed this in 5f7bfb7 May 18, 2012

Hey guys, please check #6391 for a working implementation that possible fix this.

@drogus ops, guess you were faster, thanks! :D

Owner

pixeltrix commented May 19, 2012

@carlosantoniodasilva I think we can do a bit more than just reporting the status - we should also output the path or options if available

@pixeltrix pixeltrix reopened this May 19, 2012

Member

lukaszx0 commented May 19, 2012

Yes, you're right, and I'd like to see more verbose output as well, but like you said in your first comment it's not as straightforward as it sounds because of request object passed as block argument. Consider this example:

match 'jokes/:number', :to => redirect { |params, request|
  path = (params[:number].to_i.even? ? "wheres-the-beef" : "i-love-lamp")
  "http://#{request.host_with_port}/#{path}"
}

Right now it's hard to generate this one and render on inspection as you need Rack::Request object.

I agree that it would be better to show something more, but right now, there's no simple solution for that. Do you have idea how to solve this?

Owner

pixeltrix commented May 19, 2012

I think we can so something better for simple redirects like this:

get '/foo/:id' => redirect('/bar/%{id}'), :as => :foo
get '/bar/:id' => redirect(:subdomain => 'shop', :path => '/bar/%{id}'), :as => :bar

I think you could output these as something like:

foo GET  /foo/:id(.:format)    /bar/%{id}, status: 301
bar GET  /bar/:id(.:format)    /bar/%{id}, status: 301, subdomain: 'shop'

whereas generic redirects would be something like:

foo GET  /foo/:id(.:format)    /:path, status: 301

I'm in two minds whether these need wrapping with redirect() in the output - the 3xx status is probably enough to indicate a redirect.

@pixeltrix pixeltrix closed this in ec77498 May 19, 2012

Member

drogus commented May 19, 2012

Sorry, I've missed the discussion here before I've merged it. Thanks for work on this @pixeltrix! :)

Member

lukaszx0 commented May 19, 2012

That was quick! I was about to start improving it ;-) Thanks @pixeltrix.

@pixeltrix the output is looking great, thanks :)

pixeltrix added a commit that referenced this issue May 19, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment