From 202696f913e8cd4b2b347cb2e9b958e1d0f65866 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 28 Feb 2022 10:03:24 -0800 Subject: [PATCH 1/4] Don't show lists a book is already on in add form --- bookwyrm/templates/book/book.html | 4 ++-- bookwyrm/views/books/books.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bookwyrm/templates/book/book.html b/bookwyrm/templates/book/book.html index 60e3d4fd..0e2fd5d3 100644 --- a/bookwyrm/templates/book/book.html +++ b/bookwyrm/templates/book/book.html @@ -352,7 +352,7 @@ {% endfor %} - {% if request.user.list_set.exists %} + {% if list_options.exists %}
{% csrf_token %} @@ -361,7 +361,7 @@
diff --git a/bookwyrm/views/books/books.py b/bookwyrm/views/books/books.py index e04230ba..ad7ee943 100644 --- a/bookwyrm/views/books/books.py +++ b/bookwyrm/views/books/books.py @@ -83,6 +83,7 @@ class Book(View): } if request.user.is_authenticated: + data["list_options"] = request.user.list_set.exclude(id__in=data["lists"]) data["file_link_form"] = forms.FileLinkForm() readthroughs = models.ReadThrough.objects.filter( user=request.user, From b2b3ba653e1d1139ac36e51d38541c0c9cd83a46 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 28 Feb 2022 10:29:58 -0800 Subject: [PATCH 2/4] Refactors how success/failure messages how on list page --- bookwyrm/templates/lists/list.html | 25 ++++++++++++++++++------- bookwyrm/views/list/list.py | 23 ++++++++--------------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html index 1c26733d..73ea7dba 100644 --- a/bookwyrm/templates/lists/list.html +++ b/bookwyrm/templates/lists/list.html @@ -30,13 +30,24 @@
- {% if request.GET.updated %} -
- {% if list.curation != "open" and request.user != list.user and not list.group|is_member:request.user %} - {% trans "You successfully suggested a book for this list!" %} - {% else %} - {% trans "You successfully added a book to this list!" %} - {% endif %} + {% if add_succeeded %} +
+ + + {% if list.curation != "open" and request.user != list.user and not list.group|is_member:request.user %} + {% trans "You successfully suggested a book for this list!" %} + {% else %} + {% trans "You successfully added a book to this list!" %} + {% endif %} + +
+ {% endif %} + {% if add_failed %} +
+ + + {% trans "That book is already on this list." %} +
{% endif %} diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py index fbbbee9f..0b228200 100644 --- a/bookwyrm/views/list/list.py +++ b/bookwyrm/views/list/list.py @@ -1,11 +1,10 @@ """ book list views""" from typing import Optional -from urllib.parse import urlencode from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django.core.paginator import Paginator -from django.db import IntegrityError, transaction +from django.db import transaction from django.db.models import Avg, DecimalField, Q, Max from django.db.models.functions import Coalesce from django.http import HttpResponseBadRequest, HttpResponse @@ -26,7 +25,7 @@ from bookwyrm.views.helpers import is_api_request class List(View): """book list page""" - def get(self, request, list_id): + def get(self, request, list_id, add_failed=False, add_succeeded=True): """display a book list""" book_list = get_object_or_404(models.List, id=list_id) book_list.raise_visible_to_user(request.user) @@ -110,6 +109,8 @@ class List(View): {"direction": direction, "sort_by": sort_by} ), "embed_url": embed_url, + "add_failed": add_failed, + "add_succeeded": add_succeeded, } return TemplateResponse(request, "lists/list.html", data) @@ -179,8 +180,8 @@ def add_book(request): form = forms.ListItemForm(request.POST) if not form.is_valid(): - # this shouldn't happen, there aren't validated fields - raise Exception(form.errors) + return List().get(request, book_list.id, add_failed=True) + item = form.save(commit=False) if book_list.curation == "curated": @@ -196,17 +197,9 @@ def add_book(request): ) or 0 increment_order_in_reverse(book_list.id, order_max + 1) item.order = order_max + 1 + item.save() - try: - item.save() - except IntegrityError: - # if the book is already on the list, don't flip out - pass - - path = reverse("list", args=[book_list.id]) - params = request.GET.copy() - params["updated"] = True - return redirect(f"{path}?{urlencode(params)}") + return List().get(request, book_list.id, add_succeeded=True) @require_POST From 99fc3aaf25873eca683f8db6e2b3eee7c2448f2a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 28 Feb 2022 10:31:58 -0800 Subject: [PATCH 3/4] Avoid showing success and failure --- bookwyrm/templates/lists/list.html | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html index 73ea7dba..1d5abd89 100644 --- a/bookwyrm/templates/lists/list.html +++ b/bookwyrm/templates/lists/list.html @@ -30,7 +30,14 @@
- {% if add_succeeded %} + {% if add_failed %} +
+ + + {% trans "That book is already on this list." %} + +
+ {% elif add_succeeded %}
@@ -42,14 +49,6 @@
{% endif %} - {% if add_failed %} -
- - - {% trans "That book is already on this list." %} - -
- {% endif %} {% if not items.object_list.exists %}

{% trans "This list is currently empty" %}

From 142cc5437ae6f874e2cebe26cf9dfa7e883f26c2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 28 Feb 2022 10:41:40 -0800 Subject: [PATCH 4/4] Move sorting to separate function --- bookwyrm/views/list/list.py | 67 ++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 31 deletions(-) diff --git a/bookwyrm/views/list/list.py b/bookwyrm/views/list/list.py index 0b228200..dc1843fa 100644 --- a/bookwyrm/views/list/list.py +++ b/bookwyrm/views/list/list.py @@ -25,7 +25,7 @@ from bookwyrm.views.helpers import is_api_request class List(View): """book list page""" - def get(self, request, list_id, add_failed=False, add_succeeded=True): + def get(self, request, list_id, add_failed=False, add_succeeded=False): """display a book list""" book_list = get_object_or_404(models.List, id=list_id) book_list.raise_visible_to_user(request.user) @@ -36,33 +36,10 @@ class List(View): query = request.GET.get("q") suggestions = None - # sort_by shall be "order" unless a valid alternative is given - sort_by = request.GET.get("sort_by", "order") - if sort_by not in ("order", "title", "rating"): - sort_by = "order" - - # direction shall be "ascending" unless a valid alternative is given - direction = request.GET.get("direction", "ascending") - if direction not in ("ascending", "descending"): - direction = "ascending" - - directional_sort_by = { - "order": "order", - "title": "book__title", - "rating": "average_rating", - }[sort_by] - if direction == "descending": - directional_sort_by = "-" + directional_sort_by - - items = book_list.listitem_set.prefetch_related("user", "book", "book__authors") - if sort_by == "rating": - items = items.annotate( - average_rating=Avg( - Coalesce("book__review__rating", 0.0), - output_field=DecimalField(), - ) - ) - items = items.filter(approved=True).order_by(directional_sort_by) + items = book_list.listitem_set.filter(approved=True).prefetch_related( + "user", "book", "book__authors" + ) + items = sort_list(request, items) paginated = Paginator(items, PAGE_LENGTH) @@ -105,9 +82,7 @@ class List(View): "suggested_books": suggestions, "list_form": forms.ListForm(instance=book_list), "query": query or "", - "sort_form": forms.SortListForm( - {"direction": direction, "sort_by": sort_by} - ), + "sort_form": forms.SortListForm(request.GET), "embed_url": embed_url, "add_failed": add_failed, "add_succeeded": add_succeeded, @@ -132,6 +107,36 @@ class List(View): return redirect(book_list.local_path) +def sort_list(request, items): + """helper to handle the surprisngly involved sorting""" + # sort_by shall be "order" unless a valid alternative is given + sort_by = request.GET.get("sort_by", "order") + if sort_by not in ("order", "title", "rating"): + sort_by = "order" + + # direction shall be "ascending" unless a valid alternative is given + direction = request.GET.get("direction", "ascending") + if direction not in ("ascending", "descending"): + direction = "ascending" + + directional_sort_by = { + "order": "order", + "title": "book__title", + "rating": "average_rating", + }[sort_by] + if direction == "descending": + directional_sort_by = "-" + directional_sort_by + + if sort_by == "rating": + items = items.annotate( + average_rating=Avg( + Coalesce("book__review__rating", 0.0), + output_field=DecimalField(), + ) + ) + return items.order_by(directional_sort_by) + + @require_POST @login_required def save_list(request, list_id):