Permalink
Browse files

Custom resource routes should be scoped

[#3765]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
pixeltrix authored and jeremy committed Jun 19, 2010
1 parent 2c2a5fe commit bf59717b43fc7c186f9acebac27ba9ac7d59dd93
Showing with 83 additions and 7 deletions.
  1. +36 −7 actionpack/lib/action_dispatch/routing/mapper.rb
  2. +47 −0 actionpack/test/dispatch/routing_test.rb
@@ -647,7 +647,9 @@ def new
with_scope_level(:new) do
scope(*parent_resource.new_scope) do
- yield
+ scope(action_path(:new)) do
+ yield
+ end
end
end
end
@@ -723,7 +725,17 @@ def match(*args)
options = options_for_action(args.first, options)
with_exclusive_scope do
- return match(path, options)
+ return super(path, options)
+ end
+ elsif resource_method_scope?
+ path = path_for_custom_action
+ options[:as] = name_for_action(options[:as]) if options[:as]
+ args.push(options)
+
+ with_exclusive_scope do
+ scope(path) do
+ return super
+ end
end
end
@@ -737,7 +749,7 @@ def match(*args)
def root(options={})
if @scope[:scope_level] == :resources
- with_scope_level(:collection) do
+ with_scope_level(:nested) do
scope(parent_resource.path, :name_prefix => parent_resource.collection_name) do
super(options)
end
@@ -780,12 +792,18 @@ def resource_scope?
[:resource, :resources].include?(@scope[:scope_level])
end
+ def resource_method_scope?
+ [:collection, :member, :new].include?(@scope[:scope_level])
+ end
+
def with_exclusive_scope
begin
old_name_prefix, old_path = @scope[:name_prefix], @scope[:path]
@scope[:name_prefix], @scope[:path] = nil, nil
- yield
+ with_scope_level(:exclusive) do
+ yield
+ end
ensure
@scope[:name_prefix], @scope[:path] = old_name_prefix, old_path
end
@@ -844,10 +862,8 @@ def path_for_action(action, path_names)
end
else
case @scope[:scope_level]
- when :collection
+ when :collection, :new
"#{@scope[:path]}/#{action_path(action)}(.:format)"
- when :new
- "#{@scope[:path]}/#{action_path(:new)}/#{action_path(action)}(.:format)"
else
if parent_resource.shallow?
"#{@scope[:module]}/#{parent_resource.path}/:id/#{action_path(action)}(.:format)"
@@ -858,6 +874,19 @@ def path_for_action(action, path_names)
end
end
+ def path_for_custom_action
+ case @scope[:scope_level]
+ when :collection, :new
+ @scope[:path]
+ else
+ if parent_resource.shallow?
+ "#{@scope[:module]}/#{parent_resource.path}/:id"
+ else
+ @scope[:path]
+ end
+ end
+ end
+
def action_path(name, path_names = nil)
path_names ||= @scope[:path_names]
path_names[name.to_sym] || name.to_s
@@ -180,6 +180,33 @@ def self.matches?(request)
end
end
+ resources :customers do
+ get "recent" => "customers#recent", :as => :recent, :on => :collection
+ get "profile" => "customers#profile", :as => :profile, :on => :member
+ post "preview" => "customers#preview", :as => :preview, :on => :new
+ resource :avatar do
+ get "thumbnail(.:format)" => "avatars#thumbnail", :as => :thumbnail, :on => :member
+ end
+ resources :invoices do
+ get "outstanding" => "invoices#outstanding", :as => :outstanding, :on => :collection
+ get "overdue", :to => :overdue, :on => :collection
+ get "print" => "invoices#print", :as => :print, :on => :member
+ post "preview" => "invoices#preview", :as => :preview, :on => :new
+ end
+ resources :notes, :shallow => true do
+ get "preview" => "notes#preview", :as => :preview, :on => :new
+ get "print" => "notes#print", :as => :print, :on => :member
+ end
+ end
+
+ namespace :api do
+ resources :customers do
+ get "recent" => "customers#recent", :as => :recent, :on => :collection
+ get "profile" => "customers#profile", :as => :profile, :on => :member
+ post "preview" => "customers#preview", :as => :preview, :on => :new
+ end
+ end
+
match 'sprockets.js' => ::TestRoutingMapper::SprocketsApp
match 'people/:id/update', :to => 'people#update', :as => :update_person
@@ -1295,6 +1322,26 @@ def test_shallow_nested_resources
end
end
+ def test_custom_resource_routes_are_scoped
+ with_test_routes do
+ assert_equal '/customers/recent', recent_customers_path
+ assert_equal '/customers/1/profile', profile_customer_path(:id => '1')
+ assert_equal '/customers/new/preview', preview_new_customer_path
+ assert_equal '/customers/1/avatar/thumbnail.jpg', thumbnail_customer_avatar_path(:customer_id => '1', :format => :jpg)
+ assert_equal '/customers/1/invoices/outstanding', outstanding_customer_invoices_path(:customer_id => '1')
+ assert_equal '/customers/1/invoices/2/print', print_customer_invoice_path(:customer_id => '1', :id => '2')
+ assert_equal '/customers/1/invoices/new/preview', preview_new_customer_invoice_path(:customer_id => '1')
+ assert_equal '/customers/1/notes/new/preview', preview_new_customer_note_path(:customer_id => '1')
+ assert_equal '/notes/1/print', print_note_path(:id => '1')
+ assert_equal '/api/customers/recent', recent_api_customers_path
+ assert_equal '/api/customers/1/profile', profile_api_customer_path(:id => '1')
+ assert_equal '/api/customers/new/preview', preview_new_api_customer_path
+
+ get '/customers/1/invoices/overdue'
+ assert_equal 'invoices#overdue', @response.body
+ end
+ end
+
private
def with_test_routes
yield

0 comments on commit bf59717

Please sign in to comment.