Skip to content

Commit

Permalink
Router accepts member routes on resource. [#4624 state:resolved]
Browse files Browse the repository at this point in the history
  • Loading branch information
rizwanreza authored and jeremy committed Jun 7, 2010
1 parent 83729e2 commit ac9f8e1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
19 changes: 14 additions & 5 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -622,13 +622,22 @@ def collection
end

def member
unless @scope[:scope_level] == :resources
raise ArgumentError, "can't use member outside resources scope"
unless [:resources, :resource].include?(@scope[:scope_level])
raise ArgumentError, "You can't use member action outside resources and resource scope."
end

with_scope_level(:member) do
scope(':id', :name_prefix => parent_resource.member_name, :as => "") do
yield
case @scope[:scope_level]
when :resources
with_scope_level(:member) do
scope(':id', :name_prefix => parent_resource.member_name, :as => "") do
yield
end
end
when :resource
with_scope_level(:member) do
scope(':id', :as => "") do
yield
end
end
end
end
Expand Down
13 changes: 13 additions & 0 deletions actionpack/test/dispatch/routing_test.rb
Expand Up @@ -28,6 +28,10 @@ def self.matches?(request)
post :reset

resource :info

member do
get :crush
end
end

match 'account/logout' => redirect("/logout"), :as => :logout_redirect
Expand Down Expand Up @@ -352,6 +356,15 @@ def test_session_info_nested_singleton_resource
end
end

def test_member_on_resource
with_test_routes do
get '/session/1/crush'

This comment has been minimized.

Copy link
@pixeltrix

pixeltrix Jun 7, 2010

Contributor

This isn't right - a member action on a singleton resource shouldn't have an id. It should be /session/crush.

This comment has been minimized.

Copy link
@josevalim

josevalim Jun 7, 2010

Contributor

Agreed!

This comment has been minimized.

Copy link
@josevalim

josevalim Jun 7, 2010

Contributor

In fact, I think it makes no sense to have :member at the resource level. It's redundant.

This comment has been minimized.

Copy link
@pixeltrix

pixeltrix Jun 7, 2010

Contributor

That's the current behavior, however it depends on whether actions on a new resource are ever going to be added: https://rails.lighthouseapp.com/projects/8994/tickets/4328

This comment has been minimized.

Copy link
@rizwanreza

rizwanreza Jun 7, 2010

Author Contributor

I agree that singleton resource shouldn't have an id or whatever, but if this is not the way, how does one add an action to a singleton resource when needed?

This comment has been minimized.

Copy link
@pixeltrix

pixeltrix Jun 7, 2010

Contributor

Currently you just use get, etc. directly inside the block

assert_equal 'sessions#crush', @response.body

assert_equal '/session/1/crush', crush_session_path(1)
end
end

def test_redirect_modulo
with_test_routes do
get '/account/modulo/name'
Expand Down

0 comments on commit ac9f8e1

Please sign in to comment.