Improve logic around contracting lists when destroying multiple items

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## [Unreleased]
 
+- When destroying a positioned item, first move it out of the way (position = 0) then contract the scope. Do this before destruction. Moving the item out of the way memoizes its original position to cope with the case where multiple items are destroyed with `destroy_all` as they'll have their position column cached. Thanks @james-reading for the report.
+
 ## [0.2.1] - 2024-04-08
 
 - Fetch the adapter_name from #connection_db_config (@tijn)

lib/positioning.rb

@@ -51,7 +51,7 @@ def positioned(on: [], column: :position)
 
           before_create { Mechanisms.new(self, column).create_position }
           before_update { Mechanisms.new(self, column).update_position }
-          after_destroy { Mechanisms.new(self, column).destroy_position }
+          before_destroy { Mechanisms.new(self, column).destroy_position }
 
           after_commit advisory_lock
           after_rollback advisory_lock

lib/positioning/mechanisms.rb

@@ -39,7 +39,10 @@ def update_position
     end
 
     def destroy_position
-      contract(positioning_scope, (position + 1)..) unless destroyed_via_positioning_scope?
+      unless destroyed_via_positioning_scope?
+        move_out_of_the_way
+        contract(positioning_scope, (position_was + 1)..)
+      end
     end
 
     private

test/test_positioning.rb

@@ -941,6 +941,13 @@ def test_that_an_error_is_raised_with_invalid_position
       @first_item.update position: :other
     end
   end
+
+  def test_destroying_multiple_items
+    @first_list.items.limit(2).destroy_all
+    @third_item.reload
+
+    assert_equal 1, @third_item.position
+  end
 end
 
 class TestNoScopePositioning < Minitest::Test