From d0a4c78d02d867fe267b2f723ddfa7637e8ed4d6 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Mon, 24 Jan 2022 12:01:17 -0800
Subject: [PATCH 01/22] Limit length on list item note

---
 .../migrations/0130_alter_listitem_notes.py   | 21 +++++++++++++++++++
 bookwyrm/models/list.py                       |  2 +-
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 bookwyrm/migrations/0130_alter_listitem_notes.py

diff --git a/bookwyrm/migrations/0130_alter_listitem_notes.py b/bookwyrm/migrations/0130_alter_listitem_notes.py
new file mode 100644
index 000000000..a12efd40d
--- /dev/null
+++ b/bookwyrm/migrations/0130_alter_listitem_notes.py
@@ -0,0 +1,21 @@
+# Generated by Django 3.2.10 on 2022-01-24 20:01
+
+import bookwyrm.models.fields
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0129_auto_20220117_1716"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="listitem",
+            name="notes",
+            field=bookwyrm.models.fields.TextField(
+                blank=True, max_length=300, null=True
+            ),
+        ),
+    ]
diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py
index d159bc4a8..733ee0447 100644
--- a/bookwyrm/models/list.py
+++ b/bookwyrm/models/list.py
@@ -125,7 +125,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel):
     user = fields.ForeignKey(
         "User", on_delete=models.PROTECT, activitypub_field="actor"
     )
-    notes = fields.TextField(blank=True, null=True)
+    notes = fields.TextField(blank=True, null=True, max_length=300)
     approved = models.BooleanField(default=True)
     order = fields.IntegerField()
     endorsement = models.ManyToManyField("User", related_name="endorsers")

From 5206d08dbb0d563a2d48ef6193a6d9c94f633fd5 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Mon, 24 Jan 2022 13:37:51 -0800
Subject: [PATCH 02/22] Adds demo display of comment in list entry

---
 bookwyrm/templates/lists/list.html      | 36 +++++++++++++++++--------
 bookwyrm/templates/snippets/avatar.html |  8 ++++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index c44d3fe36..a1da21e57 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -4,6 +4,7 @@
 {% load book_display_tags %}
 {% load group_tags %}
 {% load markdown %}
+{% load utilities %}
 
 {% block breadcrumbs %}
 <nav class="breadcrumb subtitle" aria-label="breadcrumbs">
@@ -46,21 +47,16 @@
         {% for item in items %}
             <li class="block mb-5">
                 <div class="card">
-                    {% with book=item.book %}
-                        <div
-                            class="
-                                card-content p-0 mb-0
-                                columns is-gapless
-                                is-mobile
-                            "
-                        >
-                            <div class="column is-3-mobile is-2-tablet is-cover align to-t">
+                    <div class="card-content">
+                        {% with book=item.book %}
+                        <div class="columns is-mobile">
+                            <div class="column is-narrow is-cover">
                                 <a href="{{ item.book.local_path }}" aria-hidden="true">
                                     {% include 'snippets/book_cover.html' with cover_class='is-w-auto is-h-m-tablet is-align-items-flex-start' size='medium' %}
                                 </a>
                             </div>
 
-                            <div class="column mx-3 my-2">
+                            <div class="column">
                                 <p>
                                     {% include 'snippets/book_titleby.html' %}
                                 </p>
@@ -73,8 +69,26 @@
                                 {% include 'snippets/shelve_button/shelve_button.html' with book=book %}
                             </div>
                         </div>
-                    {% endwith %}
+                        {% endwith %}
 
+                        <div class="media notification">
+                            <figure class="media-left">
+                                {% include "snippets/avatar.html" with user=item.user %}
+                            </figure>
+                            <div class="media-content">
+                                <div class="content">
+                                    <header>
+                                        <a href="{% url 'user-feed' user|username %}">
+                                            {{ item.user.display_name }}
+                                        </a>
+                                    </header>
+                                    <p>
+                                        Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin ornare magna eros, eu pellentesque tortor vestibulum ut. Maecenas non massa sem. Etiam finibus odio quis feugiat facilisis.
+                                    </p>
+                                </div>
+                            </div>
+                        </div>
+                    </div>
                     <div class="card-footer is-stacked-mobile has-background-white-bis is-align-items-stretch">
                         <div class="card-footer-item">
                             <p>
diff --git a/bookwyrm/templates/snippets/avatar.html b/bookwyrm/templates/snippets/avatar.html
index 5f28cd4be..48b14ab99 100644
--- a/bookwyrm/templates/snippets/avatar.html
+++ b/bookwyrm/templates/snippets/avatar.html
@@ -1,4 +1,8 @@
 {% load static %}
 
-<img class="avatar image {% if large %}is-96x96{% elif medium %}is-48x48{% else %}is-32x32{% endif %}" src="{% if user.avatar %}{% get_media_prefix %}{{ user.avatar }}{% else %}{% static "images/default_avi.jpg" %}{% endif %}" {% if ariaHide %}aria-hidden="true"{% endif %} alt="{{ user.alt_text }}">
-
+<img
+    class="avatar image {% if large %}is-96x96{% elif medium %}is-48x48{% else %}is-32x32{% endif %}"
+    src="{% if user.avatar %}{% get_media_prefix %}{{ user.avatar }}{% else %}{% static "images/default_avi.jpg" %}{% endif %}"
+    {% if ariaHide %}aria-hidden="true"{% endif %}
+    alt="{{ user.alt_text }}"
+>

From 8280386f0e98178f886ca6882fc09de400123a72 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Mon, 24 Jan 2022 13:45:52 -0800
Subject: [PATCH 03/22] Context for note block in list item

---
 bookwyrm/templates/lists/list.html | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index a1da21e57..ca2b99390 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -78,9 +78,10 @@
                             <div class="media-content">
                                 <div class="content">
                                     <header>
-                                        <a href="{% url 'user-feed' user|username %}">
-                                            {{ item.user.display_name }}
-                                        </a>
+                                        {% url 'user-feed' user|username as user_path %}
+                                        {% blocktrans trimmed with username=user|username %}
+                                        <a href="{{ user_path }}">{{ username }}</a> says:
+                                        {% endblocktrans %}
                                     </header>
                                     <p>
                                         Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin ornare magna eros, eu pellentesque tortor vestibulum ut. Maecenas non massa sem. Etiam finibus odio quis feugiat facilisis.

From a1a84b28b05122eafcd63b1ebb538793b9114fb4 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Mon, 24 Jan 2022 13:56:18 -0800
Subject: [PATCH 04/22] Adds edit box

---
 bookwyrm/templates/lists/list.html | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index ca2b99390..73172bd54 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -71,6 +71,7 @@
                         </div>
                         {% endwith %}
 
+                        {% if item.note or item.user == request.user %}
                         <div class="media notification">
                             <figure class="media-left">
                                 {% include "snippets/avatar.html" with user=item.user %}
@@ -87,8 +88,33 @@
                                         Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin ornare magna eros, eu pellentesque tortor vestibulum ut. Maecenas non massa sem. Etiam finibus odio quis feugiat facilisis.
                                     </p>
                                 </div>
+                                {% if item.user == request.user %}
+                                <div>
+                                    <details class="details-panel box">
+                                        <summary>
+                                            {% trans "Edit:" %}
+                                            <span class="details-close icon icon-pencil" aria-hidden></span>
+                                        </summary>
+                                        <form>
+                                            <div class="field">
+                                                <div class="control">
+                                                    <textarea class="textarea">{{ item.note }}</textarea>
+                                                </div>
+                                            </div>
+                                            <div class="field">
+                                                <div class="control">
+                                                    <button type="submit" class="button is-success">
+                                                        {% trans "Save" %}
+                                                    </button>
+                                                </div>
+                                            </div>
+                                        </form>
+                                    </details>
+                                </div>
+                                {% endif %}
                             </div>
                         </div>
+                        {% endif %}
                     </div>
                     <div class="card-footer is-stacked-mobile has-background-white-bis is-align-items-stretch">
                         <div class="card-footer-item">

From ca0dbeb0ab8413978e4e2cd7288bd779d5decbe9 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Mon, 24 Jan 2022 17:41:21 -0800
Subject: [PATCH 05/22] Move list views into more files

---
 bookwyrm/urls.py                  |   6 +-
 bookwyrm/views/__init__.py        |  16 ++++-
 bookwyrm/views/list/curate.py     |  55 ++++++++++++++
 bookwyrm/views/{ => list}/list.py | 115 +-----------------------------
 bookwyrm/views/list/lists.py      |  80 +++++++++++++++++++++
 5 files changed, 153 insertions(+), 119 deletions(-)
 create mode 100644 bookwyrm/views/list/curate.py
 rename bookwyrm/views/{ => list}/list.py (76%)
 create mode 100644 bookwyrm/views/list/lists.py

diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py
index 7cdfd92a0..01e864110 100644
--- a/bookwyrm/urls.py
+++ b/bookwyrm/urls.py
@@ -358,15 +358,15 @@ urlpatterns = [
     re_path(r"^list/saved/?$", views.SavedLists.as_view(), name="saved-lists"),
     re_path(r"^list/(?P<list_id>\d+)(.json)?/?$", views.List.as_view(), name="list"),
     re_path(r"^list/delete/(?P<list_id>\d+)/?$", views.delete_list, name="delete-list"),
-    re_path(r"^list/add-book/?$", views.list.add_book, name="list-add-book"),
+    re_path(r"^list/add-book/?$", views.add_book, name="list-add-book"),
     re_path(
         r"^list/(?P<list_id>\d+)/remove/?$",
-        views.list.remove_book,
+        views.remove_book,
         name="list-remove-book",
     ),
     re_path(
         r"^list-item/(?P<list_item_id>\d+)/set-position$",
-        views.list.set_book_position,
+        views.set_book_position,
         name="list-set-book-position",
     ),
     re_path(
diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py
index 3f57c274a..da9098dc3 100644
--- a/bookwyrm/views/__init__.py
+++ b/bookwyrm/views/__init__.py
@@ -61,6 +61,20 @@ from .imports.manually_review import (
     delete_import_item,
 )
 
+# lists
+from .list.curate import Curate
+from .list.lists import Lists, SavedLists, UserLists
+from .list.list import (
+    List,
+    save_list,
+    unsave_list,
+    delete_list,
+    unsafe_embed_list,
+    add_book,
+    remove_book,
+    set_book_position,
+)
+
 # misc views
 from .author import Author, EditAuthor, update_author_from_remote
 from .directory import Directory
@@ -90,8 +104,6 @@ from .group import (
 from .inbox import Inbox
 from .interaction import Favorite, Unfavorite, Boost, Unboost
 from .isbn import Isbn
-from .list import Lists, SavedLists, List, Curate, UserLists
-from .list import save_list, unsave_list, delete_list, unsafe_embed_list
 from .notifications import Notifications
 from .outbox import Outbox
 from .reading import ReadThrough, delete_readthrough, delete_progressupdate
diff --git a/bookwyrm/views/list/curate.py b/bookwyrm/views/list/curate.py
new file mode 100644
index 000000000..924f37709
--- /dev/null
+++ b/bookwyrm/views/list/curate.py
@@ -0,0 +1,55 @@
+""" book list views"""
+from django.contrib.auth.decorators import login_required
+from django.db.models import Max
+from django.shortcuts import get_object_or_404, redirect
+from django.template.response import TemplateResponse
+from django.utils.decorators import method_decorator
+from django.views import View
+
+from bookwyrm import forms, models
+from bookwyrm.views.list.list import increment_order_in_reverse
+from bookwyrm.views.list.list import normalize_book_list_ordering
+
+
+# pylint: disable=no-self-use
+@method_decorator(login_required, name="dispatch")
+class Curate(View):
+    """approve or discard list suggestsions"""
+
+    def get(self, request, list_id):
+        """display a pending list"""
+        book_list = get_object_or_404(models.List, id=list_id)
+        book_list.raise_not_editable(request.user)
+
+        data = {
+            "list": book_list,
+            "pending": book_list.listitem_set.filter(approved=False),
+            "list_form": forms.ListForm(instance=book_list),
+        }
+        return TemplateResponse(request, "lists/curate.html", data)
+
+    def post(self, request, list_id):
+        """edit a book_list"""
+        book_list = get_object_or_404(models.List, id=list_id)
+        book_list.raise_not_editable(request.user)
+
+        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)
diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list/list.py
similarity index 76%
rename from bookwyrm/views/list.py
rename to bookwyrm/views/list/list.py
index 804234acf..296b7e9f2 100644
--- a/bookwyrm/views/list.py
+++ b/bookwyrm/views/list/list.py
@@ -18,81 +18,11 @@ from django.views.decorators.clickjacking import xframe_options_exempt
 
 from bookwyrm import book_search, forms, models
 from bookwyrm.activitypub import ActivitypubResponse
-from bookwyrm.lists_stream import ListsStream
 from bookwyrm.settings import PAGE_LENGTH
-from .helpers import is_api_request
-from .helpers import get_user_from_username
+from bookwyrm.views.helpers import is_api_request
 
 
 # pylint: disable=no-self-use
-class Lists(View):
-    """book list page"""
-
-    def get(self, request):
-        """display a book list"""
-        lists = ListsStream().get_list_stream(request.user)
-        paginated = Paginator(lists, 12)
-        data = {
-            "lists": paginated.get_page(request.GET.get("page")),
-            "list_form": forms.ListForm(),
-            "path": "/list",
-        }
-        return TemplateResponse(request, "lists/lists.html", data)
-
-    @method_decorator(login_required, name="dispatch")
-    # pylint: disable=unused-argument
-    def post(self, request):
-        """create a book_list"""
-        form = forms.ListForm(request.POST)
-        if not form.is_valid():
-            return redirect("lists")
-        book_list = form.save()
-        # list should not have a group if it is not group curated
-        if not book_list.curation == "group":
-            book_list.group = None
-            book_list.save(broadcast=False)
-
-        return redirect(book_list.local_path)
-
-
-@method_decorator(login_required, name="dispatch")
-class SavedLists(View):
-    """saved book list page"""
-
-    def get(self, request):
-        """display book lists"""
-        # hide lists with no approved books
-        lists = request.user.saved_lists.order_by("-updated_date")
-
-        paginated = Paginator(lists, 12)
-        data = {
-            "lists": paginated.get_page(request.GET.get("page")),
-            "list_form": forms.ListForm(),
-            "path": "/list",
-        }
-        return TemplateResponse(request, "lists/lists.html", data)
-
-
-@method_decorator(login_required, name="dispatch")
-class UserLists(View):
-    """a user's book list page"""
-
-    def get(self, request, username):
-        """display a book list"""
-        user = get_user_from_username(request.user, username)
-        lists = models.List.privacy_filter(request.user).filter(user=user)
-        paginated = Paginator(lists, 12)
-
-        data = {
-            "user": user,
-            "is_self": request.user.id == user.id,
-            "lists": paginated.get_page(request.GET.get("page")),
-            "list_form": forms.ListForm(),
-            "path": user.local_path + "/lists",
-        }
-        return TemplateResponse(request, "user/lists.html", data)
-
-
 class List(View):
     """book list page"""
 
@@ -254,49 +184,6 @@ class EmbedList(View):
         return TemplateResponse(request, "lists/embed-list.html", data)
 
 
-@method_decorator(login_required, name="dispatch")
-class Curate(View):
-    """approve or discard list suggestsions"""
-
-    def get(self, request, list_id):
-        """display a pending list"""
-        book_list = get_object_or_404(models.List, id=list_id)
-        book_list.raise_not_editable(request.user)
-
-        data = {
-            "list": book_list,
-            "pending": book_list.listitem_set.filter(approved=False),
-            "list_form": forms.ListForm(instance=book_list),
-        }
-        return TemplateResponse(request, "lists/curate.html", data)
-
-    def post(self, request, list_id):
-        """edit a book_list"""
-        book_list = get_object_or_404(models.List, id=list_id)
-        book_list.raise_not_editable(request.user)
-
-        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)
-
-
 @require_POST
 @login_required
 def save_list(request, list_id):
diff --git a/bookwyrm/views/list/lists.py b/bookwyrm/views/list/lists.py
new file mode 100644
index 000000000..635ad4bc7
--- /dev/null
+++ b/bookwyrm/views/list/lists.py
@@ -0,0 +1,80 @@
+""" book list views"""
+from django.contrib.auth.decorators import login_required
+from django.core.paginator import Paginator
+from django.shortcuts import redirect
+from django.template.response import TemplateResponse
+from django.utils.decorators import method_decorator
+from django.views import View
+
+from bookwyrm import forms, models
+from bookwyrm.lists_stream import ListsStream
+from bookwyrm.views.helpers import get_user_from_username
+
+
+# pylint: disable=no-self-use
+class Lists(View):
+    """book list page"""
+
+    def get(self, request):
+        """display a book list"""
+        lists = ListsStream().get_list_stream(request.user)
+        paginated = Paginator(lists, 12)
+        data = {
+            "lists": paginated.get_page(request.GET.get("page")),
+            "list_form": forms.ListForm(),
+            "path": "/list",
+        }
+        return TemplateResponse(request, "lists/lists.html", data)
+
+    @method_decorator(login_required, name="dispatch")
+    # pylint: disable=unused-argument
+    def post(self, request):
+        """create a book_list"""
+        form = forms.ListForm(request.POST)
+        if not form.is_valid():
+            return redirect("lists")
+        book_list = form.save()
+        # list should not have a group if it is not group curated
+        if not book_list.curation == "group":
+            book_list.group = None
+            book_list.save(broadcast=False)
+
+        return redirect(book_list.local_path)
+
+
+@method_decorator(login_required, name="dispatch")
+class SavedLists(View):
+    """saved book list page"""
+
+    def get(self, request):
+        """display book lists"""
+        # hide lists with no approved books
+        lists = request.user.saved_lists.order_by("-updated_date")
+
+        paginated = Paginator(lists, 12)
+        data = {
+            "lists": paginated.get_page(request.GET.get("page")),
+            "list_form": forms.ListForm(),
+            "path": "/list",
+        }
+        return TemplateResponse(request, "lists/lists.html", data)
+
+
+@method_decorator(login_required, name="dispatch")
+class UserLists(View):
+    """a user's book list page"""
+
+    def get(self, request, username):
+        """display a book list"""
+        user = get_user_from_username(request.user, username)
+        lists = models.List.privacy_filter(request.user).filter(user=user)
+        paginated = Paginator(lists, 12)
+
+        data = {
+            "user": user,
+            "is_self": request.user.id == user.id,
+            "lists": paginated.get_page(request.GET.get("page")),
+            "list_form": forms.ListForm(),
+            "path": user.local_path + "/lists",
+        }
+        return TemplateResponse(request, "user/lists.html", data)

From 9902794bbdbd7773583ddc55caa9932538001a8c Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 08:44:04 -0800
Subject: [PATCH 06/22] Adds edit note view

---
 bookwyrm/forms.py                  |  5 +++++
 bookwyrm/templates/lists/list.html |  7 ++++---
 bookwyrm/urls.py                   |  1 +
 bookwyrm/views/__init__.py         |  1 +
 bookwyrm/views/list/list_item.py   | 22 ++++++++++++++++++++++
 5 files changed, 33 insertions(+), 3 deletions(-)
 create mode 100644 bookwyrm/views/list/list_item.py

diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py
index ef9bbc159..5f068a935 100644
--- a/bookwyrm/forms.py
+++ b/bookwyrm/forms.py
@@ -443,6 +443,11 @@ class ListForm(CustomForm):
         model = models.List
         fields = ["user", "name", "description", "curation", "privacy", "group"]
 
+class ListItemForm(CustomForm):
+    class Meta:
+        model = models.ListItem
+        fields = ["notes"]
+
 
 class GroupForm(CustomForm):
     class Meta:
diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index 73172bd54..6cfb9b20f 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -85,7 +85,7 @@
                                         {% endblocktrans %}
                                     </header>
                                     <p>
-                                        Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin ornare magna eros, eu pellentesque tortor vestibulum ut. Maecenas non massa sem. Etiam finibus odio quis feugiat facilisis.
+                                    {{ item.notes|to_markdown|safe }}
                                     </p>
                                 </div>
                                 {% if item.user == request.user %}
@@ -95,10 +95,11 @@
                                             {% trans "Edit:" %}
                                             <span class="details-close icon icon-pencil" aria-hidden></span>
                                         </summary>
-                                        <form>
+                                        <form name="edit-notes-{{ item.id }}" method="POST" action="{% url 'list-item' list.id item.id %}">
+                                            {% csrf_token %}
                                             <div class="field">
                                                 <div class="control">
-                                                    <textarea class="textarea">{{ item.note }}</textarea>
+                                                    <textarea class="textarea" maxlength="300" name="notes">{{ item.notes|default:'' }}</textarea>
                                                 </div>
                                             </div>
                                             <div class="field">
diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py
index 01e864110..b1485789b 100644
--- a/bookwyrm/urls.py
+++ b/bookwyrm/urls.py
@@ -357,6 +357,7 @@ urlpatterns = [
     re_path(r"^list/?$", views.Lists.as_view(), name="lists"),
     re_path(r"^list/saved/?$", views.SavedLists.as_view(), name="saved-lists"),
     re_path(r"^list/(?P<list_id>\d+)(.json)?/?$", views.List.as_view(), name="list"),
+    re_path(r"^list/(?P<list_id>\d+)/item/(?P<list_item>\d+)/?$", views.ListItem.as_view(), name="list-item"),
     re_path(r"^list/delete/(?P<list_id>\d+)/?$", views.delete_list, name="delete-list"),
     re_path(r"^list/add-book/?$", views.add_book, name="list-add-book"),
     re_path(
diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py
index da9098dc3..f4bbbfcac 100644
--- a/bookwyrm/views/__init__.py
+++ b/bookwyrm/views/__init__.py
@@ -63,6 +63,7 @@ from .imports.manually_review import (
 
 # lists
 from .list.curate import Curate
+from .list.list_item import ListItem
 from .list.lists import Lists, SavedLists, UserLists
 from .list.list import (
     List,
diff --git a/bookwyrm/views/list/list_item.py b/bookwyrm/views/list/list_item.py
new file mode 100644
index 000000000..29cfbee84
--- /dev/null
+++ b/bookwyrm/views/list/list_item.py
@@ -0,0 +1,22 @@
+""" book list views"""
+from django.contrib.auth.decorators import login_required
+from django.shortcuts import get_object_or_404, redirect
+from django.utils.decorators import method_decorator
+from django.views import View
+
+from bookwyrm import forms, models
+
+
+# pylint: disable=no-self-use
+@method_decorator(login_required, name="dispatch")
+class ListItem(View):
+    """book list page"""
+
+    def post(self, request, list_id, list_item):
+        """Edit a list item's notes"""
+        list_item = get_object_or_404(models.ListItem, id=list_item, book_list=list_id)
+        list_item.raise_not_editable(request.user)
+        form = forms.ListItemForm(request.POST, instance=list_item)
+        if form.is_valid():
+            form.save()
+        return redirect("list", list_item.book_list.id)

From 037cc35eeb028d11b92854aa2d7ac8ed0b0a54c8 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 08:45:08 -0800
Subject: [PATCH 07/22] Adds merge migration and python formatting

---
 bookwyrm/forms.py                               |  1 +
 bookwyrm/migrations/0131_merge_20220125_1644.py | 14 ++++++++++++++
 bookwyrm/urls.py                                |  6 +++++-
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 bookwyrm/migrations/0131_merge_20220125_1644.py

diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py
index 5f068a935..5a04af8c8 100644
--- a/bookwyrm/forms.py
+++ b/bookwyrm/forms.py
@@ -443,6 +443,7 @@ class ListForm(CustomForm):
         model = models.List
         fields = ["user", "name", "description", "curation", "privacy", "group"]
 
+
 class ListItemForm(CustomForm):
     class Meta:
         model = models.ListItem
diff --git a/bookwyrm/migrations/0131_merge_20220125_1644.py b/bookwyrm/migrations/0131_merge_20220125_1644.py
new file mode 100644
index 000000000..c0cd56828
--- /dev/null
+++ b/bookwyrm/migrations/0131_merge_20220125_1644.py
@@ -0,0 +1,14 @@
+# Generated by Django 3.2.10 on 2022-01-25 16:44
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('bookwyrm', '0130_alter_listitem_notes'),
+        ('bookwyrm', '0130_alter_user_preferred_language'),
+    ]
+
+    operations = [
+    ]
diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py
index b1485789b..e330408b0 100644
--- a/bookwyrm/urls.py
+++ b/bookwyrm/urls.py
@@ -357,7 +357,11 @@ urlpatterns = [
     re_path(r"^list/?$", views.Lists.as_view(), name="lists"),
     re_path(r"^list/saved/?$", views.SavedLists.as_view(), name="saved-lists"),
     re_path(r"^list/(?P<list_id>\d+)(.json)?/?$", views.List.as_view(), name="list"),
-    re_path(r"^list/(?P<list_id>\d+)/item/(?P<list_item>\d+)/?$", views.ListItem.as_view(), name="list-item"),
+    re_path(
+        r"^list/(?P<list_id>\d+)/item/(?P<list_item>\d+)/?$",
+        views.ListItem.as_view(),
+        name="list-item",
+    ),
     re_path(r"^list/delete/(?P<list_id>\d+)/?$", views.delete_list, name="delete-list"),
     re_path(r"^list/add-book/?$", views.add_book, name="list-add-book"),
     re_path(

From 6009593b6be34aeb964914498f06ccf32c17f683 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 09:17:00 -0800
Subject: [PATCH 08/22] Migration formatting

---
 bookwyrm/migrations/0131_merge_20220125_1644.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/bookwyrm/migrations/0131_merge_20220125_1644.py b/bookwyrm/migrations/0131_merge_20220125_1644.py
index c0cd56828..954ddacc4 100644
--- a/bookwyrm/migrations/0131_merge_20220125_1644.py
+++ b/bookwyrm/migrations/0131_merge_20220125_1644.py
@@ -6,9 +6,8 @@ from django.db import migrations
 class Migration(migrations.Migration):
 
     dependencies = [
-        ('bookwyrm', '0130_alter_listitem_notes'),
-        ('bookwyrm', '0130_alter_user_preferred_language'),
+        ("bookwyrm", "0130_alter_listitem_notes"),
+        ("bookwyrm", "0130_alter_user_preferred_language"),
     ]
 
-    operations = [
-    ]
+    operations = []

From 38ccc23cd5667dc14d677e1355f8a06d572a363f Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 09:40:03 -0800
Subject: [PATCH 09/22] Add notes form

---
 bookwyrm/templates/lists/list.html | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index 6cfb9b20f..6f478fa18 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -71,7 +71,7 @@
                         </div>
                         {% endwith %}
 
-                        {% if item.note or item.user == request.user %}
+                        {% if item.notes %}
                         <div class="media notification">
                             <figure class="media-left">
                                 {% include "snippets/avatar.html" with user=item.user %}
@@ -85,14 +85,14 @@
                                         {% endblocktrans %}
                                     </header>
                                     <p>
-                                    {{ item.notes|to_markdown|safe }}
+                                        {{ item.notes|to_markdown|safe }}
                                     </p>
                                 </div>
                                 {% if item.user == request.user %}
                                 <div>
                                     <details class="details-panel box">
                                         <summary>
-                                            {% trans "Edit:" %}
+                                            {% trans "Edit notes:" %}
                                             <span class="details-close icon icon-pencil" aria-hidden></span>
                                         </summary>
                                         <form name="edit-notes-{{ item.id }}" method="POST" action="{% url 'list-item' list.id item.id %}">
@@ -115,6 +115,30 @@
                                 {% endif %}
                             </div>
                         </div>
+                        {% elif item.user == request.user %}
+                        <div>
+                            <details class="details-panel box">
+                                <summary>
+                                    {% trans "Add notes:" %}
+                                    <span class="details-close icon icon-plus" aria-hidden></span>
+                                </summary>
+                                <form name="add-notes-{{ item.id }}" method="POST" action="{% url 'list-item' list.id item.id %}">
+                                    {% csrf_token %}
+                                    <div class="field">
+                                        <div class="control">
+                                            <textarea class="textarea" maxlength="300" name="notes">{{ item.notes|default:'' }}</textarea>
+                                        </div>
+                                    </div>
+                                    <div class="field">
+                                        <div class="control">
+                                            <button type="submit" class="button is-success">
+                                                {% trans "Save" %}
+                                            </button>
+                                        </div>
+                                    </div>
+                                </form>
+                            </details>
+                        </div>
                         {% endif %}
                     </div>
                     <div class="card-footer is-stacked-mobile has-background-white-bis is-align-items-stretch">

From e1599384b0d2306574bc66413106089a943c50b2 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 09:46:00 -0800
Subject: [PATCH 10/22] Fixes list views tests

---
 bookwyrm/tests/views/test_list.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py
index d36f88e59..fb374fc51 100644
--- a/bookwyrm/tests/views/test_list.py
+++ b/bookwyrm/tests/views/test_list.py
@@ -188,7 +188,7 @@ class ListViews(TestCase):
                 order=1,
             )
 
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -210,7 +210,7 @@ class ListViews(TestCase):
 
         request = self.factory.get("/?sort_by=order")
         request.user = self.local_user
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -219,7 +219,7 @@ class ListViews(TestCase):
 
         request = self.factory.get("/?sort_by=title")
         request.user = self.local_user
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -228,7 +228,7 @@ class ListViews(TestCase):
 
         request = self.factory.get("/?sort_by=rating")
         request.user = self.local_user
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -237,7 +237,7 @@ class ListViews(TestCase):
 
         request = self.factory.get("/?sort_by=sdkfh")
         request.user = self.local_user
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -250,7 +250,7 @@ class ListViews(TestCase):
         request = self.factory.get("")
         request.user = self.local_user
 
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -271,7 +271,7 @@ class ListViews(TestCase):
 
         request = self.factory.get("")
         request.user = self.anonymous_user
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id)
         self.assertIsInstance(result, TemplateResponse)
@@ -292,7 +292,7 @@ class ListViews(TestCase):
                 order=1,
             )
 
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = True
             result = view(request, self.list.id)
         self.assertIsInstance(result, ActivitypubResponse)
@@ -306,7 +306,7 @@ class ListViews(TestCase):
 
         request = self.factory.get("/?page=1")
         request.user = self.local_user
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = True
             result = view(request, self.list.id)
         self.assertIsInstance(result, ActivitypubResponse)
@@ -408,7 +408,7 @@ class ListViews(TestCase):
                 order=1,
             )
 
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             with self.assertRaises(Http404):
                 view(request, self.list.id, "")
@@ -429,7 +429,7 @@ class ListViews(TestCase):
 
         embed_key = str(self.list.embed_key.hex)
 
-        with patch("bookwyrm.views.list.is_api_request") as is_api:
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, self.list.id, embed_key)
 

From a43731b3354bb49b9c2bb6dbfb50f5ddf5b68a0b Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 10:08:54 -0800
Subject: [PATCH 11/22] Fixes tests for list actions

---
 bookwyrm/tests/views/test_list_actions.py | 34 +++++++++++------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/bookwyrm/tests/views/test_list_actions.py b/bookwyrm/tests/views/test_list_actions.py
index 7f57fae30..ece305e85 100644
--- a/bookwyrm/tests/views/test_list_actions.py
+++ b/bookwyrm/tests/views/test_list_actions.py
@@ -184,7 +184,7 @@ class ListActionViews(TestCase):
         with patch(
             "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
         ) as mock:
-            views.list.add_book(request)
+            views.add_book(request)
             self.assertEqual(mock.call_count, 1)
             activity = json.loads(mock.call_args[1]["args"][1])
             self.assertEqual(activity["type"], "Add")
@@ -219,8 +219,8 @@ class ListActionViews(TestCase):
         )
         request_two.user = self.local_user
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.list.add_book(request_one)
-            views.list.add_book(request_two)
+            views.add_book(request_one)
+            views.add_book(request_two)
 
         items = self.list.listitem_set.order_by("order").all()
         self.assertEqual(items[0].book, self.book)
@@ -261,9 +261,9 @@ class ListActionViews(TestCase):
         request_three.user = self.local_user
 
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.list.add_book(request_one)
-            views.list.add_book(request_two)
-            views.list.add_book(request_three)
+            views.add_book(request_one)
+            views.add_book(request_two)
+            views.add_book(request_three)
 
         items = self.list.listitem_set.order_by("order").all()
         self.assertEqual(items[0].book, self.book)
@@ -276,7 +276,7 @@ class ListActionViews(TestCase):
         remove_request = self.factory.post("", {"item": items[1].id})
         remove_request.user = self.local_user
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.list.remove_book(remove_request, self.list.id)
+            views.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)
@@ -312,7 +312,7 @@ class ListActionViews(TestCase):
                 approved=False,
                 order=2,
             )
-            views.list.add_book(request)
+            views.add_book(request)
 
         items = self.list.listitem_set.order_by("order").all()
         self.assertEqual(items[0].book, self.book)
@@ -427,9 +427,9 @@ class ListActionViews(TestCase):
         request_three.user = self.local_user
 
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.list.add_book(request_one)
-            views.list.add_book(request_two)
-            views.list.add_book(request_three)
+            views.add_book(request_one)
+            views.add_book(request_two)
+            views.add_book(request_three)
 
         items = self.list.listitem_set.order_by("order").all()
         self.assertEqual(items[0].book, self.book)
@@ -442,7 +442,7 @@ class ListActionViews(TestCase):
         set_position_request = self.factory.post("", {"position": 1})
         set_position_request.user = self.local_user
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.list.set_book_position(set_position_request, items[2].id)
+            views.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)
@@ -467,7 +467,7 @@ class ListActionViews(TestCase):
         with patch(
             "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
         ) as mock:
-            views.list.add_book(request)
+            views.add_book(request)
             self.assertEqual(mock.call_count, 1)
             activity = json.loads(mock.call_args[1]["args"][1])
             self.assertEqual(activity["type"], "Add")
@@ -495,7 +495,7 @@ class ListActionViews(TestCase):
         with patch(
             "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
         ) as mock:
-            views.list.add_book(request)
+            views.add_book(request)
 
         self.assertEqual(mock.call_count, 1)
         activity = json.loads(mock.call_args[1]["args"][1])
@@ -527,7 +527,7 @@ class ListActionViews(TestCase):
         with patch(
             "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
         ) as mock:
-            views.list.add_book(request)
+            views.add_book(request)
             self.assertEqual(mock.call_count, 1)
             activity = json.loads(mock.call_args[1]["args"][1])
             self.assertEqual(activity["type"], "Add")
@@ -555,7 +555,7 @@ class ListActionViews(TestCase):
         request.user = self.local_user
 
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.list.remove_book(request, self.list.id)
+            views.remove_book(request, self.list.id)
         self.assertFalse(self.list.listitem_set.exists())
 
     def test_remove_book_unauthorized(self):
@@ -569,7 +569,7 @@ class ListActionViews(TestCase):
         request.user = self.rat
 
         with self.assertRaises(PermissionDenied):
-            views.list.remove_book(request, self.list.id)
+            views.remove_book(request, self.list.id)
         self.assertTrue(self.list.listitem_set.exists())
 
     def test_save_unsave_list(self):

From f85158191bfbcb7adf0534786b0551ccedd48939 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 12:10:58 -0800
Subject: [PATCH 12/22] Use form for saving list items

---
 bookwyrm/forms.py                             |  2 +-
 bookwyrm/models/list.py                       | 17 +++++
 bookwyrm/templates/lists/add_item_modal.html  | 45 +++++++++++
 bookwyrm/templates/lists/edit_item_form.html  | 20 +++++
 .../templates/lists/item_notes_field.html     | 21 ++++++
 bookwyrm/templates/lists/list.html            | 74 ++++++-------------
 bookwyrm/views/list/list.py                   | 66 ++++++-----------
 7 files changed, 151 insertions(+), 94 deletions(-)
 create mode 100644 bookwyrm/templates/lists/add_item_modal.html
 create mode 100644 bookwyrm/templates/lists/edit_item_form.html
 create mode 100644 bookwyrm/templates/lists/item_notes_field.html

diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py
index 5a04af8c8..e442dbf45 100644
--- a/bookwyrm/forms.py
+++ b/bookwyrm/forms.py
@@ -447,7 +447,7 @@ class ListForm(CustomForm):
 class ListItemForm(CustomForm):
     class Meta:
         model = models.ListItem
-        fields = ["notes"]
+        fields = ["user", "book", "book_list", "notes"]
 
 
 class GroupForm(CustomForm):
diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py
index 733ee0447..7dff72144 100644
--- a/bookwyrm/models/list.py
+++ b/bookwyrm/models/list.py
@@ -2,6 +2,7 @@
 import uuid
 
 from django.apps import apps
+from django.core.exceptions import PermissionDenied
 from django.db import models
 from django.db.models import Q
 from django.utils import timezone
@@ -74,6 +75,22 @@ class List(OrderedCollectionMixin, BookWyrmModel):
             return
         super().raise_not_editable(viewer)
 
+    def raise_not_submittable(self, viewer):
+        """can the user submit a book to the list?"""
+        # if you can't view the list you can't submit to it
+        self.raise_visible_to_user(viewer)
+
+        # all good if you're the owner or the list is open
+        if self.user == viewer or self.curation in ["open", "curated"]:
+            return
+        if self.curation == "group":
+            is_group_member = GroupMember.objects.filter(
+                group=self.group, user=viewer
+            ).exists()
+            if is_group_member:
+                return
+        raise PermissionDenied()
+
     @classmethod
     def followers_filter(cls, queryset, viewer):
         """Override filter for "followers" privacy level to allow non-following
diff --git a/bookwyrm/templates/lists/add_item_modal.html b/bookwyrm/templates/lists/add_item_modal.html
new file mode 100644
index 000000000..5c210d462
--- /dev/null
+++ b/bookwyrm/templates/lists/add_item_modal.html
@@ -0,0 +1,45 @@
+{% extends 'components/modal.html' %}
+{% load i18n %}
+{% load utilities %}
+{% load group_tags %}
+
+{% block modal-title %}
+{% if list.curation == 'open' or request.user == list.user or list.group|is_member:request.user %}
+    {% blocktrans trimmed with title=book|book_title %}
+        Add "<em>{{ title }}</em>" to this list
+    {% endblocktrans %}
+{% else %}
+    {% blocktrans trimmed with title=book|book_title %}
+        Suggest "<em>{{ title }}</em>" for this list
+    {% endblocktrans %}
+{% endif %}
+{% endblock %}
+
+{% block modal-form-open %}
+<form
+    name="add-book-{{ book.id }}"
+    method="POST"
+    action="{% url 'list-add-book' %}{% if query %}?q={{ query }}{% endif %}"
+>
+{% endblock %}
+
+{% block modal-body %}
+    {% csrf_token %}
+    <input type="hidden" name="book" value="{{ book.id }}">
+    <input type="hidden" name="book_list" value="{{ list.id }}">
+    <input type="hidden" name="user" value="{{ request.user.id }}">
+    {% include "lists/item_notes_field.html" with form_id=id show_label=True %}
+{% endblock %}
+
+{% block modal-footer %}
+<button type="submit" class="button is-link">
+    {% if list.curation == 'open' or request.user == list.user or list.group|is_member:request.user %}
+        {% trans "Add" %}
+    {% else %}
+        {% trans "Suggest" %}
+    {% endif %}
+</button>
+<button type="button" class="button" data-modal-close>{% trans "Cancel" %}</button>
+{% endblock %}
+
+{% block modal-form-close %}</form>{% endblock %}
diff --git a/bookwyrm/templates/lists/edit_item_form.html b/bookwyrm/templates/lists/edit_item_form.html
new file mode 100644
index 000000000..2ef753e84
--- /dev/null
+++ b/bookwyrm/templates/lists/edit_item_form.html
@@ -0,0 +1,20 @@
+{% load i18n %}
+<form
+    name="edit-notes-{{ item.id }}"
+    method="POST"
+    action="{% url 'list-item' list.id item.id %}"
+>
+    {% csrf_token %}
+    <input type="hidden" name="book" value="{{ book.id }}">
+    <input type="hidden" name="book_list" value="{{ list.id }}">
+    <input type="hidden" name="user" value="{{ request.user.id }}">
+    {% include "lists/item_notes_field.html" with form_id=item.id %}
+    <div class="field">
+        <div class="control">
+            <button type="submit" class="button is-success">
+                {% trans "Save" %}
+            </button>
+        </div>
+    </div>
+</form>
+
diff --git a/bookwyrm/templates/lists/item_notes_field.html b/bookwyrm/templates/lists/item_notes_field.html
new file mode 100644
index 000000000..76c288640
--- /dev/null
+++ b/bookwyrm/templates/lists/item_notes_field.html
@@ -0,0 +1,21 @@
+{% load i18n %}
+<div class="field">
+    <label
+        for="id_notes_{{ form_id }}"
+        class="{% if show_label %}label{% else %}is-sr-only{% endif %}"
+    >
+        {% trans "Notes:" %}
+    </label>
+    <div class="control">
+        <textarea
+            class="textarea"
+            id="id_notes_{{ form_id }}"
+            maxlength="300"
+            name="notes"
+            aria-described-by="notes_description_{{ form_id }}"
+        >{{ item.notes|default:'' }}</textarea>
+    </div>
+    <p class="help" id="notes_description_{{ form_id }}">
+        {% trans "An optional note that will be displayed with the book." %}
+    </p>
+</div>
diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index 6f478fa18..0a186e8ed 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -1,8 +1,8 @@
 {% extends 'lists/layout.html' %}
 {% load i18n %}
 {% load rating_tags %}
-{% load book_display_tags %}
 {% load group_tags %}
+{% load book_display_tags %}
 {% load markdown %}
 {% load utilities %}
 
@@ -92,24 +92,12 @@
                                 <div>
                                     <details class="details-panel box">
                                         <summary>
-                                            {% trans "Edit notes:" %}
-                                            <span class="details-close icon icon-pencil" aria-hidden></span>
+                                            <span role="heading" aria-level="3">
+                                                {% trans "Edit notes" %}
+                                                <span class="details-close icon icon-pencil" aria-hidden></span>
+                                            </span>
                                         </summary>
-                                        <form name="edit-notes-{{ item.id }}" method="POST" action="{% url 'list-item' list.id item.id %}">
-                                            {% csrf_token %}
-                                            <div class="field">
-                                                <div class="control">
-                                                    <textarea class="textarea" maxlength="300" name="notes">{{ item.notes|default:'' }}</textarea>
-                                                </div>
-                                            </div>
-                                            <div class="field">
-                                                <div class="control">
-                                                    <button type="submit" class="button is-success">
-                                                        {% trans "Save" %}
-                                                    </button>
-                                                </div>
-                                            </div>
-                                        </form>
+                                        {% include "lists/edit_item_form.html" %}
                                     </details>
                                 </div>
                                 {% endif %}
@@ -119,24 +107,12 @@
                         <div>
                             <details class="details-panel box">
                                 <summary>
-                                    {% trans "Add notes:" %}
-                                    <span class="details-close icon icon-plus" aria-hidden></span>
+                                    <span role="heading" aria-level="3">
+                                        {% trans "Add notes" %}
+                                        <span class="details-close icon icon-plus" aria-hidden></span>
+                                    </span>
                                 </summary>
-                                <form name="add-notes-{{ item.id }}" method="POST" action="{% url 'list-item' list.id item.id %}">
-                                    {% csrf_token %}
-                                    <div class="field">
-                                        <div class="control">
-                                            <textarea class="textarea" maxlength="300" name="notes">{{ item.notes|default:'' }}</textarea>
-                                        </div>
-                                    </div>
-                                    <div class="field">
-                                        <div class="control">
-                                            <button type="submit" class="button is-success">
-                                                {% trans "Save" %}
-                                            </button>
-                                        </div>
-                                    </div>
-                                </form>
+                                {% include "lists/edit_item_form.html" %}
                             </details>
                         </div>
                         {% endif %}
@@ -261,23 +237,19 @@
                     <div class="column ml-3">
                         <p>{% include 'snippets/book_titleby.html' with book=book %}</p>
 
-                        <form
-                            class="mt-1"
-                            name="add-book-{{ book.id }}"
-                            method="post"
-                            action="{% url 'list-add-book' %}{% if query %}?q={{ query }}{% endif %}"
+                        {% join "add_item" list.id book.id as modal_id %}
+                        <button
+                            type="button"
+                            class="button is-small is-link"
+                            data-modal-open="{{ modal_id }}"
                         >
-                            {% csrf_token %}
-                            <input type="hidden" name="book" value="{{ book.id }}">
-                            <input type="hidden" name="list" value="{{ list.id }}">
-                            <button type="submit" class="button is-small is-link">
-                                {% if list.curation == 'open' or request.user == list.user or list.group|is_member:request.user %}
-                                    {% trans "Add" %}
-                                {% else %}
-                                    {% trans "Suggest" %}
-                                {% endif %}
-                            </button>
-                        </form>
+                            {% if list.curation == 'open' or request.user == list.user or list.group|is_member:request.user %}
+                                {% trans "Add" %}
+                            {% else %}
+                                {% trans "Suggest" %}
+                            {% endif %}
+                        </button>
+                        {% include "lists/add_item_modal.html" with id=modal_id %}
                     </div>
                 </div>
             {% endfor %}
diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py
index 296b7e9f2..fa92142ab 100644
--- a/bookwyrm/views/list/list.py
+++ b/bookwyrm/views/list/list.py
@@ -217,53 +217,35 @@ def delete_list(request, list_id):
 
 @require_POST
 @login_required
+@transaction.atomic
 def add_book(request):
     """put a book on a list"""
-    book_list = get_object_or_404(models.List, id=request.POST.get("list"))
-    is_group_member = False
-    if book_list.curation == "group":
-        is_group_member = models.GroupMember.objects.filter(
-            group=book_list.group, user=request.user
-        ).exists()
+    book_list = get_object_or_404(models.List, id=request.POST.get("book_list"))
+    # make sure the user is allowed to submit to this list
+    book_list.raise_not_submittable(request.user)
 
-    book_list.raise_visible_to_user(request.user)
+    form = forms.ListItemForm(request.POST)
+    if not form.is_valid():
+        # this shouldn't happen, there aren't validated fields
+        raise Exception(form.errors)
+    item = form.save(commit=False)
+
+    if book_list.curation == "curated":
+        # make a pending entry at the end of the list
+        order_max = (book_list.listitem_set.aggregate(Max("order"))["order__max"]) or 0
+        item.approved = False
+    else:
+        # add the book at the latest order of approved books, before 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)
+    item.order = order_max + 1
 
-    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 is_group_member
-            or book_list.curation == "open"
-        ):
-            # add the book at the latest order of approved books, before 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 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
-            return HttpResponseBadRequest()
+        item.save()
     except IntegrityError:
         # if the book is already on the list, don't flip out
         pass

From d456ab730687333982d86b8467ee1f2c5e7c5c29 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 13:21:32 -0800
Subject: [PATCH 13/22] Move lists tests into directory

---
 bookwyrm/tests/views/lists/__init__.py                | 1 +
 bookwyrm/tests/views/{ => lists}/test_list.py         | 0
 bookwyrm/tests/views/{ => lists}/test_list_actions.py | 0
 3 files changed, 1 insertion(+)
 create mode 100644 bookwyrm/tests/views/lists/__init__.py
 rename bookwyrm/tests/views/{ => lists}/test_list.py (100%)
 rename bookwyrm/tests/views/{ => lists}/test_list_actions.py (100%)

diff --git a/bookwyrm/tests/views/lists/__init__.py b/bookwyrm/tests/views/lists/__init__.py
new file mode 100644
index 000000000..b6e690fd5
--- /dev/null
+++ b/bookwyrm/tests/views/lists/__init__.py
@@ -0,0 +1 @@
+from . import *
diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/lists/test_list.py
similarity index 100%
rename from bookwyrm/tests/views/test_list.py
rename to bookwyrm/tests/views/lists/test_list.py
diff --git a/bookwyrm/tests/views/test_list_actions.py b/bookwyrm/tests/views/lists/test_list_actions.py
similarity index 100%
rename from bookwyrm/tests/views/test_list_actions.py
rename to bookwyrm/tests/views/lists/test_list_actions.py

From dd16fb2a26445eba8ee8412993b1807d88e0761b Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 13:37:57 -0800
Subject: [PATCH 14/22] Move embed view to separate file

---
 bookwyrm/views/__init__.py   |  2 +-
 bookwyrm/views/list/embed.py | 75 ++++++++++++++++++++++++++++++++++++
 bookwyrm/views/list/list.py  | 65 +------------------------------
 3 files changed, 77 insertions(+), 65 deletions(-)
 create mode 100644 bookwyrm/views/list/embed.py

diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py
index f4bbbfcac..45e5e30de 100644
--- a/bookwyrm/views/__init__.py
+++ b/bookwyrm/views/__init__.py
@@ -63,6 +63,7 @@ from .imports.manually_review import (
 
 # lists
 from .list.curate import Curate
+from .list.embed import unsafe_embed_list
 from .list.list_item import ListItem
 from .list.lists import Lists, SavedLists, UserLists
 from .list.list import (
@@ -70,7 +71,6 @@ from .list.list import (
     save_list,
     unsave_list,
     delete_list,
-    unsafe_embed_list,
     add_book,
     remove_book,
     set_book_position,
diff --git a/bookwyrm/views/list/embed.py b/bookwyrm/views/list/embed.py
new file mode 100644
index 000000000..9d0078b65
--- /dev/null
+++ b/bookwyrm/views/list/embed.py
@@ -0,0 +1,75 @@
+""" book list views"""
+from django.core.paginator import Paginator
+from django.db.models import Avg, DecimalField
+from django.db.models.functions import Coalesce
+from django.http import Http404
+from django.shortcuts import get_object_or_404
+from django.template.response import TemplateResponse
+from django.views import View
+from django.views.decorators.clickjacking import xframe_options_exempt
+
+from bookwyrm import models
+from bookwyrm.settings import PAGE_LENGTH
+
+
+# pylint: disable=no-self-use
+class EmbedList(View):
+    """embeded book list page"""
+
+    def get(self, request, list_id, list_key):
+        """display a book list"""
+        book_list = get_object_or_404(models.List, id=list_id)
+
+        embed_key = str(book_list.embed_key.hex)
+
+        if list_key != embed_key:
+            raise Http404()
+
+        # 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"
+
+        directional_sort_by = {
+            "order": "order",
+            "title": "book__title",
+            "rating": "average_rating",
+        }[sort_by]
+        if direction == "descending":
+            directional_sort_by = "-" + directional_sort_by
+
+        items = book_list.listitem_set.prefetch_related("user", "book", "book__authors")
+        if sort_by == "rating":
+            items = items.annotate(
+                average_rating=Avg(
+                    Coalesce("book__review__rating", 0.0),
+                    output_field=DecimalField(),
+                )
+            )
+        items = items.filter(approved=True).order_by(directional_sort_by)
+
+        paginated = Paginator(items, PAGE_LENGTH)
+
+        page = paginated.get_page(request.GET.get("page"))
+
+        data = {
+            "list": book_list,
+            "items": page,
+            "page_range": paginated.get_elided_page_range(
+                page.number, on_each_side=2, on_ends=1
+            ),
+        }
+        return TemplateResponse(request, "lists/embed-list.html", data)
+
+
+@xframe_options_exempt
+def unsafe_embed_list(request, *args, **kwargs):
+    """allows the EmbedList view to be loaded through unsafe iframe origins"""
+
+    embed_list_view = EmbedList.as_view()
+    return embed_list_view(request, *args, **kwargs)
diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py
index fa92142ab..c66fe2de4 100644
--- a/bookwyrm/views/list/list.py
+++ b/bookwyrm/views/list/list.py
@@ -7,14 +7,13 @@ from django.core.paginator import Paginator
 from django.db import IntegrityError, transaction
 from django.db.models import Avg, DecimalField, Q, Max
 from django.db.models.functions import Coalesce
-from django.http import HttpResponseBadRequest, HttpResponse, Http404
+from django.http import HttpResponseBadRequest, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
 from django.utils.decorators import method_decorator
 from django.views import View
 from django.views.decorators.http import require_POST
-from django.views.decorators.clickjacking import xframe_options_exempt
 
 from bookwyrm import book_search, forms, models
 from bookwyrm.activitypub import ActivitypubResponse
@@ -130,60 +129,6 @@ class List(View):
         return redirect(book_list.local_path)
 
 
-class EmbedList(View):
-    """embeded book list page"""
-
-    def get(self, request, list_id, list_key):
-        """display a book list"""
-        book_list = get_object_or_404(models.List, id=list_id)
-
-        embed_key = str(book_list.embed_key.hex)
-
-        if list_key != embed_key:
-            raise Http404()
-
-        # 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"
-
-        directional_sort_by = {
-            "order": "order",
-            "title": "book__title",
-            "rating": "average_rating",
-        }[sort_by]
-        if direction == "descending":
-            directional_sort_by = "-" + directional_sort_by
-
-        items = book_list.listitem_set.prefetch_related("user", "book", "book__authors")
-        if sort_by == "rating":
-            items = items.annotate(
-                average_rating=Avg(
-                    Coalesce("book__review__rating", 0.0),
-                    output_field=DecimalField(),
-                )
-            )
-        items = items.filter(approved=True).order_by(directional_sort_by)
-
-        paginated = Paginator(items, PAGE_LENGTH)
-
-        page = paginated.get_page(request.GET.get("page"))
-
-        data = {
-            "list": book_list,
-            "items": page,
-            "page_range": paginated.get_elided_page_range(
-                page.number, on_each_side=2, on_ends=1
-            ),
-        }
-        return TemplateResponse(request, "lists/embed-list.html", data)
-
-
 @require_POST
 @login_required
 def save_list(request, list_id):
@@ -368,11 +313,3 @@ def normalize_book_list_ordering(book_list_id, start=0, add_offset=0):
         if item.order != effective_order:
             item.order = effective_order
             item.save()
-
-
-@xframe_options_exempt
-def unsafe_embed_list(request, *args, **kwargs):
-    """allows the EmbedList view to be loaded through unsafe iframe origins"""
-
-    embed_list_view = EmbedList.as_view()
-    return embed_list_view(request, *args, **kwargs)

From 3a5289a99917d0f362d155b83d4a9c63643282d1 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 13:38:13 -0800
Subject: [PATCH 15/22] Separate lists view test files

---
 bookwyrm/tests/views/lists/test_curate.py |  68 +++++++
 bookwyrm/tests/views/lists/test_embed.py  |  89 ++++++++++
 bookwyrm/tests/views/lists/test_list.py   | 205 +---------------------
 bookwyrm/tests/views/lists/test_lists.py  | 161 +++++++++++++++++
 4 files changed, 320 insertions(+), 203 deletions(-)
 create mode 100644 bookwyrm/tests/views/lists/test_curate.py
 create mode 100644 bookwyrm/tests/views/lists/test_embed.py
 create mode 100644 bookwyrm/tests/views/lists/test_lists.py

diff --git a/bookwyrm/tests/views/lists/test_curate.py b/bookwyrm/tests/views/lists/test_curate.py
new file mode 100644
index 000000000..98a7fc016
--- /dev/null
+++ b/bookwyrm/tests/views/lists/test_curate.py
@@ -0,0 +1,68 @@
+""" test for app action functionality """
+from unittest.mock import patch
+
+from django.contrib.auth.models import AnonymousUser
+from django.template.response import TemplateResponse
+from django.test import TestCase
+from django.test.client import RequestFactory
+
+from bookwyrm import models, views
+from bookwyrm.tests.validate_html import validate_html
+
+
+# pylint: disable=unused-argument
+class ListViews(TestCase):
+    """list view"""
+
+    def setUp(self):
+        """we need basic test data and mocks"""
+        self.factory = RequestFactory()
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
+            "bookwyrm.activitystreams.populate_stream_task.delay"
+        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@local.com",
+                "mouse@mouse.com",
+                "mouseword",
+                local=True,
+                localname="mouse",
+                remote_id="https://example.com/users/mouse",
+            )
+        work = models.Work.objects.create(title="Work")
+        self.book = models.Edition.objects.create(
+            title="Example Edition",
+            remote_id="https://example.com/book/1",
+            parent_work=work,
+        )
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            self.list = models.List.objects.create(
+                name="Test List", user=self.local_user
+            )
+        self.anonymous_user = AnonymousUser
+        self.anonymous_user.is_authenticated = False
+
+        models.SiteSettings.objects.create()
+
+    def test_curate_page(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Curate.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=False,
+                order=1,
+            )
+
+        result = view(request, self.list.id)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+        request.user = self.anonymous_user
+        result = view(request, self.list.id)
+        self.assertEqual(result.status_code, 302)
diff --git a/bookwyrm/tests/views/lists/test_embed.py b/bookwyrm/tests/views/lists/test_embed.py
new file mode 100644
index 000000000..dc61736d3
--- /dev/null
+++ b/bookwyrm/tests/views/lists/test_embed.py
@@ -0,0 +1,89 @@
+""" test for app action functionality """
+from unittest.mock import patch
+
+from django.contrib.auth.models import AnonymousUser
+from django.http.response import Http404
+from django.template.response import TemplateResponse
+from django.test import TestCase
+from django.test.client import RequestFactory
+
+from bookwyrm import models, views
+from bookwyrm.tests.validate_html import validate_html
+
+
+# pylint: disable=unused-argument
+class ListViews(TestCase):
+    """list view"""
+
+    def setUp(self):
+        """we need basic test data and mocks"""
+        self.factory = RequestFactory()
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
+            "bookwyrm.activitystreams.populate_stream_task.delay"
+        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@local.com",
+                "mouse@mouse.com",
+                "mouseword",
+                local=True,
+                localname="mouse",
+                remote_id="https://example.com/users/mouse",
+            )
+        work = models.Work.objects.create(title="Work")
+        self.book = models.Edition.objects.create(
+            title="Example Edition",
+            remote_id="https://example.com/book/1",
+            parent_work=work,
+        )
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            self.list = models.List.objects.create(
+                name="Test List", user=self.local_user
+            )
+        self.anonymous_user = AnonymousUser
+        self.anonymous_user.is_authenticated = False
+
+        models.SiteSettings.objects.create()
+
+    def test_embed_call_without_key(self):
+        """there are so many views, this just makes sure it DOESN’T load"""
+        view = views.unsafe_embed_list
+        request = self.factory.get("")
+        request.user = self.anonymous_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=True,
+                order=1,
+            )
+
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
+            is_api.return_value = False
+            with self.assertRaises(Http404):
+                view(request, self.list.id, "")
+
+    def test_embed_call_with_key(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.unsafe_embed_list
+        request = self.factory.get("")
+        request.user = self.anonymous_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=True,
+                order=1,
+            )
+
+        embed_key = str(self.list.embed_key.hex)
+
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.list.id, embed_key)
+
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
diff --git a/bookwyrm/tests/views/lists/test_list.py b/bookwyrm/tests/views/lists/test_list.py
index fb374fc51..07cdbb701 100644
--- a/bookwyrm/tests/views/lists/test_list.py
+++ b/bookwyrm/tests/views/lists/test_list.py
@@ -3,7 +3,6 @@ import json
 from unittest.mock import patch
 
 from django.contrib.auth.models import AnonymousUser
-from django.http.response import Http404
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -12,9 +11,10 @@ from bookwyrm import models, views
 from bookwyrm.activitypub import ActivitypubResponse
 from bookwyrm.tests.validate_html import validate_html
 
+
 # pylint: disable=unused-argument
 class ListViews(TestCase):
-    """tag views"""
+    """list view"""
 
     def setUp(self):
         """we need basic test data and mocks"""
@@ -30,14 +30,6 @@ class ListViews(TestCase):
                 localname="mouse",
                 remote_id="https://example.com/users/mouse",
             )
-            self.rat = models.User.objects.create_user(
-                "rat@local.com",
-                "rat@rat.com",
-                "ratword",
-                local=True,
-                localname="rat",
-                remote_id="https://example.com/users/rat",
-            )
         work = models.Work.objects.create(title="Work")
         self.book = models.Edition.objects.create(
             title="Example Edition",
@@ -72,108 +64,6 @@ class ListViews(TestCase):
 
         models.SiteSettings.objects.create()
 
-    @patch("bookwyrm.lists_stream.ListsStream.get_list_stream")
-    def test_lists_page(self, _):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.Lists.as_view()
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ), patch("bookwyrm.lists_stream.add_list_task.delay"):
-            models.List.objects.create(name="Public list", user=self.local_user)
-            models.List.objects.create(
-                name="Private list", privacy="direct", user=self.local_user
-            )
-        request = self.factory.get("")
-        request.user = self.local_user
-
-        result = view(request)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-
-        request.user = self.anonymous_user
-
-        result = view(request)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-
-    def test_saved_lists_page(self):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.SavedLists.as_view()
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            booklist = models.List.objects.create(
-                name="Public list", user=self.local_user
-            )
-            models.List.objects.create(
-                name="Private list", privacy="direct", user=self.local_user
-            )
-        self.local_user.saved_lists.add(booklist)
-        request = self.factory.get("")
-        request.user = self.local_user
-
-        result = view(request)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-        self.assertEqual(result.context_data["lists"].object_list, [booklist])
-
-    def test_saved_lists_page_empty(self):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.SavedLists.as_view()
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            models.List.objects.create(name="Public list", user=self.local_user)
-            models.List.objects.create(
-                name="Private list", privacy="direct", user=self.local_user
-            )
-        request = self.factory.get("")
-        request.user = self.local_user
-
-        result = view(request)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-        self.assertEqual(len(result.context_data["lists"].object_list), 0)
-
-    def test_saved_lists_page_logged_out(self):
-        """logged out saved lists"""
-        view = views.SavedLists.as_view()
-        request = self.factory.get("")
-        request.user = self.anonymous_user
-
-        result = view(request)
-        self.assertEqual(result.status_code, 302)
-
-    def test_lists_create(self):
-        """create list view"""
-        view = views.Lists.as_view()
-        request = self.factory.post(
-            "",
-            {
-                "name": "A list",
-                "description": "wow",
-                "privacy": "unlisted",
-                "curation": "open",
-                "user": self.local_user.id,
-            },
-        )
-        request.user = self.local_user
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock:
-            result = view(request)
-
-        self.assertEqual(mock.call_count, 1)
-        activity = json.loads(mock.call_args[1]["args"][1])
-        self.assertEqual(activity["type"], "Create")
-        self.assertEqual(activity["actor"], self.local_user.remote_id)
-
-        self.assertEqual(result.status_code, 302)
-        new_list = models.List.objects.filter(name="A list").get()
-        self.assertEqual(new_list.description, "wow")
-        self.assertEqual(new_list.privacy, "unlisted")
-        self.assertEqual(new_list.curation, "open")
-
     def test_list_page(self):
         """there are so many views, this just makes sure it LOADS"""
         view = views.List.as_view()
@@ -345,94 +235,3 @@ class ListViews(TestCase):
         self.assertEqual(self.list.description, "wow")
         self.assertEqual(self.list.privacy, "direct")
         self.assertEqual(self.list.curation, "curated")
-
-    def test_curate_page(self):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.Curate.as_view()
-        request = self.factory.get("")
-        request.user = self.local_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
-                approved=False,
-                order=1,
-            )
-
-        result = view(request, self.list.id)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-
-        request.user = self.anonymous_user
-        result = view(request, self.list.id)
-        self.assertEqual(result.status_code, 302)
-
-    def test_user_lists_page(self):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.UserLists.as_view()
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            models.List.objects.create(name="Public list", user=self.local_user)
-            models.List.objects.create(
-                name="Private list", privacy="direct", user=self.local_user
-            )
-        request = self.factory.get("")
-        request.user = self.local_user
-
-        result = view(request, self.local_user.localname)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-
-    def test_user_lists_page_logged_out(self):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.UserLists.as_view()
-        request = self.factory.get("")
-        request.user = self.anonymous_user
-
-        result = view(request, self.local_user.username)
-        self.assertEqual(result.status_code, 302)
-
-    def test_embed_call_without_key(self):
-        """there are so many views, this just makes sure it DOESN’T load"""
-        view = views.unsafe_embed_list
-        request = self.factory.get("")
-        request.user = self.anonymous_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
-                approved=True,
-                order=1,
-            )
-
-        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
-            is_api.return_value = False
-            with self.assertRaises(Http404):
-                view(request, self.list.id, "")
-
-    def test_embed_call_with_key(self):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.unsafe_embed_list
-        request = self.factory.get("")
-        request.user = self.anonymous_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
-                approved=True,
-                order=1,
-            )
-
-        embed_key = str(self.list.embed_key.hex)
-
-        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
-            is_api.return_value = False
-            result = view(request, self.list.id, embed_key)
-
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
diff --git a/bookwyrm/tests/views/lists/test_lists.py b/bookwyrm/tests/views/lists/test_lists.py
new file mode 100644
index 000000000..f65d2e4c2
--- /dev/null
+++ b/bookwyrm/tests/views/lists/test_lists.py
@@ -0,0 +1,161 @@
+""" test for app action functionality """
+import json
+from unittest.mock import patch
+
+from django.contrib.auth.models import AnonymousUser
+from django.template.response import TemplateResponse
+from django.test import TestCase
+from django.test.client import RequestFactory
+
+from bookwyrm import models, views
+from bookwyrm.tests.validate_html import validate_html
+
+# pylint: disable=unused-argument
+class ListViews(TestCase):
+    """lists of lists"""
+
+    def setUp(self):
+        """we need basic test data and mocks"""
+        self.factory = RequestFactory()
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
+            "bookwyrm.activitystreams.populate_stream_task.delay"
+        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@local.com",
+                "mouse@mouse.com",
+                "mouseword",
+                local=True,
+                localname="mouse",
+                remote_id="https://example.com/users/mouse",
+            )
+        self.anonymous_user = AnonymousUser
+        self.anonymous_user.is_authenticated = False
+
+        models.SiteSettings.objects.create()
+
+    @patch("bookwyrm.lists_stream.ListsStream.get_list_stream")
+    def test_lists_page(self, _):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Lists.as_view()
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ), patch("bookwyrm.lists_stream.add_list_task.delay"):
+            models.List.objects.create(name="Public list", user=self.local_user)
+            models.List.objects.create(
+                name="Private list", privacy="direct", user=self.local_user
+            )
+        request = self.factory.get("")
+        request.user = self.local_user
+
+        result = view(request)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+        request.user = self.anonymous_user
+
+        result = view(request)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+    def test_saved_lists_page(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.SavedLists.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            booklist = models.List.objects.create(
+                name="Public list", user=self.local_user
+            )
+            models.List.objects.create(
+                name="Private list", privacy="direct", user=self.local_user
+            )
+        self.local_user.saved_lists.add(booklist)
+        request = self.factory.get("")
+        request.user = self.local_user
+
+        result = view(request)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+        self.assertEqual(result.context_data["lists"].object_list, [booklist])
+
+    def test_saved_lists_page_empty(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.SavedLists.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            models.List.objects.create(name="Public list", user=self.local_user)
+            models.List.objects.create(
+                name="Private list", privacy="direct", user=self.local_user
+            )
+        request = self.factory.get("")
+        request.user = self.local_user
+
+        result = view(request)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+        self.assertEqual(len(result.context_data["lists"].object_list), 0)
+
+    def test_saved_lists_page_logged_out(self):
+        """logged out saved lists"""
+        view = views.SavedLists.as_view()
+        request = self.factory.get("")
+        request.user = self.anonymous_user
+
+        result = view(request)
+        self.assertEqual(result.status_code, 302)
+
+    def test_user_lists_page(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.UserLists.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            models.List.objects.create(name="Public list", user=self.local_user)
+            models.List.objects.create(
+                name="Private list", privacy="direct", user=self.local_user
+            )
+        request = self.factory.get("")
+        request.user = self.local_user
+
+        result = view(request, self.local_user.localname)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+    def test_user_lists_page_logged_out(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.UserLists.as_view()
+        request = self.factory.get("")
+        request.user = self.anonymous_user
+
+        result = view(request, self.local_user.username)
+        self.assertEqual(result.status_code, 302)
+
+    def test_lists_create(self):
+        """create list view"""
+        view = views.Lists.as_view()
+        request = self.factory.post(
+            "",
+            {
+                "name": "A list",
+                "description": "wow",
+                "privacy": "unlisted",
+                "curation": "open",
+                "user": self.local_user.id,
+            },
+        )
+        request.user = self.local_user
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            result = view(request)
+
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[1]["args"][1])
+        self.assertEqual(activity["type"], "Create")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+
+        self.assertEqual(result.status_code, 302)
+        new_list = models.List.objects.filter(name="A list").get()
+        self.assertEqual(new_list.description, "wow")
+        self.assertEqual(new_list.privacy, "unlisted")
+        self.assertEqual(new_list.curation, "open")

From 3f9a401d9875b57f8e78edd4fb94d8f65841244b Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 13:43:25 -0800
Subject: [PATCH 16/22] Put list action tests in correct files

---
 bookwyrm/tests/views/lists/test_curate.py     |  61 ++
 bookwyrm/tests/views/lists/test_list.py       | 471 ++++++++++++++
 .../tests/views/lists/test_list_actions.py    | 586 ------------------
 3 files changed, 532 insertions(+), 586 deletions(-)
 delete mode 100644 bookwyrm/tests/views/lists/test_list_actions.py

diff --git a/bookwyrm/tests/views/lists/test_curate.py b/bookwyrm/tests/views/lists/test_curate.py
index 98a7fc016..9be8c2a15 100644
--- a/bookwyrm/tests/views/lists/test_curate.py
+++ b/bookwyrm/tests/views/lists/test_curate.py
@@ -1,4 +1,5 @@
 """ test for app action functionality """
+import json
 from unittest.mock import patch
 
 from django.contrib.auth.models import AnonymousUser
@@ -66,3 +67,63 @@ class ListViews(TestCase):
         request.user = self.anonymous_user
         result = view(request, self.list.id)
         self.assertEqual(result.status_code, 302)
+
+    def test_curate_approve(self):
+        """approve a pending item"""
+        view = views.Curate.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            pending = models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=False,
+                order=1,
+            )
+
+        request = self.factory.post(
+            "",
+            {"item": pending.id, "approved": "true"},
+        )
+        request.user = self.local_user
+
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            view(request, self.list.id)
+
+        self.assertEqual(mock.call_count, 2)
+        activity = json.loads(mock.call_args[1]["args"][1])
+        self.assertEqual(activity["type"], "Add")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["target"], self.list.remote_id)
+
+        pending.refresh_from_db()
+        self.assertEqual(self.list.books.count(), 1)
+        self.assertEqual(self.list.listitem_set.first(), pending)
+        self.assertTrue(pending.approved)
+
+    def test_curate_reject(self):
+        """approve a pending item"""
+        view = views.Curate.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            pending = models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=False,
+                order=1,
+            )
+
+        request = self.factory.post(
+            "",
+            {
+                "item": pending.id,
+                "approved": "false",
+            },
+        )
+        request.user = self.local_user
+
+        view(request, self.list.id)
+
+        self.assertFalse(self.list.books.exists())
+        self.assertFalse(models.ListItem.objects.exists())
diff --git a/bookwyrm/tests/views/lists/test_list.py b/bookwyrm/tests/views/lists/test_list.py
index 07cdbb701..489681547 100644
--- a/bookwyrm/tests/views/lists/test_list.py
+++ b/bookwyrm/tests/views/lists/test_list.py
@@ -3,6 +3,7 @@ import json
 from unittest.mock import patch
 
 from django.contrib.auth.models import AnonymousUser
+from django.core.exceptions import PermissionDenied
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -13,6 +14,7 @@ from bookwyrm.tests.validate_html import validate_html
 
 
 # pylint: disable=unused-argument
+# pylint: disable=too-many-public-methods
 class ListViews(TestCase):
     """list view"""
 
@@ -235,3 +237,472 @@ class ListViews(TestCase):
         self.assertEqual(self.list.description, "wow")
         self.assertEqual(self.list.privacy, "direct")
         self.assertEqual(self.list.curation, "curated")
+
+    def test_delete_list(self):
+        """delete an entire list"""
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            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=False,
+                order=2,
+            )
+        request = self.factory.post("")
+        request.user = self.local_user
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock, patch("bookwyrm.lists_stream.remove_list_task.delay") as redis_mock:
+            views.delete_list(request, self.list.id)
+        self.assertTrue(redis_mock.called)
+        activity = json.loads(mock.call_args[1]["args"][1])
+        self.assertEqual(activity["type"], "Delete")
+        self.assertEqual(activity["actor"], self.local_user.remote_id)
+        self.assertEqual(activity["object"]["id"], self.list.remote_id)
+        self.assertEqual(activity["object"]["type"], "BookList")
+
+        self.assertEqual(mock.call_count, 1)
+        self.assertFalse(models.List.objects.exists())
+        self.assertFalse(models.ListItem.objects.exists())
+
+    def test_delete_list_permission_denied(self):
+        """delete an entire list"""
+        request = self.factory.post("")
+        request.user = self.rat
+        with self.assertRaises(PermissionDenied):
+            views.delete_list(request, self.list.id)
+
+    def test_add_book(self):
+        """put a book on a list"""
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request.user = self.local_user
+
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            views.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args[1]["args"][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
+        item = self.list.listitem_set.get()
+        self.assertEqual(item.book, self.book)
+        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,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_two.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            views.add_book(request_one)
+            views.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,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_two.user = self.local_user
+
+        request_three = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_three.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            views.add_book(request_one)
+            views.add_book(request_two)
+            views.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.apply_async"):
+            views.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,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            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.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.apply_async"):
+            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.apply_async"):
+            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,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_two.user = self.local_user
+
+        request_three = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request_three.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            views.add_book(request_one)
+            views.add_book(request_two)
+            views.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.apply_async"):
+            views.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"
+        self.list.save(broadcast=False)
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request.user = self.rat
+
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            views.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args[1]["args"][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.rat.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
+        item = self.list.listitem_set.get()
+        self.assertEqual(item.book, self.book)
+        self.assertEqual(item.user, self.rat)
+        self.assertTrue(item.approved)
+
+    def test_add_book_pending(self):
+        """put a book on a list awaiting approval"""
+        self.list.curation = "curated"
+        self.list.save(broadcast=False)
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request.user = self.rat
+
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            views.add_book(request)
+
+        self.assertEqual(mock.call_count, 1)
+        activity = json.loads(mock.call_args[1]["args"][1])
+
+        self.assertEqual(activity["type"], "Add")
+        self.assertEqual(activity["actor"], self.rat.remote_id)
+        self.assertEqual(activity["target"], self.list.remote_id)
+
+        item = self.list.listitem_set.get()
+        self.assertEqual(activity["object"]["id"], item.remote_id)
+
+        self.assertEqual(item.book, self.book)
+        self.assertEqual(item.user, self.rat)
+        self.assertFalse(item.approved)
+
+    def test_add_book_self_curated(self):
+        """put a book on a list automatically approved"""
+        self.list.curation = "curated"
+        self.list.save(broadcast=False)
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "book_list": self.list.id,
+                "user": self.local_user,
+            },
+        )
+        request.user = self.local_user
+
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            views.add_book(request)
+            self.assertEqual(mock.call_count, 1)
+            activity = json.loads(mock.call_args[1]["args"][1])
+            self.assertEqual(activity["type"], "Add")
+            self.assertEqual(activity["actor"], self.local_user.remote_id)
+            self.assertEqual(activity["target"], self.list.remote_id)
+
+        item = self.list.listitem_set.get()
+        self.assertEqual(item.book, self.book)
+        self.assertEqual(item.user, self.local_user)
+        self.assertTrue(item.approved)
+
+    def test_remove_book(self):
+        """take an item off a list"""
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            item = models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                order=1,
+            )
+        self.assertTrue(self.list.listitem_set.exists())
+
+        request = self.factory.post("", {"item": item.id})
+        request.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            views.remove_book(request, self.list.id)
+        self.assertFalse(self.list.listitem_set.exists())
+
+    def test_remove_book_unauthorized(self):
+        """take an item off a list"""
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            item = models.ListItem.objects.create(
+                book_list=self.list, user=self.local_user, book=self.book, order=1
+            )
+        self.assertTrue(self.list.listitem_set.exists())
+        request = self.factory.post("", {"item": item.id})
+        request.user = self.rat
+
+        with self.assertRaises(PermissionDenied):
+            views.remove_book(request, self.list.id)
+        self.assertTrue(self.list.listitem_set.exists())
+
+    def test_save_unsave_list(self):
+        """bookmark a list"""
+        self.assertFalse(self.local_user.saved_lists.exists())
+        request = self.factory.post("")
+        request.user = self.local_user
+        views.save_list(request, self.list.id)
+        self.local_user.refresh_from_db()
+        self.assertEqual(self.local_user.saved_lists.first(), self.list)
+
+        views.unsave_list(request, self.list.id)
+        self.local_user.refresh_from_db()
+        self.assertFalse(self.local_user.saved_lists.exists())
diff --git a/bookwyrm/tests/views/lists/test_list_actions.py b/bookwyrm/tests/views/lists/test_list_actions.py
deleted file mode 100644
index ece305e85..000000000
--- a/bookwyrm/tests/views/lists/test_list_actions.py
+++ /dev/null
@@ -1,586 +0,0 @@
-""" test for app action functionality """
-import json
-from unittest.mock import patch
-
-from django.contrib.auth.models import AnonymousUser
-from django.core.exceptions import PermissionDenied
-from django.test import TestCase
-from django.test.client import RequestFactory
-
-from bookwyrm import models, views
-
-# pylint: disable=unused-argument
-class ListActionViews(TestCase):
-    """tag views"""
-
-    def setUp(self):
-        """we need basic test data and mocks"""
-        self.factory = RequestFactory()
-        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
-            "bookwyrm.activitystreams.populate_stream_task.delay"
-        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
-            self.local_user = models.User.objects.create_user(
-                "mouse@local.com",
-                "mouse@mouse.com",
-                "mouseword",
-                local=True,
-                localname="mouse",
-                remote_id="https://example.com/users/mouse",
-            )
-            self.rat = models.User.objects.create_user(
-                "rat@local.com",
-                "rat@rat.com",
-                "ratword",
-                local=True,
-                localname="rat",
-                remote_id="https://example.com/users/rat",
-            )
-
-        work = models.Work.objects.create(title="Work")
-        self.book = models.Edition.objects.create(
-            title="Example Edition",
-            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.apply_async"):
-            self.list = models.List.objects.create(
-                name="Test List", user=self.local_user
-            )
-        self.anonymous_user = AnonymousUser
-        self.anonymous_user.is_authenticated = False
-        models.SiteSettings.objects.create()
-
-    def test_delete_list(self):
-        """delete an entire list"""
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            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=False,
-                order=2,
-            )
-        request = self.factory.post("")
-        request.user = self.local_user
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock, patch("bookwyrm.lists_stream.remove_list_task.delay") as redis_mock:
-            views.delete_list(request, self.list.id)
-        self.assertTrue(redis_mock.called)
-        activity = json.loads(mock.call_args[1]["args"][1])
-        self.assertEqual(activity["type"], "Delete")
-        self.assertEqual(activity["actor"], self.local_user.remote_id)
-        self.assertEqual(activity["object"]["id"], self.list.remote_id)
-        self.assertEqual(activity["object"]["type"], "BookList")
-
-        self.assertEqual(mock.call_count, 1)
-        self.assertFalse(models.List.objects.exists())
-        self.assertFalse(models.ListItem.objects.exists())
-
-    def test_delete_list_permission_denied(self):
-        """delete an entire list"""
-        request = self.factory.post("")
-        request.user = self.rat
-        with self.assertRaises(PermissionDenied):
-            views.delete_list(request, self.list.id)
-
-    def test_curate_approve(self):
-        """approve a pending item"""
-        view = views.Curate.as_view()
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            pending = models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
-                approved=False,
-                order=1,
-            )
-
-        request = self.factory.post(
-            "",
-            {"item": pending.id, "approved": "true"},
-        )
-        request.user = self.local_user
-
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock:
-            view(request, self.list.id)
-
-        self.assertEqual(mock.call_count, 2)
-        activity = json.loads(mock.call_args[1]["args"][1])
-        self.assertEqual(activity["type"], "Add")
-        self.assertEqual(activity["actor"], self.local_user.remote_id)
-        self.assertEqual(activity["target"], self.list.remote_id)
-
-        pending.refresh_from_db()
-        self.assertEqual(self.list.books.count(), 1)
-        self.assertEqual(self.list.listitem_set.first(), pending)
-        self.assertTrue(pending.approved)
-
-    def test_curate_reject(self):
-        """approve a pending item"""
-        view = views.Curate.as_view()
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            pending = models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
-                approved=False,
-                order=1,
-            )
-
-        request = self.factory.post(
-            "",
-            {
-                "item": pending.id,
-                "approved": "false",
-            },
-        )
-        request.user = self.local_user
-
-        view(request, self.list.id)
-
-        self.assertFalse(self.list.books.exists())
-        self.assertFalse(models.ListItem.objects.exists())
-
-    def test_add_book(self):
-        """put a book on a list"""
-        request = self.factory.post(
-            "",
-            {
-                "book": self.book.id,
-                "list": self.list.id,
-            },
-        )
-        request.user = self.local_user
-
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock:
-            views.add_book(request)
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args[1]["args"][1])
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        item = self.list.listitem_set.get()
-        self.assertEqual(item.book, self.book)
-        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.apply_async"):
-            views.add_book(request_one)
-            views.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.apply_async"):
-            views.add_book(request_one)
-            views.add_book(request_two)
-            views.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.apply_async"):
-            views.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.apply_async"):
-            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.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.apply_async"):
-            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.apply_async"):
-            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.apply_async"):
-            views.add_book(request_one)
-            views.add_book(request_two)
-            views.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.apply_async"):
-            views.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"
-        self.list.save(broadcast=False)
-        request = self.factory.post(
-            "",
-            {
-                "book": self.book.id,
-                "list": self.list.id,
-            },
-        )
-        request.user = self.rat
-
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock:
-            views.add_book(request)
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args[1]["args"][1])
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.rat.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        item = self.list.listitem_set.get()
-        self.assertEqual(item.book, self.book)
-        self.assertEqual(item.user, self.rat)
-        self.assertTrue(item.approved)
-
-    def test_add_book_pending(self):
-        """put a book on a list awaiting approval"""
-        self.list.curation = "curated"
-        self.list.save(broadcast=False)
-        request = self.factory.post(
-            "",
-            {
-                "book": self.book.id,
-                "list": self.list.id,
-            },
-        )
-        request.user = self.rat
-
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock:
-            views.add_book(request)
-
-        self.assertEqual(mock.call_count, 1)
-        activity = json.loads(mock.call_args[1]["args"][1])
-
-        self.assertEqual(activity["type"], "Add")
-        self.assertEqual(activity["actor"], self.rat.remote_id)
-        self.assertEqual(activity["target"], self.list.remote_id)
-
-        item = self.list.listitem_set.get()
-        self.assertEqual(activity["object"]["id"], item.remote_id)
-
-        self.assertEqual(item.book, self.book)
-        self.assertEqual(item.user, self.rat)
-        self.assertFalse(item.approved)
-
-    def test_add_book_self_curated(self):
-        """put a book on a list automatically approved"""
-        self.list.curation = "curated"
-        self.list.save(broadcast=False)
-        request = self.factory.post(
-            "",
-            {
-                "book": self.book.id,
-                "list": self.list.id,
-            },
-        )
-        request.user = self.local_user
-
-        with patch(
-            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
-        ) as mock:
-            views.add_book(request)
-            self.assertEqual(mock.call_count, 1)
-            activity = json.loads(mock.call_args[1]["args"][1])
-            self.assertEqual(activity["type"], "Add")
-            self.assertEqual(activity["actor"], self.local_user.remote_id)
-            self.assertEqual(activity["target"], self.list.remote_id)
-
-        item = self.list.listitem_set.get()
-        self.assertEqual(item.book, self.book)
-        self.assertEqual(item.user, self.local_user)
-        self.assertTrue(item.approved)
-
-    def test_remove_book(self):
-        """take an item off a list"""
-
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            item = models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
-                order=1,
-            )
-        self.assertTrue(self.list.listitem_set.exists())
-
-        request = self.factory.post("", {"item": item.id})
-        request.user = self.local_user
-
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            views.remove_book(request, self.list.id)
-        self.assertFalse(self.list.listitem_set.exists())
-
-    def test_remove_book_unauthorized(self):
-        """take an item off a list"""
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
-            item = models.ListItem.objects.create(
-                book_list=self.list, user=self.local_user, book=self.book, order=1
-            )
-        self.assertTrue(self.list.listitem_set.exists())
-        request = self.factory.post("", {"item": item.id})
-        request.user = self.rat
-
-        with self.assertRaises(PermissionDenied):
-            views.remove_book(request, self.list.id)
-        self.assertTrue(self.list.listitem_set.exists())
-
-    def test_save_unsave_list(self):
-        """bookmark a list"""
-        self.assertFalse(self.local_user.saved_lists.exists())
-        request = self.factory.post("")
-        request.user = self.local_user
-        views.save_list(request, self.list.id)
-        self.local_user.refresh_from_db()
-        self.assertEqual(self.local_user.saved_lists.first(), self.list)
-
-        views.unsave_list(request, self.list.id)
-        self.local_user.refresh_from_db()
-        self.assertFalse(self.local_user.saved_lists.exists())

From 2eabe294012396cac43c581cd4cbeef070b89698 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 13:54:44 -0800
Subject: [PATCH 17/22] Show notes on curation page

---
 bookwyrm/templates/lists/curate.html | 12 ++++++++++++
 bookwyrm/templates/lists/list.html   |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/bookwyrm/templates/lists/curate.html b/bookwyrm/templates/lists/curate.html
index 927fb5b6a..9b484eb40 100644
--- a/bookwyrm/templates/lists/curate.html
+++ b/bookwyrm/templates/lists/curate.html
@@ -1,5 +1,6 @@
 {% extends 'lists/layout.html' %}
 {% load i18n %}
+{% load utilities %}
 
 {% block breadcrumbs %}
 <nav class="breadcrumb subtitle" aria-label="breadcrumbs">
@@ -38,6 +39,17 @@
 
                     <div class="column ml-3">
                         {% include 'snippets/book_titleby.html' %}
+                        {% if item.notes %}
+                        <div>
+                            {% url 'user-feed' item.user|username as user_path %}
+                            {% blocktrans trimmed with username=item.user.display_name %}
+                            <a href="{{ user_path }}">{{ username }}</a> says:
+                            {% endblocktrans %}
+                            <p class="notification">
+                                {{ item.notes }}
+                            </p>
+                        </div>
+                        {% endif %}
 
                         <p>
                             {% trans "Suggested by" %}
diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index 0a186e8ed..01970881c 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -22,7 +22,7 @@
 {% block panel %}
 {% if request.user == list.user and pending_count %}
 <div class="block content">
-    <p>
+    <p class="notification">
         <a href="{% url 'list-curate' list.id %}">{{ pending_count }} book{{ pending_count|pluralize }} awaiting your approval</a>
     </p>
 </div>
@@ -80,7 +80,7 @@
                                 <div class="content">
                                     <header>
                                         {% url 'user-feed' user|username as user_path %}
-                                        {% blocktrans trimmed with username=user|username %}
+                                        {% blocktrans trimmed with username=user.display_name %}
                                         <a href="{{ user_path }}">{{ username }}</a> says:
                                         {% endblocktrans %}
                                     </header>

From eddd9ccc325def0a48c85f92e46b928bfde63ce3 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Tue, 25 Jan 2022 13:57:12 -0800
Subject: [PATCH 18/22] Refer to user by id in form tests

---
 bookwyrm/tests/views/lists/test_list.py | 26 ++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/bookwyrm/tests/views/lists/test_list.py b/bookwyrm/tests/views/lists/test_list.py
index 489681547..432f23093 100644
--- a/bookwyrm/tests/views/lists/test_list.py
+++ b/bookwyrm/tests/views/lists/test_list.py
@@ -286,7 +286,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request.user = self.local_user
@@ -316,7 +316,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_one.user = self.local_user
@@ -326,7 +326,7 @@ class ListViews(TestCase):
             {
                 "book": self.book_two.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_two.user = self.local_user
@@ -350,7 +350,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_one.user = self.local_user
@@ -360,7 +360,7 @@ class ListViews(TestCase):
             {
                 "book": self.book_two.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_two.user = self.local_user
@@ -370,7 +370,7 @@ class ListViews(TestCase):
             {
                 "book": self.book_three.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_three.user = self.local_user
@@ -409,7 +409,7 @@ class ListViews(TestCase):
             {
                 "book": self.book_three.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request.user = self.local_user
@@ -520,7 +520,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_one.user = self.local_user
@@ -530,7 +530,7 @@ class ListViews(TestCase):
             {
                 "book": self.book_two.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_two.user = self.local_user
@@ -540,7 +540,7 @@ class ListViews(TestCase):
             {
                 "book": self.book_three.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request_three.user = self.local_user
@@ -579,7 +579,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request.user = self.rat
@@ -608,7 +608,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request.user = self.rat
@@ -641,7 +641,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user,
+                "user": self.local_user.id,
             },
         )
         request.user = self.local_user

From 65b9872c72365b35a1c4bb2c15e8d82183ecf56f Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Wed, 26 Jan 2022 10:37:50 -0800
Subject: [PATCH 19/22] Fixes approving curated submissions

---
 bookwyrm/tests/views/lists/test_list.py | 12 ++++++++++--
 bookwyrm/views/list/list.py             | 11 +++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/bookwyrm/tests/views/lists/test_list.py b/bookwyrm/tests/views/lists/test_list.py
index 432f23093..5469b2333 100644
--- a/bookwyrm/tests/views/lists/test_list.py
+++ b/bookwyrm/tests/views/lists/test_list.py
@@ -32,6 +32,14 @@ class ListViews(TestCase):
                 localname="mouse",
                 remote_id="https://example.com/users/mouse",
             )
+            self.rat = models.User.objects.create_user(
+                "rat@local.com",
+                "rat@rat.com",
+                "ratword",
+                local=True,
+                localname="rat",
+                remote_id="https://example.com/users/rat",
+            )
         work = models.Work.objects.create(title="Work")
         self.book = models.Edition.objects.create(
             title="Example Edition",
@@ -579,7 +587,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user.id,
+                "user": self.rat.id,
             },
         )
         request.user = self.rat
@@ -608,7 +616,7 @@ class ListViews(TestCase):
             {
                 "book": self.book.id,
                 "book_list": self.list.id,
-                "user": self.local_user.id,
+                "user": self.rat.id,
             },
         )
         request.user = self.rat
diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py
index c66fe2de4..4c740924d 100644
--- a/bookwyrm/views/list/list.py
+++ b/bookwyrm/views/list/list.py
@@ -3,6 +3,7 @@ from typing import Optional
 from urllib.parse import urlencode
 
 from django.contrib.auth.decorators import login_required
+from django.core.exceptions import PermissionDenied
 from django.core.paginator import Paginator
 from django.db import IntegrityError, transaction
 from django.db.models import Avg, DecimalField, Q, Max
@@ -167,7 +168,13 @@ def add_book(request):
     """put a book on a list"""
     book_list = get_object_or_404(models.List, id=request.POST.get("book_list"))
     # make sure the user is allowed to submit to this list
-    book_list.raise_not_submittable(request.user)
+    book_list.raise_visible_to_user(request.user)
+
+    if request.user != book_list.user and book_list.curation == "closed":
+        raise PermissionDenied()
+    is_group_member = models.GroupMember.objects.filter(
+        group=book_list.group, user=request.user
+    ).exists()
 
     form = forms.ListItemForm(request.POST)
     if not form.is_valid():
@@ -178,7 +185,7 @@ def add_book(request):
     if book_list.curation == "curated":
         # make a pending entry at the end of the list
         order_max = (book_list.listitem_set.aggregate(Max("order"))["order__max"]) or 0
-        item.approved = False
+        item.approved = is_group_member or request.user == book_list.user
     else:
         # add the book at the latest order of approved books, before pending books
         order_max = (

From 0d69d9b48b340ed7fbc0904cd8f73ed7ac201a8d Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Wed, 26 Jan 2022 11:46:42 -0800
Subject: [PATCH 20/22] Adds more unit tests for list views

---
 bookwyrm/tests/views/lists/test_list.py      | 30 +++++++++
 bookwyrm/tests/views/lists/test_list_item.py | 70 ++++++++++++++++++++
 bookwyrm/views/list/list.py                  |  5 +-
 3 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100644 bookwyrm/tests/views/lists/test_list_item.py

diff --git a/bookwyrm/tests/views/lists/test_list.py b/bookwyrm/tests/views/lists/test_list.py
index 5469b2333..a999d4cd1 100644
--- a/bookwyrm/tests/views/lists/test_list.py
+++ b/bookwyrm/tests/views/lists/test_list.py
@@ -95,6 +95,19 @@ class ListViews(TestCase):
         validate_html(result.render())
         self.assertEqual(result.status_code, 200)
 
+    def test_list_page_with_query(self):
+        """searching for a book to add"""
+        view = views.List.as_view()
+        request = self.factory.get("", {"q": "Example Edition"})
+        request.user = self.local_user
+
+        with patch("bookwyrm.views.list.list.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.list.id)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(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()
@@ -669,6 +682,23 @@ class ListViews(TestCase):
         self.assertEqual(item.user, self.local_user)
         self.assertTrue(item.approved)
 
+    def test_add_book_permission_denied(self):
+        """you can't add to that list"""
+        self.list.curation = "closed"
+        self.list.save(broadcast=False)
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "book_list": self.list.id,
+                "user": self.rat.id,
+            },
+        )
+        request.user = self.rat
+
+        with self.assertRaises(PermissionDenied):
+            views.add_book(request)
+
     def test_remove_book(self):
         """take an item off a list"""
 
diff --git a/bookwyrm/tests/views/lists/test_list_item.py b/bookwyrm/tests/views/lists/test_list_item.py
new file mode 100644
index 000000000..0d6f66abe
--- /dev/null
+++ b/bookwyrm/tests/views/lists/test_list_item.py
@@ -0,0 +1,70 @@
+""" test for app action functionality """
+from unittest.mock import patch
+
+from django.test import TestCase
+from django.test.client import RequestFactory
+
+from bookwyrm import models, views
+
+
+# pylint: disable=unused-argument
+# pylint: disable=too-many-public-methods
+class ListItemViews(TestCase):
+    """list view"""
+
+    def setUp(self):
+        """we need basic test data and mocks"""
+        self.factory = RequestFactory()
+        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
+            "bookwyrm.activitystreams.populate_stream_task.delay"
+        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
+            self.local_user = models.User.objects.create_user(
+                "mouse@local.com",
+                "mouse@mouse.com",
+                "mouseword",
+                local=True,
+                localname="mouse",
+                remote_id="https://example.com/users/mouse",
+            )
+        work = models.Work.objects.create(title="Work")
+        self.book = models.Edition.objects.create(
+            title="Example Edition",
+            remote_id="https://example.com/book/1",
+            parent_work=work,
+        )
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            self.list = models.List.objects.create(
+                name="Test List", user=self.local_user
+            )
+
+        models.SiteSettings.objects.create()
+
+    def test_add_list_item_notes(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.ListItem.as_view()
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+            item = models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=True,
+                order=1,
+            )
+        request = self.factory.post(
+            "",
+            {
+                "book_list": self.list.id,
+                "book": self.book.id,
+                "user": self.local_user.id,
+                "notes": "beep boop",
+            },
+        )
+        request.user = self.local_user
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ) as mock:
+            view(request, self.list.id, item.id)
+        self.assertEqual(mock.call_count, 1)
+
+        item.refresh_from_db()
+        self.assertEqual(item.notes, "beep boop")
diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py
index 4c740924d..fbbbee9fe 100644
--- a/bookwyrm/views/list/list.py
+++ b/bookwyrm/views/list/list.py
@@ -121,7 +121,8 @@ class List(View):
 
         form = forms.ListForm(request.POST, instance=book_list)
         if not form.is_valid():
-            return redirect("list", book_list.id)
+            # this shouldn't happen
+            raise Exception(form.errors)
         book_list = form.save()
         if not book_list.curation == "group":
             book_list.group = None
@@ -169,9 +170,9 @@ def add_book(request):
     book_list = get_object_or_404(models.List, id=request.POST.get("book_list"))
     # make sure the user is allowed to submit to this list
     book_list.raise_visible_to_user(request.user)
-
     if request.user != book_list.user and book_list.curation == "closed":
         raise PermissionDenied()
+
     is_group_member = models.GroupMember.objects.filter(
         group=book_list.group, user=request.user
     ).exists()

From d62473c4323a183087d7aadebbee0c172469bca5 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 27 Jan 2022 11:04:30 -0800
Subject: [PATCH 21/22] Tests that list item notes federate

---
 bookwyrm/tests/views/inbox/test_inbox_add.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bookwyrm/tests/views/inbox/test_inbox_add.py b/bookwyrm/tests/views/inbox/test_inbox_add.py
index a9a809825..fccd1a50f 100644
--- a/bookwyrm/tests/views/inbox/test_inbox_add.py
+++ b/bookwyrm/tests/views/inbox/test_inbox_add.py
@@ -118,6 +118,7 @@ class InboxAdd(TestCase):
                 "type": "ListItem",
                 "book": self.book.remote_id,
                 "id": "https://example.com/listbook/6189",
+                "notes": "hi hello",
                 "order": 1,
             },
             "target": "https://example.com/user/mouse/list/to-read",
@@ -130,3 +131,4 @@ class InboxAdd(TestCase):
         self.assertEqual(booklist.name, "Test List")
         self.assertEqual(booklist.books.first(), self.book)
         self.assertEqual(listitem.remote_id, "https://example.com/listbook/6189")
+        self.assertEqual(listitem.notes, "hi hello")

From a80c19d3e55c53397760fd49e92a970b2d24bc71 Mon Sep 17 00:00:00 2001
From: Mouse Reeve <mousereeve@riseup.net>
Date: Thu, 27 Jan 2022 12:55:53 -0800
Subject: [PATCH 22/22] Screen reader fixes for list view

---
 bookwyrm/templates/lists/item_notes_field.html | 2 +-
 bookwyrm/templates/lists/list.html             | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bookwyrm/templates/lists/item_notes_field.html b/bookwyrm/templates/lists/item_notes_field.html
index 76c288640..c09c50236 100644
--- a/bookwyrm/templates/lists/item_notes_field.html
+++ b/bookwyrm/templates/lists/item_notes_field.html
@@ -12,7 +12,7 @@
             id="id_notes_{{ form_id }}"
             maxlength="300"
             name="notes"
-            aria-described-by="notes_description_{{ form_id }}"
+            aria-describedby="notes_description_{{ form_id }}"
         >{{ item.notes|default:'' }}</textarea>
     </div>
     <p class="help" id="notes_description_{{ form_id }}">
diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html
index 01970881c..8e35416f6 100644
--- a/bookwyrm/templates/lists/list.html
+++ b/bookwyrm/templates/lists/list.html
@@ -73,7 +73,7 @@
 
                         {% if item.notes %}
                         <div class="media notification">
-                            <figure class="media-left">
+                            <figure class="media-left" aria-hidden="true">
                                 {% include "snippets/avatar.html" with user=item.user %}
                             </figure>
                             <div class="media-content">
@@ -263,7 +263,7 @@
                 <textarea
                     readonly
                     class="textarea is-small"
-                    aria-labelledby="embed-label"
+                    aria-describedby="embed-label"
                     data-copytext
                     data-copytext-label="{% trans 'Copy embed code' %}"
                     data-copytext-success="{% trans 'Copied!' %}"