search

mbleigh / acts-as-taggable-on

A tagging plugin for Rails applications that allows for custom tagging along dynamic contexts.
http://mbleigh.lighthouseapp.com/projects/10116-acts-as-taggable-on
MIT License
4.98k stars 1.2k forks source link

Handle RecordNotUnique error in find_or_create_all_with_like_by_name correctly #1081

Closed kuahyeow closed 1 year ago

kuahyeow commented 2 years ago

Related to https://github.com/mbleigh/acts-as-taggable-on/issues/1068, https://github.com/mbleigh/acts-as-taggable-on/issues/948

In the find_or_create_all_with_like_by_name method, instead of issuing a ROLLBACK statement, we wrap the tag creation in a transaction instead.

Issuing a ROLLBACK will break nested transactions, and also does not respect multiple databases (available since Rails 6).

Adds a spec which attempts to simulate concurrent tag creation

kuahyeow commented 2 years ago

~Note this includes a commit from https://github.com/mbleigh/acts-as-taggable-on/pull/1080~ That PR has since been merged, so rebased away

kuahyeow commented 2 years ago

This is now ready for a look !

kuahyeow commented 2 years ago

Hi @seuros - Any feedback on this PR ?

2called-chaos commented 2 years ago

Could we get the ball rolling on this please @mbleigh @seuros ? When dealing with concurrency or bulk editing this is highly annoying and frankly dangerous as the models are seemingly saved (update/update!/save/save!) when in actuality the data has been rolled back. The taggings however get created. To add to the misery the paper trail audit record gets rolled back too. Thanks

jakswa commented 1 year ago

This is what would fix our data loss I believe as well. Thank you for trying to address it. 👍

seuros commented 1 year ago

@jakswa can you try master directly ? if you confirm that it indeed resolve your issue, i will release a a new version.

I'm going to clean up the code base.

jakswa commented 1 year ago

@seuros we have been testing it today and so far so good 🤞 usually we'd have a few cases per day crop up

jakswa commented 1 year ago

Still no cases cropping back up 🎉. I'll remember to post back in here if we find any problems with this changeset, but going into the weekend feeling good! Thank you again!