Merge pull request #1852 from bookwyrm-social/shelf-fixes

Shelf date on all books page

bookwyrm/templates/shelf/shelf.html

@@ -92,7 +92,7 @@
                 </span>
                 {% with count=books.paginator.count %}
                 {% if count %}
-                <p class="help">
+                <span class="help">
                     {% blocktrans trimmed count counter=count with formatted_count=count|intcomma %}
                         {{ formatted_count }} book
                     {% plural %}
@@ -104,7 +104,7 @@
                         (showing {{ start }}-{{ end }})
                         {% endblocktrans %}
                     {% endif %}
-                </p>
+                </span>
                 {% endif %}
                 {% endwith %}
             </h2>

bookwyrm/templates/snippets/shelf_selector.html

@@ -17,7 +17,7 @@
 
 {% if shelf.editable %}
 <li role="menuitem" class="dropdown-item p-0">
-    <form name="shelve" action="/shelve/" method="post">
+    <form name="editable-shelve-{{ uuid }}" action="/shelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="change-shelf-from" value="{{ current.identifier }}">
@@ -67,7 +67,7 @@
 {% if user_shelf in book.shelves.all %}
 <li class="navbar-divider m-0" role="separator" ></li>
 <li role="menuitem" class="dropdown-item p-0">
-    <form name="shelve" action="/unshelve/" method="post">
+    <form name="shelve-{{ user_shelf.identifier }}-{{ book.id }}-{{ uuid }}" action="/unshelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="shelf" value="{{ user_shelf.id }}">
@@ -79,7 +79,7 @@
 {% else %}
 <li class="navbar-divider" role="separator" ></li>
 <li role="menuitem" class="dropdown-item p-0">
-    <form name="shelve" action="/unshelve/" method="post">
+    <form name="un-shelve-{{ shelf.identifier }}-{{ book.id }}-{{ uuid }}" action="/unshelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="shelf" value="{{ shelf.id }}">

bookwyrm/templates/snippets/shelve_button/shelve_button_dropdown_options.html

@@ -58,7 +58,7 @@
 
 {% if active_shelf.shelf %}
 <li role="menuitem" class="dropdown-item p-0" data-extra-options>
-    <form  name="shelve" action="/unshelve/" method="post">
+    <form name="unshelve-{{ uuid }}-{{ shelf.identifier }}" action="/unshelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ active_shelf.book.id }}">
         <input type="hidden" name="shelf" value="{{ active_shelf.shelf.id }}">

bookwyrm/tests/views/shelf/test_shelf.py

@@ -51,6 +51,11 @@ class ShelfViews(TestCase):
 
     def test_shelf_page_all_books(self, *_):
         """there are so many views, this just makes sure it LOADS"""
+        models.ShelfBook.objects.create(
+            book=self.book,
+            shelf=self.shelf,
+            user=self.local_user,
+        )
         view = views.Shelf.as_view()
         request = self.factory.get("")
         request.user = self.local_user
@@ -61,6 +66,41 @@ class ShelfViews(TestCase):
         validate_html(result.render())
         self.assertEqual(result.status_code, 200)
 
+    def test_shelf_page_all_books_empty(self, *_):
+        """No books shelved"""
+        view = views.Shelf.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.local_user.username)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+    def test_shelf_page_all_books_avoid_duplicates(self, *_):
+        """Make sure books aren't showing up twice on the all shelves view"""
+        models.ShelfBook.objects.create(
+            book=self.book,
+            shelf=self.shelf,
+            user=self.local_user,
+        )
+        models.ShelfBook.objects.create(
+            book=self.book,
+            shelf=self.local_user.shelf_set.first(),
+            user=self.local_user,
+        )
+        view = views.Shelf.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.local_user.username)
+        self.assertEqual(result.context_data["books"].object_list.count(), 1)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
     def test_shelf_page_all_books_json(self, *_):
         """there is no json view here"""
         view = views.Shelf.as_view()

bookwyrm/views/shelf/shelf.py

@@ -1,7 +1,7 @@
 """ shelf views """
 from collections import namedtuple
 
-from django.db.models import OuterRef, Subquery, F
+from django.db.models import OuterRef, Subquery, F, Max
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
 from django.http import HttpResponseBadRequest
@@ -72,11 +72,7 @@ class Shelf(View):
             "start_date"
         )
 
-        if shelf_identifier:
-            books = books.annotate(shelved_date=F("shelfbook__shelved_date"))
-        else:
-            # sorting by shelved date will cause duplicates in the "all books" view
-            books = books.annotate(shelved_date=F("updated_date"))
+        books = books.annotate(shelved_date=Max("shelfbook__shelved_date"))
         books = books.annotate(
             rating=Subquery(reviews.values("rating")[:1]),
             start_date=Subquery(reading.values("start_date")[:1]),