move sti counter test to the counter concern

Trying to track down some bugs around counters

lib/ancestry/instance_methods.rb

@@ -62,10 +62,12 @@ def touch_ancestors_callback
 
     # Counter Cache
     def increase_parent_counter_cache
+      # TODO: return if ancestry_callbacks_disabled?
       self.ancestry_base_class.increment_counter _counter_cache_column, parent_id
     end
 
     def decrease_parent_counter_cache
+      # TODO: ?
       # @_trigger_destroy_callback comes from activerecord, which makes sure only once decrement when concurrent deletion.
       # but @_trigger_destroy_callback began after [email protected].
       # https://github.com/rails/rails/blob/v5.2.0/activerecord/lib/active_record/persistence.rb#L340
@@ -78,14 +80,19 @@ def decrease_parent_counter_cache
     end
 
     def update_parent_counter_cache
-      changed = saved_change_to_attribute?(self.ancestry_base_class.ancestry_column)
-
-      return unless changed
+      # TODO:
+      # # @_trigger_update_callback comes from activerecord, which makes sure only once decrement when concurrent deletion.
+      # # but @_trigger_update_callback began after [email protected].
+      # # https://github.com/rails/rails/blob/v5.2.0/activerecord/lib/active_record/persistence.rb#L340
+      # # https://github.com/rails/rails/pull/27248
+      # return if defined?(@_trigger_update_callback) && !@_trigger_update_callback
+      return unless saved_change_to_attribute?(self.ancestry_base_class.ancestry_column)
 
       if parent_id_was = parent_id_before_last_save
         self.ancestry_base_class.decrement_counter _counter_cache_column, parent_id_was
       end
 
+      # TODO: use increase_parent_counter_cache instead?
       parent_id && self.ancestry_base_class.increment_counter(_counter_cache_column, parent_id)
     end
 

test/concerns/counter_cache_test.rb

@@ -19,6 +19,8 @@ def test_counter_cache_when_destroying
     end
   end
 
+  # is this a real use case?
+  # maybe test this with
   def test_counter_cache_when_reduplicate_destroying
     AncestryTestDatabase.with_model :depth => 2, :width => 2, :counter_cache => true do |_model, roots|
       parent = roots.first.first
@@ -86,4 +88,30 @@ def test_counter_cache_when_updating_record
       end
     end
   end
+
+  def test_counter_cache_on_sti
+    AncestryTestDatabase.with_model :counter_cache => true, :extra_columns => {:type => :string} do |model|
+      # NOTE rails does not properly remove classes from cache
+      #      So these names need to be unique across the suite
+      # TODO: Object.const_set "#{model.name}Subclass1", Class.new(model)
+
+      Object.const_set 'Subclass3', Class.new(model)
+      Object.const_set 'Subclass4', Class.new(model)
+
+      node1 = Subclass3.create!
+      node2 = Subclass4.create! :parent => node1
+      node3 = Subclass3.create! :parent => node1
+      node4 = Subclass4.create! :parent => node3
+      node5 = Subclass3.create! :parent => node4
+
+      assert_equal 2, node1.reload.children_count
+      assert_nil node2.reload.children_count
+      assert_equal 1, node3.reload.children_count
+      assert_equal 1, node4.reload.children_count
+      assert_nil node5.reload.children_count
+
+      Object.send :remove_const, 'Subclass3'
+      Object.send :remove_const, 'Subclass4'
+    end
+  end
 end

test/concerns/sti_support_test.rb

@@ -26,39 +26,18 @@ def test_sti_support
     end
   end
 
+  # Note: this is defining ancestry on the subclass
+  # But I don't think this does what people want
+  # The parent will still have ancestry, but pointing to the child class
+  # finders/counter caches may not find the parent class properly
   def test_sti_support_with_from_subclass
     AncestryTestDatabase.with_model :extra_columns => {:type => :string} do |model|
       subclass1 = Object.const_set 'SubclassWithAncestry', Class.new(model)
       subclass1.has_ancestry
+      # this was throwing an exception
       subclass1.create!
 
       Object.send :remove_const, 'SubclassWithAncestry'
     end
   end
-
-  def test_sti_support_for_counter_cache
-    AncestryTestDatabase.with_model :counter_cache => true, :extra_columns => {:type => :string} do |model|
-      # NOTE had to use subclasses other than Subclass1/Subclass2 from above
-      # due to (I think) Rails caching those STI classes and that not getting
-      # reset between specs
-
-      Object.const_set 'Subclass3', Class.new(model)
-      Object.const_set 'Subclass4', Class.new(model)
-
-      node1 = Subclass3.create!
-      node2 = Subclass4.create! :parent => node1
-      node3 = Subclass3.create! :parent => node1
-      node4 = Subclass4.create! :parent => node3
-      node5 = Subclass3.create! :parent => node4
-
-      assert_equal 2, node1.reload.children_count
-      assert_nil node2.reload.children_count
-      assert_equal 1, node3.reload.children_count
-      assert_equal 1, node4.reload.children_count
-      assert_nil node5.reload.children_count
-
-      Object.send :remove_const, 'Subclass3'
-      Object.send :remove_const, 'Subclass4'
-    end
-  end
 end