🐛(forum) fix sorting on sticky and announcements topics

There are different kind of topics: regular, sticky and announcements topics.
Sticky topics are supposed to stay on top of the topic listing. Announcements
are displayed in another block that is above the regular topics block and has
its own header columns. Previous commit 3f92f4a enabled sorting topics on
different header columns. This new feature introduces two different
inconstancies. First, sticky topics were sorted as well whereas they are meant
to stay on top of the listing. Second, columns of announcements block when the
order was requested in URL showed toggling arrow as if they were sorted as well.
We only want to sort regular topic discussions and not announcements.
Announcements must stay with the default order which is ordered on the creation
date. The commit introduces these two fixes.

CHANGELOG.md

@@ -25,6 +25,9 @@ Versioning](https://semver.org/spec/v2.0.0.html).
  - add django rest framework to promote/revoke moderators for current LTIContext
  - add new role moderator and permission to manage moderators
 
+### Fixed
+ - fix sorting on sticky and announcements topics
+
 ## [1.0.0-beta.5] - 2020-03-01
 
 ### Changed

src/ashley/machina_extensions/forum/views.py

@@ -72,7 +72,8 @@ def get_ordering(self):
     def get_queryset(self):
         """Returns the list of items for this view ordered by asked param."""
         query = super().get_queryset()
-        return query.order_by(self.get_ordering_column())
+        # Type of topic is kept as first order argument to keep sticky option
+        return query.order_by("-type", self.get_ordering_column())
 
     def get_context_data(self, **kwargs):
         """Returns the context data to provide to the template."""

src/ashley/templates/forum_conversation/topic_list.html

@@ -9,23 +9,39 @@
     <div class="card topiclist">
       <div class="p-0 card-header">
         <div class="row m-0 px-3 py-2">
-              <div class="pl-0 col-md-6 col-sm-9 col-12 topic-name-col sortable {{ header.0.class_attrib }}">
+              <div class="pl-0 col-md-6 col-sm-9 col-12 topic-name-col {% if topics != announces %}sortable {{ header.0.class_attrib }}{% endif %}">
                 <h3 class="m-0 card-title h5 text-dark">
+                    {% if topics == announces %}
+                      {% trans topic_list_title %}
+                    {% else %}
                       <a href="{{ header.0.url_order }}">{% trans topic_list_title %}</a>
                       {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.0%}
+                    {% endif %}
                 </h3>
               </div>
-              <div class="col-md-2 d-none d-md-block text-center text-nowrap topic-count-col sortable {{ header.1.class_attrib }}">
-                    <a href="{{ header.1.url_order }}">{% trans "Replies" %}</a>
-                    {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.1%}
+              <div class="col-md-2 d-none d-md-block text-center text-nowrap topic-count-col {% if topics != announces %}sortable {{ header.1.class_attrib }}{% endif %}">
+                    {% if topics == announces %}
+                      {% trans "Replies" %}
+                    {% else %}
+                      <a href="{{ header.1.url_order }}">{% trans "Replies" %}</a>
+                      {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.1%}
+                    {% endif %}
               </div>
-              <div class="col-md-2 d-none d-md-block text-center text-nowrap topic-count-col sortable {{ header.2.class_attrib }}">
+              <div class="col-md-2 d-none d-md-block text-center text-nowrap topic-count-col {% if topics != announces %}sortable {{ header.2.class_attrib }}{% endif %}">
+                    {% if topics == announces %}
+                      {% trans "Views" %}
+                    {% else %}
                         <a href="{{ header.2.url_order }}">{% trans "Views" %}</a>
                         {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.2%}
+                    {% endif %}
               </div>
-              <div class="pr-0 col-md-2 col-sm-3 d-none d-sm-block text-nowrap topic-last-post-col sortable {{ header.3.class_attrib }}">
+              <div class="pr-0 col-md-2 col-sm-3 d-none d-sm-block text-nowrap topic-last-post-col {% if topics != announces %}sortable {{ header.3.class_attrib }}{% endif %}">
+                    {% if topics == announces %}
+                      {% trans "Last post" %}
+                    {% else %}
                         <a href="{{ header.3.url_order }}">{% trans "Last post" %}</a>
                         {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.3%}
+                    {% endif %}
               </div>
         </div>
       </div>

tests/ashley/machina_extensions/forum/test_forum_view.py

@@ -1,8 +1,13 @@
+import lxml.html  # nosec
 from django.test import TestCase
+from lxml import etree  # nosec
 from machina.apps.forum_permission.shortcuts import assign_perm
+from machina.core.db.models import get_model
 
 from ashley.factories import ForumFactory, PostFactory, TopicFactory, UserFactory
 
+Topic = get_model("forum_conversation", "Topic")
+
 
 class TestForumView(TestCase):
     """Displays a forum and its topics overridden from django machina."""
@@ -364,3 +369,168 @@ def test_testing_query_param(self):
             '<a href="?o=3&whatever=2" class="toggle descending" title="Toggle sorting"></a>',
             html=True,
         )
+
+    def test_testing_topic_is_sticky_stay_sticky_default_order(self):
+        """
+        Request page with a sticky topic and check that the sticky topic is always the first
+        result on default order
+        """
+        forum, url_list_topic = self._get_url_list_topic_with_three_topics()
+        # Creates a post for sticky topic
+        PostFactory(
+            topic=TopicFactory(forum=forum, type=Topic.TOPIC_STICKY),
+            subject="TOPIC STICKY ONE",
+        )
+
+        # Calls page with default order
+        response = self.client.get(url_list_topic)
+
+        topicC = forum.topics.get(subject__startswith="TOPIC C the newest")
+        topicB = forum.topics.get(subject__startswith="TOPIC B the eldest")
+        topicA = forum.topics.get(subject__startswith="TOPIC A created second")
+        topicSticky = forum.topics.get(subject__startswith="TOPIC STICKY ONE")
+
+        # Controls topics have been created in the order assumed in their titles
+        self.assertGreater(topicSticky.last_post_on, topicC.last_post_on)
+        self.assertGreater(topicC.last_post_on, topicA.last_post_on)
+        self.assertGreater(topicA.last_post_on, topicB.last_post_on)
+
+        # Should be ordered by date by default with the sticky topic shown first
+        self.assertContentBefore(response, "TOPIC STICKY ONE", "TOPIC C the newest")
+        self.assertContentBefore(
+            response, "TOPIC C the newest", "TOPIC A created second"
+        )
+        self.assertContentBefore(
+            response, "TOPIC A created second", "TOPIC B the eldest"
+        )
+
+        # Reverses the order, the sticky topic should remain first
+        response = self.client.get(f"{url_list_topic}?o=-0")
+
+        self.assertContentBefore(response, "TOPIC STICKY ONE", "TOPIC B")
+        self.assertContentBefore(response, "TOPIC B", "TOPIC A ")
+        self.assertContentBefore(response, "TOPIC C", "TOPIC A")
+
+    def test_testing_topic_is_sticky_stay_sticky_other_column_than_default(self):
+        """
+        Request page with a sticky topic and check that the sticky topic is always
+        the first result even if it's sorted on column other than default one. The order
+        will be done on view counts column.
+        """
+        forum, url_list_topic = self._get_url_list_topic_with_three_topics()
+        # Creates a post for the sticky topic
+        PostFactory(
+            topic=TopicFactory(forum=forum, type=Topic.TOPIC_STICKY, views_count=7),
+            subject="TOPIC STICKY ONE",
+        )
+
+        topic12 = forum.topics.get(subject__contains="12 views_count")
+        topic9 = forum.topics.get(subject__contains="9 views_count")
+        topic6 = forum.topics.get(subject__contains="6 views_count")
+        topicSticky = forum.topics.get(subject__startswith="TOPIC STICKY ONE")
+        # Confirms that the sticky topic has neither max or lowest view_count
+        self.assertGreater(topic12.views_count, topic9.views_count)
+        self.assertGreater(topic9.views_count, topicSticky.views_count)
+        self.assertGreater(topicSticky.views_count, topic6.views_count)
+
+        # Orders on column view_post
+        response = self.client.get(f"{url_list_topic}?o=2")
+
+        # Sticky should stay first, we compare it with the max and the min views_count
+        self.assertContentBefore(response, "TOPIC STICKY ONE", "12 views_count")
+        self.assertContentBefore(response, "TOPIC STICKY ONE", "6 views_count")
+
+        # Reverses the order and confirms sticky topic stays on top
+        response = self.client.get(f"{url_list_topic}?o=-2")
+
+        # Sticky should stay first, we compare it with the max and the min views_count
+        self.assertContentBefore(response, "TOPIC STICKY ONE", "6 views_count")
+        self.assertContentBefore(response, "TOPIC STICKY ONE", "12 views_count")
+
+    def test_testing_topic_announce(self):
+        """Controls topics that are of type announcement don't have sorted options"""
+        # Creates posts for announcement topics
+        forum = ForumFactory()
+        PostFactory(topic=TopicFactory(forum=forum, type=Topic.TOPIC_ANNOUNCE))
+        PostFactory(topic=TopicFactory(forum=forum, type=Topic.TOPIC_ANNOUNCE))
+
+        user = UserFactory()
+        assign_perm("can_read_forum", user, forum)
+        self.client.force_login(user)
+
+        response = self.client.get(f"/forum/forum/{forum.slug}-{forum.pk}/")
+
+        html = lxml.html.fromstring(response.content)
+        # Select the header block of the announcement block, the first block
+        announce_block = str(
+            etree.tostring(html.cssselect(".topiclist .card-header")[0])
+        )
+
+        # Controls that announce_block is about announcements and not topics
+        self.assertIn("Announcements", announce_block)
+        self.assertNotIn("Topics", announce_block)
+        self.assertIn("Replies", announce_block)
+        self.assertIn("Views", announce_block)
+        self.assertIn("Last post", announce_block)
+
+        # There's no sortable informations
+        self.assertNotIn("sortable sorted", announce_block)
+        # There's no column that has a sorting link on
+        self.assertNotIn("<a href=", announce_block)
+        # There's no toggle sorting
+        self.assertNotIn("Toggle sorting", announce_block)
+
+    def test_testing_topic_announce_dont_get_ordered(self):
+        """
+        Controls topics that are of type announcement don't get ordered if sorting is
+        submitted in url. Orders are only applied to Topic posts.
+        """
+
+        forum = ForumFactory()
+        user = UserFactory()
+        assign_perm("can_read_forum", user, forum)
+        self.client.force_login(user)
+
+        # Creates posts for announcement topics
+        topicAnnounce1 = TopicFactory(
+            forum=forum, type=Topic.TOPIC_ANNOUNCE, views_count=100
+        )
+        topicAnnounce2 = TopicFactory(
+            forum=forum, type=Topic.TOPIC_ANNOUNCE, views_count=200
+        )
+        PostFactory(
+            topic=topicAnnounce1,
+            subject="TOPIC A TYPE ANNOUNCED",
+        )
+        PostFactory(
+            topic=topicAnnounce2,
+            subject="TOPIC B TYPE ANNOUNCED",
+        )
+        # Post of topicAnnounce2 has been created last, it should be the first one on the list
+        self.assertLess(topicAnnounce1.last_post_on, topicAnnounce2.last_post_on)
+        # Orders on column view_post
+        response = self.client.get(f"/forum/forum/{forum.slug}-{forum.pk}/?o=2")
+        # Orders is respected on default creation order
+        self.assertContentBefore(
+            response, "TOPIC B TYPE ANNOUNCED", "TOPIC A TYPE ANNOUNCED"
+        )
+        # Reverses order
+        response = self.client.get(f"/forum/forum/{forum.slug}-{forum.pk}/?o=-2")
+        # Orders of announcement topics stays the same
+        self.assertContentBefore(
+            response, "TOPIC B TYPE ANNOUNCED", "TOPIC A TYPE ANNOUNCED"
+        )
+
+        # Orders on replies column
+        response = self.client.get(f"/forum/forum/{forum.slug}-{forum.pk}/?o=-1")
+        # Shows order is respected on default creation order
+        self.assertContentBefore(
+            response, "TOPIC B TYPE ANNOUNCED", "TOPIC A TYPE ANNOUNCED"
+        )
+
+        # Reverses order
+        response = self.client.get(f"/forum/forum/{forum.slug}-{forum.pk}/?o=1")
+        # Orders of announcement topics stays the same
+        self.assertContentBefore(
+            response, "TOPIC B TYPE ANNOUNCED", "TOPIC A TYPE ANNOUNCED"
+        )