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

Not updating dynamic weight when attribute is changed #195

Open
ghost opened this issue Sep 13, 2017 · 5 comments
Open

Not updating dynamic weight when attribute is changed #195

ghost opened this issue Sep 13, 2017 · 5 comments

Comments

@ghost
Copy link

ghost commented Sep 13, 2017

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.

@mojobiri
Copy link

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

@magnusvk
Copy link
Owner

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

@kaspernj
Copy link

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

@magnusvk
Copy link
Owner

magnusvk commented Sep 15, 2017

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.

@milushov

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants