RouteMatcher is not working with multiple routes for the same controller #191

Closed
ddidier opened this Issue Nov 5, 2012 · 3 comments

Comments

Projects
None yet
2 participants
@ddidier

ddidier commented Nov 5, 2012

in route_matcher_spec.rb add:

describe Shoulda::Matchers::ActionController::RouteMatcher do
  ...

  context "given a controller with a defined route" do
    ...

    before do
      define_routes do
        match 'examples/:id', :to => 'examples#example'
        ### add this line:
        match 'examples2/:id', :to => 'examples#example'
      end
    end

    it "should accept routing the correct path to the correct parameters" do
      controller.should route(:get, '/examples/1').to(:action => 'example', :id => '1')
    end

    ### add this test:
    it "should accept routing the correct path to the correct parameters" do
      controller.should route(:get, '/examples2/1').to(:action => 'example', :id => '1')
    end

Got:

Failure/Error: controller.should route(:get, '/examples2/1').
   The generated path <"/examples/1"> did not match <"/examples2/1">.
   <"/examples2/1"> expected but was
   <"/examples/1">.
@gabebw

This comment has been minimized.

Show comment Hide comment
@gabebw

gabebw Dec 27, 2012

Contributor

This looks like a bug in the assert_routing method we use under the hood (link to code)

Here's the assert_routing documentation: Rails docs

Here's a quick test I added to route_matcher_spec.rb with 2 routes that map to the same controller and action.

it 'finds both routes that map to the same action' do
  define_controller('Examples')

  define_routes do
    match 'examples/:id', :to => 'examples#example'
    match 'examples2/:id', :to => 'examples#example'
  end

  # Passes with :path => 'examples/1'
  assert_routing({:method => :get, :path => 'examples2/1'},
    { :controller => 'examples', :action => "example", :id => "1" })
end

I'm not sure what the best course is here - I'd prefer not to monkeypatch a core Rails assertion. Can I ask what your use case is here? Why do you have 2 URLs that map to the same controller? Could you map one of them to a redirect like this?

get "real_url" => "real#action"
get "second_url" => redirect("real_url")
Contributor

gabebw commented Dec 27, 2012

This looks like a bug in the assert_routing method we use under the hood (link to code)

Here's the assert_routing documentation: Rails docs

Here's a quick test I added to route_matcher_spec.rb with 2 routes that map to the same controller and action.

it 'finds both routes that map to the same action' do
  define_controller('Examples')

  define_routes do
    match 'examples/:id', :to => 'examples#example'
    match 'examples2/:id', :to => 'examples#example'
  end

  # Passes with :path => 'examples/1'
  assert_routing({:method => :get, :path => 'examples2/1'},
    { :controller => 'examples', :action => "example", :id => "1" })
end

I'm not sure what the best course is here - I'd prefer not to monkeypatch a core Rails assertion. Can I ask what your use case is here? Why do you have 2 URLs that map to the same controller? Could you map one of them to a redirect like this?

get "real_url" => "real#action"
get "second_url" => redirect("real_url")
@ddidier

This comment has been minimized.

Show comment Hide comment
@ddidier

ddidier Dec 27, 2012

I'm using active admin which generates some routes including:

admin_root           /admin(.:format)                  admin/dashboard#index
admin_dashboard      /admin/dashboard(.:format)        admin/dashboard#index

so that's not very important but I reported it anyway, thanks

ddidier commented Dec 27, 2012

I'm using active admin which generates some routes including:

admin_root           /admin(.:format)                  admin/dashboard#index
admin_dashboard      /admin/dashboard(.:format)        admin/dashboard#index

so that's not very important but I reported it anyway, thanks

@gabebw

This comment has been minimized.

Show comment Hide comment
@gabebw

gabebw Dec 28, 2012

Contributor

Thanks for reporting. I don't see a way to fix this, plus it sounds like it's not a huge blocker for you, so I'm going to close this.

Contributor

gabebw commented Dec 28, 2012

Thanks for reporting. I don't see a way to fix this, plus it sounds like it's not a huge blocker for you, so I'm going to close this.

@gabebw gabebw closed this Dec 28, 2012

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