From 26fb722792670c01d511948599567bc95b2871b2 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Sat, 19 Jun 2010 07:23:57 +0100 Subject: [PATCH] Custom resource routes should be scoped --- actionpack/lib/action_dispatch/routing/mapper.rb | 43 ++++++++++++++++--- actionpack/test/dispatch/routing_test.rb | 47 ++++++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 95e5656..0018b64 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -647,7 +647,9 @@ module ActionDispatch 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 @@ module ActionDispatch 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 @@ module ActionDispatch 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 @@ module ActionDispatch [: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 @@ module ActionDispatch 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 @@ module ActionDispatch 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 diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 0b3bbcc..899990c 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -180,6 +180,33 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest 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 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest 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 -- 1.7.1