Forces enumeration override position to the same as its parent (#19657).

git-svn-id: http://svn.redmine.org/redmine/trunk@14627 e93f8b46-1217-0410-a6f0-8f06a7374b81

2 files changed, 71 insertions, 1 deletions

diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb
index e53ee1fd4..4f5e1e377 100644
--- a/app/models/enumeration.rb
+++ b/app/models/enumeration.rb
@@ -22,7 +22,7 @@ class Enumeration < ActiveRecord::Base
 
   belongs_to :project
 
-  acts_as_list :scope => 'type = \'#{type}\''
+  acts_as_list :scope => 'type = \'#{type}\' AND #{parent_id ? "parent_id = #{parent_id}" : "parent_id IS NULL"}'
   acts_as_customizable
   acts_as_tree
 
@@ -129,11 +129,37 @@ class Enumeration < ActiveRecord::Base
     return new == previous
   end
 
+  # Overrides acts_as_list reset_positions_in_list so that enumeration overrides
+  # get the same position as the overriden enumeration
+  def reset_positions_in_list
+    super
+    self.class.
+      where("parent_id IS NOT NULL").
+      update_all("position = (SELECT MIN(position) FROM #{self.class.table_name} p WHERE p.id = #{self.class.table_name}.parent_id)")
+  end
+
 private
   def check_integrity
     raise "Cannot delete enumeration" if self.in_use?
   end
 
+  # Overrides acts_as_list add_to_list_bottom so that enumeration overrides
+  # get the same position as the overriden enumeration
+  def add_to_list_bottom
+    if parent
+      self[position_column] = parent.position
+    else
+      super
+    end
+  end
+
+  # Overrides acts_as_list remove_from_list so that enumeration overrides
+  # get the same position as the overriden enumeration
+  def remove_from_list
+    if parent_id.blank?
+      super
+    end
+  end
 end
 
 # Force load the subclasses in development mode
diff --git a/test/unit/enumeration_test.rb b/test/unit/enumeration_test.rb
index 8f5f758d4..ac5e15c4c 100644
--- a/test/unit/enumeration_test.rb
+++ b/test/unit/enumeration_test.rb
@@ -127,4 +127,48 @@ class EnumerationTest < ActiveSupport::TestCase
       assert_equal Enumeration, klass.superclass
     end
   end
+
+  def test_list_should_be_scoped_for_each_type
+    Enumeration.delete_all
+
+    a = IssuePriority.create!(:name => 'A')
+    b = IssuePriority.create!(:name => 'B')
+    c = DocumentCategory.create!(:name => 'C')
+
+    assert_equal [1, 2, 1], [a, b, c].map(&:reload).map(&:position)
+  end
+
+  def test_override_should_be_created_with_same_position_as_parent
+    Enumeration.delete_all
+
+    a = IssuePriority.create!(:name => 'A')
+    b = IssuePriority.create!(:name => 'B')
+    override = IssuePriority.create!(:name => 'BB', :parent_id => b.id)
+
+    assert_equal [1, 2, 2], [a, b, override].map(&:reload).map(&:position)
+  end
+
+  def test_override_position_should_be_updated_with_parent_position
+    Enumeration.delete_all
+
+    a = IssuePriority.create!(:name => 'A')
+    b = IssuePriority.create!(:name => 'B')
+    override = IssuePriority.create!(:name => 'BB', :parent_id => b.id)
+    b.move_to = 'higher'
+
+    assert_equal [2, 1, 1], [a, b, override].map(&:reload).map(&:position)
+  end
+
+  def test_destroying_override_should_not_update_positions
+    Enumeration.delete_all
+
+    a = IssuePriority.create!(:name => 'A')
+    b = IssuePriority.create!(:name => 'B')
+    c = IssuePriority.create!(:name => 'C')
+    override = IssuePriority.create!(:name => 'BB', :parent_id => b.id)
+    assert_equal [1, 2, 3, 2], [a, b, c, override].map(&:reload).map(&:position)
+
+    override.destroy
+    assert_equal [1, 2, 3], [a, b, c].map(&:reload).map(&:position)
+  end
 end