Merge pull request #1462 from bookwyrm-social/fewer-active-shelf-queries

Fewer active shelf queries

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] if len(book.current_shelves) else {"book": book}
+
     shelf = (
         models.ShelfBook.objects.filter(
             shelf__user=context["request"].user,
@@ -84,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")

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)
+            with self.assertRaises(Http404):
-        self.assertEqual(result.status_code, 404)
+                view(request, 0)
 
     def test_book_page_work_id(self):
         """there are so many views, this just makes sure it LOADS"""

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,31 @@ 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):
+        user_statuses = user_statuses if request.user.is_authenticated else False
-            book = book.default_edition
+
+        # 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:
-            return HttpResponseNotFound()
+            raise Http404
 
-        work = book.parent_work
+        # all reviews for all editions of the book
-
-        # all reviews for 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

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")[
+            "books": models.Edition.viewer_aware_objects(user)
-                :limit
+            .filter(
-            ],
+                shelfbook__shelf=shelf,
+            )
+            .prefetch_related("authors")[:limit],
         }
         suggested_books.append(shelf_preview)
         book_count += len(shelf_preview["books"])

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:
@@ -49,10 +49,14 @@ class Shelf(View):
             FakeShelf = namedtuple(
                 "Shelf", ("identifier", "name", "user", "books", "privacy")
             )
-            books = models.Edition.objects.filter(
+            books = (
-                # privacy is ensured because the shelves are already filtered above
+                models.Edition.viewer_aware_objects(request.user)
-                shelfbook__shelf__in=shelves.all()
+                .filter(
-            ).distinct()
+                    # privacy is ensured because the shelves are already filtered above
+                    shelfbook__shelf__in=shelves
+                )
+                .distinct()
+            )
             shelf = FakeShelf("all", _("All books"), user, books, "public")
 
         if is_api_request(request):
@@ -82,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(