From 77775d9bf8133183d578eb51f89e35c102b81fc7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 19 Apr 2021 14:47:59 -0700 Subject: [PATCH 1/3] Remove superfluous calls to `all()` --- bookwyrm/models/list.py | 2 +- bookwyrm/models/shelf.py | 2 +- bookwyrm/templatetags/bookwyrm_tags.py | 3 +-- bookwyrm/views/feed.py | 2 +- bookwyrm/views/list.py | 2 +- bookwyrm/views/shelf.py | 2 +- bookwyrm/views/user.py | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-) diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py index 7e6de106..639f8402 100644 --- a/bookwyrm/models/list.py +++ b/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 """ diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 5bbb84b9..d37668dd 100644 --- a/bookwyrm/models/shelf.py +++ b/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 """ diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index 775c6190..bc526ba1 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -67,8 +67,7 @@ def get_replies(status): reply_parent=status, deleted=False, ) - .select_subclasses() - .all()[:10] + .select_subclasses()[:10] ) diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index cd279282..1b66b494 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -174,7 +174,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 = { diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 7fb4979c..ff20d991 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -75,7 +75,7 @@ class UserLists(View): 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) diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 740439db..2ec9e69f 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -61,7 +61,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, ) diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index 26117a92..e24db01e 100644 --- a/bookwyrm/views/user.py +++ b/bookwyrm/views/user.py @@ -64,7 +64,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(), } ) From 91aa6fa95f508713ddb8715dd8933b233bbc9b1e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 19 Apr 2021 15:01:20 -0700 Subject: [PATCH 2/3] Remove unneeded page checking logic --- bookwyrm/views/books.py | 14 ++------------ bookwyrm/views/directory.py | 8 +------- bookwyrm/views/federation.py | 7 +------ bookwyrm/views/feed.py | 15 ++------------- bookwyrm/views/invite.py | 14 ++------------ bookwyrm/views/list.py | 29 +++++++++++------------------ bookwyrm/views/reading.py | 1 + bookwyrm/views/shelf.py | 7 +------ bookwyrm/views/user.py | 7 +------ bookwyrm/views/user_admin.py | 7 +------ 10 files changed, 23 insertions(+), 86 deletions(-) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index c3ac4f49..2a47c0d2 100644 --- a/bookwyrm/views/books.py +++ b/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( diff --git a/bookwyrm/views/directory.py b/bookwyrm/views/directory.py index 2565f4ec..7919dac0 100644 --- a/bookwyrm/views/directory.py +++ b/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) diff --git a/bookwyrm/views/federation.py b/bookwyrm/views/federation.py index 8712c463..1acacf8f 100644 --- a/bookwyrm/views/federation.py +++ b/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(), } diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index 1b66b494..118473d0 100644 --- a/bookwyrm/views/feed.py +++ b/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", }, } diff --git a/bookwyrm/views/invite.py b/bookwyrm/views/invite.py index 03b31b7b..cbb189b5 100644 --- a/bookwyrm/views/invite.py +++ b/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) diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index ff20d991..a2cf7afe 100644 --- a/bookwyrm/views/list.py +++ b/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,10 +65,6 @@ 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) lists = privacy_filter(request.user, lists) @@ -82,7 +73,7 @@ class UserLists(View): 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 diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index b780dd2f..f2d5b2c2 100644 --- a/bookwyrm/views/reading.py +++ b/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) diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 2ec9e69f..446bedba 100644 --- a/bookwyrm/views/shelf.py +++ b/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 @@ -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) diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index e24db01e..9c79a77c 100644 --- a/bookwyrm/views/user.py +++ b/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 @@ -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, } diff --git a/bookwyrm/views/user_admin.py b/bookwyrm/views/user_admin.py index cc3ff841..c0d097d7 100644 --- a/bookwyrm/views/user_admin.py +++ b/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, } From 0947b79fb1e02d3aa2e4b686a25480a06e8b7bbb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 19 Apr 2021 15:06:33 -0700 Subject: [PATCH 3/3] Python formatting --- bookwyrm/templatetags/bookwyrm_tags.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index bc526ba1..52054ada 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/bookwyrm/templatetags/bookwyrm_tags.py @@ -62,13 +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 ( - models.Status.objects.filter( - reply_parent=status, - deleted=False, - ) - .select_subclasses()[:10] - ) + return models.Status.objects.filter( + reply_parent=status, + deleted=False, + ).select_subclasses()[:10] @register.filter(name="parent")