Fix bug when passing multiple options to SimplyRestful, like :new => { :preview => :get, :draft => :get }. [Rick Olson, Josh Susser, Lars Pind]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4641 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 49 insertions, 19 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index c5d8154ed2..c93ff29d7a 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fix bug when passing multiple options to SimplyRestful, like :new => { :preview => :get, :draft => :get }.  [Rick Olson, Josh Susser, Lars Pind]
+
 * Dup the options passed to map.resources so that multiple resources get the same options.  [Rick Olson]
 
 * Fixed the new_#{resource}_url route and added named route tests for Simply Restful.  [Rick Olson]
diff --git a/actionpack/lib/action_controller/resources.rb b/actionpack/lib/action_controller/resources.rb
index 93016cbcc5..2b4c9217d5 100644
--- a/actionpack/lib/action_controller/resources.rb
+++ b/actionpack/lib/action_controller/resources.rb
@@ -11,7 +11,7 @@ module ActionController
         @singular = options[:singular] || plural.to_s.singularize
         
         @options = options
-        
+
         arrange_actions
         add_default_actions
         set_prefixes
@@ -56,30 +56,17 @@ module ActionController
           @path_prefix = options.delete(:path_prefix)
           @name_prefix = options.delete(:name_prefix)
         end
-        
+
         def arrange_actions_by_methods(actions)
-          arrayize_values(flip_keys_and_values(actions || {}))
+          (actions || {}).inject({}) do |flipped_hash, (key, value)|
+            (flipped_hash[value] ||= []) << key
+            flipped_hash
+          end
         end
         
         def add_default_action(collection, method, action)
           (collection[method] ||= []).unshift(action)
         end
-
-        def flip_keys_and_values(hash)
-          hash.inject({}) do |flipped_hash, (key, value)|
-            flipped_hash[value] = key
-            flipped_hash
-          end
-        end
-
-        def arrayize_values(hash)
-          hash.each do |(key, value)|
-            unless value.is_a?(Array)
-              hash[key] = []
-              hash[key] << value
-            end
-          end
-        end
     end
     
     def resources(*entities)
diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb
index bc8fc76536..96b6431567 100644
--- a/actionpack/test/controller/resources_test.rb
+++ b/actionpack/test/controller/resources_test.rb
@@ -11,6 +11,20 @@ class CommentsController < ActionController::Base
 end
 
 class ResourcesTest < Test::Unit::TestCase
+  def test_should_arrange_actions
+    resource = ActionController::Resources::Resource.new(:messages, 
+      :collection => { :rss => :get, :reorder => :post, :csv => :post },
+      :member     => { :rss => :get, :atom => :get, :upload => :post, :fix => :post },
+      :new        => { :preview => :get, :draft => :get })
+
+    assert_resource_methods [:rss],                    resource, :collection, :get
+    assert_resource_methods [:create, :csv, :reorder], resource, :collection, :post
+    assert_resource_methods [:edit, :rss, :atom],      resource, :member,     :get
+    assert_resource_methods [:upload, :fix],           resource, :member,     :post
+    assert_resource_methods [:update],                 resource, :member,     :put
+    assert_resource_methods [:new, :preview, :draft],  resource, :new,        :get
+  end
+  
   def test_default_restful_routes
     with_restful_routing :messages do
       assert_simply_restful_for :messages
@@ -67,6 +81,25 @@ class ResourcesTest < Test::Unit::TestCase
     end
   end
 
+  def test_with_two_member_actions_with_same_method
+    [:put, :post].each do |method|
+      with_restful_routing :messages, :member => { :mark => method, :unmark => method } do
+        %w(mark unmark).each do |action|
+          action_options = {:action => action, :id => '1'}
+          action_path    = "/messages/1;#{action}"
+          assert_restful_routes_for :messages do |options|
+            assert_recognizes(options.merge(action_options), :path => action_path, :method => method)
+          end
+        
+          assert_restful_named_routes_for :messages do |options|
+            assert_named_route action_path, "#{action}_message_path".to_sym, action_options
+          end
+        end
+      end
+    end
+  end
+
+
   def test_with_new_action
     with_restful_routing :messages, :new => { :preview => :post } do
       preview_options = {:action => 'preview'}
@@ -170,4 +203,12 @@ class ResourcesTest < Test::Unit::TestCase
       actual =  @controller.send(route, options) rescue $!.class.name
       assert_equal expected, actual, "Error on route: #{route}(#{options.inspect})"
     end
+
+    def assert_resource_methods(expected, resource, action_method, method)
+      assert_equal expected.length, resource.send("#{action_method}_methods")[method].size, "#{resource.send("#{action_method}_methods")[method].inspect}"
+      expected.each do |action|
+        assert resource.send("#{action_method}_methods")[method].include?(action), 
+          "#{method} not in #{action_method} methods: #{resource.send("#{action_method}_methods")[method].inspect}"
+      end
+    end
 end