Skip to content

Commit

Permalink
Add TODO pointers to where bugs may live
Browse files Browse the repository at this point in the history
  • Loading branch information
kbrock committed Mar 28, 2023
1 parent be23abb commit 6f4e8a5
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 0 deletions.
13 changes: 13 additions & 0 deletions lib/ancestry/instance_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,28 +75,41 @@ def touch_ancestors_callback

# Counter Cache
def increase_parent_counter_cache
# TODO: return if ancestry_callbacks_disabled?
# TODO: return unless parent_id
self.class.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
# https://github.com/rails/rails/pull/14735
# https://github.com/rails/rails/pull/27248
return if defined?(@_trigger_destroy_callback) && !@_trigger_destroy_callback
return if ancestry_callbacks_disabled?
# TODO: return unless parent_id

self.class.ancestry_base_class.decrement_counter counter_cache_column, parent_id
end

def update_parent_counter_cache
# 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.class.ancestry_column)
# TODO: return if ancestry_callbacks_disabled?

if parent_id_was = parent_id_before_last_save
self.class.ancestry_base_class.decrement_counter counter_cache_column, parent_id_was
end

# NOTE: will probably be callingincrease_parent_counter_cache instead
# make sure this will not break this PR
parent_id && increase_parent_counter_cache
end

Expand Down
3 changes: 3 additions & 0 deletions test/concerns/counter_cache_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ def test_counter_cache_when_destroying
end
end

# is this a real use case?
# test with disable callbacks?
# would like to get those rails internal variables out of this code base
def test_counter_cache_when_reduplicate_destroying
AncestryTestDatabase.with_model :depth => 2, :width => 2, :counter_cache => true do |_model, roots|
parent = roots.first.first
Expand Down
2 changes: 2 additions & 0 deletions test/concerns/sti_support_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ def test_sti_support_with_from_subclass
end
end

# TODO: add some and remove some. make sure the updates work correctly
# thinking will find an issue here
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
Expand Down

0 comments on commit 6f4e8a5

Please sign in to comment.