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

Polymorphic with optimistic lock and counter cache should be destroyed #2955

Merged
merged 6 commits into from
Jan 3, 2012
Merged

Conversation

dmitry
Copy link
Contributor

@dmitry dmitry commented Sep 9, 2011

Polymorphic record with optimistic locking and counter cache should be destroyed without catching the ActiveRecord::StaleObjectError. It works fine with has_many, but not works with polymorphic has_many. It also might not work with has_one.

Issue #2954

…uld be destoyed without catching the ActiveRecord::StaleObjectError.
@dmitry
Copy link
Contributor Author

dmitry commented Sep 10, 2011

Anyone know how is better to fix it?

This code should be removed, because it's not safe:

                # No point in executing the counter update since we're going to destroy the parent anyway
                counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym
                if(o.respond_to? counter_method) then
                  class << o
                    self
                  end.send(:define_method, counter_method, Proc.new {})
                end

@dmitry
Copy link
Contributor Author

dmitry commented Dec 31, 2011

Please include this fix. Thank you!

@dmitry
Copy link
Contributor Author

dmitry commented Dec 31, 2011

@tenderlove Hope you can look into this.

tenderlove added a commit that referenced this pull request Jan 3, 2012
Polymorphic with optimistic lock and counter cache should be destroyed
@tenderlove tenderlove merged commit 7985f64 into rails:master Jan 3, 2012
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.

None yet

2 participants