Commits

Jens Diemer committed 8117b24

* Bugfix for double results in "show own posts" * Don't put user_id in GET -> use request.user * remove {% if results %} with {% for %}...{% empty %} * Refactor search()
Fixed topic search results which returned nothing.
Return a list instead of a generator in TopicFromPostResult.__getitem__() so that the pagination can work correctly.
from: https://bitbucket.org/hsoft/djangobb/changeset/1b2f31c54c516c699a7b11a1d43b170742157df5

use the slow, but correct solution for display the result as topics

used fast solution with a work-a-round for double topics

respect sort order

count only one time

better solution, but slow with woosh and many hits

change info

make it DRY

remove debug imports

remove not needed tuple()

* add "show your topics/posts" search
* add link to switch between topics/posts search
* merge {% block controls %}
* make search_posts.html useable with queryset and SearchQuerySet

Add topic <-> post switch links to other type of search, too.

not really in GET needed.

* Use PostSearchForm() in normal search
* add switch links to normal search, too.

better docstring

better text?

Comments (0)

Files changed (8)

djangobb_forum/templates/djangobb_forum/base.html

         <div class="box">
             <div class="inbox">
                 {% block controls %}
-                {% endblock %}
+                    <dl id="searchlinks" class="conl">
+                        <dt><strong>{% trans "Search links" %}</strong></dt>
+                        <dd><a href="{% url djangobb:search %}?action=show_24h">{% trans "Show recent posts" %}</a></dd>
+                        <dd><a href="{% url djangobb:search %}?action=show_unanswered">{% trans "Show unanswered posts" %}</a></dd>
+                        {% if user.is_authenticated %}
+                        <dd><a href="{% url djangobb:search %}?action=show_subscriptions">{% trans "Show your subscribed topics" %}</a></dd>
+                        <dd>{% trans "Show your" %} <a href="{% url djangobb:search %}?action=show_user&show_as=topics">{% trans "topics" %}</a>/<a href="{% url djangobb:search %}?action=show_user&show_as=posts">{% trans "posts" %}</a></dd>
+                        {% endif %}
+                    </dl>
+                {% endblock controls %}
                 <p class="conr">Powered by <a href="http://djangobb.org/">DjangoBB</a></p>
                 <div class="clearer"></div>
                 {% block lofi %}
-                {% endblock %}
+                {% endblock lofi%}
             </div>
         </div>
     </div>

djangobb_forum/templates/djangobb_forum/include/post_in_search_page.html

+{% load forum_extras %}
+{% load i18n %}
+
+<div class="blockpost searchposts roweven">
+    <h2>{% link post.topic.forum %}&nbsp;&raquo;&nbsp;{% link post.topic %}&nbsp;&raquo;&nbsp;<a href="{{ post.get_absolute_url }}">{% forum_time post.created %}</a></h2>
+
+    <div class="box">
+        <div class="inbox">
+            <div class="postleft">
+                <dl>
+                    <dt><strong><a href="{% url djangobb:forum_profile post.user %}">{{ post.user }}</a></strong></dt>
+                    <dd>{% trans "Replies:" %} {{ post.topic.post_count }}</dd>
+                    <dd><div class="icon"><div class="nosize"><!-- --></div></div></dd>
+                    <dd><p class="clearb"><a href="{{ post.get_absolute_url }}">{% trans "Go to post" %}</a></p></dd>
+                </dl>
+            </div>
+            <div class="postright">
+                <div class="postmsg">
+                    {{ post.body_html|safe }}
+                </div>
+            </div>
+            <div class="clearer"></div>
+        </div>
+    </div>
+</div>

djangobb_forum/templates/djangobb_forum/index.html

 </div>
 {% endblock %}
 
-{% block controls %}
-<dl id="searchlinks" class="conl">
-	<dt><strong>{% trans "Search links" %}</strong></dt>
-	<dd><a href="{% url djangobb:search %}?action=show_24h">{% trans "Show recent posts" %}</a></dd>
-	<dd><a href="{% url djangobb:search %}?action=show_unanswered">{% trans "Show unanswered posts" %}</a></dd>
-	{% if user.is_authenticated %}
-		<dd><a href="{% url djangobb:search %}?action=show_subscriptions">{% trans "Show your subscribed topics" %}</a></dd>
-		<dd><a href="{% url djangobb:search %}?action=show_user&amp;user_id={{ request.user.id }}">{% trans "Show your posts" %}</a></dd>
-	{% endif %}
-</dl>
-{% endblock %}
-
 {% block lofi %}
 {% if forum_settings.LOFI_SUPPORT %}
     <p class="lofi">

djangobb_forum/templates/djangobb_forum/search_form.html

 					</div>
 				</fieldset>
 			</div>
-			<p><input type="submit" name="search" value="{% trans "Submit" %}" accesskey="s" /></p>
+			<p><input type="submit" value="{% trans "Submit" %}" accesskey="s" /></p>
 		</form>
 	</div>
 </div>

djangobb_forum/templates/djangobb_forum/search_posts.html

 {% load pagination_tags %}
 
 {% block content %}
-{% autopaginate results forum_settings.FORUM_PAGE_SIZE %}
+{% autopaginate posts forum_settings.FORUM_PAGE_SIZE %}
 <div class="linkst">
 	<div class="inbox">
 		<p class="pagelink">{% paginate %}</p>
 	</div>
 </div>
 
-{% if results %}
-{% for post in results %}
-<div class="blockpost searchposts roweven">
-	<h2>{% link post.object.topic.forum %}&nbsp;&raquo;&nbsp;{% link post.object.topic %}&nbsp;&raquo;&nbsp;<a href="{{ post.object.get_absolute_url }}">{% forum_time post.created %}</a></h2>
+<div id="messages" class="block">
+    <h2><span>{% trans "Search type" %}</span></h2>
+    <div class="box">
+        <div class="inbox">
+            <a href="{{ as_topic_url }}">{% trans "Show results as topic list." %}</a>
+        </div>
+    </div>
+</div>
 
-	<div class="box">
-		<div class="inbox">
-			<div class="postleft">
-				<dl>
-					<dt><strong><a href="{% url djangobb:forum_profile post.object.user %}">{{ post.object.user }}</a></strong></dt>
-					<dd>{% trans "Replies:" %} {{ post.object.topic.post_count }}</dd>
-					<dd><div class="icon"><div class="nosize"><!-- --></div></div></dd>
-					<dd><p class="clearb"><a href="{{ post.object.get_absolute_url }}">{% trans "Go to post" %}</a></p></dd>
-				</dl>
-			</div>
-			<div class="postright">
-				<div class="postmsg">
-					{{ post.object.body_html|safe }}
-				</div>
-			</div>
-			<div class="clearer"></div>
-		</div>
-	</div>
-</div>
-{% endfor %}
+{% for post in posts %}
+{% if post.object %}
+    {% with post=post.object %}
+        {% include "djangobb_forum/include/post_in_search_page.html" %}
+    {% endwith %}
 {% else %}
+    {% include "djangobb_forum/include/post_in_search_page.html" %}
+{% endif %}
+{% empty %}
 <div id="msg" class="block">
 	<h2><span>{% trans "Info" %}</span></h2>
 	<div class="box">
 		</div>
 	</div>
 </div>
-{% endif %}
+{% endfor %}
 
 
 <div class="postlinksb">
 {% endblock %}
 
 {% block controls %}
-				<dl id="searchlinks" class="conl">
-					<dt><strong>{% trans "Search links" %}</strong></dt>
-					<dd><a href="{% url djangobb:search %}?action=show_24h">{% trans "Show recent posts" %}</a></dd>
-					<dd><a href="{% url djangobb:search %}?action=show_unanswered">{% trans "Show unanswered posts" %}</a></dd>
-					{% if user.is_authenticated %}
-					<dd><a href="{% url djangobb:search %}?action=show_subscriptions">{% trans "Show your subscribed topics" %}</a></dd>
-					<dd><a href="{% url djangobb:search %}?action=show_user&amp;user_id={{ request.user.id }}">{% trans "Show your posts" %}</a></dd>
-					{% endif %}
-				</dl>
+<dl id="searchlinks" class="conl">
+	<dt><strong>{% trans "Search links" %}</strong></dt>
+	<dd><a href="{% url djangobb:search %}?action=show_24h">{% trans "Show recent posts" %}</a></dd>
+	<dd><a href="{% url djangobb:search %}?action=show_unanswered">{% trans "Show unanswered posts" %}</a></dd>
+	{% if user.is_authenticated %}
+	<dd><a href="{% url djangobb:search %}?action=show_subscriptions">{% trans "Show your subscribed topics" %}</a></dd>
+	<dd><a href="{% url djangobb:search %}?action=show_user">{% trans "Show your topics" %}</a></dd>
+	{% endif %}
+</dl>
 {% endblock %}

djangobb_forum/templates/djangobb_forum/search_topics.html

 {% load i18n %}
 
 {% block content %}
-{% autopaginate results forum_settings.SEARCH_PAGE_SIZE %}
+{% autopaginate topics forum_settings.SEARCH_PAGE_SIZE %}
 <div class="linkst">
 	<div class="inbox">
 		<p class="pagelink">{% paginate %}</p>
 		<div class="clearer"></div>
 	</div>
 </div>
-{% if results %}
+
+<div id="messages" class="block">
+    <h2><span>{% trans "Search type" %}</span></h2>
+    <div class="box">
+        <div class="inbox">
+            <a href="{{ as_post_url }}">{% trans "Show results as post list." %}</a>
+        </div>
+    </div>
+</div>
+
 <div id="vf" class="blocktable">
 	<h2><span>{% trans "Search results" %}</span></h2>
 	<div class="box">
 				</tr>
 			</thead>
 			<tbody>
-			{% for topic in results %}
+			{% for topic in topics %}
 				<tr {% if topic|has_unreads:user %}class="inew"{% endif %} {% if topic.closed %}class="iclosed"{% endif %}>
 					<td class="tcl">
 						<div class="intd">
 					<td class="tc3">{{ topic.reply_count }}</td>
 					<td class="tcr"><a href="{{ topic.get_absolute_url }}">{% forum_time topic.updated %}</a> <span class="byuser">{% trans "by" %} {{ topic.last_post.user.username }}</span></td>
 				</tr>
+			{% empty %}
+			     <tr>
+			         <td colspan="4"><p>{% trans "Your search returned no hits." %}</p></td>
+			     </tr>
 			{% endfor %}
 			</tbody>
 			</table>
 	</div>
 </div>
 
-{% else %}
-<div id="msg" class="block">
-	<h2><span>{% trans "Info" %}</span></h2>
-	<div class="box">
-		<div class="inbox">
-		<p>{% trans "Your search returned no hits." %}</p>
-		<p><a href="javascript: history.go(-1)">{% trans "Go back" %}</a></p>
-		</div>
-	</div>
-</div>
-{% endif %}
 <div class="linksb">
 	<div class="inbox">
 		<p class="pagelink">{% paginate %}</p>
 	</div>
 </div>
 {% endblock %}
-
-{% block controls %}
-				<dl id="searchlinks" class="conl">
-					<dt><strong>{% trans "Search links" %}</strong></dt>
-					<dd><a href="{% url djangobb:search %}?action=show_24h">{% trans "Show recent posts" %}</a></dd>
-					<dd><a href="{% url djangobb:search %}?action=show_unanswered">{% trans "Show unanswered posts" %}</a></dd>
-					{% if user.is_authenticated %}
-					<dd><a href="{% url djangobb:search %}?action=show_subscriptions">{% trans "Show your subscribed topics" %}</a></dd>
-					<dd><a href="{% url djangobb:search %}?action=show_user&amp;user_id={{ request.user.id }}">{% trans "Show your posts" %}</a></dd>
-					{% endif %}
-				</dl>
-{% endblock %}

djangobb_forum/util.py

             self.html.append(data)
 
         def handle_startendtag(self, tag, attrs):
-            self.html.append('<%s%s/>' % (tag, self.__html_attrs(attrs))) 
+            self.html.append('<%s%s/>' % (tag, self.__html_attrs(attrs)))
 
         def handle_endtag(self, tag):
             self.is_ignored = False
         def __html_attrs(self, attrs):
             _attrs = ''
             if attrs:
-                _attrs = ' %s' % (' '.join([('%s="%s"' % (k,v)) for k,v in attrs]))
+                _attrs = ' %s' % (' '.join([('%s="%s"' % (k, v)) for k, v in attrs]))
             return _attrs
 
         def feed(self, data):
         paged_list_name = paginator.page(page_number).object_list
     except (InvalidPage, EmptyPage):
         raise Http404
-    return pages, paginator, paged_list_name 
+    return pages, paginator, paged_list_name
 
 def set_language(request, language):
     """
         raise Exception('Invalid markup property: %s' % markup)
     return urlize(text)
 
-
-class TopicFromPostResult(object):
-    """
-    Custom Result object to return topics from a post search.
-
-    This function uses a generator to return topic objects from
-    results given by haystack on a post query. This eliminates
-    loading a large array of topic objects into memory.
-    """
-
-    def __init__(self, posts):
-        self.posts = posts
-
-    def __len__(self):
-        return len(self.posts)
-
-    def __getitem__(self, key):
-        if isinstance(key, slice):
-            return (self.posts[i].object.topic
-                        for i in xrange(*key.indices(len(self))))
-        elif isinstance(key, int):
-            return self.posts[key].object.topic
-
-        raise TypeError('unknown type in key for __getitem__')

djangobb_forum/views.py

 from django.views.decorators.csrf import csrf_exempt
 from django.utils.translation import ugettext as _
 
-from djangobb_forum.util import build_form, paginate, set_language
-from djangobb_forum.models import Category, Forum, Topic, Post, Profile, Reputation, \
-    Attachment, PostTracking
+from djangobb_forum import settings as forum_settings
 from djangobb_forum.forms import AddPostForm, EditPostForm, UserSearchForm, \
     PostSearchForm, ReputationForm, MailToForm, EssentialsProfileForm, \
     PersonalProfileForm, MessagingProfileForm, PersonalityProfileForm, \
     DisplayProfileForm, PrivacyProfileForm, ReportForm, UploadAvatarForm
+from djangobb_forum.models import Category, Forum, Topic, Post, Profile, Reputation, \
+    Attachment, PostTracking
 from djangobb_forum.templatetags import forum_extras
-from djangobb_forum import settings as forum_settings
-from djangobb_forum.util import smiles, convert_text_to_html, TopicFromPostResult
 from djangobb_forum.templatetags.forum_extras import forum_moderated_by
+from djangobb_forum.util import build_form, paginate, set_language, smiles, convert_text_to_html
 
 from haystack.query import SearchQuerySet, SQ
 from django.contrib import messages
+from django.core.exceptions import SuspiciousOperation
 
 
 def index(request, full=True):
 
 
 def search(request):
-    # TODO: move to form
-    if 'action' in request.GET:
-        action = request.GET['action']
-        #FIXME: show_user for anonymous raise exception, 
-        #django bug http://code.djangoproject.com/changeset/14087 :|
-        groups = request.user.groups.all() or [] #removed after django > 1.2.3 release
-        topics = Topic.objects.filter(
-                   Q(forum__category__groups__in=groups) | \
-                   Q(forum__category__groups__isnull=True))
-        if action == 'show_24h':
-            date = datetime.today() - timedelta(1)
-            topics = topics.filter(created__gte=date)
-        elif action == 'show_new':
-            try:
-                last_read = PostTracking.objects.get(user=request.user).last_read
-            except PostTracking.DoesNotExist:
-                last_read = None
-            if last_read:
-                topics = topics.filter(last_post__updated__gte=last_read).all()
-            else:
-                #searching more than forum_settings.SEARCH_PAGE_SIZE in this way - not good idea :]
-                topics = [topic for topic in topics[:forum_settings.SEARCH_PAGE_SIZE] if forum_extras.has_unreads(topic, request.user)]
-        elif action == 'show_unanswered':
-            topics = topics.filter(post_count=1)
-        elif action == 'show_subscriptions':
-            topics = topics.filter(subscribers__id=request.user.id)
-        elif action == 'show_user':
-            user_id = request.GET['user_id']
-            posts = Post.objects.filter(user__id=user_id)
-            topics = [post.topic for post in posts if post.topic in topics]
-        elif action == 'search':
-            keywords = request.GET.get('keywords')
-            author = request.GET.get('author')
-            forum = request.GET.get('forum')
-            search_in = request.GET.get('search_in')
-            sort_by = request.GET.get('sort_by')
-            sort_dir = request.GET.get('sort_dir')
+    # TODO: used forms in every search type
 
-            if not (keywords or author):
-                return HttpResponseRedirect(reverse('djangobb:search'))
-
-            query = SearchQuerySet().models(Post)
-
-            if author:
-                query = query.filter(author__username=author)
-
-            if forum != u'0':
-                query = query.filter(forum__id=forum)
-
-            if keywords:
-                if search_in == 'all':
-                    query = query.filter(SQ(topic=keywords) | SQ(text=keywords))
-                elif search_in == 'message':
-                    query = query.filter(text=keywords)
-                elif search_in == 'topic':
-                    query = query.filter(topic=keywords)
-
-            # add exlusions for categories user does not have access too
-            for category in Category.objects.all():
-                if not category.has_access(request.user):
-                    query = query.exclude(category=category)
-
-            order = {'0': 'created',
-                     '1': 'author',
-                     '2': 'topic',
-                     '3': 'forum'}.get(sort_by, 'created')
-            if sort_dir == 'DESC':
-                order = '-' + order
-
-            posts = query.order_by(order)
-
-            if 'topics' in request.GET['show_as']:
-                return render(request, 'djangobb_forum/search_topics.html', {
-                    'results': TopicFromPostResult(posts)
-                })
-            elif 'posts' in request.GET['show_as']:
-                return render(request, 'djangobb_forum/search_posts.html', {'results': posts})
-
-        return render(request, 'djangobb_forum/search_topics.html', {'results': topics})
-    else:
-        form = PostSearchForm()
+    def _render_search_form(form=None):
         return render(request, 'djangobb_forum/search_form.html', {'categories': Category.objects.all(),
                 'form': form,
                 })
 
+    if not 'action' in request.GET:
+        return _render_search_form(form=PostSearchForm())
+
+    if request.GET.get("show_as") == "posts":
+        show_as_posts = True
+        template_name = 'djangobb_forum/search_posts.html'
+    else:
+        show_as_posts = False
+        template_name = 'djangobb_forum/search_topics.html'
+
+    context = {}
+
+    #FIXME: show_user for anonymous raise exception, 
+    #django bug http://code.djangoproject.com/changeset/14087 :|
+    groups = request.user.groups.all() or [] #removed after django > 1.2.3 release
+    topics = Topic.objects.filter(
+               Q(forum__category__groups__in=groups) | \
+               Q(forum__category__groups__isnull=True))
+
+    base_url = None
+    _generic_context = True
+
+    action = request.GET['action']
+    if action == 'show_24h':
+        date = datetime.today() - timedelta(1)
+        topics = topics.filter(created__gte=date)
+    elif action == 'show_new':
+        try:
+            last_read = PostTracking.objects.get(user=request.user).last_read
+        except PostTracking.DoesNotExist:
+            last_read = None
+        if last_read:
+            topics = topics.filter(last_post__updated__gte=last_read).all()
+        else:
+            #searching more than forum_settings.SEARCH_PAGE_SIZE in this way - not good idea :]
+            topics = [topic for topic in topics[:forum_settings.SEARCH_PAGE_SIZE] if forum_extras.has_unreads(topic, request.user)]
+
+    elif action == 'show_unanswered':
+        topics = topics.filter(post_count=1)
+    elif action == 'show_subscriptions':
+        topics = topics.filter(subscribers__id=request.user.id)
+    elif action == 'show_user':
+        # Show all posts from user or topics started by user
+        user_id = request.GET.get("user_id", request.user.id)
+        user_id = int(user_id)
+        posts = Post.objects.filter(user__id=user_id)
+        base_url = "?action=show_user&user_id=%s&show_as=" % user_id
+        if not show_as_posts:
+            # show as topic
+            # FIXME: This should be speed up. This is not lazy:
+            user_topics = []
+            for post in posts:
+                topic = post.topic
+                if topic in topics and topic not in user_topics:
+                    user_topics.append(topic)
+            topics = user_topics
+    elif action == 'search':
+        form = PostSearchForm(request.GET)
+        if not form.is_valid():
+            return _render_search_form(form)
+
+        keywords = form.cleaned_data['keywords']
+        author = form.cleaned_data['author']
+        forum = form.cleaned_data['forum']
+        search_in = form.cleaned_data['search_in']
+        sort_by = form.cleaned_data['sort_by']
+        sort_dir = form.cleaned_data['sort_dir']
+
+        query = SearchQuerySet().models(Post)
+
+        if author:
+            query = query.filter(author__username=author)
+
+        if forum != u'0':
+            query = query.filter(forum__id=forum)
+
+        if keywords:
+            if search_in == 'all':
+                query = query.filter(SQ(topic=keywords) | SQ(text=keywords))
+            elif search_in == 'message':
+                query = query.filter(text=keywords)
+            elif search_in == 'topic':
+                query = query.filter(topic=keywords)
+
+        # add exlusions for categories user does not have access too
+        for category in Category.objects.all():
+            if not category.has_access(request.user):
+                query = query.exclude(category=category)
+
+        order = {'0': 'created',
+                 '1': 'author',
+                 '2': 'topic',
+                 '3': 'forum'}.get(sort_by, 'created')
+        if sort_dir == 'DESC':
+            order = '-' + order
+
+        posts = query.order_by(order)
+
+        if not show_as_posts:
+            # TODO: We have here a problem to get a list of topics without double entries.
+            # Maybe we must add a search index over topics?
+
+            # Info: If whoosh backend used, setup HAYSTACK_ITERATOR_LOAD_PER_QUERY
+            #    to a higher number to speed up
+            post_pks = posts.values_list("pk", flat=True)
+            context["topics"] = Topic.objects.filter(posts__in=post_pks).distinct()
+        else:
+            context["posts"] = posts
+
+        get_query_dict = request.GET.copy()
+        get_query_dict.pop("show_as")
+        base_url = "?%s&show_as=" % get_query_dict.urlencode()
+        _generic_context = False
+
+    if _generic_context:
+        if show_as_posts:
+            context["posts"] = Post.objects.filter(topic__in=topics).order_by('-created')
+        else:
+            context["topics"] = topics
+
+    if base_url is None:
+        base_url = "?action=%s&show_as=" % action
+
+    if show_as_posts:
+        context["as_topic_url"] = base_url + "topics"
+    else:
+        context["as_post_url"] = base_url + "posts"
+
+    return render(request, template_name, context)
+
+
+
 
 @login_required
 def misc(request):