Merge pull request #1555 from bookwyrm-social/shelf-views

Shelf views

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/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/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", "/"))