Have tags default to being in creation order

taggit/managers.py

@@ -69,7 +69,9 @@ def __init__(self, through, model, instance, prefetch_cache_name, ordering=None)
         if ordering:
             self.ordering = ordering
         else:
-            self.ordering = []
+            # default to ordering by the pk of the through table entry
+            related_name = self.through.tag.field.related_query_name()
+            self.ordering = [f"{related_name}__pk"]
 
     def is_cached(self, instance):
         return self.prefetch_cache_name in instance._prefetched_objects_cache
@@ -196,58 +198,49 @@ def add(self, *tags, through_defaults=None, tag_kwargs=None, **kwargs):
     def _to_tag_model_instances(self, tags, tag_kwargs):
         """
         Takes an iterable containing either strings, tag objects, or a mixture
-        of both and returns set of tag objects.
+        of both and returns a list of tag objects while preserving order.
         """
         db = router.db_for_write(self.through, instance=self.instance)
 
-        str_tags = set()
-        tag_objs = set()
+        processed_tags = []
+        tag_objs = []
+
+        case_insensitive = getattr(settings, "TAGGIT_CASE_INSENSITIVE", False)
+        manager = self.through.tag_model()._default_manager.using(db)
 
         for t in tags:
             if isinstance(t, self.through.tag_model()):
-                tag_objs.add(t)
+                if t not in tag_objs:
+                    tag_objs.append(t)
             elif isinstance(t, str):
-                str_tags.add(t)
+                if t not in processed_tags:
+                    processed_tags.append(t)
+                    if case_insensitive:
+                        try:
+                            tag = manager.get(name__iexact=t, **tag_kwargs)
+                            tag_objs.append(tag)
+                        except self.through.tag_model().DoesNotExist:
+                            lookup = {"name__iexact": t, **tag_kwargs}
+                            tag, created = manager.get_or_create(
+                                **lookup, defaults={"name": t}
+                            )
+                            tag_objs.append(tag)
+                    else:
+                        try:
+                            tag = manager.get(name=t, **tag_kwargs)
+                            tag_objs.append(tag)
+                        except self.through.tag_model().DoesNotExist:
+                            tag, created = manager.get_or_create(
+                                name=t, defaults={"name": t}
+                            )
+                            tag_objs.append(tag)
             else:
                 raise ValueError(
                     "Cannot add {} ({}). Expected {} or str.".format(
                         t, type(t), type(self.through.tag_model())
                     )
                 )
 
-        case_insensitive = getattr(settings, "TAGGIT_CASE_INSENSITIVE", False)
-        manager = self.through.tag_model()._default_manager.using(db)
-
-        if case_insensitive:
-            # Some databases can do case-insensitive comparison with IN, which
-            # would be faster, but we can't rely on it or easily detect it.
-            existing = []
-            tags_to_create = []
-
-            for name in str_tags:
-                try:
-                    tag = manager.get(name__iexact=name, **tag_kwargs)
-                    existing.append(tag)
-                except self.through.tag_model().DoesNotExist:
-                    tags_to_create.append(name)
-        else:
-            # If str_tags has 0 elements Django actually optimizes that to not
-            # do a query.  Malcolm is very smart.
-            existing = manager.filter(name__in=str_tags, **tag_kwargs)
-
-            tags_to_create = str_tags - {t.name for t in existing}
-
-        tag_objs.update(existing)
-
-        for new_tag in tags_to_create:
-            if case_insensitive:
-                lookup = {"name__iexact": new_tag, **tag_kwargs}
-            else:
-                lookup = {"name": new_tag, **tag_kwargs}
-
-            tag, create = manager.get_or_create(**lookup, defaults={"name": new_tag})
-            tag_objs.add(tag)
-
         return tag_objs
 
     @require_instance_manager

tests/test_models.py

@@ -1,8 +1,35 @@
 from django.test import TestCase, override_settings
 
+from taggit.models import Tag
 from tests.models import TestModel
 
 
+class TestTaggableManager(TestCase):
+    def test_set_ordering(self):
+        """
+        Test that the tags are set and returned exactly
+        """
+        str_tags = ["red", "green", "delicious"]
+        sample_obj = TestModel.objects.create()
+
+        sample_obj.tags.set(str_tags)
+        for idx, tag in enumerate(sample_obj.tags.all()):
+            self.assertEqual(tag.name, str_tags[idx])
+
+    def test_set_mixed(self):
+        """
+        Test with mixed str and obj tags
+        """
+        tag_obj = Tag.objects.create(name="mcintosh")
+        str_tags = ["red", "green", "delicious"]
+        sample_obj = TestModel.objects.create()
+
+        sample_obj.tags.set(str_tags + [tag_obj])
+        results = str_tags + [tag_obj.name]
+        for idx, tag in enumerate(sample_obj.tags.all()):
+            self.assertEqual(tag.name, results[idx])
+
+
 class TestSlugification(TestCase):
     def test_unicode_slugs(self):
         """