From b75e0382557ad89069f03807d423338d906fa485 Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Wed, 25 Nov 2009 05:36:56 +0000 Subject: [PATCH] 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 --- lib/redmine/menu_manager.rb | 17 ++++---- .../lib/redmine/menu_manager/mapper_test.rb | 10 ++--- .../redmine/menu_manager/menu_helper_test.rb | 40 +++++++++---------- .../redmine/menu_manager/menu_item_test.rb | 20 +++++----- 4 files changed, 45 insertions(+), 42 deletions(-) diff --git a/lib/redmine/menu_manager.rb b/lib/redmine/menu_manager.rb index 320d4a65..af7d67b7 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 304ece69..699c0cf2 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 2d4778cb..d93a891c 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 ccb52df6..835e154d 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]