Fix issue when reordering tags and @@remove_unused_tags is true

[james-reading]

Fixes #946

When reordering taggings, the taggings get deleted and recreated. However, by the time the taggings are recreated, the tags have already been removed.

Fixed by wrapping the delete + create in a transaction, and only removing unused tags on transaction commit.

[seuros]

Can you rebase on master ? i'm hesitant to merge it as people using pgbouncer, could have transaction disabled.

[Copilot]

Pull Request Overview

This PR fixes an issue where reordering tags fails when remove_unused_tags is enabled. The fix ensures that tag deletion and creation operations happen atomically within a transaction, and that the after_destroy callback is changed to after_destroy_commit to prevent premature tag cleanup.

• Changed after_destroy to after_destroy_commit in the Tagging model to delay tag cleanup until transaction commits
• Wrapped tag destroy/create operations in a transaction to ensure atomicity
• Added test coverage for reordering tags with remove_unused_tags enabled

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
lib/acts-as-taggable-on/tagging.rb	Changed callback from after_destroy to after_destroy_commit to prevent tags from being removed before transaction commits
lib/acts-as-taggable-on/taggable/core.rb	Wrapped tag destruction and creation in a transaction block for atomicity
spec/acts_as_taggable_on/taggable_spec.rb	Added test case to verify tags can be reordered when remove_unused_tags is enabled
CHANGELOG.md	Added entry documenting the bug fix

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The comment incorrectly uses @@remove_unused_tags (class variable notation) when it should reference the configuration setting remove_unused_tags. The double @@ is misleading as this is an ActsAsTaggableOn configuration attribute, not a class variable.

Suggested change

	it 'can reorder tags successfully when @@remove_unused_tags is true' do
	it 'can reorder tags successfully when remove_unused_tags is true' do

[james-reading]

@seuros I've rebased.

I guess a fix that didn't rely on transactions would involve creating the new taggings before destroying the old taggings. I could put up a separate PR for that?

[seuros]

Active Record allow you to check if the connection support transactions.

Can you check if the versions are raising or just ignoring it.

PS: if only 7.2 is problematic, we can drop support. But if they all raise , we will need to find a proper solution.

PGbouncer disable transaction by default if i recall correctly.