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

Test case for broken counter caches #124

Closed
wants to merge 1 commit into from
Closed

Test case for broken counter caches #124

wants to merge 1 commit into from

Conversation

howardr
Copy link

@howardr howardr commented Mar 13, 2014

Counter caches are not updating correctly after a child association is restored. If I try to fix this in the model by using an after_restore hook, then a different test case breaks.

# restore child and check
child.restore!
parent.reload
assert_equal 1, parent.related_models_count
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically this one is broken right now.

<1> expected but was
<0>.

@radar
Copy link
Collaborator

radar commented Jul 28, 2014

@howardr Thanks for the test to reproduce the issue. Have you had any luck with fixing it yet?

@bluej100
Copy link
Contributor

@radar : What would you think of triggering after_create as well as after_restore when restoring? I feel like it is generally equivalent to a creation. I'm seeing the same counters issue with https://github.com/magnusvk/counter_culture .

@radar
Copy link
Collaborator

radar commented Nov 21, 2014

No, I think that's a bad approach. The record is not created. It is only restored. Please investigate an alternativez

@bluej100
Copy link
Contributor

I disagree. Restoring is a type of creation, as it adds a new record
matching a previously deleted record to the default scope. It is the
opposite of destroy. It introduces a new record which matches a deleted
record. In my opinion, the fact that this gem keeps rows of deleted records
in the same table should be thought of as an implementation detail: they
could just as well be kept in parallel archive tables.

If this gem is to interoperate with others that don't have a concept of
soft deletion, I think triggering a create event is the best solution.
On Nov 20, 2014 10:50 PM, "Ryan Bigg" notifications@github.com wrote:

No, I think that's a bad approach. The record is not created. It is only
restored. Please investigate an alternativez


Reply to this email directly or view it on GitHub
#124 (comment).

@BenMorganIO
Copy link
Collaborator

I think @excid3 just fixed this. Let me know if I'm at error :)

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

Successfully merging this pull request may close these issues.

4 participants