magnusvk/counter_culture

Not updating dynamic weight when attribute is changed

Opened this issue · 5 comments

I have the following setup:

class SalesAd
  has_one :district, through: :wine
  counter_culture :district, column_name: :active_sales_ads_count, delta_magnitude: proc { |model| model.state == "active" ? 1 : 0 }
end

I am then speccing it like this:

sales_ad
expect(district.reload.active_sales_ads_count).to eq 0
sales_ad.update!(state: "active")
expect(district.reload.active_sales_ads_count).to eq 1

Which fails with expected: 1 got: 0.

I would expect counter culture to notice the change and change the weight from 0 to 1 and add the change in weight to the active_sales_ads_count column on the district.

Did you tried to check it out in rails console?
In tests it might be because transaction fixtures are used. Try to disable transaction fixtures:

class DistrictTest < ActiveSupport::TestCase
  self.use_transactional_fixtures = false
  ...
end

Looking at the code, it looks like you're right: This doesn't currently work. There's no way for counter_culture to know which column to watch for changes as you're just passing in a proc. We'd have to add another option to specify the columns to watch, I think. I don't use this feature, so I don't think I'll have the time to work on fixing this. I'd love to see a PR though!

That being said, what you're trying to achieve does work with conditional counter caches, as documented here: https://github.com/magnusvk/counter_culture#conditional-counter-cache

@magnusvk Do you think the following could work?

  1. Hook into before_save on: :update and calculate the delta_magnitude
  2. Hook into after_save on: :update and calculate the delta_magnitude again.
  3. If there is a difference in the two, then add the difference to the column.

Then we could avoid forcing the user to have to define the columns. On the other hand it would require a bit more work if the delta_magnitude-proc did something heavy...

The value would already be changed on before_save, but you're right, counter_culture already has a way of giving you the model in its previous state, so we could calculate the delta_magnitude before and after and see if it changed.

See this for something similar.

@magnusvk, please use canonical links when you pasting links to the source code :)

image