Merge branch 'main' into group-privacy

bookwyrm/templates/author/author.html

@@ -110,8 +110,14 @@
     {% for book in books %}
         <div class="column is-one-fifth">
             {% include 'landing/small-book.html' with book=book %}
+            {% include 'snippets/shelve_button/shelve_button.html' with book=book %}
         </div>
     {% endfor %}
     </div>
 </div>
+
+<div>
+    {% include 'snippets/pagination.html' with page=books %}
+</div>
+
 {% endblock %}

bookwyrm/templates/shelf/shelf.html

@@ -19,45 +19,58 @@
     </h1>
 </header>
 
-<div class="block columns">
+<nav class="block columns is-mobile scroll-x">
-    <div class="column">
+    <div class="column pr-0">
         <div class="tabs">
             <ul>
-            <li class="{% if shelf.identifier == 'all' %}is-active{% endif %}">
+                <li class="{% if shelf.identifier == 'all' %}is-active{% endif %}">
-                <a href="{% url 'user-shelves' user|username %}"{% if shelf.identifier == 'all' %} aria-current="page"{% endif %}>
+                    <a href="{% url 'user-shelves' user|username %}"{% if shelf.identifier == 'all' %} aria-current="page"{% endif %}>
-                    {% trans "All books" %}
+                        {% trans "All books" %}
-                </a>
+                    </a>
-            </li>
+                </li>
-            {% for shelf_tab in shelves %}
+                {% for shelf_tab in shelves %}
-            <li class="{% if shelf_tab.identifier == shelf.identifier %}is-active{% endif %}">
+                <li class="{% if shelf_tab.identifier == shelf.identifier %}is-active{% endif %}">
-                <a
+                    <a
-                    href="{{ shelf_tab.local_path }}"
+                        href="{{ shelf_tab.local_path }}"
-                    {% if shelf_tab.identifier == shelf.identifier %} aria-current="page"{% endif %}
+                        {% if shelf_tab.identifier == shelf.identifier %} aria-current="page"{% endif %}
-                >
+                    >
-                    {% if shelf_tab.identifier == 'to-read' %}
+                        {% if shelf_tab.identifier == 'to-read' %}
-                        {% trans "To Read" %}
+                            {% trans "To Read" %}
-                    {% elif shelf_tab.identifier == 'reading' %}
+                        {% elif shelf_tab.identifier == 'reading' %}
-                        {% trans "Currently Reading" %}
+                            {% trans "Currently Reading" %}
-                    {% elif shelf_tab.identifier == 'read' %}
+                        {% elif shelf_tab.identifier == 'read' %}
-                        {% trans "Read" %}
+                            {% trans "Read" %}
-                    {% else %}
+                        {% else %}
-                        {{ shelf_tab.name }}
+                            {{ shelf_tab.name }}
-                    {% endif %}
+                        {% endif %}
-                </a>
+                    </a>
-            </li>
+                </li>
-            {% endfor %}
+                {% endfor %}
+
             </ul>
         </div>
     </div>
 
     {% if is_self %}
+    <div class="column is-narrow pl-0">
+        <div class="tabs">
+            <ul>
+                <li>
+                    <a href="{% url 'import' %}">
+                        <span class="icon icon-list" aria-hidden="true"></span>
+                        <span>{% trans "Import Books" %}</span>
+                    </a>
+                </li>
+            </ul>
+        </div>
+    </div>
+
     <div class="column is-narrow">
         {% trans "Create shelf" as button_text %}
         {% include 'snippets/toggle/open_button.html' with text=button_text icon_with_text="plus" controls_text="create_shelf_form" focus="create_shelf_form_header" %}
-        <a class="button" href="{% url 'import' %}">{% trans "Import Books" %}</a>
     </div>
     {% endif %}
-</div>
+</nav>
 
 <div class="block">
     {% include 'shelf/create_shelf_form.html' with controls_text='create_shelf_form' %}

bookwyrm/tests/views/shelf/__init__.py

@@ -0,0 +1 @@
+from . import *

bookwyrm/tests/views/shelf/test_shelf.py

@@ -0,0 +1,165 @@
+""" 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.activitypub import ActivitypubResponse
+from bookwyrm.tests.validate_html import validate_html
+
+
+@patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay")
+@patch("bookwyrm.suggested_users.rerank_suggestions_task.delay")
+@patch("bookwyrm.activitystreams.populate_stream_task.delay")
+@patch("bookwyrm.activitystreams.add_book_statuses_task.delay")
+@patch("bookwyrm.activitystreams.remove_book_statuses_task.delay")
+class ShelfViews(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"
+        ):
+            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.work = models.Work.objects.create(title="Test Work")
+        self.book = models.Edition.objects.create(
+            title="Example Edition",
+            remote_id="https://example.com/book/1",
+            parent_work=self.work,
+        )
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            self.shelf = models.Shelf.objects.create(
+                name="Test Shelf", identifier="test-shelf", user=self.local_user
+            )
+        models.SiteSettings.objects.create()
+
+        self.anonymous_user = AnonymousUser
+        self.anonymous_user.is_authenticated = False
+
+    def test_shelf_page_all_books(self, *_):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Shelf.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.local_user.username)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+    def test_shelf_page_all_books_anonymous(self, *_):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Shelf.as_view()
+        request = self.factory.get("")
+        request.user = self.anonymous_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.local_user.username)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+    def test_shelf_page_sorted(self, *_):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Shelf.as_view()
+        shelf = self.local_user.shelf_set.first()
+        request = self.factory.get("", {"sort": "author"})
+        request.user = self.local_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.local_user.username, shelf.identifier)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+    def test_shelf_page(self, *_):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Shelf.as_view()
+        shelf = self.local_user.shelf_set.first()
+        request = self.factory.get("")
+        request.user = self.local_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, self.local_user.username, shelf.identifier)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
+        self.assertEqual(result.status_code, 200)
+
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = True
+            result = view(request, self.local_user.username, shelf.identifier)
+        self.assertIsInstance(result, ActivitypubResponse)
+        self.assertEqual(result.status_code, 200)
+
+        request = self.factory.get("/?page=1")
+        request.user = self.local_user
+        with patch("bookwyrm.views.shelf.shelf.is_api_request") as is_api:
+            is_api.return_value = True
+            result = view(request, self.local_user.username, shelf.identifier)
+        self.assertIsInstance(result, ActivitypubResponse)
+        self.assertEqual(result.status_code, 200)
+
+    def test_edit_shelf_privacy(self, *_):
+        """set name or privacy on shelf"""
+        view = views.Shelf.as_view()
+        shelf = self.local_user.shelf_set.get(identifier="to-read")
+        self.assertEqual(shelf.privacy, "public")
+
+        request = self.factory.post(
+            "",
+            {
+                "privacy": "unlisted",
+                "user": self.local_user.id,
+                "name": "To Read",
+            },
+        )
+        request.user = self.local_user
+        view(request, self.local_user.username, shelf.identifier)
+        shelf.refresh_from_db()
+
+        self.assertEqual(shelf.privacy, "unlisted")
+
+    def test_edit_shelf_name(self, *_):
+        """change the name of an editable shelf"""
+        view = views.Shelf.as_view()
+        shelf = models.Shelf.objects.create(name="Test Shelf", user=self.local_user)
+        self.assertEqual(shelf.privacy, "public")
+
+        request = self.factory.post(
+            "", {"privacy": "public", "user": self.local_user.id, "name": "cool name"}
+        )
+        request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            view(request, request.user.username, shelf.identifier)
+        shelf.refresh_from_db()
+
+        self.assertEqual(shelf.name, "cool name")
+        self.assertEqual(shelf.identifier, f"testshelf-{shelf.id}")
+
+    def test_edit_shelf_name_not_editable(self, *_):
+        """can't change the name of an non-editable shelf"""
+        view = views.Shelf.as_view()
+        shelf = self.local_user.shelf_set.get(identifier="to-read")
+        self.assertEqual(shelf.privacy, "public")
+
+        request = self.factory.post(
+            "", {"privacy": "public", "user": self.local_user.id, "name": "cool name"}
+        )
+        request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            view(request, request.user.username, shelf.identifier)
+
+        self.assertEqual(shelf.name, "To Read")

bookwyrm/tests/views/shelf/test_shelf_actions.py

@@ -3,13 +3,10 @@ import json
 from unittest.mock import patch
 
 from django.core.exceptions import PermissionDenied
-from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
 
 from bookwyrm import forms, models, views
-from bookwyrm.activitypub import ActivitypubResponse
-from bookwyrm.tests.validate_html import validate_html
 
 
 @patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay")
@@ -17,7 +14,7 @@ from bookwyrm.tests.validate_html import validate_html
 @patch("bookwyrm.activitystreams.populate_stream_task.delay")
 @patch("bookwyrm.activitystreams.add_book_statuses_task.delay")
 @patch("bookwyrm.activitystreams.remove_book_statuses_task.delay")
-class ShelfViews(TestCase):
+class ShelfActionViews(TestCase):
     """tag views"""
 
     def setUp(self):
@@ -46,85 +43,6 @@ class ShelfViews(TestCase):
             )
         models.SiteSettings.objects.create()
 
-    def test_shelf_page(self, *_):
-        """there are so many views, this just makes sure it LOADS"""
-        view = views.Shelf.as_view()
-        shelf = self.local_user.shelf_set.first()
-        request = self.factory.get("")
-        request.user = self.local_user
-        with patch("bookwyrm.views.shelf.is_api_request") as is_api:
-            is_api.return_value = False
-            result = view(request, self.local_user.username, shelf.identifier)
-        self.assertIsInstance(result, TemplateResponse)
-        validate_html(result.render())
-        self.assertEqual(result.status_code, 200)
-
-        with patch("bookwyrm.views.shelf.is_api_request") as is_api:
-            is_api.return_value = True
-            result = view(request, self.local_user.username, shelf.identifier)
-        self.assertIsInstance(result, ActivitypubResponse)
-        self.assertEqual(result.status_code, 200)
-
-        request = self.factory.get("/?page=1")
-        request.user = self.local_user
-        with patch("bookwyrm.views.shelf.is_api_request") as is_api:
-            is_api.return_value = True
-            result = view(request, self.local_user.username, shelf.identifier)
-        self.assertIsInstance(result, ActivitypubResponse)
-        self.assertEqual(result.status_code, 200)
-
-    def test_edit_shelf_privacy(self, *_):
-        """set name or privacy on shelf"""
-        view = views.Shelf.as_view()
-        shelf = self.local_user.shelf_set.get(identifier="to-read")
-        self.assertEqual(shelf.privacy, "public")
-
-        request = self.factory.post(
-            "",
-            {
-                "privacy": "unlisted",
-                "user": self.local_user.id,
-                "name": "To Read",
-            },
-        )
-        request.user = self.local_user
-        view(request, self.local_user.username, shelf.identifier)
-        shelf.refresh_from_db()
-
-        self.assertEqual(shelf.privacy, "unlisted")
-
-    def test_edit_shelf_name(self, *_):
-        """change the name of an editable shelf"""
-        view = views.Shelf.as_view()
-        shelf = models.Shelf.objects.create(name="Test Shelf", user=self.local_user)
-        self.assertEqual(shelf.privacy, "public")
-
-        request = self.factory.post(
-            "", {"privacy": "public", "user": self.local_user.id, "name": "cool name"}
-        )
-        request.user = self.local_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            view(request, request.user.username, shelf.identifier)
-        shelf.refresh_from_db()
-
-        self.assertEqual(shelf.name, "cool name")
-        self.assertEqual(shelf.identifier, f"testshelf-{shelf.id}")
-
-    def test_edit_shelf_name_not_editable(self, *_):
-        """can't change the name of an non-editable shelf"""
-        view = views.Shelf.as_view()
-        shelf = self.local_user.shelf_set.get(identifier="to-read")
-        self.assertEqual(shelf.privacy, "public")
-
-        request = self.factory.post(
-            "", {"privacy": "public", "user": self.local_user.id, "name": "cool name"}
-        )
-        request.user = self.local_user
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            view(request, request.user.username, shelf.identifier)
-
-        self.assertEqual(shelf.name, "To Read")
-
     def test_shelve(self, *_):
         """shelve a book"""
         request = self.factory.post(
@@ -182,6 +100,30 @@ class ShelfViews(TestCase):
         # make sure the book is on the shelf
         self.assertEqual(shelf.books.get(), self.book)
 
+    def test_shelve_read_with_change_shelf(self, *_):
+        """special behavior for the read shelf"""
+        previous_shelf = models.Shelf.objects.get(identifier="reading")
+        models.ShelfBook.objects.create(
+            shelf=previous_shelf, user=self.local_user, book=self.book
+        )
+        shelf = models.Shelf.objects.get(identifier="read")
+
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "shelf": shelf.identifier,
+                "change-shelf-from": previous_shelf.identifier,
+            },
+        )
+        request.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.shelve(request)
+        # make sure the book is on the shelf
+        self.assertEqual(shelf.books.get(), self.book)
+        self.assertEqual(list(previous_shelf.books.all()), [])
+
     def test_unshelve(self, *_):
         """remove a book from a shelf"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):

bookwyrm/tests/views/test_author.py

@@ -1,6 +1,7 @@
 """ test for app action functionality """
 from unittest.mock import patch
-from django.contrib.auth.models import Group, Permission
+
+from django.contrib.auth.models import AnonymousUser, Group, Permission
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import PermissionDenied
 from django.template.response import TemplateResponse
@@ -44,6 +45,8 @@ class AuthorViews(TestCase):
             parent_work=self.work,
         )
 
+        self.anonymous_user = AnonymousUser
+        self.anonymous_user.is_authenticated = False
         models.SiteSettings.objects.create()
 
     def test_author_page(self):
@@ -51,15 +54,33 @@ class AuthorViews(TestCase):
         view = views.Author.as_view()
         author = models.Author.objects.create(name="Jessica")
         request = self.factory.get("")
+        request.user = self.local_user
         with patch("bookwyrm.views.author.is_api_request") as is_api:
             is_api.return_value = False
             result = view(request, author.id)
         self.assertIsInstance(result, TemplateResponse)
         validate_html(result.render())
         self.assertEqual(result.status_code, 200)
+
+    def test_author_page_logged_out(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Author.as_view()
+        author = models.Author.objects.create(name="Jessica")
+        request = self.factory.get("")
+        request.user = self.anonymous_user
+        with patch("bookwyrm.views.author.is_api_request") as is_api:
+            is_api.return_value = False
+            result = view(request, author.id)
+        self.assertIsInstance(result, TemplateResponse)
+        validate_html(result.render())
         self.assertEqual(result.status_code, 200)
 
+    def test_author_page_api_response(self):
+        """there are so many views, this just makes sure it LOADS"""
+        view = views.Author.as_view()
+        author = models.Author.objects.create(name="Jessica")
         request = self.factory.get("")
+        request.user = self.local_user
         with patch("bookwyrm.views.author.is_api_request") as is_api:
             is_api.return_value = True
             result = view(request, author.id)
@@ -78,7 +99,6 @@ class AuthorViews(TestCase):
         self.assertIsInstance(result, TemplateResponse)
         validate_html(result.render())
         self.assertEqual(result.status_code, 200)
-        self.assertEqual(result.status_code, 200)
 
     def test_edit_author(self):
         """edit an author"""
@@ -126,5 +146,5 @@ class AuthorViews(TestCase):
         resp = view(request, author.id)
         author.refresh_from_db()
         self.assertEqual(author.name, "Test Author")
-        resp.render()
+        validate_html(resp.render())
         self.assertEqual(resp.status_code, 200)

bookwyrm/views/__init__.py

@@ -38,6 +38,11 @@ from .landing.login import Login, Logout
 from .landing.register import Register, ConfirmEmail, ConfirmEmailCode, resend_link
 from .landing.password import PasswordResetRequest, PasswordReset
 
+# shelves
+from .shelf.shelf import Shelf
+from .shelf.shelf_actions import create_shelf, delete_shelf
+from .shelf.shelf_actions import shelve, unshelve
+
 # misc views
 from .author import Author, EditAuthor
 from .directory import Directory
@@ -69,9 +74,6 @@ from .reading import create_readthrough, delete_readthrough, delete_progressupda
 from .reading import ReadingStatus
 from .rss_feed import RssFeed
 from .search import Search
-from .shelf import Shelf
-from .shelf import create_shelf, delete_shelf
-from .shelf import shelve, unshelve
 from .status import CreateStatus, EditStatus, DeleteStatus, update_progress
 from .status import edit_readthrough
 from .updates import get_notification_count, get_unread_status_count

bookwyrm/views/author.py

@@ -1,6 +1,7 @@
 """ the good people stuff! the authors! """
 from django.contrib.auth.decorators import login_required, permission_required
-from django.db.models import Q
+from django.core.paginator import Paginator
+from django.db.models import OuterRef, Subquery, F, Q
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
@@ -8,7 +9,8 @@ from django.views import View
 
 from bookwyrm import forms, models
 from bookwyrm.activitypub import ActivitypubResponse
-from .helpers import is_api_request
+from bookwyrm.settings import PAGE_LENGTH
+from bookwyrm.views.helpers import is_api_request
 
 
 # pylint: disable= no-self-use
@@ -22,12 +24,27 @@ class Author(View):
         if is_api_request(request):
             return ActivitypubResponse(author.to_activity())
 
-        books = models.Work.objects.filter(
+        default_editions = models.Edition.objects.filter(
-            Q(authors=author) | Q(editions__authors=author)
+            parent_work=OuterRef("parent_work")
-        ).distinct()
+        ).order_by("-edition_rank")
+
+        books = (
+            models.Edition.viewer_aware_objects(request.user)
+            .filter(Q(authors=author) | Q(parent_work__authors=author))
+            .annotate(default_id=Subquery(default_editions.values("id")[:1]))
+            .filter(default_id=F("id"))
+            .order_by("-first_published_date", "-published_date", "-created_date")
+            .prefetch_related("authors")
+        )
+
+        paginated = Paginator(books, PAGE_LENGTH)
+        page = paginated.get_page(request.GET.get("page"))
         data = {
             "author": author,
-            "books": [b.default_edition for b in books],
+            "books": page,
+            "page_range": paginated.get_elided_page_range(
+                page.number, on_each_side=2, on_ends=1
+            ),
         }
         return TemplateResponse(request, "author/author.html", data)
 

bookwyrm/views/shelf/shelf.py

@@ -1,7 +1,6 @@
 """ shelf views """
 from collections import namedtuple
 
-from django.db import IntegrityError, transaction
 from django.db.models import OuterRef, Subquery, F
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
@@ -11,12 +10,11 @@ from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
 from django.utils.translation import gettext_lazy as _
 from django.views import View
-from django.views.decorators.http import require_POST
 
 from bookwyrm import forms, models
 from bookwyrm.activitypub import ActivitypubResponse
 from bookwyrm.settings import PAGE_LENGTH
-from .helpers import is_api_request, get_user_from_username
+from bookwyrm.views.helpers import is_api_request, get_user_from_username
 
 
 # pylint: disable=no-self-use
@@ -128,102 +126,6 @@ class Shelf(View):
         return redirect(shelf.local_path)
 
 
-@login_required
-@require_POST
-def create_shelf(request):
-    """user generated shelves"""
-    form = forms.ShelfForm(request.POST)
-    if not form.is_valid():
-        return redirect(request.headers.get("Referer", "/"))
-
-    shelf = form.save()
-    return redirect(shelf.local_path)
-
-
-@login_required
-@require_POST
-def delete_shelf(request, shelf_id):
-    """user generated shelves"""
-    shelf = get_object_or_404(models.Shelf, id=shelf_id)
-    shelf.raise_not_deletable(request.user)
-
-    shelf.delete()
-    return redirect("user-shelves", request.user.localname)
-
-
-@login_required
-@require_POST
-@transaction.atomic
-def shelve(request):
-    """put a book on a user's shelf"""
-    book = get_object_or_404(models.Edition, id=request.POST.get("book"))
-    desired_shelf = get_object_or_404(
-        request.user.shelf_set, identifier=request.POST.get("shelf")
-    )
-
-    # first we need to remove from the specified shelf
-    change_from_current_identifier = request.POST.get("change-shelf-from")
-    if change_from_current_identifier:
-        # find the shelfbook obj and delete it
-        get_object_or_404(
-            models.ShelfBook,
-            book=book,
-            user=request.user,
-            shelf__identifier=change_from_current_identifier,
-        ).delete()
-
-    # A book can be on multiple shelves, but only on one read status shelf at a time
-    if desired_shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS:
-        # figure out where state shelf it's currently on (if any)
-        current_read_status_shelfbook = (
-            models.ShelfBook.objects.select_related("shelf")
-            .filter(
-                shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
-                user=request.user,
-                book=book,
-            )
-            .first()
-        )
-        if current_read_status_shelfbook is not None:
-            if (
-                current_read_status_shelfbook.shelf.identifier
-                != desired_shelf.identifier
-            ):
-                current_read_status_shelfbook.delete()
-            else:  # It is already on the shelf
-                return redirect(request.headers.get("Referer", "/"))
-
-        # create the new shelf-book entry
-        models.ShelfBook.objects.create(
-            book=book, shelf=desired_shelf, user=request.user
-        )
-    else:
-        # we're putting it on a custom shelf
-        try:
-            models.ShelfBook.objects.create(
-                book=book, shelf=desired_shelf, user=request.user
-            )
-        # The book is already on this shelf.
-        # Might be good to alert, or reject the action?
-        except IntegrityError:
-            pass
-    return redirect(request.headers.get("Referer", "/"))
-
-
-@login_required
-@require_POST
-def unshelve(request):
-    """put a on a user's shelf"""
-    book = get_object_or_404(models.Edition, id=request.POST.get("book"))
-    shelf_book = get_object_or_404(
-        models.ShelfBook, book=book, shelf__id=request.POST["shelf"]
-    )
-    shelf_book.raise_not_deletable(request.user)
-
-    shelf_book.delete()
-    return redirect(request.headers.get("Referer", "/"))
-
-
 def sort_books(books, sort):
     """Books in shelf sorting"""
     sort_fields = [

bookwyrm/views/shelf/shelf_actions.py

@@ -0,0 +1,103 @@
+""" shelf views """
+from django.db import IntegrityError, transaction
+from django.contrib.auth.decorators import login_required
+from django.shortcuts import get_object_or_404, redirect
+from django.views.decorators.http import require_POST
+
+from bookwyrm import forms, models
+
+
+@login_required
+@require_POST
+def create_shelf(request):
+    """user generated shelves"""
+    form = forms.ShelfForm(request.POST)
+    if not form.is_valid():
+        return redirect(request.headers.get("Referer", "/"))
+
+    shelf = form.save()
+    return redirect(shelf.local_path)
+
+
+@login_required
+@require_POST
+def delete_shelf(request, shelf_id):
+    """user generated shelves"""
+    shelf = get_object_or_404(models.Shelf, id=shelf_id)
+    shelf.raise_not_deletable(request.user)
+
+    shelf.delete()
+    return redirect("user-shelves", request.user.localname)
+
+
+@login_required
+@require_POST
+@transaction.atomic
+def shelve(request):
+    """put a book on a user's shelf"""
+    book = get_object_or_404(models.Edition, id=request.POST.get("book"))
+    desired_shelf = get_object_or_404(
+        request.user.shelf_set, identifier=request.POST.get("shelf")
+    )
+
+    # first we need to remove from the specified shelf
+    change_from_current_identifier = request.POST.get("change-shelf-from")
+    if change_from_current_identifier:
+        # find the shelfbook obj and delete it
+        get_object_or_404(
+            models.ShelfBook,
+            book=book,
+            user=request.user,
+            shelf__identifier=change_from_current_identifier,
+        ).delete()
+
+    # A book can be on multiple shelves, but only on one read status shelf at a time
+    if desired_shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS:
+        # figure out where state shelf it's currently on (if any)
+        current_read_status_shelfbook = (
+            models.ShelfBook.objects.select_related("shelf")
+            .filter(
+                shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
+                user=request.user,
+                book=book,
+            )
+            .first()
+        )
+        if current_read_status_shelfbook is not None:
+            if (
+                current_read_status_shelfbook.shelf.identifier
+                != desired_shelf.identifier
+            ):
+                current_read_status_shelfbook.delete()
+            else:  # It is already on the shelf
+                return redirect(request.headers.get("Referer", "/"))
+
+        # create the new shelf-book entry
+        models.ShelfBook.objects.create(
+            book=book, shelf=desired_shelf, user=request.user
+        )
+    else:
+        # we're putting it on a custom shelf
+        try:
+            models.ShelfBook.objects.create(
+                book=book, shelf=desired_shelf, user=request.user
+            )
+        # The book is already on this shelf.
+        # Might be good to alert, or reject the action?
+        except IntegrityError:
+            pass
+    return redirect(request.headers.get("Referer", "/"))
+
+
+@login_required
+@require_POST
+def unshelve(request):
+    """remove a book from a user's shelf"""
+    book = get_object_or_404(models.Edition, id=request.POST.get("book"))
+    shelf_book = get_object_or_404(
+        models.ShelfBook, book=book, shelf__id=request.POST["shelf"]
+    )
+    shelf_book.raise_not_deletable(request.user)
+
+    shelf_book.delete()
+    return redirect(request.headers.get("Referer", "/"))