Updated menus from JPL's feedback.

* Updated Mapper#push documentation
* Renamed :parent_menu to :parent
* Renamed the external API for :child_menus to :children.  Internally it needs
  to stay :child_menus because Tree::TreeNode already defines a children
  method for another purpose

  #4250

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@3092 e93f8b46-1217-0410-a6f0-8f06a7374b81

4 files changed, 45 insertions, 42 deletions

diff --git a/lib/redmine/menu_manager.rb b/lib/redmine/menu_manager.rb
index 320d4a655..af7d67b77 100644
--- a/lib/redmine/menu_manager.rb
+++ b/lib/redmine/menu_manager.rb
@@ -313,13 +313,16 @@ module Redmine
       #   * a String
       #   * a Proc that can take the project as argument
       # * before, after: specify where the menu item should be inserted (eg. :after => :activity)
+      # * parent: menu item will be added as a child of another named menu (eg. :parent => :issues)
+      # * children: a Proc that is called before rendering the item. The Proc should return an array of MenuItems, which will be added as children to this item.
+      #   eg. :children => Proc.new {|project| [Redmine::MenuManager::MenuItem.new(...)] }
       # * last: menu item will stay at the end (eg. :last => true)
       # * html_options: a hash of html options that are passed to link_to
       def push(name, url, options={})
         options = options.dup
 
-        if options[:parent_menu]
-          subtree = self.find(options[:parent_menu])
+        if options[:parent]
+          subtree = self.find(options[:parent])
           if subtree
             target_root = subtree
           else
@@ -383,13 +386,13 @@ module Redmine
     
     class MenuItem < Tree::TreeNode
       include Redmine::I18n
-      attr_reader :name, :url, :param, :condition, :parent_menu, :child_menus
+      attr_reader :name, :url, :param, :condition, :parent, :child_menus
       
       def initialize(name, url, options)
         raise ArgumentError, "Invalid option :if for menu item '#{name}'" if options[:if] && !options[:if].respond_to?(:call)
         raise ArgumentError, "Invalid option :html for menu item '#{name}'" if options[:html] && !options[:html].is_a?(Hash)
-        raise ArgumentError, "Cannot set the :parent_menu to be the same as this item" if options[:parent_menu] == name.to_sym
-        raise ArgumentError, "Invalid option :child_menus for menu item '#{name}'" if options[:child_menus] && !options[:child_menus].respond_to?(:call)
+        raise ArgumentError, "Cannot set the :parent to be the same as this item" if options[:parent] == name.to_sym
+        raise ArgumentError, "Invalid option :children for menu item '#{name}'" if options[:children] && !options[:children].respond_to?(:call)
         @name = name
         @url = url
         @condition = options[:if]
@@ -398,8 +401,8 @@ module Redmine
         @html_options = options[:html] || {}
         # Adds a unique class to each menu item based on its name
         @html_options[:class] = [@html_options[:class], @name.to_s.dasherize].compact.join(' ')
-        @parent_menu = options[:parent_menu]
-        @child_menus = options[:child_menus]
+        @parent = options[:parent]
+        @child_menus = options[:children]
         super @name.to_sym
       end
       
diff --git a/test/unit/lib/redmine/menu_manager/mapper_test.rb b/test/unit/lib/redmine/menu_manager/mapper_test.rb
index 304ece697..699c0cf2c 100644
--- a/test/unit/lib/redmine/menu_manager/mapper_test.rb
+++ b/test/unit/lib/redmine/menu_manager/mapper_test.rb
@@ -32,7 +32,7 @@ class Redmine::MenuManager::MapperTest < Test::Unit::TestCase
   def test_push_onto_parent
     menu_mapper = Redmine::MenuManager::Mapper.new(:test_menu, {})
     menu_mapper.push :test_overview, { :controller => 'projects', :action => 'show'}, {}
-    menu_mapper.push :test_child, { :controller => 'projects', :action => 'show'}, {:parent_menu => :test_overview}
+    menu_mapper.push :test_child, { :controller => 'projects', :action => 'show'}, {:parent => :test_overview}
 
     assert menu_mapper.exists?(:test_child)
     assert_equal :test_child, menu_mapper.find(:test_child).name
@@ -41,13 +41,13 @@ class Redmine::MenuManager::MapperTest < Test::Unit::TestCase
   def test_push_onto_grandparent
     menu_mapper = Redmine::MenuManager::Mapper.new(:test_menu, {})
     menu_mapper.push :test_overview, { :controller => 'projects', :action => 'show'}, {}
-    menu_mapper.push :test_child, { :controller => 'projects', :action => 'show'}, {:parent_menu => :test_overview}
-    menu_mapper.push :test_grandchild, { :controller => 'projects', :action => 'show'}, {:parent_menu => :test_child}
+    menu_mapper.push :test_child, { :controller => 'projects', :action => 'show'}, {:parent => :test_overview}
+    menu_mapper.push :test_grandchild, { :controller => 'projects', :action => 'show'}, {:parent => :test_child}
 
     assert menu_mapper.exists?(:test_grandchild)
     grandchild = menu_mapper.find(:test_grandchild)
     assert_equal :test_grandchild, grandchild.name
-    assert_equal :test_child, grandchild.parent_menu
+    assert_equal :test_child, grandchild.parent.name
   end
 
   def test_push_first
@@ -122,7 +122,7 @@ class Redmine::MenuManager::MapperTest < Test::Unit::TestCase
   def test_exists_for_child_node
     menu_mapper = Redmine::MenuManager::Mapper.new(:test_menu, {})
     menu_mapper.push :test_overview, { :controller => 'projects', :action => 'show'}, {}
-    menu_mapper.push :test_child, { :controller => 'projects', :action => 'show'}, {:parent_menu => :test_overview }
+    menu_mapper.push :test_child, { :controller => 'projects', :action => 'show'}, {:parent => :test_overview }
 
     assert menu_mapper.exists?(:test_child)
   end
diff --git a/test/unit/lib/redmine/menu_manager/menu_helper_test.rb b/test/unit/lib/redmine/menu_manager/menu_helper_test.rb
index 2d4778cbe..d93a891c7 100644
--- a/test/unit/lib/redmine/menu_manager/menu_helper_test.rb
+++ b/test/unit/lib/redmine/menu_manager/menu_helper_test.rb
@@ -101,20 +101,20 @@ class Redmine::MenuManager::MenuHelperTest < HelperTestCase
     
   end
 
-  def test_render_menu_node_with_child_menus
+  def test_render_menu_node_with_children
     User.current = User.find(2)
     
     parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
                                                      '/test',
                                                      {
-                                                       :child_menus => Proc.new {|p|
-                                                         child_menus = []
+                                                       :children => Proc.new {|p|
+                                                         children = []
                                                          3.times do |time|
-                                                           child_menus << Redmine::MenuManager::MenuItem.new("test_child_#{time}",
+                                                           children << Redmine::MenuManager::MenuItem.new("test_child_#{time}",
                                                                                                              {:controller => 'issues', :action => 'index'},
                                                                                                              {})
                                                          end
-                                                         child_menus
+                                                         children
                                                        }
                                                      })
     @response.body = render_menu_node(parent_node, Project.find(1))
@@ -129,30 +129,30 @@ class Redmine::MenuManager::MenuHelperTest < HelperTestCase
     end
   end
 
-  def test_render_menu_node_with_nested_items_and_child_menus
+  def test_render_menu_node_with_nested_items_and_children
     User.current = User.find(2)
 
     parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
                                                      '/test',
                                                      {
-                                                       :child_menus => Proc.new {|p|
-                                                         child_menus = []
+                                                       :children => Proc.new {|p|
+                                                         children = []
                                                          3.times do |time|
-                                                           child_menus << Redmine::MenuManager::MenuItem.new("test_child_#{time}", {:controller => 'issues', :action => 'index'}, {})
+                                                           children << Redmine::MenuManager::MenuItem.new("test_child_#{time}", {:controller => 'issues', :action => 'index'}, {})
                                                          end
-                                                         child_menus
+                                                         children
                                                        }
                                                      })
 
     parent_node << Redmine::MenuManager::MenuItem.new(:child_node,
                                                      '/test',
                                                      {
-                                                       :child_menus => Proc.new {|p|
-                                                         child_menus = []
+                                                       :children => Proc.new {|p|
+                                                         children = []
                                                          6.times do |time|
-                                                            child_menus << Redmine::MenuManager::MenuItem.new("test_dynamic_child_#{time}", {:controller => 'issues', :action => 'index'}, {})
+                                                            children << Redmine::MenuManager::MenuItem.new("test_dynamic_child_#{time}", {:controller => 'issues', :action => 'index'}, {})
                                                          end
-                                                         child_menus
+                                                         children
                                                        }
                                                      })
 
@@ -177,26 +177,26 @@ class Redmine::MenuManager::MenuHelperTest < HelperTestCase
     end
   end
 
-  def test_render_menu_node_with_child_menus_without_an_array
+  def test_render_menu_node_with_children_without_an_array
     parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
                                                      '/test',
                                                      {
-                                                       :child_menus => Proc.new {|p| Redmine::MenuManager::MenuItem.new("test_child", "/testing", {})}
+                                                       :children => Proc.new {|p| Redmine::MenuManager::MenuItem.new("test_child", "/testing", {})}
                                                      })
 
-    assert_raises Redmine::MenuManager::MenuError, ":child_menus must be an array of MenuItems" do
+    assert_raises Redmine::MenuManager::MenuError, ":children must be an array of MenuItems" do
       @response.body = render_menu_node(parent_node, Project.find(1))
     end
   end
     
-  def test_render_menu_node_with_incorrect_child_menus
+  def test_render_menu_node_with_incorrect_children
     parent_node = Redmine::MenuManager::MenuItem.new(:parent_node,
                                                      '/test',
                                                      {
-                                                       :child_menus => Proc.new {|p| ["a string"] }
+                                                       :children => Proc.new {|p| ["a string"] }
                                                      })
 
-    assert_raises Redmine::MenuManager::MenuError, ":child_menus must be an array of MenuItems" do
+    assert_raises Redmine::MenuManager::MenuError, ":children must be an array of MenuItems" do
       @response.body = render_menu_node(parent_node, Project.find(1))
     end
 
diff --git a/test/unit/lib/redmine/menu_manager/menu_item_test.rb b/test/unit/lib/redmine/menu_manager/menu_item_test.rb
index ccb52df6d..835e154d8 100644
--- a/test/unit/lib/redmine/menu_manager/menu_item_test.rb
+++ b/test/unit/lib/redmine/menu_manager/menu_item_test.rb
@@ -28,9 +28,9 @@ class Redmine::MenuManager::MenuItemTest < Test::Unit::TestCase
   include RedmineMenuTestHelper
 
   Redmine::MenuManager.map :test_menu do |menu|
-    menu.push(:parent_menu, '/test', { })
-    menu.push(:child_menu, '/test', { :parent_menu => :parent_menu})
-    menu.push(:child2_menu, '/test', { :parent_menu => :parent_menu})
+    menu.push(:parent, '/test', { })
+    menu.push(:child_menu, '/test', { :parent => :parent})
+    menu.push(:child2_menu, '/test', { :parent => :parent})
   end
   
   context "MenuItem#caption" do
@@ -92,28 +92,28 @@ class Redmine::MenuManager::MenuItemTest < Test::Unit::TestCase
                                               })
   end
 
-  def test_new_menu_item_should_require_a_proc_to_use_the_child_menus_option
+  def test_new_menu_item_should_require_a_proc_to_use_the_children_option
     assert_raises ArgumentError do
       Redmine::MenuManager::MenuItem.new(:test_error, '/test',
                                          {
-                                           :child_menus => ['not_a_proc']
+                                           :children => ['not_a_proc']
                                          })
     end
 
-    assert Redmine::MenuManager::MenuItem.new(:test_good_child_menus, '/test',
+    assert Redmine::MenuManager::MenuItem.new(:test_good_children, '/test',
                                               {
-                                                :child_menus => Proc.new{}
+                                                :children => Proc.new{}
                                               })
   end
 
-  def test_new_should_not_allow_setting_the_parent_menu_item_to_the_current_item
+  def test_new_should_not_allow_setting_the_parent_item_to_the_current_item
     assert_raises ArgumentError do
-      Redmine::MenuManager::MenuItem.new(:test_error, '/test', { :parent_menu => :test_error })
+      Redmine::MenuManager::MenuItem.new(:test_error, '/test', { :parent => :test_error })
     end
   end
 
   def test_has_children
-    parent_item = get_menu_item(:test_menu, :parent_menu)
+    parent_item = get_menu_item(:test_menu, :parent)
     assert parent_item.hasChildren?
     assert_equal 2, parent_item.children.size
     assert_equal get_menu_item(:test_menu, :child_menu), parent_item.children[0]