diff --git a/lib/counter_culture/extensions.rb b/lib/counter_culture/extensions.rb index b354905f..96795152 100644 --- a/lib/counter_culture/extensions.rb +++ b/lib/counter_culture/extensions.rb @@ -51,6 +51,12 @@ def counter_culture(relation, options = {}) raise ":column_names must be a Hash of conditions and column names" end + if options[:execute_after_commit] + ActiveSupport::Deprecation.warn( + 'execute_after_commit is deprecated and will be removed from '\ + 'counter_culture 2.0') + end + # add the counter to our collection @after_commit_counter_cache << Counter.new(self, relation, options) end @@ -89,61 +95,42 @@ def counter_culture_fix_counts(options = {}) end private - # need to make sure counter_culture is only activated once - # per commit; otherwise, if we do an update in an after_create, - # we would be triggered twice within the same transaction -- once - # for the create, once for the update - def _wrap_in_counter_culture_active(&block) - if @_counter_culture_active - # don't do anything; we are already active for this transaction - else - block.call - execute_after_commit { @_counter_culture_active = false} - end - end - # called by after_create callback def _update_counts_after_create - _wrap_in_counter_culture_active do - @_counter_culture_active = true - self.class.after_commit_counter_cache.each do |counter| - # increment counter cache - counter.change_counter_cache(self, :increment => true) - end + @_counter_culture_active = true + self.class.after_commit_counter_cache.each do |counter| + # increment counter cache + counter.change_counter_cache(self, :increment => true) end end # called by after_destroy callback def _update_counts_after_destroy - _wrap_in_counter_culture_active do - @_counter_culture_active = true - self.class.after_commit_counter_cache.each do |counter| - # decrement counter cache - counter.change_counter_cache(self, :increment => false) - end + @_counter_culture_active = true + self.class.after_commit_counter_cache.each do |counter| + # decrement counter cache + counter.change_counter_cache(self, :increment => false) end end # called by after_update callback def _update_counts_after_update - _wrap_in_counter_culture_active do - self.class.after_commit_counter_cache.each do |counter| - # figure out whether the applicable counter cache changed (this can happen - # with dynamic column names) - counter_cache_name_was = counter.counter_cache_name_for(counter.previous_model(self)) - counter_cache_name = counter.counter_cache_name_for(self) - - if counter.first_level_relation_changed?(self) || - (counter.delta_column && counter.attribute_changed?(self, counter.delta_column)) || - counter_cache_name != counter_cache_name_was - - @_counter_culture_active = true - - # increment the counter cache of the new value - counter.change_counter_cache(self, :increment => true, :counter_column => counter_cache_name) - # decrement the counter cache of the old value - counter.change_counter_cache(self, :increment => false, :was => true, :counter_column => counter_cache_name_was) - end + self.class.after_commit_counter_cache.each do |counter| + # figure out whether the applicable counter cache changed (this can happen + # with dynamic column names) + counter_cache_name_was = counter.counter_cache_name_for(counter.previous_model(self)) + counter_cache_name = counter.counter_cache_name_for(self) + + if counter.first_level_relation_changed?(self) || + (counter.delta_column && counter.attribute_changed?(self, counter.delta_column)) || + counter_cache_name != counter_cache_name_was + + @_counter_culture_active = true + + # increment the counter cache of the new value + counter.change_counter_cache(self, :increment => true, :counter_column => counter_cache_name) + # decrement the counter cache of the old value + counter.change_counter_cache(self, :increment => false, :was => true, :counter_column => counter_cache_name_was) end end end diff --git a/spec/counter_culture_spec.rb b/spec/counter_culture_spec.rb index 7ccc2ca0..97be1b52 100644 --- a/spec/counter_culture_spec.rb +++ b/spec/counter_culture_spec.rb @@ -148,6 +148,34 @@ expect(user2.reload.review_approvals_count).to eq(69) end + it "works with multiple saves in one transcation" do + user = User.create + product = Product.create + + expect(user.reviews_count).to eq(0) + expect(user.review_approvals_count).to eq(0) + + Review.transaction do + review1 = Review.create!(user_id: user.id, product_id: product.id, approvals: 0) + + user.reload + expect(user.reviews_count).to eq(1) + expect(user.review_approvals_count).to eq(0) + + review1.update_attributes!(approvals: 42) + + user.reload + expect(user.reviews_count).to eq(1) + expect(user.review_approvals_count).to eq(42) + + review2 = Review.create!(user_id: user.id, product_id: product.id, approvals: 1) + + user.reload + expect(user.reviews_count).to eq(2) + expect(user.review_approvals_count).to eq(43) + end + end + it "treats null delta column values as 0" do user = User.create product = Product.create