fixing merge conflicts

CHANGELOG.rst

@@ -8,10 +8,11 @@ Changelog
   The previous behavior for this was that by default tag items were not ordered. In practice tag items often end up ordered by creation date anyways, just due to how databases work, but this was not a guarantee.
   If you wish to have the old behavior, set ``ordering=[]`` to your ``TaggableManager`` instance.
   We believe that this should not cause a noticable performance change, and the number of queries involved should not change.
+* Added the ability to merge tags via the admin
+* Added functionality to remove orphaned tags via management command
 * Add Django 5.0 support (no code changes were needed, but now we test this release).
 * Add Python 3.12 support
-* Added functionality for tag merging
-* Added functionality to remove orphaned tags
+* Add support for dumpdata/loaddata using natural keys
 
 5.0.1 (2023-10-26)
 ~~~~~~~~~~~~~~~~~~

docs/admin.rst

@@ -42,10 +42,9 @@ method to the :class:`~django.contrib.admin.ModelAdmin`, using
 
 
 Merging tags in the admin
-=========================
+~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Functionality has been added to the admin app to allow for tag "merging".
-Really what is happening is a "find and replace" where the selected tags are being used.
+Functionality has been added to the admin app to allow for tag "merging". Multiple tags can be selected, and all of their usages will be replaced by the tag that you choose to use.
 
 To merge your tags follow these steps:
 

docs/index.rst

@@ -13,6 +13,7 @@ tagging to your project easy and fun.
    forms
    admin
    serializers
+   testing
    api
    faq
    custom_tagging

docs/testing.rst

@@ -0,0 +1,14 @@
+Testing
+=======
+
+Natural Key Support
+-------------------
+We have added `natural key support <https://docs.djangoproject.com/en/5.0/topics/serialization/#natural-keys>`_ to the Tag model in the Django taggit library. This allows you to identify objects by human-readable identifiers rather than by their database ID::
+
+    python manage.py dumpdata taggit.Tag --natural-foreign --natural-primary > tags.json
+
+    python manage.py loaddata tags.json
+
+By default tags use the name field as the natural key.
+
+You can customize this in your own custom tag model by setting the ``natural_key_fields`` property on your model the required fields.

taggit/admin.py

@@ -27,7 +27,7 @@ def get_urls(self):
             path(
                 "merge-tags/",
                 self.admin_site.admin_view(self.merge_tags_view),
-                name="merge_tags",
+                name="taggit_tag_merge_tags",
             ),
         ]
         return custom_urls + urls
@@ -58,9 +58,18 @@ def merge_tags_view(self, request):
                         tag = Tag.objects.get(id=tag_id)
                         tagged_items = TaggedItem.objects.filter(tag=tag)
                         for tagged_item in tagged_items:
-                            tagged_item.tag = new_tag
-                            TaggedItem.objects.filter(tag=tag).update(tag=new_tag)
-                        # tag.delete()  #this will delete the selected tags after merge...leaving out for now
+                            if TaggedItem.objects.filter(
+                                tag=new_tag,
+                                content_type=tagged_item.content_type,
+                                object_id=tagged_item.object_id,
+                            ).exists():
+                                # we have the new tag as well, so we can just
+                                # remove the tag association
+                                tagged_item.delete()
+                            else:
+                                # point this taggedItem to the new one
+                                tagged_item.tag = new_tag
+                                tagged_item.save()
 
                 self.message_user(request, "Tags have been merged", level="success")
                 # clear the selected_tag_ids from session after merge is complete

taggit/models.py

@@ -15,7 +15,25 @@ def unidecode(tag):
         return tag
 
 
-class TagBase(models.Model):
+class NaturalKeyManager(models.Manager):
+    def get_by_natural_key(self, *args):
+        if len(args) != len(self.model.natural_key_fields):
+            raise ValueError(
+                "Number of arguments does not match number of natural key fields."
+            )
+        lookup_kwargs = dict(zip(self.model.natural_key_fields, args))
+        return self.get(**lookup_kwargs)
+
+
+class NaturalKeyModel(models.Model):
+    def natural_key(self):
+        return [getattr(self, field) for field in self.natural_key_fields]
+
+    class Meta:
+        abstract = True
+
+
+class TagBase(NaturalKeyModel):
     name = models.CharField(
         verbose_name=pgettext_lazy("A tag name", "name"), unique=True, max_length=100
     )
@@ -26,6 +44,9 @@ class TagBase(models.Model):
         allow_unicode=True,
     )
 
+    natural_key_fields = ["name"]
+    objects = NaturalKeyManager()
+
     def __str__(self):
         return self.name
 
@@ -91,13 +112,15 @@ class Meta:
         app_label = "taggit"
 
 
-class ItemBase(models.Model):
+class ItemBase(NaturalKeyModel):
     def __str__(self):
         return gettext("%(object)s tagged with %(tag)s") % {
             "object": self.content_object,
             "tag": self.tag,
         }
 
+    objects = NaturalKeyManager()
+
     class Meta:
         abstract = True
 
@@ -170,13 +193,15 @@ def tags_for(cls, model, instance=None, **extra_filters):
 
 class GenericTaggedItemBase(CommonGenericTaggedItemBase):
     object_id = models.IntegerField(verbose_name=_("object ID"), db_index=True)
+    natural_key_fields = ["object_id"]
 
     class Meta:
         abstract = True
 
 
 class GenericUUIDTaggedItemBase(CommonGenericTaggedItemBase):
     object_id = models.UUIDField(verbose_name=_("object ID"), db_index=True)
+    natural_key_fields = ["object_id"]
 
     class Meta:
         abstract = True

taggit/templates/admin/taggit/merge_tags_form.html

@@ -6,7 +6,7 @@
         <form
           id="merge-tags-form"
           method="post"
-          action="{% url 'admin:merge_tags' %}"
+          action="{% url 'admin:taggit_tag_merge_tags' %}"
         >
           {% csrf_token %} {% for field in form %}
           <div>

tests/test_admin.py

@@ -2,6 +2,8 @@
 from django.test import TestCase
 from django.urls import reverse
 
+from taggit.models import Tag
+
 from .models import Food
 
 
@@ -10,6 +12,11 @@ def setUp(self):
         super().setUp()
         self.apple = Food.objects.create(name="apple")
         self.apple.tags.add("Red", "red")
+        self.pear = Food.objects.create(name="pear")
+        self.pear.tags.add("red", "RED")
+        self.peach = Food.objects.create(name="peach")
+        self.peach.tags.add("red", "Yellow")
+
         user = User.objects.create_superuser(
             username="admin", email="[email protected]", password="password"
         )
@@ -40,3 +47,33 @@ def test_get_change(self):
             reverse("admin:tests_food_change", args=(self.apple.pk,))
         )
         self.assertEqual(response.status_code, 200)
+
+    def test_tag_merging(self):
+        response = self.client.get(reverse("admin:taggit_tag_changelist"))
+
+        # merging red and RED into Red
+        pks_to_select = [Tag.objects.get(name="red").pk, Tag.objects.get(name="RED").pk]
+        response = self.client.post(
+            reverse("admin:taggit_tag_changelist"),
+            data={"action": "render_tag_form", "_selected_action": pks_to_select},
+        )
+        # we're redirecting
+        self.assertEqual(response.status_code, 302)
+        # make sure what we expected got into the session keys
+        assert "selected_tag_ids" in self.client.session.keys()
+        self.assertEqual(
+            self.client.session["selected_tag_ids"], ",".join(map(str, pks_to_select))
+        )
+
+        # let's do the actual merge operation
+        response = self.client.post(
+            reverse("admin:taggit_tag_merge_tags"), {"new_tag_name": "Red"}
+        )
+        self.assertEqual(response.status_code, 302)
+
+        # time to check the result of the merges
+        self.assertSetEqual({tag.name for tag in self.apple.tags.all()}, {"Red"})
+        self.assertSetEqual({tag.name for tag in self.pear.tags.all()}, {"Red"})
+        self.assertSetEqual(
+            {tag.name for tag in self.peach.tags.all()}, {"Yellow", "Red"}
+        )

tests/test_remove_orphaned_tags.py

@@ -0,0 +1,58 @@
+from django.core.management import call_command
+from django.test import TestCase
+
+from taggit.models import Tag
+from tests.models import Food, HousePet
+
+
+class RemoveOrphanedTagsTests(TestCase):
+    def setUp(self):
+        # Create some tags, some orphaned and some not
+        self.orphan_tag1 = Tag.objects.create(name="Orphan1")
+        self.orphan_tag2 = Tag.objects.create(name="Orphan2")
+        self.used_tag = Tag.objects.create(name="Used")
+
+        # Create instances of Food and HousePet and tag them
+        self.food_item = Food.objects.create(name="Apple")
+        self.pet_item = HousePet.objects.create(name="Fido")
+
+        self.food_item.tags.add(self.used_tag)
+        self.pet_item.tags.add(self.used_tag)
+
+    def test_remove_orphaned_tags(self):
+        # Ensure the setup is correct
+        self.assertEqual(Tag.objects.count(), 3)
+        self.assertEqual(Tag.objects.filter(taggit_taggeditem_items=None).count(), 2)
+
+        # Call the management command to remove orphaned tags
+        call_command("remove_orphaned_tags")
+
+        # Check the count of tags after running the command
+        self.assertEqual(Tag.objects.count(), 1)
+        self.assertEqual(Tag.objects.filter(taggit_taggeditem_items=None).count(), 0)
+
+        # Ensure that the used tag still exists
+        self.assertTrue(Tag.objects.filter(name="Used").exists())
+        self.assertFalse(Tag.objects.filter(name="Orphan1").exists())
+        self.assertFalse(Tag.objects.filter(name="Orphan2").exists())
+
+    def test_no_orphaned_tags(self):
+        # Ensure the setup is correct
+        self.assertEqual(Tag.objects.count(), 3)
+        self.assertEqual(Tag.objects.filter(taggit_taggeditem_items=None).count(), 2)
+
+        # Add used_tag to food_item to make no tags orphaned
+        self.food_item.tags.add(self.orphan_tag1)
+        self.food_item.tags.add(self.orphan_tag2)
+
+        # Call the management command to remove orphaned tags
+        call_command("remove_orphaned_tags")
+
+        # Check the count of tags after running the command
+        self.assertEqual(Tag.objects.count(), 3)
+        self.assertEqual(Tag.objects.filter(taggit_taggeditem_items=None).count(), 0)
+
+        # Ensure all tags still exist
+        self.assertTrue(Tag.objects.filter(name="Used").exists())
+        self.assertTrue(Tag.objects.filter(name="Orphan1").exists())
+        self.assertTrue(Tag.objects.filter(name="Orphan2").exists())

tests/tests.py

@@ -1,3 +1,4 @@
+import os
 from io import StringIO
 from unittest import mock
 
@@ -1398,3 +1399,111 @@ def test_tests_have_no_pending_migrations(self):
         out = StringIO()
         call_command("makemigrations", "tests", dry_run=True, stdout=out)
         self.assertEqual(out.getvalue().strip(), "No changes detected in app 'tests'")
+
+
+class NaturalKeyTests(TestCase):
+    @classmethod
+    def setUpTestData(cls):
+        cls.tag_names = ["circle", "square", "triangle", "rectangle", "pentagon"]
+        cls.filename = "test_data_dump.json"
+        cls.tag_count = len(cls.tag_names)
+
+    def setUp(self):
+        self.tags = self._create_tags()
+
+    def tearDown(self):
+        self._clear_existing_tags()
+        try:
+            os.remove(self.filename)
+        except FileNotFoundError:
+            pass
+
+    @property
+    def _queryset(self):
+        return Tag.objects.filter(name__in=self.tag_names)
+
+    def _create_tags(self):
+        return Tag.objects.bulk_create(
+            [Tag(name=shape, slug=shape) for shape in self.tag_names],
+            ignore_conflicts=True,
+        )
+
+    def _clear_existing_tags(self):
+        self._queryset.delete()
+
+    def _dump_model(self, model):
+        model_label = model._meta.label
+        with open(self.filename, "w") as f:
+            call_command(
+                "dumpdata",
+                model_label,
+                natural_primary=True,
+                use_natural_foreign_keys=True,
+                stdout=f,
+            )
+
+    def _load_model(self):
+        call_command("loaddata", self.filename)
+
+    def test_tag_natural_key(self):
+        """
+        Test that tags can be dumped and loaded using natural keys.
+        """
+
+        # confirm count in the DB
+        self.assertEqual(self._queryset.count(), self.tag_count)
+
+        # dump all tags to a file
+        self._dump_model(Tag)
+
+        # Delete all tags
+        self._clear_existing_tags()
+
+        # confirm all tags clear
+        self.assertEqual(self._queryset.count(), 0)
+
+        # load the tags from the file
+        self._load_model()
+
+        # confirm count in the DB
+        self.assertEqual(self._queryset.count(), self.tag_count)
+
+    def test_tag_reloading_with_changed_pk(self):
+        """Test that tags are not reliant on the primary key of the tag model.
+
+        Test that data is correctly loaded after database state has changed.
+
+        """
+        original_shape = self._queryset.first()
+        original_pk = original_shape.pk
+        original_shape_name = original_shape.name
+        new_shape_name = "hexagon"
+
+        # dump all tags to a file
+        self._dump_model(Tag)
+
+        # Delete the tag
+        self._clear_existing_tags()
+
+        # create new tag with the same PK
+        Tag.objects.create(name=new_shape_name, slug=new_shape_name, pk=original_pk)
+
+        # Load the tags from the file
+        self._load_model()
+
+        # confirm that load did not overwrite the new_shape
+        self.assertEqual(Tag.objects.get(pk=original_pk).name, new_shape_name)
+
+        # confirm that the original shape was reloaded with a different PK
+        self.assertNotEqual(Tag.objects.get(name=original_shape_name).pk, original_pk)
+
+    def test_get_by_natural_key(self):
+        # Test retrieval of tags by their natural key
+        for name in self.tag_names:
+            tag = Tag.objects.get_by_natural_key(name)
+            self.assertEqual(tag.name, name)
+
+    def test_wrong_number_of_args(self):
+        # Test that get_by_natural_key raises an error when the wrong number of args is passed
+        with self.assertRaises(ValueError):
+            Tag.objects.get_by_natural_key()