Lot of small improvement in the forum

forum/models.py

@@ -6,6 +6,7 @@ from django.core.exceptions import ValidationError
 from django.db import IntegrityError, transaction
 from django.core.urlresolvers import reverse
 from django.utils import timezone
+from django.utils.functional import cached_property
 
 from datetime import datetime
 import pytz
@@ -16,6 +17,10 @@ from club.models import Club
 class Forum(models.Model):
     """
     The Forum class, made as a tree to allow nice tidy organization
+
+    owner_club allows club members to moderate there own topics
+    edit_groups allows to put any group as a forum admin
+    view_groups allows some groups to view a forum
     """
     name = models.CharField(_('name'), max_length=64)
     description = models.CharField(_('description'), max_length=256, default="")
@@ -77,6 +82,10 @@ class Forum(models.Model):
     def get_absolute_url(self):
         return reverse('forum:view_forum', kwargs={'forum_id': self.id})
 
+    @cached_property
+    def parent_list(self):
+        return self.get_parent_list()
+
     def get_parent_list(self):
         l = []
         p = self.parent
@@ -85,22 +94,27 @@ class Forum(models.Model):
             p = p.parent
         return l
 
+    @cached_property
+    def topic_number(self):
+        return self.get_topic_number()
+
     def get_topic_number(self):
         number = self.topics.all().count()
         for c in self.children.all():
-            number += c.get_topic_number()
+            number += c.topic_number
         return number
 
+    @cached_property
+    def last_message(self):
+        return self.get_last_message()
+
     def get_last_message(self):
         last_msg = None
-        for m in ForumMessage.objects.order_by('-id'):
+        for m in ForumMessage.objects.select_related('topic__forum', 'author').order_by('-id'):
             forum = m.topic.forum
-            if self in (forum.get_parent_list() + [forum]):
+            if self in (forum.parent_list + [forum]):
                 return m
                 last_msg = m
-        try:
-            pass
-        except: pass
         return last_msg
 
 class ForumTopic(models.Model):
@@ -145,26 +159,34 @@ class ForumMessage(models.Model):
         ordering = ['id']
 
     def __str__(self):
-        return "%s" % (self.title)
+        return "%s - %s" % (self.id, self.title)
 
-    def is_owned_by(self, user):
+    def is_owned_by(self, user): # Anyone can create a topic: it's better to
-        return self.topic.is_owned_by(user) or user.id == self.author.id
+                                 # check the rights at the forum level, since it's more controlled
+        return self.topic.forum.is_owned_by(user) or user.id == self.author.id
 
     def can_be_edited_by(self, user):
-        return user.is_owner(self.topic)
+        return user.can_edit(self.topic.forum)
 
     def can_be_viewed_by(self, user):
         return user.can_view(self.topic)
 
     def can_be_moderated_by(self, user):
-        return user.is_in_group(settings.SITH_GROUP_FORUM_ADMIN_ID)
+        return self.topic.forum.is_owned_by(user)
 
     def get_absolute_url(self):
-        return self.topic.get_absolute_url() + "#first_unread"
+        return self.topic.get_absolute_url() + "#msg_" + str(self.id)
 
     def mark_as_read(self, user):
         self.readers.add(user)
 
+    def is_read(self, user):
+        return (self.date < user.forum_infos.last_read_date) or (user in self.readers.all())
+
+    @cached_property
+    def deleted(self):
+        return self.is_deleted()
+
     def is_deleted(self):
         meta = self.metas.exclude(action="EDIT").order_by('-date').first()
         if meta:
@@ -184,7 +206,12 @@ class ForumMessageMeta(models.Model):
     action = models.CharField(_("action"), choices=MESSAGE_META_ACTIONS, max_length=16)
 
 class ForumUserInfo(models.Model):
-    user = models.OneToOneField(User, related_name="_forum_infos")
+    """
+    This currently stores only the last date a user clicked "Mark all as read".
+    However, this can be extended with lot of user preferences dedicated to a
+    user, such as the favourite topics, the signature, and so on...
+    """
+    user = models.OneToOneField(User, related_name="_forum_infos") # TODO: see to move that to the User class in order to reduce the number of db queries
     last_read_date = models.DateTimeField(_('last read date'), default=datetime(year=settings.SITH_SCHOOL_START_YEAR,
         month=1, day=1, tzinfo=pytz.UTC))
 

forum/templates/forum/macros.jinja

@@ -23,13 +23,12 @@
     {% if not forum.is_category %}
     <div class="ib w_small">
         <div class="ib w_medium">
-            {{ forum.get_topic_number() }}
+            {{ forum.topic_number }}
         </div>
         <div class="ib w_medium">
-            {% set last_msg = forum.get_last_message() %}
+            {% if forum.last_message %}
-            {% if last_msg %}
+            {{ forum.last_message.author }} <br/>
-            {{ last_msg.author }} <br/>
+            {{ forum.last_message.date|date(DATETIME_FORMAT) }} {{ forum.last_message.date|time(DATETIME_FORMAT) }}
-            {{ last_msg.date|date(DATETIME_FORMAT) }} {{ last_msg.date|time(DATETIME_FORMAT) }}
             {% endif %}
         </div>
     </div>
@@ -64,7 +63,7 @@
             </div>
         </div>
         <div class="ib w_medium" style="text-align: center;">
-            {% set last_msg = topic.messages.order_by('id').last() %}
+            {% set last_msg = topic.messages.order_by('id').select_related('author').last() %}
             {{ user_profile_link(last_msg.author) }} <br/>
             {{ last_msg.date|date(DATETIME_FORMAT) }} {{ last_msg.date|time(DATETIME_FORMAT) }}
         </div>

forum/templates/forum/topic.jinja

@@ -37,14 +37,11 @@
     <p>{{ topic.description }}</p>
     <p><a href="{{ url('forum:new_message', topic_id=topic.id) }}">Reply</a></p>
 
-    {% set vars = {'unread': False} %} {# ugly hack to counter Jinja scopes #}
+    {% for m in topic.messages.select_related('author__profile_pict').all() %}
-    {% for m in topic.messages.all() %}
+    {% if m.id >= first_unread_message_id %}
-    {% if m.id == first_unread_message_id and vars.update({'unread': True}) %} {# idem #}
+    <div id="msg_{{ m.id }}" class="message unread">
-        <div class="message unread" id="first_unread">
-    {% elif vars.unread %}
-        <div class="message unread">
     {% else %}
-        <div class="message">
+    <div id="msg_{{ m.id }}" class="message">
     {% endif %}
         <div class="msg_author">
                 {% if m.author.profile_pict %}
@@ -55,7 +52,7 @@
             <br/>
             <strong>{{ user_profile_link(m.author) }}</strong>
         </div>
-        <div style="display: inline-block; width: 80%; vertical-align: top;">
+        <div {% if m.id == first_unread_message_id %}id="first_unread"{% endif %} style="display: inline-block; width: 80%; vertical-align: top;">
             <div style="display: inline-block; width: 74%;">
                 {% if m.title %}
                 <h5>{{ m.title }}</h5>
@@ -68,7 +65,7 @@
                 <span> <a href="{{ url('forum:edit_message', message_id=m.id) }}">{% trans %}Edit{% endtrans %}</a></span>
                 {% endif %}
                 {% if m.can_be_moderated_by(user) %}
-                {% if m.is_deleted() %}
+                {% if m.deleted %}
                 <span> <a href="{{ url('forum:undelete_message', message_id=m.id) }}">{% trans %}Undelete{% endtrans %}</a></span>
                 {% else %}
                 <span> <a href="{{ url('forum:delete_message', message_id=m.id) }}">{% trans %}Delete{% endtrans %}</a></span>
@@ -81,17 +78,20 @@
             <div>
             {{ m.message|markdown }}
             </div>
+            {% if m.can_be_moderated_by(user) %}
             <ul>
-            {% for meta in m.metas.all() %}
+            {% for meta in m.metas.select_related('user').all() %}
                 <li>{{ meta.get_action_display() }} {{ meta.user.get_display_name() }}
                     {% trans %} at {% endtrans %}{{ meta.date|time(DATETIME_FORMAT) }}
                     {% trans %} the {% endtrans %}{{ meta.date|date(DATETIME_FORMAT)}}</li>
             {% endfor %}
             </ul>
+            {% endif %}
         </div>
     </div>
     {{ m.mark_as_read(user) or "" }}
     {% endfor %}
+    <p><a href="{{ url('forum:new_message', topic_id=topic.id) }}">Reply</a></p>
 {% endblock %}
 
 

forum/views.py

@@ -10,6 +10,8 @@ from django import forms
 from django.db import models
 from django.core.exceptions import PermissionDenied
 
+from math import inf
+
 from core.views import CanViewMixin, CanEditMixin, CanEditPropMixin, CanCreateMixin, TabedViewMixin
 from forum.models import Forum, ForumMessage, ForumTopic, ForumMessageMeta
 
@@ -36,9 +38,7 @@ class ForumLastUnread(ListView):
     def get_queryset(self):
         l = ForumMessage.objects.exclude(readers=self.request.user).filter(
                 date__gt=self.request.user.forum_infos.last_read_date).values_list('topic') # TODO try to do better
-        return self.model.objects.filter(id__in=l).annotate(models.Max('messages__date')).order_by('-messages__date__max')
+        return self.model.objects.filter(id__in=l).annotate(models.Max('messages__date')).order_by('-messages__date__max').select_related('author')
-        # return self.model.objects.exclude(messages__readers=self.request.user).distinct().annotate(
-                # models.Max('messages__date')).order_by('-messages__date__max')
 
 class ForumCreateView(CanCreateMixin, CreateView):
     model = Forum
@@ -118,14 +118,15 @@ class ForumTopicDetailView(CanViewMixin, DetailView):
     pk_url_kwarg = "topic_id"
     template_name = "forum/topic.jinja"
     context_object_name = "topic"
+    queryset = ForumTopic.objects.select_related('forum__parent')
 
     def get_context_data(self, **kwargs):
         kwargs = super(ForumTopicDetailView, self).get_context_data(**kwargs)
-        msg = self.object.messages.exclude(readers=self.request.user).order_by('id').first()
+        msg = self.object.messages.exclude(readers=self.request.user).filter(date__gte=self.request.user.forum_infos.last_read_date).order_by('id').first()
         try:
             kwargs['first_unread_message_id'] = msg.id
         except:
-            kwargs['first_unread_message_id'] = -1
+            kwargs['first_unread_message_id'] = inf
         return kwargs
 
 class ForumMessageEditView(CanEditMixin, UpdateView):
@@ -144,6 +145,7 @@ class ForumMessageDeleteView(SingleObjectMixin, RedirectView):
     permanent = False
 
     def get_redirect_url(self, *args, **kwargs):
+        self.object = self.get_object()
         if self.object.can_be_moderated_by(self.request.user):
             ForumMessageMeta(message=self.object, user=self.request.user, action="DELETE").save()
         return self.object.get_absolute_url()
@@ -154,6 +156,7 @@ class ForumMessageUndeleteView(SingleObjectMixin, RedirectView):
     permanent = False
 
     def get_redirect_url(self, *args, **kwargs):
+        self.object = self.get_object()
         if self.object.can_be_moderated_by(self.request.user):
             ForumMessageMeta(message=self.object, user=self.request.user, action="UNDELETE").save()
         return self.object.get_absolute_url()
@@ -172,10 +175,14 @@ class ForumMessageCreateView(CanCreateMixin, CreateView):
     def get_initial(self):
         init = super(ForumMessageCreateView, self).get_initial()
         try:
-            init['message'] = "\n".join([
+            message = ForumMessage.objects.select_related('author').filter(id=self.request.GET['quote_id']).first()
-                "> " + line for line in ForumMessage.objects.filter(id=self.request.GET['quote_id']).first().message.split('\n')
+            init['message'] = "> ##### %s\n" % (_("%(author)s said") % {'author': message.author.get_short_name()})
+            init['message'] += "\n".join([
+                "> " + line for line in message.message.split('\n')
                 ])
-        except: pass
+            init['message'] += "\n\n"
+        except Exception as e:
+            print(repr(e))
         return init
 
     def form_valid(self, form):
@@ -183,3 +190,4 @@ class ForumMessageCreateView(CanCreateMixin, CreateView):
         form.instance.author = self.request.user
         return super(ForumMessageCreateView, self).form_valid(form)
 
+