reset_counters() breaks with multiple belongs_to having same foreign keys #5200

Closed
Pliny opened this Issue Feb 28, 2012 · 10 comments

Comments

Projects
None yet
4 participants
Contributor

Pliny commented Feb 28, 2012

Given this code:

class Friendship < ActiveRecord::Base
  belongs_to :friend, class_name: 'User'
  belongs_to :follower, foreign_key: 'friend_id', class_name: 'User', counter_cache: :followers_count
end
class User < ActiveRecord::Base
  has_many :followers, foreign_key: 'friend_id', class_name: 'Friendship'
end

User.reset_counters(id, :follower) crashes

Contributor

isaacsanders commented May 5, 2012

Is this still an issue?

Contributor

Pliny commented May 5, 2012

Yes, issue still exists with the latest master.

Contributor

isaacsanders commented May 7, 2012

It looks like you have done a lot of work on this, do you want to make a pull request?

Contributor

Pliny commented May 7, 2012

I did request one.. but in a different thread. #5210. It's been a few months though.

Owner

rafaelfranca commented May 7, 2012

@Pliny we will take a look in the pull request. Thanks

Contributor

Pliny commented May 10, 2012

Thanks @rafaelfranca. Please let me know if anything else is needed from me.

Contributor

isaacsanders commented May 12, 2012

Can we close this now that the PR is getting love?

Owner

rafaelfranca commented May 12, 2012

Lets wait the pull request getting merged. This issue was referenced in the pull request so went they will merged I will close this one.

@isaacsanders thank you for your help with the issues triage.

Contributor

isaacsanders commented May 12, 2012

No problem!

Member

schneems commented Aug 6, 2012

Can we get some more eyes on #5210? It needs to be merged or closed along with this issue.

@Pliny Pliny added a commit to Pliny/rails that referenced this issue Aug 7, 2012

@Pliny Pliny This closes issue #5200. reset_counters() was crashing when there wer…
…e multiple belongs_to associations with the same foreign key.
5717654

@Pliny Pliny added a commit to Pliny/rails that referenced this issue Aug 7, 2012

@Pliny Pliny Added required test files for #5200 0ce9f2d

@rafaelfranca rafaelfranca added a commit that referenced this issue Aug 21, 2012

@rafaelfranca rafaelfranca Merge pull request #5210 from Pliny/masteri
Fix for #5200

Conflicts:
	activerecord/CHANGELOG.md
	activerecord/lib/active_record/counter_cache.rb
648c5a1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment