Rails Ruby on Rails

Comments and changes to this ticket

• You flagged this item as spam.
  

  Wincent Colaiuta November 21st, 2010 @ 09:10 PM

  One minor correction to what I wrote above. The route is actually visible in the rake routes output:

      issues GET    /issues/page/:page(.:format)                              {:action=>"index", :controller=>"issues", :page=>/\d+/}
             GET    /issues(.:format)                                         {:action=>"index", :controller=>"issues"}
             POST   /issues(.:format)                                         {:action=>"create", :controller=>"issues"}
   new_issue GET    /issues/new(.:format)                                     {:action=>"new", :controller=>"issues"}
  edit_issue GET    /issues/:id/edit(.:format)                                {:action=>"edit", :controller=>"issues"}
       issue GET    /issues/:id(.:format)                                     {:action=>"show", :controller=>"issues"}
             PUT    /issues/:id(.:format)                                     {:action=>"update", :controller=>"issues"}
             DELETE /issues/:id(.:format)                                     {:action=>"destroy", :controller=>"issues"}
  

  Here's a quick test demo in Rails console of how this blows up:

  >> include Rails.application.routes.url_helpers
  => Object
  >> issues_path
  ActionController::RoutingError: No route matches {:controller=>"issues"}
  

• You flagged this item as spam.
  

  Wincent Colaiuta November 21st, 2010 @ 09:13 PM

  For comparison, the output of rake routes under Rails v3.0.1:

      issues GET    /issues/page/:page(.:format)                              {:page=>/\d+/, :controller=>"issues", :action=>"index"}
      issues GET    /issues(.:format)                                         {:controller=>"issues", :action=>"index"}
      issues POST   /issues(.:format)                                         {:controller=>"issues", :action=>"create"}
   new_issue GET    /issues/new(.:format)                                     {:controller=>"issues", :action=>"new"}
  edit_issue GET    /issues/:id/edit(.:format)                                {:controller=>"issues", :action=>"edit"}
       issue GET    /issues/:id(.:format)                                     {:controller=>"issues", :action=>"show"}
       issue PUT    /issues/:id(.:format)                                     {:controller=>"issues", :action=>"update"}
       issue DELETE /issues/:id(.:format)                                     {:controller=>"issues", :action=>"destroy"}
  

• You flagged this item as spam.
  

  Andrés Mejía November 22nd, 2010 @ 11:47 AM

  I think this is the intended behavior, and not a bug:

  When you say
  

  get 'page/:page' => 'issues#index', :page => %r{\d+}
  

  you are saying that a page parameter is required. If you pass the page parameter it works as expected:

  ruby-1.9.2-p0 > issues_path(:page => 666)
   => "/issues/page/666"
  

  If you want the page parameter to be optional, do
  

  get '(page/:page)' => 'issues#index', :page => %r{\d+}
  

  This makes it work as you want:

  ruby-1.9.2-p0 > issues_path
   => "/issues" 
  ruby-1.9.2-p0 > issues_path(:page => 1)
   => "/issues/page/1" 
  ruby-1.9.2-p0 > issues_path(:page => 666)
   => "/issues/page/666"
  

  Maybe the bug was on Rails 3.0.1 that didn't enforce the presence of the page parameter?

• You flagged this item as spam.
  

  Wincent Colaiuta November 22nd, 2010 @ 11:56 AM

  Thanks, I'll apply that as a workaround for now.

  As for whether it's intended or not, I don't know. It would be good to have clarification from one of the people who designed the router.

  I always thought the basic idea in the routing file was that rules would have a kind of top-down precedence, so whatever gets declared first and can match, wins. So declaring the "page" rule after the non-paged case was consistent with that idea:

  ie. the non-paged case would be the default which would usually apply, but in the presence of a page parameter the more specific paged case would apply

• You flagged this item as spam.
  

  Andrés Mejía November 22nd, 2010 @ 12:27 PM

  By the way, it's not working properly for me on Rails 3.0.1, either:

  On routes.rb :

    resources :issues do
      collection do
        get 'page/:page' => 'issues#index', :page => %r{\d+}
      end
    end
  

  This is what I get:

  $ rails c
  Loading development environment (Rails 3.0.1)
  ruby-1.9.2-p0 > include Rails.application.routes.url_helpers
   => Object 
  ruby-1.9.2-p0 > issues_path
   => "/issues" 
  ruby-1.9.2-p0 > issues_path(:page => 666)
   => "/issues?page=666"
  

  So as you see, the it didn't use the page/:page route.

  Do you get the same?

• You flagged this item as spam.
  

  Wincent Colaiuta November 22nd, 2010 @ 02:24 PM

  I do, but I don't generate the page links using the helper in my app; I have an lib module that does that (via string concatenation, gasp)...

• You flagged this item as spam.
  

  Andrés Mejía November 22nd, 2010 @ 02:32 PM

  All right. I think there's nothing to fix here. Can you change this ticket's state to "invalid"?

• You flagged this item as spam.
  

  Wincent Colaiuta November 23rd, 2010 @ 08:03 AM

  Don't think I'm the one to do that, as I'm not sure that it is invalid. Better for someone from the core team, or someone more heavily involved in the router design, to make that decision.

• 

  Prem Sichanugrist (sikachu) November 23rd, 2010 @ 01:44 PM

  • State changed from “new” to “open”
  • Assigned user set to “Prem Sichanugrist (sikachu)”
  • Importance changed from “” to “Low”

  Hi,

  I can confirm that the behavior has been changed. That commits were making sure that no duplicate named route should be created. That's why in 3.0.3 you would see issues only appearing on the first line.

  However, I think there's a bug here. It seems like get 'page/:page' => 'issues#index', :page => %r{\d+} got added as a named route, which shouldn't be. I'll make a failing test case and a patch for it :)

  Thank you for reporting in. Meanwhile, I think you can solve your problem by adding :as to your route like this:

  resources :issues do
    collection do
      get 'page/:page' => 'issues#index', :page => %r{\d+}, :as => 'issues_page'
    end
  end
  

• 

  Prem Sichanugrist (sikachu) November 23rd, 2010 @ 01:44 PM

  • Importance changed from “Low” to “Medium”
• 

  José Valim November 23rd, 2010 @ 01:59 PM

  Prem is right, Rails in fact tries to generate routes automatically, but in this case it is generating the wrong one.

• 

  Repository November 25th, 2010 @ 10:50 AM

  • State changed from “open” to “resolved”

  (from [731ca00b484379661786fac36c17db7e085603c4]) Dynamically generaeted helpers on collection should not clobber resources url helper [#6028 state:resolved] https://github.com/rails/rails/commit/731ca00b484379661786fac36c17d...

• 

  Repository November 25th, 2010 @ 10:53 AM

  (from [7e903a3d3a661c6ed4164d6a563bcf54e4497db3]) Dynamically generaeted helpers on collection should not clobber resources url helper [#6028 state:resolved] https://github.com/rails/rails/commit/7e903a3d3a661c6ed4164d6a563bc...

• You flagged this item as spam.
  

  Wincent Colaiuta November 25th, 2010 @ 11:38 AM

  Thanks for the fix.