Merge pull request #4103 from jwhitlock/taggit_ci_1331643

bug 1331643, 1330357: Avoid duplicate tags with TAGGIT_CASE_INSENSITIVE=True

kuma/core/managers.py

@@ -51,22 +51,9 @@ def all_ns(self, namespace=None):
 
         if namespace is not None:
             # Namespace requested, so generate filtered set
-            # TODO: Do this in the DB query? Might not be worth it.
-            #
-            # On databases with case-insensitive collation, we can end
-            # up with duplicate tags (the same tag, differing only by
-            # case, like 'javascript' and 'JavaScript') in some
-            # cases. The most common instance of this is user
-            # tags, which are coerced to lowercase on save to avoid
-            # the problem, but because there are a large number of
-            # these duplicates already existing, we do a quick filter
-            # here to ensure we don't return a bunch of dupes that
-            # differ only by case.
-            seen = []
             results = []
             for tag in tags:
-                if tag.name.startswith(namespace) and tag.name.lower() not in seen:
-                    seen.append(tag.name.lower())
+                if tag.name.startswith(namespace):
                     results.append(tag)
             return results
 

kuma/core/tests/test_taggit_extras.py

@@ -1,8 +1,12 @@
 from django.test import TestCase
 
+from taggit.models import Tag
+import pytest
+
 from .taggit_extras.models import Food
 
 
+@pytest.mark.tags
 class NamespacedTaggableManagerTest(TestCase):
     food_model = Food
 
@@ -74,3 +78,19 @@ def test_add_ns(self):
         apple.tags.add_ns('a:', *tags)
 
         self.assert_tags_equal(apple.tags.all(), ['a:%s' % t for t in tags])
+
+    def test_duplicate_names_to_create(self):
+        apple = self.food_model.objects.create(name="apple")
+        tags = ['tasty', 'Tasty']
+        apple.tags.add_ns('a:', *tags)
+        assert apple.tags.count() == 1
+        tag = apple.tags.get()
+        assert tag.name in ('a:tasty', 'a:Tasty')
+
+    def test_duplicate_names_existing(self):
+        apple = self.food_model.objects.create(name="apple")
+        Tag.objects.create(name='a:Red')
+        Tag.objects.create(name='a:Tasty')
+        tags = ['tasty', 'Tasty', 'Red', 'red']
+        apple.tags.add_ns('a:', *tags)
+        self.assert_tags_equal(apple.tags.all(), ['a:Tasty', 'a:Red'])

kuma/settings/common.py

@@ -1449,3 +1449,6 @@ def get_user_url(user):
 # Tell django-recaptcha we want to use "No CAPTCHA".
 # Note: The API keys are located in Django constance.
 NOCAPTCHA = True  # Note: Using No Captcha implies SSL.
+
+# Tell django-taggit to use case-insensitive search for existing tags
+TAGGIT_CASE_INSENSITIVE = True

kuma/static/js/users.js

@@ -87,8 +87,7 @@
     $(document).ready(function(){
 
         // Convert interests text field into a tag-it widget
-        // also lowercase to deal with database weirdness
-        var interests = $('#id_user-interests').val().toLowerCase();
+        var interests = $('#id_user-interests').val();
         $('#id_user-interests').hide()
             .val(interests)
             .after('<ul id="tagit-interests"></ul>')
@@ -108,9 +107,8 @@
         $('#tagit-interests .tagit-new input').attr('placeholder', gettext('New interest...'));
 
         // Convert the expertise text field into tag list
-        // lowercase to deal with database weirdness
         // checkboxes sync'd to interests
-        var expertise = $('#id_user-expertise').val().toLowerCase();
+        var expertise = $('#id_user-expertise').val();
         $('#id_user-expertise').hide()
         .val(expertise)
         .after('<ul id="tags-expertise" class="tags"></ul>');

kuma/users/views.py

@@ -429,7 +429,7 @@ def user_edit(request, username):
             # Update tags from form fields
             for field, tag_ns in field_to_tag_ns:
                 field_value = user_form.cleaned_data.get(field, '')
-                tags = [tag.lower() for tag in parse_tags(field_value)]
+                tags = parse_tags(field_value)
                 new_user.tags.set_ns(tag_ns, *tags)
 
             return redirect(edit_user)

kuma/wiki/tests/test_models.py

@@ -25,7 +25,8 @@
 from ..events import EditDocumentInTreeEvent
 from ..exceptions import (DocumentRenderedContentNotAvailable,
                           DocumentRenderingInProgress, PageMoveError)
-from ..models import Document, Revision, RevisionIP, TaggedDocument
+from ..models import (Document, DocumentTag, Revision, RevisionIP,
+                      TaggedDocument)
 from ..templatetags.jinja_helpers import absolutify
 from ..utils import tidy_content
 from ..signals import render_done
@@ -470,10 +471,10 @@ def test_parent_trees_watched_by(self):
         assert 2 == len(grandchild_doc.parent_trees_watched_by(testuser2))
 
 
+@pytest.mark.tags
 class TaggedDocumentTests(UserTestCase):
     """Tests for tags in Documents and Revisions"""
 
-    @pytest.mark.tags
     def test_revision_tags(self):
         """Change tags on Document by creating Revisions"""
         rev = revision(is_approved=True, save=True, content='Sample document')
@@ -495,6 +496,23 @@ def test_revision_tags(self):
         eq_(0, Document.objects.filter(tags__name='foo').count())
         eq_(1, Document.objects.filter(tags__name='alpha').count())
 
+    def test_duplicate_tags_with_creation(self):
+        rev = revision(
+            is_approved=True, save=True, content='Sample document',
+            tags="test Test")
+        assert rev.document.tags.count() == 1
+        tag = rev.document.tags.get()
+        assert tag.name in ('test', 'Test')
+
+    def test_duplicate_tags_with_existing(self):
+        dt = DocumentTag.objects.create(name='Test')
+        rev = revision(
+            is_approved=True, save=True, content='Sample document',
+            tags="test Test")
+        assert rev.document.tags.count() == 1
+        tag = rev.document.tags.get()
+        assert tag == dt
+
 
 class RevisionTests(UserTestCase):
     """Tests for the Revision model"""

requirements/default.txt

@@ -126,9 +126,9 @@ django-sundial==1.0.2 \
     --hash=sha256:3745891337d726041b37aed95005b9c70377116d232157726e48b46aefef1961
 
 # Simple tagging support
-django-taggit==0.18.0 \
-    --hash=sha256:7010a1f7597d954aaf8b8174563f2e3455520db949d9c5d360dd9e4f906dd32a \
-    --hash=sha256:4ddd61928a4fc6ba7ccb1145d8d3a6d5c2829de8d8738eea49924ade8969f0ce
+django-taggit==0.21.6 \
+    --hash=sha256:5d0a2d7c3933badb36bcbe1cdee62514fbc8949cd2b92ed1c351b724282318fd \
+    --hash=sha256:80e22fdf41fba66a3c6f4807e283f38c4557838e35766a942370fb90f21d7435
 
 # Send async emails in response to events
 django-tidings==1.1 \