fixed error in related tag counts, found by Dario

6 files changed, 162 insertions, 82 deletions

diff --git a/askbot/models/question.py b/askbot/models/question.py
index 5f914bd5..63c924fd 100644
--- a/askbot/models/question.py
+++ b/askbot/models/question.py
@@ -70,19 +70,31 @@ class QuestionManager(models.Manager):
         return question
 
     def run_advanced_search(
-                            self,
-                            request_user = None,
-                            scope_selector = const.DEFAULT_POST_SCOPE,#unanswered/all/favorite (for logged in)
-                            search_query = None,
-                            tag_selector = None,
-                            author_selector = None,#???question or answer author or just contributor
-                            sort_method = const.DEFAULT_POST_SORT_METHOD
-                            ):
+                        self,
+                        request_user = None,
+                        search_state = None
+                    ):
         """all parameters are guaranteed to be clean
         however may not relate to database - in that case
         a relvant filter will be silently dropped
         """
 
+        scope_selector = getattr(
+                            search_state,
+                            'scope',
+                            const.DEFAULT_POST_SCOPE
+                        )
+
+        search_query = search_state.query
+        tag_selector = search_state.tags
+        author_selector = search_state.author
+
+        sort_method = getattr(
+                            search_state, 
+                            'sort',
+                            const.DEFAULT_POST_SORT_METHOD
+                        )
+
         qs = self.filter(deleted=False)#todo - add a possibility to see deleted questions
 
         #return metadata
@@ -125,6 +137,7 @@ class QuestionManager(models.Manager):
         #user contributed questions & answers
         if author_selector:
             try:
+                #todo maybe support selection by multiple authors
                 u = User.objects.get(id=int(author_selector))
                 qs = qs.filter(
                             models.Q(author=u, deleted=False) \
@@ -135,46 +148,63 @@ class QuestionManager(models.Manager):
                 meta_data['author_name'] = None
 
         #get users tag filters
+        ignored_tag_names = None
         if request_user and request_user.is_authenticated():
             uid_str = str(request_user.id)
             #mark questions tagged with interesting tags
-            qs = qs.extra(
-                select = SortedDict([
-                    (
-                        'interesting_score', 
-                        'SELECT COUNT(1) FROM askbot_markedtag, question_tags '
-                         + 'WHERE askbot_markedtag.user_id = %s '
-                         + 'AND askbot_markedtag.tag_id = question_tags.tag_id '
-                         + 'AND askbot_markedtag.reason = \'good\' '
-                         + 'AND question_tags.question_id = question.id'
-                    ),
-                        ]),
-                select_params = (uid_str,),
-             )
-            if request_user.hide_ignored_questions:
-                #exclude ignored tags if the user wants to
-                ignored_tags = Tag.objects.filter(user_selections__reason='bad',
-                                                user_selections__user = request_user)
-                qs = qs.exclude(tags__in=ignored_tags)
-            else:
-                #annotate questions tagged with ignored tags
-                qs = qs.extra(
-                            select = SortedDict([
-                                (
-                                    'ignored_score', 
-                                    'SELECT COUNT(1) FROM askbot_markedtag, question_tags '
-                                      + 'WHERE askbot_markedtag.user_id = %s '
-                                      + 'AND askbot_markedtag.tag_id = question_tags.tag_id '
-                                      + 'AND askbot_markedtag.reason = \'bad\' '
-                                      + 'AND question_tags.question_id = question.id'
+            #a kind of fancy annotation, would be nice to avoid it
+            interesting_tags = Tag.objects.filter(
+                                    user_selections__user=request_user,
+                                    user_selections__reason='good'
+                                )
+            ignored_tags = Tag.objects.filter(
+                                    user_selections__user=request_user,
+                                    user_selections__reason='bad'
                                 )
-                                    ]),
-                            select_params = (uid_str, )
-                         )
+
+            meta_data['interesting_tag_names'] = [tag.name for tag in interesting_tags]
+
+            ignored_tag_names = [tag.name for tag in ignored_tags]
+            meta_data['ignored_tag_names'] = ignored_tag_names
+
+            if interesting_tags:
+                #expensive query
+                qs = qs.extra(
+                    select = SortedDict([
+                        (
+                            'interesting_score', 
+                            'SELECT COUNT(1) FROM askbot_markedtag, question_tags '
+                             + 'WHERE askbot_markedtag.user_id = %s '
+                             + 'AND askbot_markedtag.tag_id = question_tags.tag_id '
+                             + 'AND askbot_markedtag.reason = \'good\' '
+                             + 'AND question_tags.question_id = question.id'
+                        ),
+                            ]),
+                    select_params = (uid_str,),
+                 )
             # get the list of interesting and ignored tags (interesting_tag_names, ignored_tag_names) = (None, None)
-            pt = MarkedTag.objects.filter(user=request_user)
-            meta_data['interesting_tag_names'] = pt.filter(reason='good').values_list('tag__name', flat=True)
-            meta_data['ignored_tag_names'] = pt.filter(reason='bad').values_list('tag__name', flat=True)
+
+            if ignored_tags:
+                if request_user.hide_ignored_questions:
+                    #exclude ignored tags if the user wants to
+                    qs = qs.exclude(tags__in=ignored_tags)
+                else:
+                    #annotate questions tagged with ignored tags
+                    #expensive query
+                    qs = qs.extra(
+                        select = SortedDict([
+                            (
+                                'ignored_score', 
+                                'SELECT COUNT(1) '
+                                  + 'FROM askbot_markedtag, question_tags '
+                                  + 'WHERE askbot_markedtag.user_id = %s '
+                                  + 'AND askbot_markedtag.tag_id = question_tags.tag_id '
+                                  + 'AND askbot_markedtag.reason = \'bad\' '
+                                  + 'AND question_tags.question_id = question.id'
+                            )
+                                ]),
+                        select_params = (uid_str, )
+                     )
 
         #qs = qs.select_related(depth=1)
         #todo: fix orderby here
@@ -191,7 +221,13 @@ class QuestionManager(models.Manager):
                         'last_activity_by__silver',
                         'last_activity_by__bronze'
                     )
-        return qs, meta_data
+
+        related_tags = Tag.objects.get_related_to_search(
+                                                questions = qs,
+                                                search_state = search_state
+                                            )
+
+        return qs, meta_data, related_tags
 
     #todo: this function is similar to get_response_receivers
     #profile this function against the other one
@@ -200,7 +236,11 @@ class QuestionManager(models.Manager):
         answer_list = []
         #question_list = list(question_list)#important for MySQL, b/c it does not support
         from askbot.models.answer import Answer
-        q_id = list(question_list.values_list('id', flat=True))
+        if isinstance(question_list, list):
+            q_id = [question.id for question in question_list]
+        else:
+            q_id = list(question_list.values_list('id', flat=True))
+
         a_id = list(Answer.objects.filter(question__in=q_id).values_list('id', flat=True))
         u_id = set(self.filter(id__in=q_id).values_list('author', flat=True))
         u_id = u_id.union(
diff --git a/askbot/models/tag.py b/askbot/models/tag.py
index 73712e3c..f1c7ca92 100644
--- a/askbot/models/tag.py
+++ b/askbot/models/tag.py
@@ -50,11 +50,42 @@ class TagManager(models.Manager):
         cursor.execute(query, [tag.id for tag in tags])
         transaction.commit_unless_managed()
 
-    def get_tags_by_questions(self, questions):
-        related_tags = self.filter(
-                            questions__in = list(questions)
-                        ).distinct()
-        return related_tags
+    def get_related_to_search(
+                            self,
+                            questions=None,
+                            search_state=None
+                        ):
+        """must return at least tag names, along with use counts
+        handle several cases to optimize the query performance
+        """
+
+        if search_state.is_default() or \
+                questions.count() > search_state.page_size * 3:
+            """if we have too many questions or 
+            search query is the most common - just return a list
+            of top tags"""
+            cheating = True
+            tags = Tag.objects.all().order_by('-used_count')
+        else:
+            cheating = False
+            #getting id's is necessary to avoid hitting a heavy query
+            #on entire selection of questions. We actually want
+            #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
+                ).annotate(
+                    local_used_count=models.Count('id')
+                ).order_by(
+                    '-local_used_count'
+                )
+
+        tags = tags[:50]#magic number
+        if cheating:
+            for tag in tags:
+                tag.local_used_count = tag.used_count
+
+        return tags
 
 class Tag(DeletableContent):
     name            = models.CharField(max_length=255, unique=True)
diff --git a/askbot/search/state_manager.py b/askbot/search/state_manager.py
index f8643e31..5edde679 100644
--- a/askbot/search/state_manager.py
+++ b/askbot/search/state_manager.py
@@ -42,6 +42,21 @@ class SearchState(object):
         out += 'logged_in=%s\n' % str(self.logged_in)
         return out
 
+    def is_default(self):
+        """True if search state is default
+        False otherwise, but with a few exceptions
+        notably page_size has no effect here
+        """
+        if self.scope != const.DEFAULT_POST_SCOPE:
+            return False
+        if self.author:
+            return False
+        if self.query:
+            return False
+        if self.tags:
+            return False
+        return True
+
     def set_logged_out(self):
         if self.scope == 'favorite':
             self.scope = None
diff --git a/askbot/skins/default/templates/questions.html b/askbot/skins/default/templates/questions.html
index 30aa9ee6..fffd20db 100644
--- a/askbot/skins/default/templates/questions.html
+++ b/askbot/skins/default/templates/questions.html
@@ -155,7 +155,7 @@
     </div>
 {% endif %}
 <div id="listA">
-{% cache 60 "questions" questions search_tags scope sort query context.page context.page_size language_code %}
+{% cache 0 "questions" questions search_tags scope sort query context.page context.page_size language_code %}
     {% for question in questions.object_list %}
     <div class="short-summary">
         <div class="counts">
@@ -294,7 +294,7 @@
                         rel="tag" 
                         title="{% trans tag_name=tag.name %}see questions tagged '{{ tag_name }}'{% endtrans %}"  
                         href="{% url questions %}?tags={{tag.name|urlencode}}">{{ tag.name }}</a>
-                    <span class="tag-number">&#215; {{ tag.used_count|intcomma }}</span>
+                    <span class="tag-number">&#215; {{ tag.local_used_count|intcomma }}</span>
                     <br />
                 {% endfor %}
             </div>
diff --git a/askbot/skins/default/templates/users_questions.html b/askbot/skins/default/templates/users_questions.html
index 4ef829a0..05087b6c 100644
--- a/askbot/skins/default/templates/users_questions.html
+++ b/askbot/skins/default/templates/users_questions.html
@@ -6,7 +6,7 @@
             {% if question.favorited_myself %}
                 <div class="favorites-count">
                     <img 
-                        title="{% trans cnt=question.favorite_count %}this questions was selected as favorite {{cnt}} time{% pluralize %}this questions was selected as favorite {{cnt}} times{% endtrans %}"
+                        title="{% trans cnt=question.favourite_count %}this questions was selected as favorite {{cnt}} time{% pluralize %}this questions was selected as favorite {{cnt}} times{% endtrans %}"
                         alt="{% trans %}thumb-up on{% endtrans %}"
                         src="{{"/images/vote-favorite-on.png"|media}}"/>
                     <div><b>{{question.favourite_count|intcomma}}</b></div>
@@ -14,7 +14,7 @@
             {% else %}
                 <div class="favorites-count-off">
                     <img 
-                        title="{% trans cnt=question.favorite_count %}this questions was selected as favorite {{cnt}} time{% pluralize %}this questions was selected as favorite {{cnt}} times{% endtrans %}"
+                        title="{% trans cnt=question.favourite_count %}this questions was selected as favorite {{cnt}} time{% pluralize %}this questions was selected as favorite {{cnt}} times{% endtrans %}"
                         alt="{% trans %}thumb-up off{% endtrans %}"
                         src="{{"/images/vote-favorite-off.png"|media}}"/>
                     <div><b>{{question.favourite_count|intcomma}}</b></div>
diff --git a/askbot/views/readers.py b/askbot/views/readers.py
index 5579983e..c669f02d 100644
--- a/askbot/views/readers.py
+++ b/askbot/views/readers.py
@@ -104,9 +104,9 @@ def questions(request):
     #todo: form is used only for validation...
     if form.is_valid():
         search_state.update_from_user_input(
-                                                form.cleaned_data, 
-                                                request.GET, 
-                                            )
+                                    form.cleaned_data,
+                                    request.GET,
+                                )
         #todo: better put these in separately then analyze
         #what neesd to be done, otherwise there are two routines
         #that take request.GET I don't like this use of parameters
@@ -120,42 +120,36 @@ def questions(request):
     #search_state.reset()
     #request.session.modified = True
 
-    #have this call implemented for sphinx, mysql and pgsql
-    (qs, meta_data) = Question.objects.run_advanced_search(
-                            request_user = request.user,
-                            scope_selector = search_state.scope,#unanswered/all/favorite (for logged in)
-                            search_query = search_state.query,
-                            tag_selector = search_state.tags,
-                            author_selector = search_state.author,
-                            sort_method = search_state.sort
-                        )
+    #todo: have this call implemented for sphinx, mysql and pgsql
+    (qs, meta_data, related_tags) = Question.objects.run_advanced_search(
+                                            request_user = request.user,
+                                            search_state = search_state,
+                                        )
 
-    objects_list = Paginator(qs, search_state.page_size)
+    paginator = Paginator(qs, search_state.page_size)
 
-    if objects_list.num_pages < search_state.page:
+    if paginator.num_pages < search_state.page:
         raise Http404
 
-    questions = objects_list.page(search_state.page)
+    page = paginator.page(search_state.page)
 
-    #todo maybe do this search on query the set instead
-    related_tags = Tag.objects.get_tags_by_questions(questions.object_list)
-    contributors = Question.objects.get_question_and_answer_contributors(questions.object_list)
+    contributors = Question.objects.get_question_and_answer_contributors(page.object_list)
 
     paginator_context = {
-        'is_paginated' : (objects_list.count > search_state.page_size),
-        'pages': objects_list.num_pages,
+        'is_paginated' : (paginator.count > search_state.page_size),
+        'pages': paginator.num_pages,
         'page': search_state.page,
-        'has_previous': questions.has_previous(),
-        'has_next': questions.has_next(),
-        'previous': questions.previous_page_number(),
-        'next': questions.next_page_number(),
+        'has_previous': page.has_previous(),
+        'has_next': page.has_next(),
+        'previous': page.previous_page_number(),
+        'next': page.next_page_number(),
         'base_url' : request.path + '?sort=%s&amp;' % search_state.sort,#todo in T sort=>sort_method
         'page_size' : search_state.page_size,#todo in T pagesize -> page_size
     }
 
     if request.is_ajax():
 
-        q_count = objects_list.count
+        q_count = paginator.count
         question_counter = ungettext(
                                 '%(q_num)s question',
                                 '%(q_num)s questions',
@@ -224,7 +218,7 @@ def questions(request):
         views_color_min_fg = askbot_settings.COLORS_VIEW_COUNTER_MIN_FG
         views_bgcolor_min = askbot_settings.COLORS_VIEW_COUNTER_MIN_BG
 
-        for question in questions.object_list:
+        for question in page.object_list:
             timestamp = question.last_activity_at
             author = question.last_activity_by
 
@@ -318,11 +312,11 @@ def questions(request):
         'reset_method_count': reset_method_count,
         'view_name': 'questions',
         'active_tab': 'questions',
-        'questions' : questions,
+        'questions' : page,
         'contributors' : contributors,
         'author_name' : meta_data.get('author_name',None),
         'tab_id' : search_state.sort,
-        'questions_count' : objects_list.count,
+        'questions_count' : paginator.count,
         'tags' : related_tags,
         'query': search_state.query,
         'search_tags' : search_state.tags,