From 6d06edc2c7a9233922b7f131c8a8cecfe0cb74ec Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 10:58:34 -0700 Subject: [PATCH 01/10] User viewer aware books for all books shelf view --- bookwyrm/views/shelf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index ba9f6a3c1..ae67a0c49 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -49,7 +49,7 @@ class Shelf(View): FakeShelf = namedtuple( "Shelf", ("identifier", "name", "user", "books", "privacy") ) - books = models.Edition.objects.filter( + books = models.Edition.viewer_aware_objects(request.user).filter( # privacy is ensured because the shelves are already filtered above shelfbook__shelf__in=shelves.all() ).distinct() From 995e2c47dbdff8b93bbd2d00c0318b37a8b4ffb5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 11:13:47 -0700 Subject: [PATCH 02/10] User viewer aware objects for book page Plus other refactors for that view --- bookwyrm/views/books.py | 36 ++++++++++++++++++++++-------------- bookwyrm/views/shelf.py | 12 ++++++++---- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index a31a99e71..7e7e90ed8 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -8,7 +8,7 @@ from django.core.files.base import ContentFile from django.core.paginator import Paginator from django.db import transaction from django.db.models import Avg, Q -from django.http import HttpResponseBadRequest, HttpResponseNotFound +from django.http import HttpResponseBadRequest, Http404 from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.utils.datastructures import MultiValueDictKeyError @@ -30,25 +30,33 @@ class Book(View): def get(self, request, book_id, user_statuses=False): """info about a book""" - user_statuses = user_statuses if request.user.is_authenticated else False - try: - book = models.Book.objects.select_subclasses().get(id=book_id) - except models.Book.DoesNotExist: - return HttpResponseNotFound() - if is_api_request(request): + book = get_object_or_404( + models.Book.objects.select_subclasses(), id=book_id + ) return ActivitypubResponse(book.to_activity()) - if isinstance(book, models.Work): - book = book.default_edition + user_statuses = user_statuses if request.user.is_authenticated else False + + try: + book = models.Edition.viewer_aware_objects(request.user).filter(id=book_id) + except models.Edition.DoesNotExist: + book = ( + models.Edition.viewer_aware_objects(request.user) + .filter( + parent_work__id=book_id, + ) + .order_by("-edition_rank") + ) + book = book.select_related("parent_work").prefetch_related("authors") + book = book.first() + if not book or not book.parent_work: - return HttpResponseNotFound() + raise Http404 - work = book.parent_work - - # all reviews for the book + # all reviews for all editions of the book reviews = privacy_filter( - request.user, models.Review.objects.filter(book__in=work.editions.all()) + request.user, models.Review.objects.filter(book__parent_work__editions=book) ) # the reviews to show diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index ae67a0c49..26d0f48de 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -49,10 +49,14 @@ class Shelf(View): FakeShelf = namedtuple( "Shelf", ("identifier", "name", "user", "books", "privacy") ) - books = models.Edition.viewer_aware_objects(request.user).filter( - # privacy is ensured because the shelves are already filtered above - shelfbook__shelf__in=shelves.all() - ).distinct() + books = ( + models.Edition.viewer_aware_objects(request.user) + .filter( + # privacy is ensured because the shelves are already filtered above + shelfbook__shelf__in=shelves.all() + ) + .distinct() + ) shelf = FakeShelf("all", _("All books"), user, books, "public") if is_api_request(request): From 35131262ffa59d3036aa18e0f0df6bad178740e8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 11:17:53 -0700 Subject: [PATCH 03/10] CHeck for current shelves attr before querying for it --- bookwyrm/templatetags/bookwyrm_tags.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index 1c9151577..de3948868 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -70,6 +70,9 @@ def related_status(notification): @register.simple_tag(takes_context=True) def active_shelf(context, book): """check what shelf a user has a book on, if any""" + if hasattr(book, "current_shelves"): + return book.current_shelves[0] + shelf = ( models.ShelfBook.objects.filter( shelf__user=context["request"].user, From cb089ed81768582ad909815dedf114155a701f98 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 11:23:55 -0700 Subject: [PATCH 04/10] One query to get book for book view --- bookwyrm/views/books.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 7e7e90ed8..6ab938db2 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -38,18 +38,16 @@ class Book(View): user_statuses = user_statuses if request.user.is_authenticated else False - try: - book = models.Edition.viewer_aware_objects(request.user).filter(id=book_id) - except models.Edition.DoesNotExist: - book = ( - models.Edition.viewer_aware_objects(request.user) - .filter( - parent_work__id=book_id, - ) - .order_by("-edition_rank") - ) - book = book.select_related("parent_work").prefetch_related("authors") - book = book.first() + # it's safe to use this OR because edition and work and subclasses of the same + # table, so they never have clashing IDs + book = ( + models.Edition.viewer_aware_objects(request.user) + .filter(Q(id=book_id) | Q(parent_work__id=book_id)) + .order_by("-edition_rank") + .select_related("parent_work") + .prefetch_related("authors") + .first() + ) if not book or not book.parent_work: raise Http404 From 8e8f46ee516822c6c572b0ae6173cafa7eca34bc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 11:27:46 -0700 Subject: [PATCH 05/10] Only call .all once in shelf view --- bookwyrm/views/shelf.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 26d0f48de..414ace39e 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -31,9 +31,9 @@ class Shelf(View): is_self = user == request.user if is_self: - shelves = user.shelf_set + shelves = user.shelf_set.all() else: - shelves = privacy_filter(request.user, user.shelf_set) + shelves = privacy_filter(request.user, user.shelf_set).all() # get the shelf and make sure the logged in user should be able to see it if shelf_identifier: @@ -53,7 +53,7 @@ class Shelf(View): models.Edition.viewer_aware_objects(request.user) .filter( # privacy is ensured because the shelves are already filtered above - shelfbook__shelf__in=shelves.all() + shelfbook__shelf__in=shelves ) .distinct() ) @@ -86,7 +86,7 @@ class Shelf(View): data = { "user": user, "is_self": is_self, - "shelves": shelves.all(), + "shelves": shelves, "shelf": shelf, "books": page, "page_range": paginated.get_elided_page_range( From ba4df5b38be6988b2d16d9a67882038cb1df415d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 11:29:53 -0700 Subject: [PATCH 06/10] Safely return if there are no current shelves --- bookwyrm/templatetags/bookwyrm_tags.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index de3948868..07e518cee 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -71,7 +71,7 @@ def related_status(notification): def active_shelf(context, book): """check what shelf a user has a book on, if any""" if hasattr(book, "current_shelves"): - return book.current_shelves[0] + return book.current_shelves[0] if len(book.current_shelves) else {"book": book} shelf = ( models.ShelfBook.objects.filter( From 36c00c49ab2b00cf163e99b4d08e2921b91fbb54 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 11:35:32 -0700 Subject: [PATCH 07/10] User viewer aware objects for feed suggested books --- bookwyrm/views/feed.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index 98a1c836c..09d45065b 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -168,9 +168,11 @@ def get_suggested_books(user, max_books=5): shelf_preview = { "name": shelf.name, "identifier": shelf.identifier, - "books": shelf.books.order_by("shelfbook").prefetch_related("authors")[ - :limit - ], + "books": models.Edition.viewer_aware_objects(user) + .filter( + shelfbook__shelf=shelf, + ) + .prefetch_related("authors")[:limit], } suggested_books.append(shelf_preview) book_count += len(shelf_preview["books"]) From dd3850a3ba3bf98009cbcbc614bef58cb3730931 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 12:26:56 -0700 Subject: [PATCH 08/10] Updates books test --- bookwyrm/templatetags/bookwyrm_tags.py | 1 + bookwyrm/tests/views/test_book.py | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index 07e518cee..b66f68c74 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -70,6 +70,7 @@ def related_status(notification): @register.simple_tag(takes_context=True) def active_shelf(context, book): """check what shelf a user has a book on, if any""" + print(hasattr(book, "current_shelves")) if hasattr(book, "current_shelves"): return book.current_shelves[0] if len(book.current_shelves) else {"book": book} diff --git a/bookwyrm/tests/views/test_book.py b/bookwyrm/tests/views/test_book.py index 99022ec5e..8c423e16a 100644 --- a/bookwyrm/tests/views/test_book.py +++ b/bookwyrm/tests/views/test_book.py @@ -9,6 +9,7 @@ import responses from django.contrib.auth.models import Group, Permission from django.contrib.contenttypes.models import ContentType from django.core.files.uploadedfile import SimpleUploadedFile +from django.http import Http404 from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory @@ -133,8 +134,8 @@ class BookViews(TestCase): request.user = self.local_user with patch("bookwyrm.views.books.is_api_request") as is_api: is_api.return_value = False - result = view(request, 0) - self.assertEqual(result.status_code, 404) + with self.assertRaises(Http404): + view(request, 0) def test_book_page_work_id(self): """there are so many views, this just makes sure it LOADS""" From d7a54b0b10a5e1e88ddbed75117c69140e130c3f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 12:56:40 -0700 Subject: [PATCH 09/10] Removes stray print --- bookwyrm/templatetags/bookwyrm_tags.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index b66f68c74..07e518cee 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -70,7 +70,6 @@ def related_status(notification): @register.simple_tag(takes_context=True) def active_shelf(context, book): """check what shelf a user has a book on, if any""" - print(hasattr(book, "current_shelves")) if hasattr(book, "current_shelves"): return book.current_shelves[0] if len(book.current_shelves) else {"book": book} From 1efe62a70e8e9c9619d1709fca6adef85326fe9e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 13:08:11 -0700 Subject: [PATCH 10/10] Check for readthrough annotation --- bookwyrm/templatetags/bookwyrm_tags.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index 07e518cee..e683f9c24 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -87,6 +87,9 @@ def active_shelf(context, book): @register.simple_tag(takes_context=False) def latest_read_through(book, user): """the most recent read activity""" + if hasattr(book, "active_readthroughs"): + return book.active_readthroughs[0] if len(book.active_readthroughs) else None + return ( models.ReadThrough.objects.filter(user=user, book=book, is_active=True) .order_by("-start_date")