Merge pull request #965 from bookwyrm-social/pagination-cleanup

Pagination cleanup

bookwyrm/models/list.py

@@ -47,7 +47,7 @@ class List(OrderedCollectionMixin, BookWyrmModel):
     @property
     def collection_queryset(self):
         """ list of books for this shelf, overrides OrderedCollectionMixin  """
-        return self.books.filter(listitem__approved=True).all().order_by("listitem")
+        return self.books.filter(listitem__approved=True).order_by("listitem")
 
     class Meta:
         """ default sorting """

bookwyrm/models/shelf.py

@@ -48,7 +48,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
     @property
     def collection_queryset(self):
         """ list of books for this shelf, overrides OrderedCollectionMixin  """
-        return self.books.all().order_by("shelfbook")
+        return self.books.order_by("shelfbook")
 
     def get_remote_id(self):
         """ shelf identifier instead of id """

bookwyrm/templatetags/bookwyrm_tags.py

@@ -62,14 +62,10 @@ def get_notification_count(user):
 def get_replies(status):
     """ get all direct replies to a status """
     # TODO: this limit could cause problems
-    return (
+    return models.Status.objects.filter(
-        models.Status.objects.filter(
         reply_parent=status,
         deleted=False,
-        )
+    ).select_subclasses()[:10]
-        .select_subclasses()
-        .all()[:10]
-    )
 
 
 @register.filter(name="parent")

bookwyrm/views/books.py

@@ -30,11 +30,6 @@ class Book(View):
 
     def get(self, request, book_id):
         """ info about a book """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         try:
             book = models.Book.objects.select_subclasses().get(id=book_id)
         except models.Book.DoesNotExist:
@@ -60,7 +55,7 @@ class Book(View):
         paginated = Paginator(
             reviews.exclude(Q(content__isnull=True) | Q(content="")), PAGE_LENGTH
         )
-        reviews_page = paginated.get_page(page)
+        reviews_page = paginated.get_page(request.GET.get("page"))
 
         user_tags = readthroughs = user_shelves = other_edition_shelves = []
         if request.user.is_authenticated:
@@ -266,11 +261,6 @@ class Editions(View):
         """ list of editions of a book """
         work = get_object_or_404(models.Work, id=book_id)
 
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         if is_api_request(request):
             return ActivitypubResponse(work.to_edition_list(**request.GET))
         filters = {}
@@ -285,7 +275,7 @@ class Editions(View):
 
         paginated = Paginator(editions.filter(**filters).all(), PAGE_LENGTH)
         data = {
-            "editions": paginated.get_page(page),
+            "editions": paginated.get_page(request.GET.get("page")),
             "work": work,
             "languages": languages,
             "formats": set(

bookwyrm/views/directory.py

@@ -15,12 +15,6 @@ class Directory(View):
 
     def get(self, request):
         """ lets see your cute faces """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
-        # filters
         filters = {}
         software = request.GET.get("software")
         if not software or software == "bookwyrm":
@@ -39,7 +33,7 @@ class Directory(View):
         paginated = Paginator(users, 12)
 
         data = {
-            "users": paginated.get_page(page),
+            "users": paginated.get_page(request.GET.get("page")),
         }
         return TemplateResponse(request, "directory/directory.html", data)
 

bookwyrm/views/federation.py

@@ -24,11 +24,6 @@ class Federation(View):
 
     def get(self, request):
         """ list of servers """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         servers = models.FederatedServer.objects
 
         sort = request.GET.get("sort")
@@ -40,7 +35,7 @@ class Federation(View):
         paginated = Paginator(servers, PAGE_LENGTH)
 
         data = {
-            "servers": paginated.get_page(page),
+            "servers": paginated.get_page(request.GET.get("page")),
             "sort": sort,
             "form": forms.ServerForm(),
         }

bookwyrm/views/feed.py

@@ -22,11 +22,6 @@ class Feed(View):
 
     def get(self, request, tab):
         """ user's homepage with activity feed """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         if not tab in STREAMS:
             tab = "home"
 
@@ -39,7 +34,7 @@ class Feed(View):
             **feed_page_data(request.user),
             **{
                 "user": request.user,
-                "activities": paginated.get_page(page),
+                "activities": paginated.get_page(request.GET.get("page")),
                 "suggested_users": suggested_users,
                 "tab": tab,
                 "goal_form": forms.GoalForm(),
@@ -55,11 +50,6 @@ class DirectMessage(View):
 
     def get(self, request, username=None):
         """ like a feed but for dms only """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         # remove fancy subclasses of status, keep just good ol' notes
         queryset = models.Status.objects.filter(
             review__isnull=True,
@@ -82,13 +72,12 @@ class DirectMessage(View):
         ).order_by("-published_date")
 
         paginated = Paginator(activities, PAGE_LENGTH)
-        activity_page = paginated.get_page(page)
         data = {
             **feed_page_data(request.user),
             **{
                 "user": request.user,
                 "partner": user,
-                "activities": activity_page,
+                "activities": paginated.get_page(request.GET.get("page")),
                 "path": "/direct-messages",
             },
         }
@@ -174,7 +163,7 @@ def get_suggested_books(user, max_books=5):
         )
         shelf = user.shelf_set.get(identifier=preset)
 
-        shelf_books = shelf.shelfbook_set.order_by("-updated_date").all()[:limit]
+        shelf_books = shelf.shelfbook_set.order_by("-updated_date")[:limit]
         if not shelf_books:
             continue
         shelf_preview = {

bookwyrm/views/invite.py

@@ -30,11 +30,6 @@ class ManageInvites(View):
 
     def get(self, request):
         """ invite management page """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         paginated = Paginator(
             models.SiteInvite.objects.filter(user=request.user).order_by(
                 "-created_date"
@@ -43,7 +38,7 @@ class ManageInvites(View):
         )
 
         data = {
-            "invites": paginated.get_page(page),
+            "invites": paginated.get_page(request.GET.get("page")),
             "form": forms.CreateInviteForm(),
         }
         return TemplateResponse(request, "settings/manage_invites.html", data)
@@ -93,11 +88,6 @@ class ManageInviteRequests(View):
     def get(self, request):
         """ view a list of requests """
         ignored = request.GET.get("ignored", False)
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         sort = request.GET.get("sort")
         sort_fields = [
             "created_date",
@@ -136,7 +126,7 @@ class ManageInviteRequests(View):
         data = {
             "ignored": ignored,
             "count": paginated.count,
-            "requests": paginated.get_page(page),
+            "requests": paginated.get_page(request.GET.get("page")),
             "sort": sort,
         }
         return TemplateResponse(request, "settings/manage_invite_requests.html", data)

bookwyrm/views/list.py

@@ -26,11 +26,6 @@ class Lists(View):
 
     def get(self, request):
         """ display a book list """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         # hide lists with no approved books
         lists = (
             models.List.objects.annotate(
@@ -47,7 +42,7 @@ class Lists(View):
 
         paginated = Paginator(lists, 12)
         data = {
-            "lists": paginated.get_page(page),
+            "lists": paginated.get_page(request.GET.get("page")),
             "list_form": forms.ListForm(),
             "path": "/list",
         }
@@ -70,19 +65,15 @@ class UserLists(View):
 
     def get(self, request, username):
         """ display a book list """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
         user = get_user_from_username(request.user, username)
-        lists = models.List.objects.filter(user=user).all()
+        lists = models.List.objects.filter(user=user)
         lists = privacy_filter(request.user, lists)
         paginated = Paginator(lists, 12)
 
         data = {
             "user": user,
             "is_self": request.user.id == user.id,
-            "lists": paginated.get_page(page),
+            "lists": paginated.get_page(request.GET.get("page")),
             "list_form": forms.ListForm(),
             "path": user.local_path + "/lists",
         }
@@ -114,8 +105,6 @@ class List(View):
         if direction not in ("ascending", "descending"):
             direction = "ascending"
 
-        page = request.GET.get("page", 1)
-
         internal_sort_by = {
             "order": "order",
             "title": "book__title",
@@ -163,7 +152,7 @@ class List(View):
 
         data = {
             "list": book_list,
-            "items": paginated.get_page(page),
+            "items": paginated.get_page(request.GET.get("page")),
             "pending_count": book_list.listitem_set.filter(approved=False).count(),
             "suggested_books": suggestions,
             "list_form": forms.ListForm(instance=book_list),
@@ -212,7 +201,8 @@ class Curate(View):
         suggestion = get_object_or_404(models.ListItem, id=request.POST.get("item"))
         approved = request.POST.get("approved") == "true"
         if approved:
-            # update the book and set it to be the last in the order of approved books, before any pending books
+            # update the book and set it to be the last in the order of approved books,
+            # before any pending books
             suggestion.approved = True
             order_max = (
                 book_list.listitem_set.filter(approved=True).aggregate(Max("order"))[
@@ -241,7 +231,7 @@ def add_book(request):
     # do you have permission to add to the list?
     try:
         if request.user == book_list.user or book_list.curation == "open":
-            # add the book at the latest order of approved books, before any pending books
+            # add the book at the latest order of approved books, before pending books
             order_max = (
                 book_list.listitem_set.filter(approved=True).aggregate(Max("order"))[
                     "order__max"
@@ -327,7 +317,7 @@ def set_book_position(request, list_item_id):
         original_order = list_item.order
         if original_order == int_position:
             return HttpResponse(status=204)
-        elif original_order > int_position:
+        if original_order > int_position:
             list_item.order = -1
             list_item.save()
             increment_order_in_reverse(book_list.id, int_position, original_order)
@@ -346,6 +336,7 @@ def set_book_position(request, list_item_id):
 def increment_order_in_reverse(
     book_list_id: int, start: int, end: Optional[int] = None
 ):
+    """ increase the order nu,ber for every item in a list """
     try:
         book_list = models.List.objects.get(id=book_list_id)
     except models.List.DoesNotExist:
@@ -361,6 +352,7 @@ def increment_order_in_reverse(
 
 @transaction.atomic
 def decrement_order(book_list_id, start, end):
+    """ decrement the order value for every item in a list """
     try:
         book_list = models.List.objects.get(id=book_list_id)
     except models.List.DoesNotExist:
@@ -375,10 +367,11 @@ def decrement_order(book_list_id, start, end):
 
 @transaction.atomic
 def normalize_book_list_ordering(book_list_id, start=0, add_offset=0):
+    """ gives each book in a list the proper sequential order number """
     try:
         book_list = models.List.objects.get(id=book_list_id)
     except models.List.DoesNotExist:
-        return HttpResponseNotFound()
+        return
     items = book_list.listitem_set.filter(order__gt=start).order_by("order")
     for i, item in enumerate(items, start):
         effective_order = i + add_offset

bookwyrm/views/reading.py

@@ -145,6 +145,7 @@ def create_readthrough(request):
 
 
 def load_date_in_user_tz_as_utc(date_str: str, user: models.User) -> datetime:
+    """ ensures that data is stored consistently in the UTC timezone """
     user_tz = dateutil.tz.gettz(user.preferred_timezone)
     start_date = dateutil.parser.parse(date_str, ignoretz=True)
     return start_date.replace(tzinfo=user_tz).astimezone(dateutil.tz.UTC)

bookwyrm/views/shelf.py

@@ -30,11 +30,6 @@ class Shelf(View):
         except models.User.DoesNotExist:
             return HttpResponseNotFound()
 
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         shelves = privacy_filter(request.user, user.shelf_set)
 
         # get the shelf and make sure the logged in user should be able to see it
@@ -61,7 +56,7 @@ class Shelf(View):
             return ActivitypubResponse(shelf.to_activity(**request.GET))
 
         paginated = Paginator(
-            shelf.books.order_by("-updated_date").all(),
+            shelf.books.order_by("-updated_date"),
             PAGE_LENGTH,
         )
 
@@ -70,7 +65,7 @@ class Shelf(View):
             "is_self": is_self,
             "shelves": shelves.all(),
             "shelf": shelf,
-            "books": paginated.get_page(page),
+            "books": paginated.get_page(request.GET.get("page")),
         }
 
         return TemplateResponse(request, "user/shelf.html", data)

bookwyrm/views/user.py

@@ -40,11 +40,6 @@ class User(View):
             return ActivitypubResponse(user.to_activity())
         # otherwise we're at a UI view
 
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         shelf_preview = []
 
         # only show other shelves that should be visible
@@ -64,7 +59,7 @@ class User(View):
                 {
                     "name": user_shelf.name,
                     "local_path": user_shelf.local_path,
-                    "books": user_shelf.books.all()[:3],
+                    "books": user_shelf.books[:3],
                     "size": user_shelf.books.count(),
                 }
             )
@@ -87,7 +82,7 @@ class User(View):
             "is_self": is_self,
             "shelves": shelf_preview,
             "shelf_count": shelves.count(),
-            "activities": paginated.get_page(page),
+            "activities": paginated.get_page(request.GET.get("page", 1)),
             "goal": goal,
         }
 

bookwyrm/views/user_admin.py

@@ -20,11 +20,6 @@ class UserAdmin(View):
 
     def get(self, request):
         """ list of users """
-        try:
-            page = int(request.GET.get("page", 1))
-        except ValueError:
-            page = 1
-
         filters = {}
         server = request.GET.get("server")
         if server:
@@ -50,7 +45,7 @@ class UserAdmin(View):
 
         paginated = Paginator(users, PAGE_LENGTH)
         data = {
-            "users": paginated.get_page(page),
+            "users": paginated.get_page(request.GET.get("page")),
             "sort": sort,
             "server": server,
         }