diff --git a/CHANGELOG.md b/CHANGELOG.md index ad10eb9a..7c5c17ac 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/ashley/machina_extensions/forum/views.py b/src/ashley/machina_extensions/forum/views.py index b5a633e1..2a46e8c1 100644 --- a/src/ashley/machina_extensions/forum/views.py +++ b/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.""" diff --git a/src/ashley/templates/forum_conversation/topic_list.html b/src/ashley/templates/forum_conversation/topic_list.html index 77b545e5..3fa2516f 100644 --- a/src/ashley/templates/forum_conversation/topic_list.html +++ b/src/ashley/templates/forum_conversation/topic_list.html @@ -9,23 +9,39 @@
-
+

+ {% if topics == announces %} + {% trans topic_list_title %} + {% else %} {% trans topic_list_title %} {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.0%} + {% endif %}

-
- {% trans "Replies" %} - {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.1%} +
+ {% if topics == announces %} + {% trans "Replies" %} + {% else %} + {% trans "Replies" %} + {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.1%} + {% endif %}
-
+
+ {% if topics == announces %} + {% trans "Views" %} + {% else %} {% trans "Views" %} {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.2%} + {% endif %}
-
+
+ {% if topics == announces %} + {% trans "Last post" %} + {% else %} {% trans "Last post" %} {% include "forum_conversation/partials/topic_list_header_sort.html" with col=header.3%} + {% endif %}
diff --git a/tests/ashley/machina_extensions/forum/test_forum_view.py b/tests/ashley/machina_extensions/forum/test_forum_view.py index fd803eca..19ddebd0 100644 --- a/tests/ashley/machina_extensions/forum/test_forum_view.py +++ b/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): '', 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("