Merge pull request #27 from LukasSkywalker/main

Fix healer when model uses an ordering default scope

Author: brendon

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## [Unreleased]
 
+- Fix healing a list with a default scope `:order` and/or `:select`. Thanks @LukasSkywalker!
+
 ## [0.4.4] - 2024-11-20
 
 - Add `funding_uri` to gemspec.

lib/positioning/healer.rb

@@ -8,7 +8,7 @@ def initialize(model, column, order)
 
     def heal
       if scope_columns.present?
-        @model.select(*scope_columns).distinct.each do |scope_record|
+        @model.unscope(:order).reselect(*scope_columns).distinct.each do |scope_record|
           @model.transaction do
             if scope_associations.present?
               scope_associations.each do |scope_association|
@@ -40,7 +40,7 @@ def scope_associations
     end
 
     def sequence(scope)
-      scope.reorder(@order).each.with_index(1) do |record, index|
+      scope.unscope(:select).reorder(@order).each.with_index(1) do |record, index|
         record.update_columns @column => index
       end
     end

test/models/default_scope_item.rb

@@ -0,0 +1,7 @@
+class DefaultScopeItem < ActiveRecord::Base
+  belongs_to :list
+
+  positioned on: :list
+
+  default_scope -> { select(:name, :position).order(:position) }
+end

test/models/list.rb

@@ -1,6 +1,7 @@
 class List < ActiveRecord::Base
   has_many :items, -> { order(:position) }, dependent: :destroy
   has_many :new_items, -> { order(:position) }, dependent: :destroy
+  has_many :default_scope_items, -> { order(:position) }, dependent: :destroy
   has_many :composite_primary_key_items, -> { order(:position) }, dependent: :destroy
   has_many :authors, -> { order(:position) }, dependent: :destroy
 end

test/support/active_record.rb

@@ -36,6 +36,14 @@
       t.references :list, null: false
     end
 
+    create_table :default_scope_items, force: true do |t|
+      t.string :name
+      t.integer :position, null: false
+      t.references :list, null: false
+    end
+
+    add_index :default_scope_items, [:list_id, :position], unique: true
+
     create_table :composite_primary_key_items, primary_key: [:item_id, :account_id], force: true do |t|
       t.integer :item_id, null: false
       t.integer :account_id, null: false

test/test_positioning.rb

@@ -3,6 +3,7 @@
 require_relative "models/list"
 require_relative "models/item"
 require_relative "models/new_item"
+require_relative "models/default_scope_item"
 require_relative "models/composite_primary_key_item"
 require_relative "models/category"
 require_relative "models/categorised_item"
@@ -1760,4 +1761,20 @@ def test_heal_position_with_no_scope
 
     assert_equal [1, 2, 3], [third_category.reload, second_category.reload, first_category.reload].map(&:position)
   end
+
+  def test_heal_position_with_default_scope
+    first_list = List.create name: "First List"
+
+    first_item = first_list.default_scope_items.create name: "First Item"
+    second_item = first_list.default_scope_items.create name: "Second Item"
+    third_item = first_list.default_scope_items.create name: "Third Item"
+
+    first_item.update_columns position: 10
+    second_item.update_columns position: 15
+    third_item.update_columns position: 5
+
+    DefaultScopeItem.heal_position_column!
+
+    assert_equal [1, 2, 3], [third_item.reload, first_item.reload, second_item.reload].map(&:position)
+  end
 end