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

added example when removed from habtm relation #359

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

exocode
Copy link

@exocode exocode commented Nov 17, 2022

I've added rspec example for a habtm relation.

  • Given a product.create("A product", categories: [old_category])
  • When product.update(categories: [new_category1, new_category2]) is updating,
  • The old_category relation is deleted, but the counter for old_category stays untouched.

(The new_categories are counted properly!)

The other spec tests are mostly sanity checks

@exocode
Copy link
Author

exocode commented Nov 17, 2022

Regarding to this issue #358

@magnusvk
Copy link
Owner

magnusvk commented Dec 7, 2022

Ok, so I think the issue here must be that the callbacks on the has-and-belongs-to-many model aren't firing when you're updating the relation directly. Doing a little bit of googling on this, it seems like the way to do this would be to add after_remove and after_add callbacks on the relation. But then I'd imagine it getting tricky in that we'd have to make sure that only one or the other callback fires, not both, to prevent double-decrementing or -incrementing.

This unfortunately looks like it might end up being a pretty big change, so I won't have time to work on that. If you're willing to invest the time, I'd be happy to look at a PR that adds this. Alternatively, I think if you stopped using update and manipulated the has-and-belongs-to-many model directly instead, that it would end up counting correctly for you.

Thanks for opening this, it does make the problem easy to understand.

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.

2 participants