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

Don't update counters twice if non-slotted counter #10

Merged
merged 1 commit into from
Jan 17, 2023

Conversation

danielwestendorf
Copy link
Contributor

@danielwestendorf danielwestendorf commented Jan 11, 2023

What is the purpose of this pull request?

Fix compatibility with non-slotted counter cache columns

Fixes #9

What changes did you make? (overview)

Don't double increment when a counter column is not registered as a slotted counter

Is there anything you'd like reviewers to focus on?

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated a documentation

Copy link
Member

@Envek Envek left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the pull request with the fix!

You have checked “I've added tests for this change” checkbox, but I can't see any test changes in this pull request, perhaps you forgot to commit it?

And don't hesitate to add a change log entry praising you 😉

Also optional nitpick below ↓

lib/activerecord_slotted_counters/has_slotted_counter.rb Outdated Show resolved Hide resolved
@danielwestendorf
Copy link
Contributor Author

danielwestendorf commented Jan 12, 2023

Sorry, I was in a hurry and just threw the PR up quickly. I've revised.

  • Fixed test which was providing false positive results
  • Updated style as suggested
  • Added changelog

Thanks for the gem!

@Envek Envek requested a review from egor-lukin January 12, 2023 03:30
@palkan palkan merged commit 47e6bc8 into evilmartians:master Jan 17, 2023
@palkan
Copy link
Member

palkan commented Jan 17, 2023

Thanks!

Released in 0.1.1

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.

Non-slotted counters get incremented twice after including this gem
4 participants