Skip to content

Fix counter_cache double increment bug#24713

Merged
sgrif merged 1 commit intorails:masterfrom
tomkadwill:fix_counter_cache_increment
May 1, 2016
Merged

Fix counter_cache double increment bug#24713
sgrif merged 1 commit intorails:masterfrom
tomkadwill:fix_counter_cache_increment

Conversation

@tomkadwill
Copy link
Copy Markdown
Contributor

Summary

Fixes counter cache bug, reported in #24183. Counter cache is incrementing twice when replacing the association.

Other Information

@rails-bot
Copy link
Copy Markdown

r? @rafaelfranca

(@rails-bot has picked a reviewer for you, use r? to override)

Copy link
Copy Markdown
Member

@eileencodes eileencodes Apr 24, 2016

Choose a reason for hiding this comment

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

While we don't accept PR's to change hash syntax only, new tests and code should use the new hash syntax Topic.create(title: 'Zoom-zoom-zoom)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks @eileencodes, I've updated the PR to fix the hash syntax

@tomkadwill
Copy link
Copy Markdown
Contributor Author

My PR is failing on Travis but it looks like Rails is currently red, is that correct? The failures seem unrelated to my change. Sorry if this is a silly question, I am a new contributor :). cc/ @eileencodes @vipulnsward @rafaelfranca

@sgrif sgrif merged commit a4b3c78 into rails:master May 1, 2016
@sgrif
Copy link
Copy Markdown
Contributor

sgrif commented May 1, 2016

Please don't explicitly ping contributors who aren't assigned without specific reason in the future

@adambedford
Copy link
Copy Markdown

What version of Rails would this have been fixed in? I'm running 5.0.0.1 and I'm running into either this issue or a very similar one

@redtachyons
Copy link
Copy Markdown

redtachyons commented Sep 19, 2017

@adambedford As you can see here, this fix is present from 5.0 onwards. So bug you are facing may be different

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants