Merge pull request #42 from pyromaniac/fix-nil-scope

Fix optional scope healing

Author: brendon

.gitignore

@@ -8,3 +8,5 @@
 /tmp/
 /db/
 Gemfile.lock
+.ruby-version
+.ruby-gemset

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## [Unreleased]
 
+- Fix healing for positioning with nullable scope (parent in trees, for example). Thanks @pyromaniac!
+
 ## [0.4.5] - 2024-12-04
 
 - Fix healing a list with a default scope `:order` and/or `:select`. Thanks @LukasSkywalker!

lib/positioning/healer.rb

@@ -12,7 +12,7 @@ def heal
           @model.transaction do
             if scope_associations.present?
               scope_associations.each do |scope_association|
-                scope_record.send(scope_association).lock!
+                scope_record.send(scope_association)&.lock!
               end
             else
               @model.where(scope_record.slice(*scope_columns)).lock!

test/models/category.rb

@@ -1,7 +1,5 @@
 class Category < ActiveRecord::Base
-  has_many :categorised_items, dependent: :destroy
+  belongs_to :parent, class_name: "Category", optional: true
 
-  positioned
-
-  default_scope { order(:position) }
+  positioned on: :parent
 end

test/models/product.rb

@@ -0,0 +1,5 @@
+class Product < ActiveRecord::Base
+  positioned
+
+  default_scope { order(:position) }
+end

test/support/active_record.rb

@@ -1,3 +1,4 @@
+require "logger"
 require "active_record"
 
 ENV["DB"] = "mysql" unless ENV["DB"]
@@ -57,9 +58,17 @@
     create_table :categories, force: true do |t|
       t.string :name
       t.integer :position, null: false
+      t.references :parent
     end
 
-    add_index :categories, :position, unique: true
+    add_index :categories, [:parent_id, :position], unique: true
+
+    create_table :products, force: true do |t|
+      t.string :name
+      t.integer :position, null: false
+    end
+
+    add_index :products, :position, unique: true
 
     create_table :categorised_items, force: true do |t|
       t.string :name

test/test_healing.rb

@@ -0,0 +1,99 @@
+require "test_helper"
+
+class TestHealing < Minitest::Test
+  include Minitest::Hooks
+
+  def around
+    ActiveRecord::Base.transaction do
+      super
+      raise ActiveRecord::Rollback
+    end
+  end
+
+  def test_heal_position
+    first_list = List.create name: "First List"
+    second_list = List.create name: "Second List"
+
+    first_item = first_list.new_items.create name: "First Item"
+    second_item = first_list.new_items.create name: "Second Item"
+    third_item = first_list.new_items.create name: "Third Item"
+
+    fourth_item = second_list.new_items.create name: "Fourth Item"
+    fifth_item = second_list.new_items.create name: "Fifth Item"
+    sixth_item = second_list.new_items.create name: "Sixth Item"
+
+    first_item.update_columns position: 9
+    second_item.update_columns position: nil
+    third_item.update_columns position: -42
+
+    fourth_item.update_columns position: 0
+    fifth_item.update_columns position: 998
+    sixth_item.update_columns position: 800
+
+    NewItem.heal_position_column!
+
+    if ENV["DB"] == "postgresql"
+      assert_equal [1, 2, 3], [third_item.reload, first_item.reload, second_item.reload].map(&:position)
+    else
+      assert_equal [1, 2, 3], [second_item.reload, third_item.reload, first_item.reload].map(&:position)
+    end
+
+    assert_equal [1, 2, 3], [fourth_item.reload, sixth_item.reload, fifth_item.reload].map(&:position)
+
+    NewItem.heal_position_column! name: :desc
+
+    assert_equal [1, 2, 3], [third_item.reload, second_item.reload, first_item.reload].map(&:position)
+    assert_equal [1, 2, 3], [sixth_item.reload, fourth_item.reload, fifth_item.reload].map(&:position)
+  end
+
+  def test_heal_position_on_a_tree
+    first_category = Category.create name: "First Category"
+    second_category = Category.create name: "Second Category"
+    third_category = Category.create name: "Third Category", parent: first_category
+    fourth_category = Category.create name: "Fourth Category", parent: second_category
+    fifth_category = Category.create name: "Fifth Category", parent: second_category
+    sixth_category = Category.create name: "Sixth Category", parent: second_category
+
+    first_category.update_columns position: 9
+    second_category.update_columns position: 0
+    third_category.update_columns position: -42
+    fourth_category.update_columns position: 998
+    fifth_category.update_columns position: 800
+    sixth_category.update_columns position: 1000
+
+    Category.heal_position_column!
+
+    assert_equal [1, 2, 1], [second_category.reload, first_category.reload, third_category.reload].map(&:position)
+    assert_equal [1, 2, 3], [fifth_category.reload, fourth_category.reload, sixth_category.reload].map(&:position)
+  end
+
+  def test_heal_position_with_no_scope
+    first_product = Product.create name: "First Product"
+    second_product = Product.create name: "Second Product"
+    third_product = Product.create name: "Third Product"
+
+    first_product.update_columns position: 9
+    second_product.update_columns position: 0
+    third_product.update_columns position: -42
+
+    Product.heal_position_column!
+
+    assert_equal [1, 2, 3], [third_product.reload, second_product.reload, first_product.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

test/test_helper.rb

@@ -5,3 +5,18 @@
 require "minitest/hooks/test"
 require "minitest/autorun"
 require "mocha/minitest"
+
+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/product"
+require_relative "models/category"
+require_relative "models/categorised_item"
+require_relative "models/author"
+require_relative "models/author/student"
+require_relative "models/author/teacher"
+require_relative "models/blog"
+require_relative "models/post"
+require_relative "models/entity"