Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

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
Closed

rake routes for a redirect is unhelpful #6369

Jaym3s opened this issue May 17, 2012 · 10 comments
Assignees

Comments

@Jaym3s
Copy link

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
@pixeltrix
Copy link
Contributor

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.

@markmcspadden
Copy link
Contributor

@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 as completed in 5f7bfb7 May 18, 2012
@carlosantoniodasilva
Copy link
Member

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

@carlosantoniodasilva
Copy link
Member

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

@pixeltrix
Copy link
Contributor

@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
@lukaszx0
Copy link
Member

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?

@pixeltrix
Copy link
Contributor

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.

@drogus
Copy link
Member

drogus commented May 19, 2012

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

@lukaszx0
Copy link
Member

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

@carlosantoniodasilva
Copy link
Member

@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
Projects
None yet
Development

No branches or pull requests

6 participants