Attempt to namespace a controller

[romanbsd]

I have a view in admin namespace served by Admin::DashboardController#show. This view has url_for(:controller => 'stats', :action => 'show')

In journey 1.0.3 this works as expected. In journey 1.0.4 it fails, because it tries to namespace the controller:

ActionController::RoutingError (No route matches {:controller=>"admin/stats", :action=>"show"})

[jefmathiot]

We also encountered this issue with a similar namespace setup. We had to downgrade to 1.0.3.

[ccocchi]

Same here with API namespace. Worked fine in journey 1.0.3.

Test looks like

describe Api::V1::UsersController do
  describe "GET show" do
    it "should not throw an error"
      get :show, id: '1'
    end
  end

And the error

No route matches {:controller=>"api/v1/users", :action=>"show"}

The route is correctly handle by rake routes

GET      /users/:id(.:format)                                api/v1/users#show

[schneems]

Are you able to use url_for and pass in any parameters that work? Such as :controller => '/stats'. To me specifying a controller should be explicit and this would not be expected. The journey team is quite busy, any chance someone could fork journey, add a test to the suite that reproduces this error?

That would likely help if this regression is unwanted. If you do so, can you reference and close this PR?

[romanbsd]

It's a pity that Rails 3.2.7 depends on journey 1.0.4, it makes it impossible to upgrade to 3.2.6

[tenderlove]

Can someone post any kind of test? Even an entire rails repo with a failing test would be a great help. Thanks.

[schneems]

I made a super simple rails app that demonstrates the behavior https://github.com/heroku/journey-namespace-issue you can run

$ rake

you should receive a failure:

test_should_get_index(Admin::BarsControllerTest):
ActionView::Template::Error: No route matches {:controller=>"admin/foos"}
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_dispatch/routing/route_set.rb:532:in `raise_routing_error'
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_dispatch/routing/route_set.rb:528:in `rescue in generate'
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_dispatch/routing/route_set.rb:520:in `generate'
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_dispatch/routing/route_set.rb:561:in `generate'
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_dispatch/routing/route_set.rb:586:in `url_for'
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_dispatch/routing/url_for.rb:148:in `url_for'
    /Users/schneems/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.7/lib/action_view/helpers/url_helper.rb:107:in `url_for'

Hope this helps, let me know if you want something changed.

[pixeltrix]

@schneems is the double declaration of resources :foos necessary to generate the error?

[pixeltrix]

Are you sure it generated the correct url before? The only change between 1.0.3 and 1.0.4 with respect to url generation is c70e5484551a77aac94bad1bb49831fff69df784 and if I undo that the url generated is /assets?controller=admin%2Ffoos.

[pixeltrix]

Okay, I've confirmed that even in Rails-3.2.0/Journey-1.0.0 the url generated was /assets?controller=admin%2Ffoos so the question is this a regression from Rails-3.1 and Rack::Mount.

[pixeltrix]

Just checked and this how it's meant to be - Rails 3.1 and Rack::Mount generate a routing error when used with @schneems example app. If we look at the relevant code in Rails it's obviously intentional - the initial commit was 2 years ago and @kennyj made a fix that allowed using '/' to specify an absolute controller 6 months ago.

[schneems]

Great sleuthing, you think this behavior warrants documentation in url_for ?

[romanbsd]

I second @schneems

This is a unexpected and breaking change.

[schneems]

@romanbsd I don't disagree with the change, though expected behaviors should be documented. I try to never use url_for directly.

[pixeltrix]

@romanbsd it isn't really a change - it's just stopped generating an incorrect url. At least this way you'll now know about the error. Like @schneems I try to use named routes wherever possible - much quicker than having Journey find the best match.

@schneems yes, some documentation about the use of relative controller paths would be useful - especially about the use of a leading slash to specify an absolute path.

[romanbsd]

@pixeltrix the code in question is a code of a helper, which is shared among the controllers. With journey 1.0.3 it was generating the correct (in my opinion) urls when used from a regular controllers, as well as from namespaced controllers. The change in journey 1.0.4 takes into account the current namespacing of the controller, which I consider a breaking change. Regarding the named routes - this is impossible in my case, as the url is generated automagically by the helper.

[pixeltrix]

@romanbsd you'll have to post some code that generates a correct url on Rails-3.2.6/Journey-1.0.3 then, because this is what I get:

Testapp::Application.routes.draw do
  namespace :admin do
    get '/dashboard', :to => 'dashboard#show', :as => :dashboard
  end

  get '/stats', :to => 'stats#show', :as => :stats
end

$ rails c
Loading development environment (Rails 3.2.6)
>> app.get '/admin/dashboard'
=> 200
>> app.url_for :controller => 'stats', :action => 'show'
=> "http://www.example.com/assets?action=show&controller=admin%2Fstats"
>> app.url_for :controller => '/stats', :action => 'show'
=> "http://www.example.com/stats"

$ rails c
Loading development environment (Rails 3.2.7)
>> app.get '/admin/dashboard'
=> 200
>> app.url_for :controller => 'stats', :action => 'show'
ActionController::RoutingError: No route matches {:controller=>"admin/stats", :action=>"show"}
>> app.url_for :controller => '/stats', :action => 'show'
=> "http://www.example.com/stats"

[keo]

If I have a namespaced route:

  scope '/account' do
    resources :email_accounts
  end

then in a view I do:

if current_page? controller: "email_accounts"

and it says: No route matches {:controller=>"devise/email_accounts"}

if I prepend a slash to it it works:

if current_page? controller: "/email_accounts"

This looks a bit odd because it is namespaced with /account in the URL.

Shouldn't it be doing the following: look for the controller in the current scope, then if not found, look for the same controller in the root scope?

[pixeltrix]

Shouldn't it be doing the following: look for the controller in the current scope, then if not found, look for the same controller in the root scope?

That would often generate incorrect urls which would be likely to go unnoticed - this way it's brought to the developers attention. You can use named route helpers with current_page? if that would be better, e.g:

if current_page? email_accounts_url

This looks a bit odd because it is namespaced with /account in the URL.

But your controller isn't namespaced - if you namespace the controller it will work because the namespace depth of the current controller and the new controller are the same, e.g:

scope '/account', :module => 'account' do
  resources :email_accounts
end

this will now work:

if current_page? controller: 'account/email_accounts'

[schneems]

Added docs for this functionality: https://github.com/lifo/docrails/commit/e4633519696f539bffacf6985caa3969ad717c85