Merge pull request #1165 from bookwyrm-social/nojs-modals

Refactors want to read/start reading/finish reading modals

bookwyrm/static/js/bookwyrm.js

@@ -3,7 +3,7 @@
 
 let BookWyrm = new class {
     constructor() {
-        this.MAX_FILE_SIZE_BYTES = 10 * 1000000
+        this.MAX_FILE_SIZE_BYTES = 10 * 1000000;
         this.initOnDOMLoaded();
         this.initReccuringTasks();
         this.initEventListeners();
@@ -45,14 +45,14 @@ let BookWyrm = new class {
      * Execute code once the DOM is loaded.
      */
     initOnDOMLoaded() {
-        const bookwyrm = this
+        const bookwyrm = this;
 
         window.addEventListener('DOMContentLoaded', function() {
             document.querySelectorAll('.tab-group')
                 .forEach(tabs => new TabGroup(tabs));
             document.querySelectorAll('input[type="file"]').forEach(
                 bookwyrm.disableIfTooLarge.bind(bookwyrm)
-            )
+            );
         });
     }
 
@@ -138,6 +138,7 @@ let BookWyrm = new class {
      * @return {undefined}
      */
     toggleAction(event) {
+        event.preventDefault();
         let trigger = event.currentTarget;
         let pressed = trigger.getAttribute('aria-pressed') === 'false';
         let targetId = trigger.dataset.controls;
@@ -182,6 +183,8 @@ let BookWyrm = new class {
         if (focus) {
             this.toggleFocus(focus);
         }
+
+        return false;
     }
 
     /**
@@ -298,25 +301,25 @@ let BookWyrm = new class {
     }
 
     disableIfTooLarge(eventOrElement) {
-        const { addRemoveClass, MAX_FILE_SIZE_BYTES } = this
-        const element = eventOrElement.currentTarget || eventOrElement
+        const { addRemoveClass, MAX_FILE_SIZE_BYTES } = this;
+        const element = eventOrElement.currentTarget || eventOrElement;
 
-        const submits = element.form.querySelectorAll('[type="submit"]')
-        const warns = element.parentElement.querySelectorAll('.file-too-big')
+        const submits = element.form.querySelectorAll('[type="submit"]');
+        const warns = element.parentElement.querySelectorAll('.file-too-big');
         const isTooBig = element.files &&
             element.files[0] &&
-            element.files[0].size > MAX_FILE_SIZE_BYTES
+            element.files[0].size > MAX_FILE_SIZE_BYTES;
 
         if (isTooBig) {
-            submits.forEach(submitter => submitter.disabled = true)
+            submits.forEach(submitter => submitter.disabled = true);
             warns.forEach(
                 sib => addRemoveClass(sib, 'is-hidden', false)
-            )
+            );
         } else {
-            submits.forEach(submitter => submitter.disabled = false)
+            submits.forEach(submitter => submitter.disabled = false);
             warns.forEach(
                 sib => addRemoveClass(sib, 'is-hidden', true)
-            )
+            );
         }
     }
-}
+}();

bookwyrm/static/js/localstorage.js

@@ -17,7 +17,7 @@ let LocalStorageTools = new class {
      * @return {undefined}
      */
     updateDisplay(event) {
-        // used in set reading goal
+        // Used in set reading goal
         let key = event.target.dataset.id;
         let value = event.target.dataset.value;
 
@@ -34,10 +34,10 @@ let LocalStorageTools = new class {
      * @return {undefined}
      */
     setDisplay(node) {
-        // used in set reading goal
+        // Used in set reading goal
         let key = node.dataset.hide;
         let value = window.localStorage.getItem(key);
 
         BookWyrm.addRemoveClass(node, 'is-hidden', value);
     }
-}
+}();

bookwyrm/templates/components/modal.html

@@ -1,7 +1,7 @@
 {% load i18n %}
 <div
     role="dialog"
-    class="modal is-hidden"
+    class="modal {% if active %}is-active{% else %}is-hidden{% endif %}"
     id="{{ controls_text }}-{{ controls_uid }}"
     aria-labelledby="modal-card-title-{{ controls_text }}-{{ controls_uid }}"
     aria-modal="true"

bookwyrm/templates/reading_progress/finish.html

@@ -0,0 +1,14 @@
+{% extends 'layout.html' %}
+{% load i18n %}
+
+{% block title %}
+{% blocktrans trimmed with book_title=book.title %}
+Finish "{{ book_title }}"
+{% endblocktrans %}
+{% endblock %}
+
+{% block content %}
+
+{% include "snippets/shelve_button/finish_reading_modal.html" with book=book active=True %}
+
+{% endblock %}

bookwyrm/templates/reading_progress/start.html

@@ -0,0 +1,14 @@
+{% extends 'layout.html' %}
+{% load i18n %}
+
+{% block title %}
+{% blocktrans trimmed with book_title=book.title %}
+Start "{{ book_title }}"
+{% endblocktrans %}
+{% endblock %}
+
+{% block content %}
+
+{% include "snippets/shelve_button/start_reading_modal.html" with book=book active=True %}
+
+{% endblock %}

bookwyrm/templates/reading_progress/want.html

@@ -0,0 +1,14 @@
+{% extends 'layout.html' %}
+{% load i18n %}
+
+{% block title %}
+{% blocktrans trimmed with book_title=book.title %}
+Want to Read "{{ book_title }}"
+{% endblocktrans %}
+{% endblock %}
+
+{% block content %}
+
+{% include "snippets/shelve_button/want_to_read_modal.html" with book=book active=True no_body=True %}
+
+{% endblock %}

bookwyrm/templates/snippets/shelve_button/finish_reading_modal.html

@@ -7,7 +7,7 @@
 
 
 {% block modal-form-open %}
-<form name="finish-reading" action="/finish-reading/{{ book.id }}" method="post">
+<form name="finish-reading" action="{% url 'finish-reading' book.id %}" method="post">
 {% endblock %}
 
 {% block modal-body %}

bookwyrm/templates/snippets/shelve_button/shelve_button_options.html

@@ -7,16 +7,25 @@
 {% if dropdown %}<li role="menuitem" class="dropdown-item p-0">{% endif %}
     <div class="{% if not dropdown and active_shelf.shelf.identifier|next_shelf != shelf.identifier %}is-hidden{% endif %}">
     {% if shelf.identifier == 'reading' %}{% if not dropdown or active_shelf.shelf.identifier|next_shelf != shelf.identifier %}
+
         {% trans "Start reading" as button_text %}
-        {% include 'snippets/toggle/toggle_button.html' with class=class text=button_text controls_text="start-reading" controls_uid=button_uuid focus="modal-title-start-reading" disabled=is_current %}
+        {% url 'start-reading' book.id as fallback_url %}
+        {% include 'snippets/toggle/toggle_button.html' with class=class text=button_text controls_text="start-reading" controls_uid=button_uuid focus="modal-title-start-reading" disabled=is_current fallback_url=fallback_url %}
+
     {% endif %}{% elif shelf.identifier == 'read' and active_shelf.shelf.identifier == 'read' %}{% if not dropdown or active_shelf.shelf.identifier|next_shelf != shelf.identifier %}
     <button type="button" class="button {{ class }}" disabled><span>{% trans "Read" %}</span>
     {% endif %}{% elif shelf.identifier == 'read' %}{% if not dropdown or active_shelf.shelf.identifier|next_shelf != shelf.identifier %}
+
         {% trans "Finish reading" as button_text %}
-        {% include 'snippets/toggle/toggle_button.html' with class=class text=button_text controls_text="finish-reading" controls_uid=button_uuid focus="modal-title-finish-reading" disabled=is_current %}
+        {% url 'finish-reading' book.id as fallback_url %}
+        {% include 'snippets/toggle/toggle_button.html' with class=class text=button_text controls_text="finish-reading" controls_uid=button_uuid focus="modal-title-finish-reading" disabled=is_current fallback_url=fallback_url %}
+
     {% endif %}{% elif shelf.identifier == 'to-read' %}{% if not dropdown or active_shelf.shelf.identifier|next_shelf != shelf.identifier %}
+
         {% trans "Want to read" as button_text %}
-        {% include 'snippets/toggle/toggle_button.html' with class=class text=button_text controls_text="want-to-read" controls_uid=button_uuid focus="modal-title-want-to-read" disabled=is_current %}
+        {% url 'to-read' book.id as fallback_url %}
+        {% include 'snippets/toggle/toggle_button.html' with class=class text=button_text controls_text="want-to-read" controls_uid=button_uuid focus="modal-title-want-to-read" disabled=is_current fallback_url=fallback_url %}
+
     {% endif %}{% elif shelf.editable %}
         <form name="shelve" action="/shelve/" method="post">
             {% csrf_token %}
@@ -44,7 +53,9 @@
         {% csrf_token %}
         <input type="hidden" name="book" value="{{ active_shelf.book.id }}">
         <input type="hidden" name="shelf" value="{{ active_shelf.shelf.id }}">
-        <button class="button is-fullwidth is-small{% if dropdown %} is-radiusless{% endif %} is-danger is-light" type="submit">{% blocktrans with name=active_shelf.shelf.name %}Remove from {{ name }}{% endblocktrans %}</button>
+        <button class="button is-fullwidth is-small{% if dropdown %} is-radiusless{% endif %} is-danger is-light" type="submit">
+            {% blocktrans with name=active_shelf.shelf.name %}Remove from {{ name }}{% endblocktrans %}
+        </button>
     </form>
 </li>
 {% endif %}

bookwyrm/templates/snippets/shelve_button/start_reading_modal.html

@@ -2,11 +2,13 @@
 {% load i18n %}
 
 {% block modal-title %}
-{% blocktrans with book_title=book.title %}Start "<em>{{ book_title }}</em>"{% endblocktrans %}
+{% blocktrans trimmed with book_title=book.title %}
+Start "<em>{{ book_title }}</em>"
+{% endblocktrans %}
 {% endblock %}
 
 {% block modal-form-open %}
-<form name="start-reading" action="/start-reading/{{ book.id }}" method="post">
+<form name="start-reading" action="{% url 'start-reading' book.id %}" method="post">
 {% endblock %}
 
 {% block modal-body %}

bookwyrm/templates/snippets/shelve_button/want_to_read_modal.html

@@ -6,7 +6,7 @@
 {% endblock %}
 
 {% block modal-form-open %}
-<form name="shelve" action="/shelve/" method="post">
+<form name="shelve" action="{% url 'to-read' book.id %}" method="post">
     {% csrf_token %}
     <input type="hidden" name="book" value="{{ active_shelf.book.id }}">
     <input type="hidden" name="shelf" value="to-read">

bookwyrm/templates/snippets/toggle/toggle_button.html

@@ -1,5 +1,12 @@
+{% if fallback_url %}
+<form name="fallback-form-{{ controls_uuid}}" method="GET" action="{{ fallback_url }}">
+{% endif %}
 <button
-    type="button"
+    {% if not fallback_url %}
+        type="button"
+    {% else %}
+        type="submit"
+    {% endif %}
     class="{% if not nonbutton %}button {% endif %}{{ class }}{% if button_type %} {{ button_type }}{% endif %}"
     data-controls="{{ controls_text }}{% if controls_uid %}-{{ controls_uid }}{% endif %}"
     {% if focus %}data-focus-target="{{ focus }}{% if controls_uid %}-{{ controls_uid }}{% endif %}"{% endif %}
@@ -20,3 +27,6 @@
     <span>{{ text }}</span>
     {% endif %}
 </button>
+{% if fallback_url %}
+</form>
+{% endif %}

bookwyrm/tests/views/test_reading.py

@@ -56,7 +56,7 @@ class ReadingViews(TestCase):
         )
         request.user = self.local_user
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            views.start_reading(request, self.book.id)
+            views.StartReading.as_view()(request, self.book.id)
 
         self.assertEqual(shelf.books.get(), self.book)
 
@@ -86,7 +86,7 @@ class ReadingViews(TestCase):
         request = self.factory.post("")
         request.user = self.local_user
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            views.start_reading(request, self.book.id)
+            views.StartReading.as_view()(request, self.book.id)
 
         self.assertFalse(to_read_shelf.books.exists())
         self.assertEqual(shelf.books.get(), self.book)
@@ -112,7 +112,7 @@ class ReadingViews(TestCase):
         request.user = self.local_user
 
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            views.finish_reading(request, self.book.id)
+            views.FinishReading.as_view()(request, self.book.id)
 
         self.assertEqual(shelf.books.get(), self.book)
 

bookwyrm/tests/views/test_readthrough.py

@@ -57,7 +57,6 @@ class ReadThrough(TestCase):
             "/start-reading/{}".format(self.edition.id),
             {
                 "start_date": "2020-11-27",
-                "progress": 50,
             },
         )
 
@@ -66,15 +65,8 @@ class ReadThrough(TestCase):
         self.assertEqual(
             readthroughs[0].start_date, datetime(2020, 11, 27, tzinfo=timezone.utc)
         )
-        self.assertEqual(readthroughs[0].progress, 50)
         self.assertEqual(readthroughs[0].finish_date, None)
 
-        progress_updates = readthroughs[0].progressupdate_set.all()
-        self.assertEqual(len(progress_updates), 1)
-        self.assertEqual(progress_updates[0].mode, models.ProgressMode.PAGE)
-        self.assertEqual(progress_updates[0].progress, 50)
-        self.assertEqual(delay_mock.call_count, 1)
-
         # Update progress
         self.client.post(
             "/edit-readthrough",
@@ -87,9 +79,9 @@ class ReadThrough(TestCase):
         progress_updates = (
             readthroughs[0].progressupdate_set.order_by("updated_date").all()
         )
-        self.assertEqual(len(progress_updates), 2)
-        self.assertEqual(progress_updates[1].mode, models.ProgressMode.PAGE)
-        self.assertEqual(progress_updates[1].progress, 100)
+        self.assertEqual(len(progress_updates), 1)
+        self.assertEqual(progress_updates[0].mode, models.ProgressMode.PAGE)
+        self.assertEqual(progress_updates[0].progress, 100)
 
         # Edit doesn't publish anything
         self.assertEqual(delay_mock.call_count, 1)

bookwyrm/urls.py

@@ -223,7 +223,7 @@ urlpatterns = [
     re_path(
         r"^list/(?P<list_id>\d+)/curate/?$", views.Curate.as_view(), name="list-curate"
     ),
-    # Uyser books
+    # User books
     re_path(r"%s/books/?$" % user_path, views.Shelf.as_view(), name="user-shelves"),
     re_path(
         r"^%s/(helf|books)/(?P<shelf_identifier>[\w-]+)(.json)?/?$" % user_path,
@@ -315,8 +315,20 @@ urlpatterns = [
     re_path(r"^delete-readthrough/?$", views.delete_readthrough),
     re_path(r"^create-readthrough/?$", views.create_readthrough),
     re_path(r"^delete-progressupdate/?$", views.delete_progressupdate),
-    re_path(r"^start-reading/(?P<book_id>\d+)/?$", views.start_reading),
-    re_path(r"^finish-reading/(?P<book_id>\d+)/?$", views.finish_reading),
+    # shelve actions
+    re_path(
+        r"^to-read/(?P<book_id>\d+)/?$", views.WantToRead.as_view(), name="to-read"
+    ),
+    re_path(
+        r"^start-reading/(?P<book_id>\d+)/?$",
+        views.StartReading.as_view(),
+        name="start-reading",
+    ),
+    re_path(
+        r"^finish-reading/(?P<book_id>\d+)/?$",
+        views.FinishReading.as_view(),
+        name="finish-reading",
+    ),
     # following
     re_path(r"^follow/?$", views.follow, name="follow"),
     re_path(r"^unfollow/?$", views.unfollow, name="unfollow"),

bookwyrm/views/__init__.py

@@ -25,7 +25,7 @@ from .list import Lists, List, Curate, UserLists
 from .notifications import Notifications
 from .outbox import Outbox
 from .reading import edit_readthrough, create_readthrough, delete_readthrough
-from .reading import start_reading, finish_reading, delete_progressupdate
+from .reading import WantToRead, StartReading, FinishReading, delete_progressupdate
 from .reports import Report, Reports, make_report, resolve_report, suspend_user
 from .rss_feed import RssFeed
 from .password import PasswordResetRequest, PasswordReset, ChangePassword

bookwyrm/views/reading.py

@@ -7,95 +7,107 @@ from dateutil.parser import ParserError
 from django.contrib.auth.decorators import login_required
 from django.http import HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import get_object_or_404, redirect
+from django.template.response import TemplateResponse
+from django.utils.decorators import method_decorator
+from django.views import View
 from django.views.decorators.http import require_POST
 
 from bookwyrm import models
 from .helpers import get_edition, handle_reading_status
-from .shelf import handle_unshelve
 
 
-# pylint: disable= no-self-use
-@login_required
-@require_POST
-def start_reading(request, book_id):
-    """begin reading a book"""
+@method_decorator(login_required, name="dispatch")
+# pylint: disable=no-self-use
+class WantToRead(View):
+    """consider reading a book"""
+
+    def get(self, request, book_id):
+        """modal page"""
+        book = get_edition(book_id)
+        return TemplateResponse(request, "reading_progress/want.html", {"book": book})
+
+    def post(self, request, book_id):
+        """desire a book"""
+        desired_shelf = models.Shelf.objects.filter(
+            identifier=models.Shelf.TO_READ, user=request.user
+        ).first()
+        return handle_shelve(request, book_id, desired_shelf)
+
+
+@method_decorator(login_required, name="dispatch")
+# pylint: disable=no-self-use
+class StartReading(View):
+    """begin a book"""
+
+    def get(self, request, book_id):
+        """modal page"""
+        book = get_edition(book_id)
+        return TemplateResponse(request, "reading_progress/start.html", {"book": book})
+
+    def post(self, request, book_id):
+        """begin reading a book"""
+        desired_shelf = models.Shelf.objects.filter(
+            identifier=models.Shelf.READING, user=request.user
+        ).first()
+        return handle_shelve(request, book_id, desired_shelf)
+
+
+@method_decorator(login_required, name="dispatch")
+# pylint: disable=no-self-use
+class FinishReading(View):
+    """finish a book"""
+
+    def get(self, request, book_id):
+        """modal page"""
+        book = get_edition(book_id)
+        return TemplateResponse(request, "reading_progress/finish.html", {"book": book})
+
+    def post(self, request, book_id):
+        """a user completed a book, yay"""
+        desired_shelf = models.Shelf.objects.filter(
+            identifier=models.Shelf.READ_FINISHED, user=request.user
+        ).first()
+        return handle_shelve(request, book_id, desired_shelf)
+
+
+def handle_shelve(request, book_id, desired_shelf):
+    """these are all basically the same"""
     book = get_edition(book_id)
-    reading_shelf = models.Shelf.objects.filter(
-        identifier=models.Shelf.READING, user=request.user
-    ).first()
 
-    # create a readthrough
-    readthrough = update_readthrough(request, book=book)
-    if readthrough:
-        readthrough.save()
+    reshelve_book(request.user, book, desired_shelf)
 
-        # create a progress update if we have a page
-        readthrough.create_update()
+    if desired_shelf.identifier != models.Shelf.TO_READ:
+        # update or create a readthrough
+        readthrough = update_readthrough(request, book=book)
+        if readthrough:
+            readthrough.save()
 
+    # post about it (if you want)
+    if request.POST.get("post-status"):
+        privacy = request.POST.get("privacy")
+        handle_reading_status(request.user, desired_shelf, book, privacy)
+
+    return redirect(request.headers.get("Referer", "/"))
+
+
+def reshelve_book(user, book, desired_shelf):
+    """move a book to a new shelf"""
     current_status_shelfbook = (
         models.ShelfBook.objects.select_related("shelf")
         .filter(
             shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
-            user=request.user,
+            user=user,
             book=book,
         )
         .first()
     )
     if current_status_shelfbook is not None:
-        if current_status_shelfbook.shelf.identifier != models.Shelf.READING:
-            handle_unshelve(book, current_status_shelfbook.shelf)
+        if current_status_shelfbook.shelf.identifier != desired_shelf.identifier:
+            current_status_shelfbook.delete()
         else:  # It already was on the shelf
-            return redirect(request.headers.get("Referer", "/"))
+            return
 
-    models.ShelfBook.objects.create(book=book, shelf=reading_shelf, user=request.user)
-
-    # post about it (if you want)
-    if request.POST.get("post-status"):
-        privacy = request.POST.get("privacy")
-        handle_reading_status(request.user, reading_shelf, book, privacy)
-
-    return redirect(request.headers.get("Referer", "/"))
-
-
-@login_required
-@require_POST
-def finish_reading(request, book_id):
-    """a user completed a book, yay"""
-    book = get_edition(book_id)
-    finished_read_shelf = models.Shelf.objects.filter(
-        identifier=models.Shelf.READ_FINISHED, user=request.user
-    ).first()
-
-    # update or create a readthrough
-    readthrough = update_readthrough(request, book=book)
-    if readthrough:
-        readthrough.save()
-
-    current_status_shelfbook = (
-        models.ShelfBook.objects.select_related("shelf")
-        .filter(
-            shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
-            user=request.user,
-            book=book,
-        )
-        .first()
-    )
-    if current_status_shelfbook is not None:
-        if current_status_shelfbook.shelf.identifier != models.Shelf.READ_FINISHED:
-            handle_unshelve(book, current_status_shelfbook.shelf)
-        else:  # It already was on the shelf
-            return redirect(request.headers.get("Referer", "/"))
-
-    models.ShelfBook.objects.create(
-        book=book, shelf=finished_read_shelf, user=request.user
-    )
-
-    # post about it (if you want)
-    if request.POST.get("post-status"):
-        privacy = request.POST.get("privacy")
-        handle_reading_status(request.user, finished_read_shelf, book, privacy)
-
-    return redirect(request.headers.get("Referer", "/"))
+    models.ShelfBook.objects.create(book=book, shelf=desired_shelf, user=user)
 
 
 @login_required

bookwyrm/views/shelf.py

@@ -20,7 +20,7 @@ from .helpers import is_api_request, get_edition, get_user_from_username
 from .helpers import handle_reading_status, privacy_filter
 
 
-# pylint: disable= no-self-use
+# pylint: disable=no-self-use
 class Shelf(View):
     """shelf page"""
 
@@ -178,11 +178,6 @@ def shelve(request):
         models.ShelfBook.objects.create(
             book=book, shelf=desired_shelf, user=request.user
         )
-        if desired_shelf.identifier == models.Shelf.TO_READ and request.POST.get(
-            "post-status"
-        ):
-            privacy = request.POST.get("privacy") or desired_shelf.privacy
-            handle_reading_status(request.user, desired_shelf, book, privacy=privacy)
     else:
         try:
             models.ShelfBook.objects.create(
@@ -206,7 +201,6 @@ def unshelve(request):
     return redirect(request.headers.get("Referer", "/"))
 
 
-# pylint: disable=unused-argument
 def handle_unshelve(book, shelf):
     """unshelve a book"""
     row = models.ShelfBook.objects.get(book=book, shelf=shelf)