From 92e2c70baf331e8e8bd214dccfaa9785a3fb20ba Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Mon, 10 May 2021 18:28:31 -0700
Subject: [PATCH] Fixes list sorting by rating

---
 bookwyrm/tests/views/test_list.py | 49 +++++++++++++++++++++++++++++++
 bookwyrm/views/list.py            | 28 +++++++-----------
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py
index d767e2b9..d6ad0e86 100644
--- a/bookwyrm/tests/views/test_list.py
+++ b/bookwyrm/tests/views/test_list.py
@@ -138,6 +138,55 @@ class ListViews(TestCase):
         result.render()
         self.assertEqual(result.status_code, 200)
 
+    def test_list_page_sorted(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.List.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            for (i, book) in enumerate([self.book, self.book_two, self.book_three]):
+                models.ListItem.objects.create(
+                    book_list=self.list,
+                    user=self.local_user,
+                    book=book,
+                    approved=True,
+                    order=i + 1,
+                )
+
+        request = self.factory.get("/?sort_by=order")
+        request.user = self.local_user
+        with patch("bookwyrm.views.list.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.list.id)
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+        self.assertEqual(result.status_code, 200)
+
+        request = self.factory.get("/?sort_by=title")
+        request.user = self.local_user
+        with patch("bookwyrm.views.list.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.list.id)
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+        self.assertEqual(result.status_code, 200)
+
+        request = self.factory.get("/?sort_by=rating")
+        request.user = self.local_user
+        with patch("bookwyrm.views.list.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.list.id)
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+        self.assertEqual(result.status_code, 200)
+
+        request = self.factory.get("/?sort_by=sdkfh")
+        request.user = self.local_user
+        with patch("bookwyrm.views.list.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.list.id)
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+        self.assertEqual(result.status_code, 200)
+
     def test_list_page_empty(self):
         """there are so many views, this just makes sure it LOADS"""
         view = views.List.as_view()
diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py
index bfd61790..75bb5d48 100644
--- a/bookwyrm/views/list.py
+++ b/bookwyrm/views/list.py
@@ -5,7 +5,7 @@ from urllib.parse import urlencode
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
 from django.db import IntegrityError, transaction
-from django.db.models import Avg, Count, Q, Max
+from django.db.models import Avg, Count, DecimalField, Q, Max
 from django.db.models.functions import Coalesce
 from django.http import HttpResponseNotFound, HttpResponseBadRequest, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
@@ -108,31 +108,23 @@ class List(View):
         if direction not in ("ascending", "descending"):
             direction = "ascending"
 
-        internal_sort_by = {
+        directional_sort_by = {
             "order": "order",
             "title": "book__title",
             "rating": "average_rating",
-        }
-        directional_sort_by = internal_sort_by[sort_by]
+        }[sort_by]
         if direction == "descending":
             directional_sort_by = "-" + directional_sort_by
 
-        if sort_by == "order":
-            items = book_list.listitem_set.filter(approved=True).order_by(
-                directional_sort_by
-            )
-        elif sort_by == "title":
-            items = book_list.listitem_set.filter(approved=True).order_by(
-                directional_sort_by
-            )
-        elif sort_by == "rating":
-            items = (
-                book_list.listitem_set.annotate(
-                    average_rating=Avg(Coalesce("book__review__rating", 0))
+        items = book_list.listitem_set
+        if sort_by == "rating":
+            items = items.annotate(
+                average_rating=Avg(
+                    Coalesce("book__review__rating", 0.0),
+                    output_field=DecimalField(),
                 )
-                .filter(approved=True)
-                .order_by(directional_sort_by)
             )
+        items = items.filter(approved=True).order_by(directional_sort_by)
 
         paginated = Paginator(items, PAGE_LENGTH)