From daf69d2375ec17100234cfe24660cfa6a368f1ef Mon Sep 17 00:00:00 2001 From: Pablo Barton Date: Thu, 25 Mar 2021 19:57:20 -0400 Subject: [PATCH] 716 books can be on multiple shelves, but only on one shelf for reading status --- .gitignore | 4 ++ bookwyrm/models/shelf.py | 6 ++ .../templates/snippets/shelf_selector.html | 1 + bookwyrm/tests/views/test_reading.py | 6 +- bookwyrm/views/reading.py | 64 ++++++++++++------- bookwyrm/views/shelf.py | 62 +++++++++++++----- 6 files changed, 102 insertions(+), 41 deletions(-) diff --git a/.gitignore b/.gitignore index 4b5b7fef2..00dd239d1 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ *.pyc *.swp **/__pycache__ +.local # VSCode /.vscode @@ -17,3 +18,6 @@ # Testing .coverage + +#PyCharm +.idea diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 3185b47f7..739c0599a 100644 --- a/bookwyrm/models/shelf.py +++ b/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( diff --git a/bookwyrm/templates/snippets/shelf_selector.html b/bookwyrm/templates/snippets/shelf_selector.html index 4f736c0e3..47ea73a0f 100644 --- a/bookwyrm/templates/snippets/shelf_selector.html +++ b/bookwyrm/templates/snippets/shelf_selector.html @@ -12,6 +12,7 @@
{% csrf_token %} +
diff --git a/bookwyrm/tests/views/test_reading.py b/bookwyrm/tests/views/test_reading.py index 4661e487e..b1ae6b88b 100644 --- a/bookwyrm/tests/views/test_reading.py +++ b/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( diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index fb63f2a3e..1cf9b4dc4 100644 --- a/bookwyrm/views/reading.py +++ b/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 - if request.POST.get("reshelve", True): - try: - current_shelf = models.Shelf.objects.get(user=request.user, edition=book) - handle_unshelve(request.user, book, current_shelf) - except models.Shelf.DoesNotExist: - # this just means it isn't currently on the user's shelves - pass - models.ShelfBook.objects.create(book=book, shelf=shelf, user=request.user) + current_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_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 - if request.POST.get("reshelve", True): - try: - current_shelf = models.Shelf.objects.get(user=request.user, edition=book) - handle_unshelve(request.user, book, current_shelf) - except models.Shelf.DoesNotExist: - # this just means it isn't currently on the user's shelves - pass - models.ShelfBook.objects.create(book=book, shelf=shelf, user=request.user) + current_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_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", "/")) diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 6256eac7f..1a952c718 100644 --- a/bookwyrm/views/shelf.py +++ b/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") + if change_from_current_identifier is not None: + current_shelf = models.Shelf.objects.get( + user=request.user, identifier=change_from_current_identifier + ) + handle_unshelve(book, current_shelf) + + # 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: + 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: try: - current_shelf = models.Shelf.objects.get(user=request.user, edition=book) - handle_unshelve(request.user, book, current_shelf) - except models.Shelf.DoesNotExist: - # this just means it isn't currently on the user's shelves + 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 - models.ShelfBook.objects.create(book=book, shelf=desired_shelf, user=request.user) - - # post about "want to read" shelves - if desired_shelf.identifier == "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) - - return redirect("/") + 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()