[DONE] Improve sort & menu order (EsupPortail#1003)

* Improve video sort

* Improve playlist sort

* Fix titles

* Reorganize menu buttons

* Fix playlist test case

* Improve playlist page tests

* Add tests for playlists and flake8

pod/main/static/js/filter-aside-element-list-refresh.js

@@ -21,7 +21,7 @@ function updateSortDirectionTitle(sortDirectionAsc) {
   let newTitle = sortDirectionTitle[+sortDirectionAsc];
   document
     .getElementById("sort_direction_label")
-    .setAttribute("title", newTitle);
+    .setAttribute("data-bs-original-title", newTitle);
 }
 
 /**

pod/main/templates/navbar.html

@@ -61,7 +61,12 @@ <h5 class="offcanvas-title" id="pod-navbar__menuLabel">{% trans "Main menu" %}</
           </li>
           <li>
             <span class="progress">
-              <span class="text-bg-primary progress-bar progress-bar-striped progress-bar-animated" role="progressbar" aria-valuenow="75" aria-valuemin="0" aria-valuemax="100"></span>
+              <span class="text-bg-primary progress-bar progress-bar-striped progress-bar-animated"
+                role="progressbar"
+                aria-valuenow="75"
+                aria-valuemin="0"
+                aria-valuemax="100">
+              </span>
             </span>
           </li>
         {% endif %}

pod/playlist/static/playlist/js/filter-aside-playlist-list-refresh.js

@@ -47,7 +47,7 @@ function refreshPlaylistsSearch() {
 }
 
 let playlist_list = document.getElementById("playlists_list");
-if (playlist_list.dataset.numberVideos >= 2) {
+if (playlist_list.dataset.numberPlaylists >= 2) {
     // Add trigger event to manage sort direction.
     document
         .getElementById('sort_direction_label')

pod/playlist/templates/playlist/filter_aside.html

@@ -9,7 +9,8 @@ <h2 class="h3 card-title pod-card__title">
       id="resetFilters"
       href="{% url 'playlist:list' %}"
       class="btn btn-outline-primary"
-      title="{% trans "Reset the filters" %}"
+      title="{% trans 'Reset the filters' %}"
+      data-bs-toggle="tooltip"
     >
       {% trans 'Reset' %}
     </a>

pod/playlist/templates/playlist/playlists.html

@@ -35,7 +35,7 @@ <h2 class="page_title col-xxl-4 mb-2">{% trans "No playlists found"%}</h2>
     </a>
   </div>
 
-  <div class="infinite-container pod-infinite-container" id="playlists_list" data-number-video={{ playlists|length }}>
+  <div class="infinite-container pod-infinite-container" id="playlists_list" data-number-playlists={{ playlists|length }}>
     {% for playlist in playlists %}
       <div class="infinite-item card-group">
         {% include "playlist/playlist_card.html" %}

pod/playlist/tests/test_views.py

@@ -3,6 +3,7 @@
 *  run with 'python manage.py test pod.playlist.tests.test_views'
 """
 from django.contrib.auth.models import User
+from django.http import JsonResponse
 from django.urls import reverse
 from django.utils.translation import ugettext as _
 from django.test import override_settings, TestCase
@@ -86,6 +87,7 @@ def setUp(self) -> None:
             name="Second public playlist",
             visibility="public",
             owner=self.second_user,
+            promoted=True,
         )
         self.playlist_content_1 = PlaylistContent.objects.create(
             playlist=self.public_playlist,
@@ -110,8 +112,8 @@ def setUp(self) -> None:
         self.url = reverse("playlist:list")
 
     @override_settings(USE_PLAYLIST=True)
-    def test_video_counter(self) -> None:
-        """Test if the video counter works correctly."""
+    def test_playlist_card_counter(self) -> None:
+        """Test if the number of cards in playlist list page is correct."""
         importlib.reload(context_processors)
         self.client.force_login(self.first_user)
         response = self.client.get(self.url)
@@ -120,24 +122,12 @@ def test_video_counter(self) -> None:
             200,
             "Test if status code equal 200.",
         )
-        self.assertTrue("4" in response.content.decode(), "Test if '4' is visible.")
-        self.client.logout()
-        print(" --->  test_video_counter ok")
-
-    @override_settings(USE_PLAYLIST=True)
-    def test_playlist_counter(self) -> None:
-        """Test if the playlist counter works correctly."""
-        importlib.reload(context_processors)
-        self.client.force_login(self.first_user)
-        response = self.client.get(self.url)
-        self.assertEqual(
-            response.status_code,
-            200,
-            "Test if status code equal 200.",
+        self.assertTrue(
+            "data-number-playlists=5" in response.content.decode(),
+            "Test if there's 5 playlists visible in the playlist page.",
         )
-        self.assertTrue("4" in response.content.decode(), "Test if '4' is visible.")
         self.client.logout()
-        print(" --->  test_playlist_counter ok")
+        print(" --->  test_playlist_card_counter ok")
 
     @override_settings(USE_PLAYLIST=True)
     def test_card_titles(self) -> None:
@@ -178,6 +168,10 @@ def test_private_filter(self) -> None:
             "bi-globe-americas" in response.content.decode(),
             "Test if public icon isn't visible.",
         )
+        self.assertTrue(
+            "data-number-playlists=2" in response.content.decode(),
+            "Test if there's 2 private playlists in the playlist page.",
+        )
         self.client.logout()
         print(" --->  test_private_filter ok")
 
@@ -202,6 +196,10 @@ def test_protected_filter(self) -> None:
             "bi-globe-americas" in response.content.decode(),
             "Test if public icon isn't visible.",
         )
+        self.assertTrue(
+            "data-number-playlists=1" in response.content.decode(),
+            "Test if there's 1 protected playlist in the playlist page.",
+        )
         self.client.logout()
         print(" --->  test_protected_filter ok")
 
@@ -225,9 +223,71 @@ def test_public_filter(self) -> None:
         self.assertFalse(
             "bi-lock" in response.content.decode(), "Test if protected icon isn't visible"
         )
+        self.assertTrue(
+            "data-number-playlists=1" in response.content.decode(),
+            "Test if there's 1 playlist visible in the playlist page.",
+        )
         self.client.logout()
         print(" --->  test_public_filter ok")
 
+    @override_settings(USE_PLAYLIST=True)
+    def test_allpublic_filter(self) -> None:
+        """
+        Test if all public playlists are visible when the allpublic filter is active.
+        """
+        importlib.reload(context_processors)
+        self.client.force_login(self.first_user)
+        response = self.client.get(f"{self.url}?visibility=allpublic")
+        self.assertEqual(
+            response.status_code,
+            200,
+            "Test if status code equal 200.",
+        )
+        self.assertFalse(
+            "bi-incognito" in response.content.decode(),
+            "Test if private icon isn't visible.",
+        )
+        self.assertFalse(
+            "bi-lock" in response.content.decode(), "Test if protected icon isn't visible"
+        )
+        self.assertTrue(
+            "data-number-playlists=2" in response.content.decode(),
+            "Test if there's 2 playlists visible in the playlist page.",
+        )
+        self.client.logout()
+        print(" --->  test_allpublic_filter ok")
+
+    @override_settings(USE_PLAYLIST=True)
+    def test_promoted_filter(self) -> None:
+        """
+        Test if all promoted playlist are visible when the promoted filter is active.
+        """
+        importlib.reload(context_processors)
+        self.client.force_login(self.first_user)
+        response = self.client.get(f"{self.url}?visibility=promoted")
+        self.assertEqual(
+            response.status_code,
+            200,
+            "Test if status code equal 200.",
+        )
+        self.assertFalse(
+            "bi-incognito" in response.content.decode(),
+            "Test if private icon isn't visible.",
+        )
+        self.assertFalse(
+            "bi-lock" in response.content.decode(), "Test if protected icon isn't visible"
+        )
+        self.assertTrue(
+            "promoted-icon" in response.content.decode(),
+            "Test if promoted icon is visible.",
+        )
+        self.assertTrue(
+            "data-number-playlists=1" in response.content.decode(),
+            "Test if there's 1 promoted playlist in the playlist page.",
+        )
+        self.client.logout()
+        print(" --->  test_promoted_filter ok")
+
 
 class TestPlaylistsPageLinkTestCase(TestCase):
     """Playlists page link test case."""
@@ -488,7 +548,6 @@ def test_link_in_playlists_page(self) -> None:
             200,
             "Test if status code equal 200.",
         )
-        print(response.content.decode())
         self.assertTrue(
             f"/playlist/start-playlist/{self.simple_playlist.slug}"
             in response.content.decode(),
@@ -569,6 +628,12 @@ def setUp(self) -> None:
             kwargs={"slug": get_favorite_playlist_for_user(self.user2).slug},
         )
 
+        self.playlist_content_1 = PlaylistContent.objects.create(
+            playlist=self.playlist_user1,
+            video=self.video,
+            rank=1,
+        )
+
     @override_settings(USE_FAVORITES=True)
     def test_playlist_video_list(self):
         """Test if the favorite video list has a correct number of video in it."""
@@ -659,6 +724,74 @@ def test_manage_section_for_editable_playlists(self) -> None:
         self.client.logout()
         print(" --->  test_manage_section_for_editable_playlists ok")
 
+    @override_settings(USE_PLAYLIST=True)
+    def test_remove_video_in_playlist(self):
+        """Test if remove a video from a playlist works."""
+        importlib.reload(context_processors)
+        self.client.force_login(self.user)
+        url_content = reverse(
+            "playlist:content", kwargs={"slug": self.playlist_user1.slug}
+        )
+        response = self.client.get(url_content)
+        self.assertTrue(
+            'data-countvideos="1"' in response.content.decode(),
+            "Test if there's 1 video in the playlist.",
+        )
+        url = reverse(
+            "playlist:remove-video",
+            kwargs={
+                "slug": self.playlist_user1.slug,
+                "video_slug": self.video.slug,
+            },
+        )
+        response = self.client.get(url, HTTP_REFERER=url_content)
+        self.assertEqual(response.status_code, 302)
+
+        redirected_url = response.url
+        response = self.client.get(redirected_url)
+        self.assertTrue(
+            'data-countvideos="0"' in response.content.decode(),
+            "Test if there's 0 video in the playlist.",
+        )
+        print(" --->  test_remove_video_in_playlist ok")
+
+    @override_settings(USE_PLAYLIST=True)
+    def test_remove_video_in_playlist_json(self):
+        """Test if remove a video from a playlist works with JSON."""
+        importlib.reload(context_processors)
+        self.client.force_login(self.user)
+        url_content = reverse(
+            "playlist:content", kwargs={"slug": self.playlist_user1.slug}
+        )
+        response = self.client.get(url_content)
+        self.assertTrue(
+            'data-countvideos="1"' in response.content.decode(),
+            "Test if there's 1 video in the playlist.",
+        )
+
+        url = (
+            reverse(
+                "playlist:remove-video",
+                kwargs={
+                    "slug": self.playlist_user1.slug,
+                    "video_slug": self.video.slug,
+                },
+            )
+            + "?json=1"
+        )
+
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        data = JsonResponse({"state": "out-playlist"}).content.decode("utf-8")
+        self.assertEqual(response.content.decode("utf-8"), data)
+
+        response = self.client.get(url_content)
+        self.assertTrue(
+            'data-countvideos="0"' in response.content.decode(),
+            "Test if there's 0 video in the playlist.",
+        )
+        print(" --->  test_remove_video_in_playlist_json ok")
+
 
 class TestStatsInfoTestCase(TestCase):
     """Statistics informations test case."""

pod/playlist/utils.py

@@ -1,6 +1,7 @@
 """Esup-Pod playlist utilities."""
 from django.contrib.auth.models import User
 from django.contrib.sites.models import Site
+from django.db.models.functions import Lower
 from django.db.models import Max
 from django.urls import reverse
 from django.core.handlers.wsgi import WSGIRequest
@@ -350,18 +351,24 @@ def sort_playlist_list(playlist_list: list, sort_field: str, sort_direction="")
     Returns:
         list (:class:`list(pod.playlist.models.Playlist)`): The list of playlist
     """
-    if sort_field and sort_field in [
+    if sort_field and sort_field in {
         "id",
         "name",
         "visibility",
         "slug",
         "owner",
         "date_created",
         "date_updated",
-    ]:
-        if not sort_direction:
+    }:
+        if sort_field in {"name"}:
+            sort_field = Lower(sort_field)
+            if not sort_direction:
+                sort_field = sort_field.desc()
+
+        elif not sort_direction:
             sort_field = "-" + sort_field
         playlist_list = playlist_list.order_by(sort_field)
+
     return playlist_list.distinct()
 
 

pod/video/utils.py

@@ -1,4 +1,5 @@
 """Esup-Pod video utilities."""
+from django.db.models.functions import Lower
 import os
 import re
 import shutil
@@ -201,7 +202,7 @@ def sort_videos_list(videos_list, sort_field, sort_direction=""):
 
     Sorted by specific column name and ascending or descending direction
     """
-    if sort_field and sort_field in [
+    if sort_field and sort_field in {
         "category",
         "channel",
         "cursus",
@@ -224,10 +225,16 @@ def sort_videos_list(videos_list, sort_field, sort_direction=""):
         "type",
         "viewcount",
         "rank",
-    ]:
-        if not sort_direction:
+    }:
+        if sort_field in {"title", "title_fr", "title_en"}:
+            sort_field = Lower(sort_field)
+            if not sort_direction:
+                sort_field = sort_field.desc()
+
+        elif not sort_direction:
             sort_field = "-" + sort_field
         videos_list = videos_list.order_by(sort_field)
+
     return videos_list.distinct()