Merge pull request #923 from SavinaRoja/584-sorting-lists

584 sorting of lists

bookwyrm/forms.py

@@ -3,7 +3,7 @@ import datetime
 from collections import defaultdict
 
 from django import forms
-from django.forms import ModelForm, PasswordInput, widgets
+from django.forms import ModelForm, PasswordInput, widgets, ChoiceField
 from django.forms.widgets import Textarea
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
@@ -287,3 +287,20 @@ class ServerForm(CustomForm):
     class Meta:
         model = models.FederatedServer
         exclude = ["remote_id"]
+
+
+class SortListForm(forms.Form):
+    sort_by = ChoiceField(
+        choices=(
+            ("order", _("List Order")),
+            ("title", _("Book Title")),
+            ("rating", _("Rating")),
+        ),
+        label=_("Sort By"),
+    )
+    direction = ChoiceField(
+        choices=(
+            ("ascending", _("Ascending")),
+            ("descending", _("Descending")),
+        ),
+    )

bookwyrm/migrations/0067_denullify_list_item_order.py

@@ -0,0 +1,30 @@
+from django.db import migrations
+
+
+def forwards_func(apps, schema_editor):
+    # Set all values for ListItem.order
+    BookList = apps.get_model("bookwyrm", "List")
+    db_alias = schema_editor.connection.alias
+    for book_list in BookList.objects.using(db_alias).all():
+        for i, item in enumerate(book_list.listitem_set.order_by("id"), 1):
+            item.order = i
+            item.save()
+
+
+def reverse_func(apps, schema_editor):
+    # null all values for ListItem.order
+    BookList = apps.get_model("bookwyrm", "List")
+    db_alias = schema_editor.connection.alias
+    for book_list in BookList.objects.using(db_alias).all():
+        for item in book_list.listitem_set.order_by("id"):
+            item.order = None
+            item.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0066_user_deactivation_reason"),
+    ]
+
+    operations = [migrations.RunPython(forwards_func, reverse_func)]

bookwyrm/migrations/0068_ordering_for_list_items.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.1.6 on 2021-04-08 16:15
+
+import bookwyrm.models.fields
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0067_denullify_list_item_order"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="listitem",
+            name="order",
+            field=bookwyrm.models.fields.IntegerField(),
+        ),
+        migrations.AlterUniqueTogether(
+            name="listitem",
+            unique_together={("order", "book_list"), ("book", "book_list")},
+        ),
+    ]

bookwyrm/models/list.py

@@ -67,7 +67,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
     )
     notes = fields.TextField(blank=True, null=True)
     approved = models.BooleanField(default=True)
-    order = fields.IntegerField(blank=True, null=True)
+    order = fields.IntegerField()
     endorsement = models.ManyToManyField("User", related_name="endorsers")
 
     activity_serializer = activitypub.ListItem
@@ -93,7 +93,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
             )
 
     class Meta:
-        """ an opinionated constraint! you can't put a book on a list twice """
-
-        unique_together = ("book", "book_list")
+        # A book may only be placed into a list once, and each order in the list may be used only
+        # once
+        unique_together = (("book", "book_list"), ("order", "book_list"))
         ordering = ("-created_date",)

bookwyrm/templates/lists/list.html

@@ -13,10 +13,10 @@
 
 <div class="columns mt-3">
     <section class="column is-three-quarters">
-        {% if not items.exists %}
+        {% if not items.object_list.exists %}
         <p>{% trans "This list is currently empty" %}</p>
         {% else %}
-        <ol>
+        <ol start="{{ items.start_index }}">
         {% for item in items %}
             <li class="block pb-3">
                 <div class="card">
@@ -40,6 +40,16 @@
                             <input type="hidden" name="item" value="{{ item.id }}">
                             <button type="submit" class="button is-small is-danger">{% trans "Remove" %}</button>
                         </form>
+                    </div>
+                    <div class="card-footer has-background-white-bis">
+                        <div>
+                            <form name="set-position" method="post" action="{% url 'list-set-book-position' item.id %}" class="card-footer-item">
+                                {% csrf_token %}
+                                <label for="input-list-position" class="is-sr-only">{% trans "List position" %}</label>
+                                <input id="input-list-position" class="input" type="number" min="1" name="position" value="{{ item.order }}">
+                                <button type="submit" class="button is-small is-info">{% trans "List position" %}</button>
+                            </form>
+                        </div>
                         {% endif %}
                     </div>
                 </div>
@@ -47,10 +57,27 @@
         {% endfor %}
         </ol>
         {% endif %}
+        {% include "snippets/pagination.html" with page=items %}
     </section>
 
-    {% if request.user.is_authenticated and not list.curation == 'closed' or request.user == list.user %}
     <section class="column is-one-quarter content">
+        <h2>{% trans "Sort List" %}</h2>
+        <form name="sort" action="{% url 'list' list.id %}" method="GET" class="block">
+            <label class="label" for="id_sort_by">{% trans "Sort By" %}</label>
+            <div class="select is-fullwidth">
+                {{ sort_form.sort_by }}
+            </div>
+            <label class="label" for="id_direction">{% trans "Direction" %}</label>
+            <div class="select is-fullwidth">
+                {{ sort_form.direction }}
+            </div>
+            <div>
+                <button class="button is-primary is-fullwidth" type="submit">
+                    {% trans "Sort List" %}
+                </button>
+            </div>
+        </form>
+    {% if request.user.is_authenticated and not list.curation == 'closed' or request.user == list.user %}
         <h2>{% if list.curation == 'open' or request.user == list.user %}{% trans "Add Books" %}{% else %}{% trans "Suggest Books" %}{% endif %}</h2>
         <form name="search" action="{% url 'list' list.id %}" method="GET" class="block">
             <div class="field has-addons">
@@ -93,7 +120,7 @@
         </div>
         {% endif %}
         {% endfor %}
-    </section>
     {% endif %}
+    </section>
 </div>
 {% endblock %}

bookwyrm/tests/models/test_list.py

@@ -51,6 +51,7 @@ class List(TestCase):
             book_list=book_list,
             book=self.book,
             user=self.local_user,
+            order=1,
         )
 
         self.assertTrue(item.approved)
@@ -65,7 +66,11 @@ class List(TestCase):
             )
 
         item = models.ListItem.objects.create(
-            book_list=book_list, book=self.book, user=self.local_user, approved=False
+            book_list=book_list,
+            book=self.book,
+            user=self.local_user,
+            approved=False,
+            order=1,
         )
 
         self.assertFalse(item.approved)

bookwyrm/tests/views/inbox/test_inbox_add.py

@@ -94,6 +94,7 @@ class InboxAdd(TestCase):
                 "type": "ListItem",
                 "book": self.book.remote_id,
                 "id": "https://bookwyrm.social/listbook/6189",
+                "order": 1,
             },
             "target": "https://bookwyrm.social/user/mouse/list/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",

bookwyrm/tests/views/inbox/test_inbox_remove.py

@@ -80,6 +80,7 @@ class InboxRemove(TestCase):
                 user=self.local_user,
                 book=self.book,
                 book_list=booklist,
+                order=1,
             )
         self.assertEqual(booklist.books.count(), 1)
 

bookwyrm/tests/views/test_list.py

@@ -39,6 +39,25 @@ class ListViews(TestCase):
             remote_id="https://example.com/book/1",
             parent_work=work,
         )
+        work_two = models.Work.objects.create(title="Labori")
+        self.book_two = models.Edition.objects.create(
+            title="Example Edition 2",
+            remote_id="https://example.com/book/2",
+            parent_work=work_two,
+        )
+        work_three = models.Work.objects.create(title="Trabajar")
+        self.book_three = models.Edition.objects.create(
+            title="Example Edition 3",
+            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
@@ -194,6 +213,7 @@ class ListViews(TestCase):
                 user=self.local_user,
                 book=self.book,
                 approved=False,
+                order=1,
             )
 
         request = self.factory.post(
@@ -208,7 +228,7 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             view(request, self.list.id)
 
-        self.assertEqual(mock.call_count, 1)
+        self.assertEqual(mock.call_count, 2)
         activity = json.loads(mock.call_args[0][1])
         self.assertEqual(activity["type"], "Add")
         self.assertEqual(activity["actor"], self.local_user.remote_id)
@@ -228,6 +248,7 @@ class ListViews(TestCase):
                 user=self.local_user,
                 book=self.book,
                 approved=False,
+                order=1,
             )
 
         request = self.factory.post(
@@ -268,6 +289,261 @@ class ListViews(TestCase):
         self.assertEqual(item.user, self.local_user)
         self.assertTrue(item.approved)
 
+    def test_add_two_books(self):
+        """
+        Putting two books on the list. The first should have an order value of
+        1 and the second should have an order value of 2.
+        """
+        request_one = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "list": self.list.id,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "list": self.list.id,
+            },
+        )
+        request_two.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.add_book(request_one)
+            views.list.add_book(request_two)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+
+    def test_add_three_books_and_remove_second(self):
+        """
+        Put three books on a list and then remove the one in the middle. The
+        ordering of the list should adjust to not have a gap.
+        """
+        request_one = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "list": self.list.id,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "list": self.list.id,
+            },
+        )
+        request_two.user = self.local_user
+
+        request_three = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "list": self.list.id,
+            },
+        )
+        request_three.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.add_book(request_one)
+            views.list.add_book(request_two)
+            views.list.add_book(request_three)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[2].book, self.book_three)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+        self.assertEqual(items[2].order, 3)
+
+        remove_request = self.factory.post("", {"item": items[1].id})
+        remove_request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.remove_book(remove_request, self.list.id)
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_three)
+        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
+        position.
+        """
+        request_one = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "list": self.list.id,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "list": self.list.id,
+            },
+        )
+        request_two.user = self.local_user
+
+        request_three = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "list": self.list.id,
+            },
+        )
+        request_three.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.add_book(request_one)
+            views.list.add_book(request_two)
+            views.list.add_book(request_three)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[2].book, self.book_three)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+        self.assertEqual(items[2].order, 3)
+
+        set_position_request = self.factory.post("", {"position": 1})
+        set_position_request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.set_book_position(set_position_request, items[2].id)
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book_three)
+        self.assertEqual(items[1].book, self.book)
+        self.assertEqual(items[2].book, self.book_two)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+        self.assertEqual(items[2].order, 3)
+
     def test_add_book_outsider(self):
         """ put a book on a list """
         self.list.curation = "open"
@@ -358,6 +634,7 @@ class ListViews(TestCase):
                 book_list=self.list,
                 user=self.local_user,
                 book=self.book,
+                order=1,
             )
         self.assertTrue(self.list.listitem_set.exists())
 
@@ -377,9 +654,7 @@ class ListViews(TestCase):
         """ take an item off a list """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             item = models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
+                book_list=self.list, user=self.local_user, book=self.book, order=1
             )
         self.assertTrue(self.list.listitem_set.exists())
         request = self.factory.post(

bookwyrm/urls.py

@@ -184,6 +184,11 @@ urlpatterns = [
         views.list.remove_book,
         name="list-remove-book",
     ),
+    re_path(
+        r"^list-item/(?P<list_item_id>\d+)/set-position$",
+        views.list.set_book_position,
+        name="list-set-book-position",
+    ),
     re_path(
         r"^list/(?P<list_id>\d+)/curate/?$", views.Curate.as_view(), name="list-curate"
     ),

bookwyrm/views/list.py

@@ -1,9 +1,12 @@
 """ 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
-from django.db.models import Count, Q
-from django.http import HttpResponseNotFound, HttpResponseBadRequest
+from django.db import IntegrityError, transaction
+from django.db.models import Avg, Count, 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
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
@@ -16,6 +19,7 @@ from bookwyrm.connectors import connector_manager
 from .helpers import is_api_request, privacy_filter
 from .helpers import get_user_from_username
 
+
 # pylint: disable=no-self-use
 class Lists(View):
     """ book list page """
@@ -35,7 +39,6 @@ class Lists(View):
             .filter(item_count__gt=0)
             .order_by("-updated_date")
             .distinct()
-            .all()
         )
 
         lists = privacy_filter(
@@ -100,6 +103,47 @@ class List(View):
 
         query = request.GET.get("q")
         suggestions = None
+
+        # sort_by shall be "order" unless a valid alternative is given
+        sort_by = request.GET.get("sort_by", "order")
+        if sort_by not in ("order", "title", "rating"):
+            sort_by = "order"
+
+        # direction shall be "ascending" unless a valid alternative is given
+        direction = request.GET.get("direction", "ascending")
+        if direction not in ("ascending", "descending"):
+            direction = "ascending"
+
+        page = request.GET.get("page", 1)
+
+        internal_sort_by = {
+            "order": "order",
+            "title": "book__title",
+            "rating": "average_rating",
+        }
+        directional_sort_by = internal_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))
+                )
+                .filter(approved=True)
+                .order_by(directional_sort_by)
+            )
+
+        paginated = Paginator(items, 12)
+
         if query and request.user.is_authenticated:
             # search for books
             suggestions = connector_manager.local_search(query, raw=True)
@@ -119,11 +163,14 @@ class List(View):
 
         data = {
             "list": book_list,
-            "items": book_list.listitem_set.filter(approved=True),
+            "items": paginated.get_page(page),
             "pending_count": book_list.listitem_set.filter(approved=False).count(),
             "suggested_books": suggestions,
             "list_form": forms.ListForm(instance=book_list),
             "query": query or "",
+            "sort_form": forms.SortListForm(
+                {"direction": direction, "sort_by": sort_by}
+            ),
         }
         return TemplateResponse(request, "lists/list.html", data)
 
@@ -165,10 +212,21 @@ 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
+            order_max = (
+                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()
         else:
+            deleted_order = suggestion.order
             suggestion.delete(broadcast=False)
+            normalize_book_list_ordering(book_list.id, start=deleted_order)
         return redirect("list-curate", book_list.id)
 
 
@@ -183,19 +241,30 @@ def add_book(request):
     # 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,
                 user=request.user,
+                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,
                 book_list=book_list,
                 user=request.user,
+                order=order_max + 1,
             )
         else:
             # you can't add to this list, what were you THINKING
@@ -209,12 +278,110 @@ def add_book(request):
 
 @require_POST
 def remove_book(request, list_id):
-    """ put a book on a list """
+    """ remove a book from a list """
+    with transaction.atomic():
         book_list = get_object_or_404(models.List, id=list_id)
         item = get_object_or_404(models.ListItem, id=request.POST.get("item"))
 
         if not book_list.user == request.user and not item.user == request.user:
             return HttpResponseNotFound()
 
+        deleted_order = item.order
         item.delete()
+    normalize_book_list_ordering(book_list.id, start=deleted_order)
     return redirect("list", 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.
+    """
+    with transaction.atomic():
+        list_item = get_object_or_404(models.ListItem, id=list_item_id)
+        try:
+            int_position = int(request.POST.get("position"))
+        except ValueError:
+            return HttpResponseBadRequest(
+                "bad value for position. should be an integer"
+            )
+
+        if int_position < 1:
+            return HttpResponseBadRequest("position cannot be less than 1")
+
+        book_list = list_item.book_list
+
+        # 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
+
+        if request.user not in (book_list.user, list_item.user):
+            return HttpResponseNotFound()
+
+        original_order = list_item.order
+        if original_order == int_position:
+            return HttpResponse(status=204)
+        elif original_order > int_position:
+            list_item.order = -1
+            list_item.save()
+            increment_order_in_reverse(book_list.id, int_position, original_order)
+        else:
+            list_item.order = -1
+            list_item.save()
+            decrement_order(book_list.id, original_order, int_position)
+
+        list_item.order = int_position
+        list_item.save()
+
+    return redirect("list", book_list.id)
+
+
+@transaction.atomic
+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)
+    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()
+
+
+@transaction.atomic
+def decrement_order(book_list_id, start, end):
+    try:
+        book_list = models.List.objects.get(id=book_list_id)
+    except models.List.DoesNotExist:
+        return
+    items = book_list.listitem_set.filter(order__gt=start, order__lte=end).order_by(
+        "order"
+    )
+    for item in items:
+        item.order -= 1
+        item.save()
+
+
+@transaction.atomic
+def normalize_book_list_ordering(book_list_id, start=0, add_offset=0):
+    try:
+        book_list = models.List.objects.get(id=book_list_id)
+    except models.List.DoesNotExist:
+        return HttpResponseNotFound()
+    items = book_list.listitem_set.filter(order__gt=start).order_by("order")
+    for i, item in enumerate(items, start):
+        effective_order = i + add_offset
+        if item.order != effective_order:
+            item.order = effective_order
+            item.save()