insert_at respects unique not null check (>= 0) db constraints

lib/acts_as_list/active_record/acts/list.rb

@@ -317,32 +317,53 @@ def increment_positions_on_all_items
         # Reorders intermediate items to support moving an item from old_position to new_position.
         def shuffle_positions_on_intermediate_items(old_position, new_position, avoid_id = nil)
           return if old_position == new_position
-          scope = acts_as_list_list
+          scope = acts_as_list_list.select(:id)
 
           if avoid_id
             scope = scope.where("#{quoted_table_name}.#{self.class.primary_key} != ?", self.class.connection.quote(avoid_id))
           end
 
+          # unique constraint prevents regular increment_all and forces to do increments one by one
+          # http://stackoverflow.com/questions/7703196/sqlite-increment-unique-integer-field
+          # both SQLite and PostgreSQL (and most probably MySQL too) has same issue
+          update_one_by_one = acts_as_list_list.connection.index_exists?(acts_as_list_list.table_name, position_column, unique: true)
+
           if old_position < new_position
             # Decrement position of intermediate items
             #
             # e.g., if moving an item from 2 to 5,
             # move [3, 4, 5] to [2, 3, 4]
-            scope.where(
+            items = scope.where(
               "#{quoted_position_column_with_table_name} > ?", old_position
             ).where(
               "#{quoted_position_column_with_table_name} <= ?", new_position
-            ).decrement_all
+            )
+
+            if update_one_by_one
+              items.order("#{quoted_position_column_with_table_name} ASC").pluck(:id).each do |id|
+                acts_as_list_list.find(id).decrement!(position_column)
+              end
+            else
+              items.decrement_all
+            end
           else
             # Increment position of intermediate items
             #
             # e.g., if moving an item from 5 to 2,
             # move [2, 3, 4] to [3, 4, 5]
-            scope.where(
+            items = scope.where(
               "#{quoted_position_column_with_table_name} >= ?", new_position
             ).where(
               "#{quoted_position_column_with_table_name} < ?", old_position
-            ).increment_all
+            )
+
+            if update_one_by_one
+              items.order("#{quoted_position_column_with_table_name} DESC").pluck(:id).each do |id|
+                acts_as_list_list.find(id).increment!(position_column)
+              end
+            else
+              items.increment_all
+            end
           end
         end
 
@@ -352,23 +373,19 @@ def insert_at_position(position)
             if in_list?
               old_position = send(position_column).to_i
               return if position == old_position
-              shuffle_positions_on_intermediate_items(old_position, position)
+              # temporary move after bottom with gap, avoiding duplicate values
+              # gap is required to leave room for position increments
+              # positive number will be valid with unique not null check (>= 0) db constraint
+              tmp_position_after_everything = acts_as_list_class.unscoped.maximum(position_column).to_i + 2
+              set_list_position(tmp_position_after_everything)
+              shuffle_positions_on_intermediate_items(old_position, position, id)
             else
               increment_positions_on_lower_items(position)
             end
             set_list_position(position)
           end
         end
 
-        # used by insert_at_position instead of remove_from_list, as postgresql raises error if position_column has non-null constraint
-        def store_at_0
-          if in_list?
-            old_position = send(position_column).to_i
-            set_list_position(0)
-            decrement_positions_on_lower_items(old_position)
-          end
-        end
-
         def update_positions
           old_position = send("#{position_column}_was") || bottom_position_in_list + 1
           new_position = send(position_column).to_i

test/test_list.rb

@@ -6,9 +6,12 @@
 ActiveRecord::Schema.verbose = false
 
 def setup_db(position_options = {})
+  # sqlite cannot drop/rename/alter columns and add constraints after table creation
+  sqlite = ENV.fetch("DB", "sqlite") == "sqlite"
+
   # AR caches columns options like defaults etc. Clear them!
   ActiveRecord::Base.connection.create_table :mixins do |t|
-    t.column :pos, :integer, position_options
+    t.column :pos, :integer, position_options unless position_options[:positive] && sqlite
     t.column :active, :boolean, default: true
     t.column :parent_id, :integer
     t.column :parent_type, :string
@@ -17,6 +20,20 @@ def setup_db(position_options = {})
     t.column :state, :integer
   end
 
+  if position_options[:unique] && !(sqlite && position_options[:positive])
+    ActiveRecord::Base.connection.add_index :mixins, :pos, unique: true
+  end
+
+  if position_options[:positive]
+    if sqlite
+      # SQLite cannot add constraint after table creation, also cannot add unique inside ADD COLUMN
+      ActiveRecord::Base.connection.execute('ALTER TABLE mixins ADD COLUMN pos integer8 NOT NULL CHECK (pos > 0) DEFAULT 1')
+      ActiveRecord::Base.connection.execute('CREATE UNIQUE INDEX index_mixins_on_pos ON mixins(pos)')
+    else
+      ActiveRecord::Base.connection.execute('ALTER TABLE mixins ADD CONSTRAINT pos_check CHECK (pos > 0)')
+    end
+  end
+
   # This table is used to test table names and column names quoting
   ActiveRecord::Base.connection.create_table 'table-name' do |t|
     t.column :order, :integer
@@ -784,3 +801,24 @@ def test_nil_position_ordering
     assert_equal [2, 3, 1], DefaultScopedMixin.all.map(&:id)
   end
 end
+
+class DefaultScopedNotNullUniquePositiveConstraintsTest < ActsAsListTestCase
+  def setup
+    setup_db null: false, unique: true, positive: true
+    (1..4).each { |counter| DefaultScopedMixin.create!({pos: counter}) }
+  end
+
+  def test_insert_at
+    new = DefaultScopedMixin.create
+    assert_equal 5, new.pos
+
+    new.insert_at(1)
+    assert_equal 1, new.pos
+
+    new.insert_at(5)
+    assert_equal 5, new.pos
+
+    new.insert_at(3)
+    assert_equal 3, new.pos
+  end
+end