Custom perms function for shelf view

bookwyrm/models/shelf.py

@@ -1,5 +1,6 @@
 """ puttin' books on shelves """
 import re
+from django.core.exceptions import PermissionDenied
 from django.db import models
 from django.utils import timezone
 
@@ -57,6 +58,12 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
         identifier = self.identifier or self.get_identifier()
         return f"{base_path}/books/{identifier}"
 
+    def raise_not_deletable(self, viewer):
+        """don't let anyone delete a default shelf"""
+        super().raise_not_deletable(viewer)
+        if not self.editable:
+            raise PermissionDenied()
+
     class Meta:
         """user/shelf unqiueness"""
 

bookwyrm/views/shelf.py

@@ -1,11 +1,11 @@
-""" shelf views"""
+""" shelf views """
 from collections import namedtuple
 
-from django.db import IntegrityError
+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
-from django.http import HttpResponseBadRequest, HttpResponseNotFound
+from django.http import HttpResponseBadRequest
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
@@ -16,7 +16,7 @@ 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_edition, get_user_from_username
+from .helpers import is_api_request, get_user_from_username
 from .helpers import privacy_filter
 
 
@@ -37,15 +37,11 @@ class Shelf(View):
 
         # get the shelf and make sure the logged in user should be able to see it
         if shelf_identifier:
-            try:
-                shelf = user.shelf_set.get(identifier=shelf_identifier)
-            except models.Shelf.DoesNotExist:
-                return HttpResponseNotFound()
-            if not shelf.visible_to_user(request.user):
-                return HttpResponseNotFound()
+            shelf = get_object_or_404(user.shelf_set, identifier=shelf_identifier)
+            shelf.raise_visible_to_user(request.user)
             books = shelf.books
-        # this is a constructed "all books" view, with a fake "shelf" obj
         else:
+            # this is a constructed "all books" view, with a fake "shelf" obj
             FakeShelf = namedtuple(
                 "Shelf", ("identifier", "name", "user", "books", "privacy")
             )
@@ -100,13 +96,11 @@ class Shelf(View):
     # pylint: disable=unused-argument
     def post(self, request, username, shelf_identifier):
         """edit a shelf"""
-        try:
-            shelf = request.user.shelf_set.get(identifier=shelf_identifier)
-        except models.Shelf.DoesNotExist:
-            return HttpResponseNotFound()
+        user = get_user_from_username(request.user, username)
+        shelf = get_object_or_404(user.shelf_set, identifier=shelf_identifier)
+        shelf.raise_not_editable(request.user)
 
-        if request.user != shelf.user:
-            return HttpResponseBadRequest()
+        # you can't change the name of the default shelves
         if not shelf.editable and request.POST.get("name") != shelf.name:
             return HttpResponseBadRequest()
 
@@ -134,8 +128,7 @@ def create_shelf(request):
 def delete_shelf(request, shelf_id):
     """user generated shelves"""
     shelf = get_object_or_404(models.Shelf, id=shelf_id)
-    if request.user != shelf.user or not shelf.editable:
-        return HttpResponseBadRequest()
+    shelf.raise_not_deletable()
 
     shelf.delete()
     return redirect("user-shelves", request.user.localname)
@@ -143,25 +136,28 @@ def delete_shelf(request, shelf_id):
 
 @login_required
 @require_POST
+@transaction.atomic
 def shelve(request):
     """put a book on a user's shelf"""
-    book = get_edition(request.POST.get("book"))
-
-    desired_shelf = models.Shelf.objects.filter(
-        identifier=request.POST.get("shelf"), user=request.user
-    ).first()
-    if not desired_shelf:
-        return HttpResponseNotFound()
-
-    change_from_current_identifier = request.POST.get("change-shelf-from")
-    if change_from_current_identifier is not None:
-        current_shelf = models.Shelf.objects.get(
-            user=request.user, identifier=change_from_current_identifier
+    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")
     )
-        handle_unshelve(book, current_shelf)
+
+    # first we need to remove from the specified shelf
+    change_from_current_identifier = request.POST.get("change-shelf-from")
+    if not 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(
@@ -176,14 +172,16 @@ def shelve(request):
                 current_read_status_shelfbook.shelf.identifier
                 != desired_shelf.identifier
             ):
-                handle_unshelve(book, current_read_status_shelfbook.shelf)
+                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
@@ -199,14 +197,11 @@ def shelve(request):
 @require_POST
 def unshelve(request):
     """put a on a user's shelf"""
-    book = models.Edition.objects.get(id=request.POST["book"])
-    current_shelf = models.Shelf.objects.get(id=request.POST["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)
 
-    handle_unshelve(book, current_shelf)
+    shelf_book.delete()
     return redirect(request.headers.get("Referer", "/"))
-
-
-def handle_unshelve(book, shelf):
-    """unshelve a book"""
-    row = models.ShelfBook.objects.get(book=book, shelf=shelf)
-    row.delete()