Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix increase_parent_counter_cache callback #618

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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