716 books can be on multiple shelves, but only on one shelf for reading status

.gitignore

@@ -3,6 +3,7 @@
 *.pyc
 *.swp
 **/__pycache__
+.local
 
 # VSCode
 /.vscode
@@ -17,3 +18,6 @@
 
 # Testing
 .coverage
+
+#PyCharm
+.idea

bookwyrm/models/shelf.py

@@ -11,6 +11,12 @@ from . import fields
 class Shelf(OrderedCollectionMixin, BookWyrmModel):
     """ a list of books owned by a user """
 
+    TO_READ = "to-read"
+    READING = "reading"
+    READ_FINISHED = "read"
+
+    READ_STATUS_IDENTIFIERS = (TO_READ, READING, READ_FINISHED)
+
     name = fields.CharField(max_length=100)
     identifier = models.CharField(max_length=100)
     user = fields.ForeignKey(

bookwyrm/templates/snippets/shelf_selector.html

@@ -12,6 +12,7 @@
     <form class="dropdown-item pt-0 pb-0" name="shelve" action="/shelve/" method="post">
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ book.id }}">
+        <input type="hidden" name="change-shelf-from" value={{ current.identifier }}>
         <input type="hidden" name="shelf" value="{{ shelf.identifier }}">
         <button class="button is-fullwidth is-small" type="submit">{{ shelf.name }}</button>
     </form>

bookwyrm/tests/views/test_reading.py

@@ -42,7 +42,7 @@ class ReadingViews(TestCase):
 
     def test_start_reading(self, _):
         """ begin a book """
-        shelf = self.local_user.shelf_set.get(identifier="reading")
+        shelf = self.local_user.shelf_set.get(identifier=models.Shelf.READING)
         self.assertFalse(shelf.books.exists())
         self.assertFalse(models.Status.objects.exists())
 
@@ -73,7 +73,7 @@ class ReadingViews(TestCase):
 
     def test_start_reading_reshelf(self, _):
         """ begin a book """
-        to_read_shelf = self.local_user.shelf_set.get(identifier="to-read")
+        to_read_shelf = self.local_user.shelf_set.get(identifier=models.Shelf.TO_READ)
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             models.ShelfBook.objects.create(
                 shelf=to_read_shelf, book=self.book, user=self.local_user
@@ -93,7 +93,7 @@ class ReadingViews(TestCase):
 
     def test_finish_reading(self, _):
         """ begin a book """
-        shelf = self.local_user.shelf_set.get(identifier="read")
+        shelf = self.local_user.shelf_set.get(identifier=models.Shelf.READ_FINISHED)
         self.assertFalse(shelf.books.exists())
         self.assertFalse(models.Status.objects.exists())
         readthrough = models.ReadThrough.objects.create(

bookwyrm/views/reading.py

@@ -19,7 +19,9 @@ from .shelf import handle_unshelve
 def start_reading(request, book_id):
     """ begin reading a book """
     book = get_edition(book_id)
-    shelf = models.Shelf.objects.filter(identifier="reading", user=request.user).first()
+    reading_shelf = models.Shelf.objects.filter(
+        identifier=models.Shelf.READING, user=request.user
+    ).first()
 
     # create a readthrough
     readthrough = update_readthrough(request, book=book)
@@ -29,20 +31,27 @@ def start_reading(request, book_id):
         # create a progress update if we have a page
         readthrough.create_update()
 
-    # shelve the book
+    current_status_shelfbook = (
-    if request.POST.get("reshelve", True):
+        models.ShelfBook.objects.select_related("shelf")
-        try:
+        .filter(
-            current_shelf = models.Shelf.objects.get(user=request.user, edition=book)
+            shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
-            handle_unshelve(request.user, book, current_shelf)
+            user=request.user,
-        except models.Shelf.DoesNotExist:
+            book=book,
-            # this just means it isn't currently on the user's shelves
+        )
-            pass
+        .first()
-    models.ShelfBook.objects.create(book=book, shelf=shelf, user=request.user)
+    )
+    if current_status_shelfbook is not None:
+        if current_status_shelfbook.shelf.identifier != models.Shelf.READING:
+            handle_unshelve(book, current_status_shelfbook.shelf)
+        else:  # It already was on the shelf
+            return redirect(request.headers.get("Referer", "/"))
+
+    models.ShelfBook.objects.create(book=book, shelf=reading_shelf, user=request.user)
 
     # post about it (if you want)
     if request.POST.get("post-status"):
         privacy = request.POST.get("privacy")
-        handle_reading_status(request.user, shelf, book, privacy)
+        handle_reading_status(request.user, reading_shelf, book, privacy)
 
     return redirect(request.headers.get("Referer", "/"))
 
@@ -52,27 +61,38 @@ def start_reading(request, book_id):
 def finish_reading(request, book_id):
     """ a user completed a book, yay """
     book = get_edition(book_id)
-    shelf = models.Shelf.objects.filter(identifier="read", user=request.user).first()
+    finished_read_shelf = models.Shelf.objects.filter(
+        identifier=models.Shelf.READ_FINISHED, user=request.user
+    ).first()
 
     # update or create a readthrough
     readthrough = update_readthrough(request, book=book)
     if readthrough:
         readthrough.save()
 
-    # shelve the book
+    current_status_shelfbook = (
-    if request.POST.get("reshelve", True):
+        models.ShelfBook.objects.select_related("shelf")
-        try:
+        .filter(
-            current_shelf = models.Shelf.objects.get(user=request.user, edition=book)
+            shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
-            handle_unshelve(request.user, book, current_shelf)
+            user=request.user,
-        except models.Shelf.DoesNotExist:
+            book=book,
-            # this just means it isn't currently on the user's shelves
+        )
-            pass
+        .first()
-    models.ShelfBook.objects.create(book=book, shelf=shelf, user=request.user)
+    )
+    if current_status_shelfbook is not None:
+        if current_status_shelfbook.shelf.identifier != models.Shelf.READ_FINISHED:
+            handle_unshelve(book, current_status_shelfbook.shelf)
+        else:  # It already was on the shelf
+            return redirect(request.headers.get("Referer", "/"))
+
+    models.ShelfBook.objects.create(
+        book=book, shelf=finished_read_shelf, user=request.user
+    )
 
     # post about it (if you want)
     if request.POST.get("post-status"):
         privacy = request.POST.get("privacy")
-        handle_reading_status(request.user, shelf, book, privacy)
+        handle_reading_status(request.user, finished_read_shelf, book, privacy)
 
     return redirect(request.headers.get("Referer", "/"))
 

bookwyrm/views/shelf.py

@@ -1,4 +1,5 @@
 """ shelf views"""
+from django.db import IntegrityError
 from django.contrib.auth.decorators import login_required
 from django.http import HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import get_object_or_404, redirect
@@ -118,7 +119,7 @@ def delete_shelf(request, shelf_id):
 @login_required
 @require_POST
 def shelve(request):
-    """ put a  on a user's shelf """
+    """ put a book on a user's shelf """
     book = get_edition(request.POST.get("book"))
 
     desired_shelf = models.Shelf.objects.filter(
@@ -127,21 +128,50 @@ def shelve(request):
     if not desired_shelf:
         return HttpResponseNotFound()
 
-    if request.POST.get("reshelve", True):
+    change_from_current_identifier = request.POST.get("change-shelf-from")
-        try:
+    if change_from_current_identifier is not None:
-            current_shelf = models.Shelf.objects.get(user=request.user, edition=book)
+        current_shelf = models.Shelf.objects.get(
-            handle_unshelve(request.user, book, current_shelf)
+            user=request.user, identifier=change_from_current_identifier
-        except models.Shelf.DoesNotExist:
+        )
-            # this just means it isn't currently on the user's shelves
+        handle_unshelve(book, current_shelf)
-            pass
-    models.ShelfBook.objects.create(book=book, shelf=desired_shelf, user=request.user)
 
-    # post about "want to read" shelves
+    # A book can be on multiple shelves, but only on one read status shelf at a time
-    if desired_shelf.identifier == "to-read" and request.POST.get("post-status"):
+    if desired_shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS:
+        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
+            ):
+                handle_unshelve(book, current_read_status_shelfbook.shelf)
+            else:  # It is already on the shelf
+                return redirect(request.headers.get("Referer", "/"))
+
+        models.ShelfBook.objects.create(
+            book=book, shelf=desired_shelf, user=request.user
+        )
+        if desired_shelf.identifier == models.Shelf.TO_READ and request.POST.get(
+            "post-status"
+        ):
             privacy = request.POST.get("privacy") or desired_shelf.privacy
             handle_reading_status(request.user, desired_shelf, book, privacy=privacy)
-
+    else:
-    return redirect("/")
+        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
@@ -151,12 +181,12 @@ def unshelve(request):
     book = models.Edition.objects.get(id=request.POST["book"])
     current_shelf = models.Shelf.objects.get(id=request.POST["shelf"])
 
-    handle_unshelve(request.user, book, current_shelf)
+    handle_unshelve(book, current_shelf)
     return redirect(request.headers.get("Referer", "/"))
 
 
 # pylint: disable=unused-argument
-def handle_unshelve(user, book, shelf):
+def handle_unshelve(book, shelf):
     """ unshelve a book """
     row = models.ShelfBook.objects.get(book=book, shelf=shelf)
     row.delete()