unused tags are automatically deleted

8 files changed, 99 insertions, 76 deletions

diff --git a/askbot/__init__.py b/askbot/__init__.py
index fcedc653..125dd0be 100644
--- a/askbot/__init__.py
+++ b/askbot/__init__.py
@@ -22,7 +22,7 @@ def get_version():
     """returns version of the askbot app
     this version is meaningful for pypi only
     """
-    return '0.6.33'
+    return '0.6.34'
 
 #todo: maybe send_mail functions belong to models
 #or the future API
diff --git a/askbot/forms.py b/askbot/forms.py
index c72fb7cd..5231bd4b 100644
--- a/askbot/forms.py
+++ b/askbot/forms.py
@@ -395,7 +395,7 @@ class CloseForm(forms.Form):
     reason = forms.ChoiceField(choices=const.CLOSE_REASONS)
 
 class RetagQuestionForm(forms.Form):
-    tags   = TagNamesField()
+    tags = TagNamesField()
     # initialize the default values
     def __init__(self, question, *args, **kwargs):
         super(RetagQuestionForm, self).__init__(*args, **kwargs)
diff --git a/askbot/models/question.py b/askbot/models/question.py
index ba4237c9..2dff5694 100644
--- a/askbot/models/question.py
+++ b/askbot/models/question.py
@@ -60,7 +60,7 @@ class QuestionManager(models.Manager):
             question.wikified_at = added_at
 
         question.parse_and_save(author = author)
-        question.update_tags(tagnames, author)
+        question.update_tags(tagnames = tagnames, user = author, timestamp = added_at)
 
         question.add_revision(
             author=author,
@@ -376,35 +376,67 @@ class Question(content.Content, DeletableContent):
         others_tags = set(other_question.get_tag_names())
         return len(my_tags & others_tags)
 
-    def update_tags(self, tagnames, user):
+    def update_tags(self, tagnames = None, user = None, timestamp = None):
         """
         Updates Tag associations for a question to match the given
         tagname string.
-
-        Returns ``True`` if tag usage counts were updated as a result,
-        ``False`` otherwise.
         """
 
-        current_tags = list(self.tags.all())
-        current_tagnames = set(t.name for t in current_tags)
-        updated_tagnames = set(t for t in tagnames.split(' ') if t)
-        modified_tags = []
+        previous_tags = list(self.tags.all())
+
+        previous_tagnames = set([tag.name for tag in previous_tags])
+        updated_tagnames = set(t for t in tagnames.split(' '))
+
+        removed_tagnames = previous_tagnames - updated_tagnames
+        added_tagnames = updated_tagnames - previous_tagnames
 
-        removed_tags = [t for t in current_tags
-                        if t.name not in updated_tagnames]
-        if removed_tags:
-            modified_tags.extend(removed_tags)
+        modified_tags = list()
+        #remove tags from the question's tags many2many relation
+        if removed_tagnames:
+            removed_tags = [tag for tag in previous_tags if tag.name in removed_tagnames]
             self.tags.remove(*removed_tags)
 
-        added_tagnames = updated_tagnames - current_tagnames
+            #if any of the removed tags reached use count == 1 that means they must be deleted
+            for tag in removed_tags:
+                if tag.used_count == 1:
+                    #we won't modify used count b/c it's done below anyway
+                    removed_tags.remove(tag)
+                    tag.delete()#auto-delete tags whose use count dwindled
+
+            #remember modified tags, we'll need to update use counts on them
+            modified_tags = removed_tags
+
+        #add new tags to the relation
         if added_tagnames:
-            added_tags = Tag.objects.get_or_create_multiple(
-                                                    added_tagnames,
-                                                    user
-                                                )
-            modified_tags.extend(added_tags)
+            #find reused tags
+            reused_tags = Tag.objects.filter(name__in = added_tagnames)
+            #undelete them, because we are using them
+            reused_count = reused_tags.update(
+                                    deleted = False,
+                                    deleted_by = None,
+                                    deleted_at = None
+                                )
+            #if there are brand new tags, create them and finalize the added tag list
+            if reused_count < len(added_tagnames):
+                added_tags = list(reused_tags)
+
+                reused_tagnames = set([tag.name for tag in reused_tags])
+                new_tagnames = added_tagnames - reused_tagnames
+                for name in new_tagnames:
+                    new_tag = Tag.objects.create(
+                                            name = name,
+                                            created_by = user,
+                                            used_count = 1
+                                        )
+                    added_tags.append(new_tag)
+            else:
+                added_tags = reused_tags
+
+            #finally add tags to the relation and extend the modified list
             self.tags.add(*added_tags)
+            modified_tags.extend(added_tags)
 
+        #if there are any modified tags, update their use counts
         if modified_tags:
             Tag.objects.update_use_counts(modified_tags)
             return True
@@ -477,7 +509,7 @@ class Question(content.Content, DeletableContent):
         self.save()
 
         # Update the Question's tag associations
-        tags_updated = self.update_tags(tagnames, retagged_by)
+        self.update_tags(tagnames = tagnames, user = retagged_by, timestamp = retagged_at)
 
         # Create a new revision
         latest_revision = self.get_latest_revision()
@@ -527,7 +559,7 @@ class Question(content.Content, DeletableContent):
 
         # Update the Question tag associations
         if latest_revision.tagnames != tags:
-            tags_updated = self.update_tags(tags, edited_by)
+            self.update_tags(tagnames = tags, user = edited_by, timestamp = edited_at)
 
         # Create a new revision
         self.add_revision(
@@ -560,25 +592,25 @@ class Question(content.Content, DeletableContent):
             text       = text
         )
 
-    def save(self, **kwargs):
-        """
-        Overridden to manually manage addition of tags when the object
-        is first saved.
-
-        This is required as we're using ``tagnames`` as the sole means of
-        adding and editing tags.
-        """
-        initial_addition = (self.pk is None)
-
-        super(Question, self).save(**kwargs)
-
-        if initial_addition:
-            tags = Tag.objects.get_or_create_multiple(
-                                       self.get_tag_names(),
-                                       self.author
-                                    )
-            self.tags.add(*tags)
-            Tag.objects.update_use_counts(tags)
+#    def save(self, **kwargs):
+#        """
+#        Overridden to manually manage addition of tags when the object
+#        is first saved.
+#
+#        This is required as we're using ``tagnames`` as the sole means of
+#        adding and editing tags.
+#        """
+#        initial_addition = (self.pk is None)
+#
+#        super(Question, self).save(**kwargs)
+#
+#        if initial_addition:
+#            tags = Tag.objects.get_or_create_multiple(
+#                                       self.get_tag_names(),
+#                                       self.author
+#                                    )
+#            self.tags.add(*tags)
+#            Tag.objects.update_use_counts(tags)
 
     def get_tag_names(self):
         """Creates a list of Tag names from the ``tagnames`` attribute."""
diff --git a/askbot/models/tag.py b/askbot/models/tag.py
index dd92edba..473c3cd7 100644
--- a/askbot/models/tag.py
+++ b/askbot/models/tag.py
@@ -6,41 +6,20 @@ from askbot.models.base import DeletableContent
 
 
 class TagManager(models.Manager):
-    UPDATE_USED_COUNTS_QUERY = (
-        'UPDATE tag '
-        'SET used_count = ('
-            'SELECT COUNT(*) FROM question_tags '
-            'INNER JOIN question ON question_id=question.id '
-            'WHERE tag_id = tag.id AND NOT question.deleted'
-        ') '
-        'WHERE id IN (%s)')
+    UPDATE_USED_COUNTS_QUERY ="""
+        UPDATE tag 
+        SET used_count = (
+            SELECT COUNT(*) FROM question_tags 
+            INNER JOIN question ON question_id=question.id
+            WHERE tag_id = tag.id AND NOT question.deleted
+        ) 
+        WHERE id IN (%s);
+    """
 
     def get_valid_tags(self, page_size):
         tags = self.all().filter(deleted=False).exclude(used_count=0).order_by("-id")[:page_size]
         return tags
 
-    def get_or_create_multiple(self, names, user):
-        """
-        Fetches a list of Tags with the given names, creating any Tags
-        which don't exist when necesssary.
-        """
-        tags = list(self.filter(name__in=names))
-        #Set all these tag visible
-        for tag in tags:
-            if tag.deleted:
-                tag.deleted = False
-                tag.deleted_by = None
-                tag.deleted_at = None
-                tag.save()
-
-        if len(tags) < len(names):
-            existing_names = set(tag.name for tag in tags)
-            new_names = [name for name in names if name not in existing_names]
-            tags.extend([self.create(name=name, created_by=user)
-                         for name in new_names if self.filter(name=name).count() == 0 and len(name.strip()) > 0])
-
-        return tags
-
     def update_use_counts(self, tags):
         """Updates the given Tags with their current use counts."""
         if not tags:
@@ -74,7 +53,7 @@ class TagManager(models.Manager):
             #the big questions query to hit only the page to be displayed
             q_id_list = questions.values_list('id', flat=True)
             tags = self.filter(
-                    questions__id__in = q_id_list
+                    questions__id__in = q_id_list,
                 ).annotate(
                     local_used_count=models.Count('id')
                 ).order_by(
@@ -84,6 +63,8 @@ class TagManager(models.Manager):
         if ignored_tag_names:
             tags = tags.exclude(name__in=ignored_tag_names)
 
+        tags = tags.exclude(deleted = True)
+
         tags = tags[:50]#magic number
         if cheating:
             for tag in tags:
diff --git a/askbot/skins/default/templates/question.html b/askbot/skins/default/templates/question.html
index 8ef973f0..db1b15c1 100644
--- a/askbot/skins/default/templates/question.html
+++ b/askbot/skins/default/templates/question.html
@@ -364,7 +364,7 @@
 {% endblock %}
 
 {% block sidebar %}
-{% cache 600 "questions_tags" questions_tags question.id language_code %}
+{% cache 0 "questions_tags" questions_tags question.id language_code %}
 <div class="boxC">
     <p>
         {% trans %}Question tags{% endtrans %}:
diff --git a/askbot/skins/default/templates/questions.html b/askbot/skins/default/templates/questions.html
index a73d1a7f..323bca56 100644
--- a/askbot/skins/default/templates/questions.html
+++ b/askbot/skins/default/templates/questions.html
@@ -212,7 +212,7 @@
     {% endif %}
 
     {% if tags %}
-        {% cache 600 "tags" tags search_tags scope sort query context.page context.page_size language_code %}
+        {% cache 0 "tags" tags search_tags scope sort query context.page context.page_size language_code %}
         <div class="boxC">
             <h3 class="subtitle">{% trans %}Related tags{% endtrans %}</h3>
             <div id="related-tags" class="tags">
diff --git a/askbot/tests/db_api_tests.py b/askbot/tests/db_api_tests.py
index edd1cca9..c4c31fc4 100644
--- a/askbot/tests/db_api_tests.py
+++ b/askbot/tests/db_api_tests.py
@@ -98,3 +98,13 @@ class DBApiTests(AskbotTestCase):
         self.assertTrue(answer_count == 1)
         saved_question = models.Question.objects.get(id = self.question.id)
         self.assertTrue(saved_question.answer_count == 1)
+
+    def test_unused_tag_is_auto_deleted(self):
+        self.user.retag_question(self.question, tags = 'one-tag')
+        tag = models.Tag.objects.get(name='one-tag')
+        self.assertEquals(tag.used_count, 1)
+        self.assertEquals(tag.deleted, False)
+        self.user.retag_question(self.question, tags = 'two-tag')
+
+        count = models.Tag.objects.filter(name='one-tag').count()
+        self.assertEquals(count, 0)
diff --git a/askbot/views/readers.py b/askbot/views/readers.py
index 2191dd8b..7877cf38 100644
--- a/askbot/views/readers.py
+++ b/askbot/views/readers.py
@@ -304,7 +304,7 @@ def questions(request):
         'show_sort_by_relevance': askbot.should_show_sort_by_relevance(),
         'scope': search_state.scope,
         'context' : paginator_context,
-        })
+    })
 
     assert(request.is_ajax() == False)
     #ajax request is handled in a separate branch above