Fix counter cache column name if :inverse_of is specified on belongs_to #4631
Conversation
|
This seems good to me but I am a little concerned about breaking existing code where people are relying on the current logic. @tenderlove wdyt? |
|
Seems fine for master. I don't think we should backport to 3-2-stable though. |
|
Alrighty. @jm could you add something to the CHANGELOG please? |
|
Crap I didn't even see these comments until right now. I'll add something. Sorry about that. |
|
@jm hey, would you mind adding that Changelog entry, so we can get this merged in? Thanks! |
| @@ -391,6 +391,20 @@ def test_custom_counter_cache | |||
| reply[:replies_count] = 17 | |||
| assert_equal 17, reply.replies.size | |||
| end | |||
|
|
|||
| def test_counter_cache_with_class_name | |||
There was a problem hiding this comment.
I think the test could be test_counter_cache_with_inverse_of, wdyt?
There was a problem hiding this comment.
Agreed. That's a crappily named test. :)
|
@jm any chance you could update that test name and add a CHANGELOG entry so we can merge this? |
|
Oh, and removing the merge commit would be good too. |
|
Feeling a little out place, but trying to help out the community at the same time. I came across this PR from http://issuetriage.herokuapp.com/, and just adding a comment to bring it up again. @jm have you lost the energy behind this change? Should the PR be closed? |
|
Yes, with two months and no answer, I'm giving it a close. @jm, you know how to re-open a new pull request if you think this is still a needed change. |
This gist explains what problem I'm fixing: https://gist.github.com/1665503
This patch adds a test and a small amount of code to fix the issue. Sorry for the stupid merging master commit. I can't seem to figure out how to get it to behave and don't have time to wrestle with git to fix it. :)