diff --git a/taggit/managers.py b/taggit/managers.py index 85542c90..19b6cfb1 100644 --- a/taggit/managers.py +++ b/taggit/managers.py @@ -66,8 +66,12 @@ def __init__(self, through, model, instance, prefetch_cache_name, ordering=None) self.model = model self.instance = instance self.prefetch_cache_name = prefetch_cache_name - if ordering: + if ordering is not None: self.ordering = ordering + elif instance: + # 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"] else: self.ordering = [] @@ -76,13 +80,15 @@ def is_cached(self, instance): def get_queryset(self, extra_filters=None): try: - return self.instance._prefetched_objects_cache[self.prefetch_cache_name] + qs = self.instance._prefetched_objects_cache[self.prefetch_cache_name] except (AttributeError, KeyError): kwargs = extra_filters if extra_filters else {} - return self.through.tags_for(self.model, self.instance, **kwargs).order_by( + qs = self.through.tags_for(self.model, self.instance, **kwargs).order_by( *self.ordering ) + return qs + def get_prefetch_queryset(self, instances, queryset=None): if queryset is not None: raise ValueError("Custom queryset can't be used for this lookup.") @@ -196,18 +202,53 @@ 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 case_insensitive: + if t.name.lower() not in processed_tags: + tag_objs.append(t) + processed_tags.append(t.name.lower()) + else: + if t.name not in processed_tags: + tag_objs.append(t) + processed_tags.append(t.name) elif isinstance(t, str): - str_tags.add(t) + if case_insensitive: + if t.lower() not in processed_tags: + 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) + finally: + processed_tags.append(t.lower()) + else: + if t not in processed_tags: + try: + tag = manager.get(name=t, **tag_kwargs) + tag_objs.append(tag) + except self.through.tag_model().DoesNotExist: + lookup = {"name": t, **tag_kwargs} + tag, created = manager.get_or_create( + **lookup, defaults={"name": t} + ) + tag_objs.append(tag) + finally: + processed_tags.append(t) else: raise ValueError( "Cannot add {} ({}). Expected {} or str.".format( @@ -215,39 +256,6 @@ def _to_tag_model_instances(self, tags, tag_kwargs): ) ) - 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 diff --git a/tests/test_models.py b/tests/test_models.py index 33d44a53..2228f1ee 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,8 +1,39 @@ 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) + self.assertEqual(str_tags, [tag.name for tag in sample_obj.tags.all()]) + + 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] + self.assertEqual(results, [tag.name for tag in sample_obj.tags.all()]) + + def test_duplicates(self): + sample_obj = TestModel.objects.create() + sample_obj.tags.set(["green", "green"]) + desired_result = ["green"] + self.assertEqual(desired_result, [tag.name for tag in sample_obj.tags.all()]) + + class TestSlugification(TestCase): def test_unicode_slugs(self): """