counter_culture icon indicating copy to clipboard operation
counter_culture copied to clipboard

Not updating dynamic weight when attribute is changed

Open ghost opened this issue 7 years ago • 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.

ghost avatar Sep 13 '17 18:09 ghost

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

mojobiri avatar Sep 14 '17 11:09 mojobiri

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 avatar Sep 14 '17 16:09 magnusvk

@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...

kaspernj avatar Sep 15 '17 11:09 kaspernj

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 avatar Sep 15 '17 13:09 magnusvk

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

image

milushov avatar Feb 26 '18 03:02 milushov