Replaces acts_as_list with an implementation that handles #position= (#12909).

Objects are reordered using the regular attribute writer #position= and AR callbacks.

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

6 files changed, 30 insertions, 25 deletions

diff --git a/app/models/board.rb b/app/models/board.rb
index d62f863c4..104c85749 100644
--- a/app/models/board.rb
+++ b/app/models/board.rb
@@ -21,7 +21,7 @@ class Board < ActiveRecord::Base
   has_many :messages, lambda {order("#{Message.table_name}.created_on DESC")}, :dependent => :destroy
   belongs_to :last_message, :class_name => 'Message'
   acts_as_tree :dependent => :nullify
-  acts_as_list :scope => '(project_id = #{project_id} AND parent_id #{parent_id ? "= #{parent_id}" : "IS NULL"})'
+  acts_as_positioned :scope => [:project_id, :parent_id]
   acts_as_watchable
 
   validates_presence_of :name, :description
diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb
index f5f15a4a1..511299523 100644
--- a/app/models/custom_field.rb
+++ b/app/models/custom_field.rb
@@ -24,7 +24,7 @@ class CustomField < ActiveRecord::Base
            :dependent => :delete_all
   has_many :custom_values, :dependent => :delete_all
   has_and_belongs_to_many :roles, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "custom_field_id"
-  acts_as_list :scope => 'type = \'#{self.class}\''
+  acts_as_positioned
   serialize :possible_values
   store :format_store
 
diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb
index eac403452..fc8486174 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}\' AND #{parent_id ? "parent_id = #{parent_id}" : "parent_id IS NULL"}'
+  acts_as_positioned :scope => :parent_id
   acts_as_customizable
   acts_as_tree
 
@@ -129,33 +129,38 @@ 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
-    acts_as_list_class.where(scope_condition).reorder("#{position_column} ASC, id ASC").each_with_index do |item, i|
-      acts_as_list_class.where("id = :id OR parent_id = :id", :id => item.id).
-        update_all({position_column => (i + 1)})
-    end
-  end
+  private
 
-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
+  # Overrides Redmine::Acts::Positioned#set_default_position 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
+  def set_default_position
+    if position.nil? && parent
+      self.position = parent.position
+    end
+    super
+  end
+
+  # Overrides Redmine::Acts::Positioned#update_position so that overrides get the same
+  # position as the overriden enumeration
+  def update_position
+    super
+    if position_changed?
+      self.class.where.not(:parent_id => nil).update_all(
+        "position = coalesce((
+          select position
+          from (select id, position from enumerations) as parent
+          where parent_id = parent.id), 1)"
+      )
     end
   end
 
-  # Overrides acts_as_list remove_from_list so that enumeration overrides
+  # Overrides Redmine::Acts::Positioned#remove_position so that enumeration overrides
   # get the same position as the overriden enumeration
-  def remove_from_list
+  def remove_position
     if parent_id.blank?
       super
     end
diff --git a/app/models/issue_status.rb b/app/models/issue_status.rb
index 7855f5b92..31c0f031c 100644
--- a/app/models/issue_status.rb
+++ b/app/models/issue_status.rb
@@ -19,7 +19,7 @@ class IssueStatus < ActiveRecord::Base
   before_destroy :check_integrity
   has_many :workflows, :class_name => 'WorkflowTransition', :foreign_key => "old_status_id"
   has_many :workflow_transitions_as_new_status, :class_name => 'WorkflowTransition', :foreign_key => "new_status_id"
-  acts_as_list
+  acts_as_positioned
 
   after_update :handle_is_closed_change
   before_destroy :delete_workflow_rules
diff --git a/app/models/role.rb b/app/models/role.rb
index b7d048e2a..defbc311d 100644
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -70,7 +70,7 @@ class Role < ActiveRecord::Base
 
   has_many :member_roles, :dependent => :destroy
   has_many :members, :through => :member_roles
-  acts_as_list
+  acts_as_positioned :scope => :builtin
 
   serialize :permissions, ::Role::PermissionsAttributeCoder
   attr_protected :builtin
@@ -223,10 +223,10 @@ private
   def self.find_or_create_system_role(builtin, name)
     role = where(:builtin => builtin).first
     if role.nil?
-      role = create(:name => name, :position => 0) do |r|
+      role = create(:name => name) do |r|
         r.builtin = builtin
       end
-      raise "Unable to create the #{name} role." if role.new_record?
+      raise "Unable to create the #{name} role (#{role.errors.full_messages.join(',')})." if role.new_record?
     end
     role
   end
diff --git a/app/models/tracker.rb b/app/models/tracker.rb
index dd802efdf..73cf569fc 100644
--- a/app/models/tracker.rb
+++ b/app/models/tracker.rb
@@ -34,7 +34,7 @@ class Tracker < ActiveRecord::Base
 
   has_and_belongs_to_many :projects
   has_and_belongs_to_many :custom_fields, :class_name => 'IssueCustomField', :join_table => "#{table_name_prefix}custom_fields_trackers#{table_name_suffix}", :association_foreign_key => 'custom_field_id'
-  acts_as_list
+  acts_as_positioned
 
   attr_protected :fields_bits