diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index c9464c85b..2f0e7d678 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -57,7 +57,7 @@
{% endfor %}
{% endif %}
- {% include "snippets/booklist-pagination.html" with page=items %}
+ {% include "snippets/pagination.html" with page=items %}
diff --git a/bookwyrm/templates/snippets/booklist-pagination.html b/bookwyrm/templates/snippets/booklist-pagination.html
deleted file mode 100644
index 161c743cb..000000000
--- a/bookwyrm/templates/snippets/booklist-pagination.html
+++ /dev/null
@@ -1,58 +0,0 @@
-{% load i18n %}
-{% if page.has_other_pages %}
-
-{% endif %}
diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py
index 02581939b..215f3e61e 100644
--- a/bookwyrm/tests/views/test_list.py
+++ b/bookwyrm/tests/views/test_list.py
@@ -51,6 +51,13 @@ class ListViews(TestCase):
remote_id="https://example.com/book/3",
parent_work=work_three,
)
+ work_four = models.Work.objects.create(title="Travailler")
+ self.book_four = models.Edition.objects.create(
+ title="Example Edition 4",
+ remote_id="https://example.com/book/4",
+ parent_work=work_four,
+ )
+
with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
self.list = models.List.objects.create(
name="Test List", user=self.local_user
@@ -369,6 +376,117 @@ class ListViews(TestCase):
self.assertEqual(items[0].order, 1)
self.assertEqual(items[1].order, 2)
+ def test_adding_book_with_a_pending_book(self):
+ """
+ When a list contains any pending books, the pending books should have
+ be at the end of the list by order. If a book is added while a book is
+ pending, its order should precede the pending books.
+ """
+ request = self.factory.post(
+ "",
+ {
+ "book": self.book_three.id,
+ "list": self.list.id,
+ },
+ )
+ request.user = self.local_user
+ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+ models.ListItem.objects.create(
+ book_list=self.list,
+ user=self.local_user,
+ book=self.book,
+ approved=True,
+ order=1,
+ )
+ models.ListItem.objects.create(
+ book_list=self.list,
+ user=self.rat,
+ book=self.book_two,
+ approved=False,
+ order=2,
+ )
+ views.list.add_book(request)
+
+ items = self.list.listitem_set.order_by("order").all()
+ self.assertEqual(items[0].book, self.book)
+ self.assertEqual(items[0].order, 1)
+ self.assertTrue(items[0].approved)
+
+ self.assertEqual(items[1].book, self.book_three)
+ self.assertEqual(items[1].order, 2)
+ self.assertTrue(items[1].approved)
+
+ self.assertEqual(items[2].book, self.book_two)
+ self.assertEqual(items[2].order, 3)
+ self.assertFalse(items[2].approved)
+
+ def test_approving_one_pending_book_from_multiple(self):
+ """
+ When a list contains any pending books, the pending books should have
+ be at the end of the list by order. If a pending book is approved, then
+ its order should be at the end of the approved books and before the
+ remaining pending books.
+ """
+ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+ models.ListItem.objects.create(
+ book_list=self.list,
+ user=self.local_user,
+ book=self.book,
+ approved=True,
+ order=1,
+ )
+ models.ListItem.objects.create(
+ book_list=self.list,
+ user=self.local_user,
+ book=self.book_two,
+ approved=True,
+ order=2,
+ )
+ models.ListItem.objects.create(
+ book_list=self.list,
+ user=self.rat,
+ book=self.book_three,
+ approved=False,
+ order=3,
+ )
+ to_be_approved = models.ListItem.objects.create(
+ book_list=self.list,
+ user=self.rat,
+ book=self.book_four,
+ approved=False,
+ order=4,
+ )
+
+ view = views.Curate.as_view()
+ request = self.factory.post(
+ "",
+ {
+ "item": to_be_approved.id,
+ "approved": "true",
+ },
+ )
+ request.user = self.local_user
+
+ with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+ view(request, self.list.id)
+
+ items = self.list.listitem_set.order_by("order").all()
+ self.assertEqual(items[0].book, self.book)
+ self.assertEqual(items[0].order, 1)
+ self.assertTrue(items[0].approved)
+
+ self.assertEqual(items[1].book, self.book_two)
+ self.assertEqual(items[1].order, 2)
+ self.assertTrue(items[1].approved)
+
+ self.assertEqual(items[2].book, self.book_four)
+ self.assertEqual(items[2].order, 3)
+ self.assertTrue(items[2].approved)
+
+ self.assertEqual(items[3].book, self.book_three)
+ self.assertEqual(items[3].order, 4)
+ self.assertFalse(items[3].approved)
+
def test_add_three_books_and_move_last_to_first(self):
"""
Put three books on the list and move the last book to the first
diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py
index 2f01410d2..7fb4979cb 100644
--- a/bookwyrm/views/list.py
+++ b/bookwyrm/views/list.py
@@ -1,4 +1,6 @@
""" book list views"""
+from typing import Optional
+
from django.contrib.auth.decorators import login_required
from django.core.paginator import Paginator
from django.db import IntegrityError, transaction
@@ -140,7 +142,7 @@ class List(View):
.order_by(directional_sort_by)
)
- paginated = Paginator(items, 25)
+ paginated = Paginator(items, 12)
if query and request.user.is_authenticated:
# search for books
@@ -210,14 +212,17 @@ class Curate(View):
suggestion = get_object_or_404(models.ListItem, id=request.POST.get("item"))
approved = request.POST.get("approved") == "true"
if approved:
+ # update the book and set it to be the last in the order of approved books, before any pending books
suggestion.approved = True
- current_order = suggestion.order
order_max = (
- book_list.listitem_set.aggregate(Max("order"))["order__max"] or 0
+ book_list.listitem_set.filter(approved=True).aggregate(Max("order"))[
+ "order__max"
+ ]
+ or 0
) + 1
suggestion.order = order_max
+ increment_order_in_reverse(book_list.id, order_max)
suggestion.save()
- normalize_book_list_ordering(book_list.id, start=current_order)
else:
deleted_order = suggestion.order
suggestion.delete(broadcast=False)
@@ -232,13 +237,17 @@ def add_book(request):
if not book_list.visible_to_user(request.user):
return HttpResponseNotFound()
- order_max = book_list.listitem_set.aggregate(Max("order"))["order__max"] or 0
-
book = get_object_or_404(models.Edition, id=request.POST.get("book"))
# do you have permission to add to the list?
try:
if request.user == book_list.user or book_list.curation == "open":
- # go ahead and add it
+ # add the book at the latest order of approved books, before any pending books
+ order_max = (
+ book_list.listitem_set.filter(approved=True).aggregate(Max("order"))[
+ "order__max"
+ ]
+ ) or 0
+ increment_order_in_reverse(book_list.id, order_max + 1)
models.ListItem.objects.create(
book=book,
book_list=book_list,
@@ -246,7 +255,10 @@ def add_book(request):
order=order_max + 1,
)
elif book_list.curation == "curated":
- # make a pending entry
+ # make a pending entry at the end of the list
+ order_max = (
+ book_list.listitem_set.aggregate(Max("order"))["order__max"]
+ ) or 0
models.ListItem.objects.create(
approved=False,
book=book,
@@ -283,7 +295,8 @@ def remove_book(request, list_id):
@require_POST
def set_book_position(request, list_item_id):
"""
- Action for when the list user manually specifies a list position, takes special care with the unique ordering per list
+ Action for when the list user manually specifies a list position, takes
+ special care with the unique ordering per list.
"""
with transaction.atomic():
list_item = get_object_or_404(models.ListItem, id=list_item_id)
@@ -298,7 +311,12 @@ def set_book_position(request, list_item_id):
return HttpResponseBadRequest("position cannot be less than 1")
book_list = list_item.book_list
- order_max = book_list.listitem_set.aggregate(Max("order"))["order__max"]
+
+ # the max position to which a book may be set is the highest order for
+ # books which are approved
+ order_max = book_list.listitem_set.filter(approved=True).aggregate(
+ Max("order")
+ )["order__max"]
if int_position > order_max:
int_position = order_max
@@ -325,14 +343,17 @@ def set_book_position(request, list_item_id):
@transaction.atomic
-def increment_order_in_reverse(book_list_id, start, end):
+def increment_order_in_reverse(
+ book_list_id: int, start: int, end: Optional[int] = None
+):
try:
book_list = models.List.objects.get(id=book_list_id)
except models.List.DoesNotExist:
return
- items = book_list.listitem_set.filter(order__gte=start, order__lt=end).order_by(
- "-order"
- )
+ items = book_list.listitem_set.filter(order__gte=start)
+ if end is not None:
+ items = items.filter(order__lt=end)
+ items = items.order_by("-order")
for item in items:
item.order += 1
item.save()