diff options
author | Evgeny Fadeev <evgeny.fadeev@gmail.com> | 2010-10-05 23:07:33 -0400 |
---|---|---|
committer | Evgeny Fadeev <evgeny.fadeev@gmail.com> | 2010-10-05 23:07:33 -0400 |
commit | 91c5ebf35c28183ddbab9ed4dcea4e7dfc1fca0a (patch) | |
tree | 273eebfd8202154a87e573cd2021202382aef5ed | |
parent | 8d3c67d11b51cf6aef7f064fa3763ed1aeaf58ad (diff) | |
download | askbot-91c5ebf35c28183ddbab9ed4dcea4e7dfc1fca0a.tar.gz askbot-91c5ebf35c28183ddbab9ed4dcea4e7dfc1fca0a.tar.bz2 askbot-91c5ebf35c28183ddbab9ed4dcea4e7dfc1fca0a.zip |
fixed error in related tag counts, found by Dario
-rw-r--r-- | askbot/models/question.py | 130 | ||||
-rw-r--r-- | askbot/models/tag.py | 41 | ||||
-rw-r--r-- | askbot/search/state_manager.py | 15 | ||||
-rw-r--r-- | askbot/skins/default/templates/questions.html | 4 | ||||
-rw-r--r-- | askbot/skins/default/templates/users_questions.html | 4 | ||||
-rw-r--r-- | askbot/views/readers.py | 50 |
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">× {{ tag.used_count|intcomma }}</span> + <span class="tag-number">× {{ 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&' % 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, |