Redirects in engine routes works strange

[wildchild]

Assuming engine routes:

Pages::Engine.routes.draw do
  namespace :admin do
    root to: redirect("/admin/map")

    resource :map
  end
end

And an application:

Rails.application.routes.draw do
  mount Pages::Engine => "/pages"
end

GET "/pages/admin" should redirect to "/pages/admin/map", but redirecting to "/admin/map"

This works in plain rails apps, thus I think, that engines should behave the same way.

[pixeltrix]

Does the following working in a plain application

Rails.application.routes.draw do
  namespace :pages do
    namespace :admin
      root to: redirect("/admin/map")
      resource :map
    end
  end
end

I haven't checked but I think /pages/admin will redirect to /admin/map - can you check this please.

[wildchild]

@pixeltrix, yes, it redirects to /admin/map.

[pixeltrix]

The following should work for engine routes:

Pages::Engine.routes.draw do
  namespace :admin do
    root to: redirect do |params, request|
      "#{request.script_name}/admin/map"
    end

    resource :map
  end
end

There's an argument that it should automatically prepend the SCRIPT_NAME to take account of situation where the Rails application is running in a subdirectory on the web server. However this would be a breaking change so it would be Rails 4+ only.

/cc @tenderlove @rafaelfranca @josevalim what do you think? Should we automatically prepend the SCRIPT_NAME by default and require a custom block to break out of it?

[wildchild]

However this would be a breaking change so it would be Rails 4+ only.

Rails 4 is on the way, what is a decision about it?

[wildchild]

@pixeltrix, could you shed some light?

[rafaelfranca]

@pixeltrix if we automatically prepend the SCRIPT_NAME how will be the block code to remove it?

[pixeltrix]

@rafaelfranca that's the problem - I've got the code and tests to fix this but can't think of a good way to break out of the SCRIPT_NAME constraints.

[JonRowe]

Any progress on how to remove the SCRIPT_NAME blocker?

[JonRowe]

Although given Rails 4 has shipped, is this now even feasible to change?

[pixeltrix]

@JonRowe it'd be feasible for merging to master but I still don't have a good way of redirecting to a url outside of SCRIPT_NAME so we'd be switching one problem for another. I did think that by restricting it to path redirects and option redirects and requiring a full url for custom redirects would work but on second thoughts it's non-obvious - at least with the current behaviour it's consistent.

One possible solution is to support relative paths so if the redirect returns a url starting with '/' it's treated as an absolute url and 'path/to/action' would have the SCRIPT_NAME prepended - wdyt?

[JonRowe]

I think it's kind of counter intuitive to not take account of script_name (even though it's consistent), simply because an engine is designed to be mounted on another endpoint and thus is most likly to want consistent paths relative to it's own root.

However I like the idea of /path generating from the Rails root, and engine/path being relative to the engine. That makes sense as a starting slash means "absolute" path in most interpretations :)

[pixeltrix]

@rafaelfranca wdyt? By using paths starting with a slash to indicate an absolute path then it should be backwards compatible.

[rafaelfranca]

@pixeltrix I like this approach. 👍 from my side

[Systho]

How are we supposed to redirect_to engine root path ? I've tried redirect_to '' and redirect_to '/' but both redirect to app root

[pixeltrix]

I would guess that redirect '' should work - I'll look at fixing it

[vikassy]

I think we need to change

path && !path.empty? && path[0] != '/' => path && (path.empty? || path[0] != '/')

in https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/redirection.rb#L58

I tried this out, but I got 1 failing test(i.e test_redirect_hash_with_domain_and_path) where

expected => "http://www.example-documentation.com" and actual => "http://www.example-documentation.com/"

Should that be a problem?? @pixeltrix

[tmandry]

For anyone arriving via Google, the correct code to do this in Rails 3 is

    root(to: redirect do |params, request|
      "#{request.script_name}/admin/map"
    end)

(note the necessary parentheses), or

    root to: redirect { |params, request| "#{request.script_name}/admin/map" }

which I find much cleaner.