diff --git a/CHANGELOG.md b/CHANGELOG.md index 49b83b75..8de6c854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ As such, _Breaking Changes_ are major. _Features_ would map to either major or m * Reduce packaged gem size * Fixes * [@iainbeeston Fix mismatched indentations warning message](https://github.com/mbleigh/acts-as-taggable-on/pull/1150) + * [@james-reading Fix issue when reordering tags and @@remove_unused_tags is true](https://github.com/mbleigh/acts-as-taggable-on/pull/1154) ### [v12.0.0) / 2024-11-09](https://github.com/mbleigh/acts-as-taggable-on/compare/v11.0.0...v12.0.0) diff --git a/lib/acts-as-taggable-on/taggable/core.rb b/lib/acts-as-taggable-on/taggable/core.rb index f3451616..8b897d9e 100644 --- a/lib/acts-as-taggable-on/taggable/core.rb +++ b/lib/acts-as-taggable-on/taggable/core.rb @@ -272,15 +272,17 @@ def save_tags # Delete discarded tags and create new tags end - # Destroy old taggings: - taggings.not_owned.by_context(context).where(tag_id: old_tags).destroy_all if old_tags.present? - - # Create new taggings: - new_tags.each do |tag| - if taggable_tenant - taggings.create!(tag_id: tag.id, context: context.to_s, taggable: self, tenant: taggable_tenant) - else - taggings.create!(tag_id: tag.id, context: context.to_s, taggable: self) + transaction do + # Destroy old taggings: + taggings.not_owned.by_context(context).where(tag_id: old_tags).destroy_all if old_tags.present? + + # Create new taggings: + new_tags.each do |tag| + if taggable_tenant + taggings.create!(tag_id: tag.id, context: context.to_s, taggable: self, tenant: taggable_tenant) + else + taggings.create!(tag_id: tag.id, context: context.to_s, taggable: self) + end end end end diff --git a/lib/acts-as-taggable-on/tagging.rb b/lib/acts-as-taggable-on/tagging.rb index e8e062dd..50188385 100644 --- a/lib/acts-as-taggable-on/tagging.rb +++ b/lib/acts-as-taggable-on/tagging.rb @@ -23,7 +23,7 @@ class Tagging < ActsAsTaggableOn.base_class.constantize # :nodoc: validates_uniqueness_of :tag_id, scope: %i[taggable_type taggable_id context tagger_id tagger_type] - after_destroy :remove_unused_tags + after_destroy_commit :remove_unused_tags private diff --git a/spec/acts_as_taggable_on/taggable_spec.rb b/spec/acts_as_taggable_on/taggable_spec.rb index a708df75..d054cf1f 100644 --- a/spec/acts_as_taggable_on/taggable_spec.rb +++ b/spec/acts_as_taggable_on/taggable_spec.rb @@ -92,6 +92,23 @@ ids = @taggable.tags.map { |t| t.taggings.first.id } expect(ids).to eq(ids.sort) end + + it 'can reorder tags successfully when @@remove_unused_tags is true' do + previous_setting = ActsAsTaggableOn.remove_unused_tags + ActsAsTaggableOn.remove_unused_tags = true + + @taggable.tag_list = 'pow, ruby, rails' + @taggable.save! + @taggable.reload + expect(@taggable.tag_list).to eq(%w(pow ruby rails)) + + @taggable.tag_list = 'rails, pow, ruby' + @taggable.save! + @taggable.reload + expect(@taggable.tag_list).to eq(%w(rails pow ruby)) + ensure + ActsAsTaggableOn.remove_unused_tags = previous_setting + end end RSpec.describe 'Taggable' do