diff --git a/bookwyrm/templates/shelf/shelf.html b/bookwyrm/templates/shelf/shelf.html index fa1e91f9..662d7507 100644 --- a/bookwyrm/templates/shelf/shelf.html +++ b/bookwyrm/templates/shelf/shelf.html @@ -19,45 +19,58 @@ -
-
+
+
{% include 'shelf/create_shelf_form.html' with controls_text='create_shelf_form' %} diff --git a/bookwyrm/tests/views/shelf/__init__.py b/bookwyrm/tests/views/shelf/__init__.py new file mode 100644 index 00000000..b6e690fd --- /dev/null +++ b/bookwyrm/tests/views/shelf/__init__.py @@ -0,0 +1 @@ +from . import * diff --git a/bookwyrm/tests/views/shelf/test_shelf.py b/bookwyrm/tests/views/shelf/test_shelf.py new file mode 100644 index 00000000..71df3631 --- /dev/null +++ b/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") diff --git a/bookwyrm/tests/views/test_shelf.py b/bookwyrm/tests/views/shelf/test_shelf_actions.py similarity index 70% rename from bookwyrm/tests/views/test_shelf.py rename to bookwyrm/tests/views/shelf/test_shelf_actions.py index b78e241c..3efae0f4 100644 --- a/bookwyrm/tests/views/test_shelf.py +++ b/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"): diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index b502bd8d..e1dd8355 100644 --- a/bookwyrm/views/__init__.py +++ b/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 diff --git a/bookwyrm/views/shelf/__init__.py b/bookwyrm/views/shelf/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf/shelf.py similarity index 60% rename from bookwyrm/views/shelf.py rename to bookwyrm/views/shelf/shelf.py index 0b830d90..f8cffe93 100644 --- a/bookwyrm/views/shelf.py +++ b/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 = [ diff --git a/bookwyrm/views/shelf/shelf_actions.py b/bookwyrm/views/shelf/shelf_actions.py new file mode 100644 index 00000000..702b72c1 --- /dev/null +++ b/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", "/"))