Merge pull request #1455 from bookwyrm-social/refactor-readthroughs

Refactor read-throughs

bookwyrm/migrations/0099_readthrough_is_active.py

@@ -0,0 +1,37 @@
+# Generated by Django 3.2.4 on 2021-09-22 16:53
+
+from django.db import migrations, models
+
+
+def set_active_readthrough(apps, schema_editor):
+    """best-guess for deactivation date"""
+    db_alias = schema_editor.connection.alias
+    apps.get_model("bookwyrm", "ReadThrough").objects.using(db_alias).filter(
+        start_date__isnull=False,
+        finish_date__isnull=True,
+    ).update(is_active=True)
+
+
+def reverse_func(apps, schema_editor):
+    """noop"""
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0098_auto_20210918_2238"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="readthrough",
+            name="is_active",
+            field=models.BooleanField(default=False),
+        ),
+        migrations.RunPython(set_active_readthrough, reverse_func),
+        migrations.AlterField(
+            model_name="readthrough",
+            name="is_active",
+            field=models.BooleanField(default=True),
+        ),
+    ]

bookwyrm/models/book.py

@@ -3,8 +3,8 @@ import re
 
 from django.contrib.postgres.search import SearchVectorField
 from django.contrib.postgres.indexes import GinIndex
-from django.db import models
+from django.db import models, transaction
-from django.db import transaction
+from django.db.models import Prefetch
 from django.dispatch import receiver
 from model_utils import FieldTracker
 from model_utils.managers import InheritanceManager
@@ -307,6 +307,27 @@ class Edition(Book):
 
         return super().save(*args, **kwargs)
 
+    @classmethod
+    def viewer_aware_objects(cls, viewer):
+        """annotate a book query with metadata related to the user"""
+        queryset = cls.objects
+        if not viewer or not viewer.is_authenticated:
+            return queryset
+
+        queryset = queryset.prefetch_related(
+            Prefetch(
+                "shelfbook_set",
+                queryset=viewer.shelfbook_set.all(),
+                to_attr="current_shelves",
+            ),
+            Prefetch(
+                "readthrough_set",
+                queryset=viewer.readthrough_set.filter(is_active=True).all(),
+                to_attr="active_readthroughs",
+            ),
+        )
+        return queryset
+
 
 def isbn_10_to_13(isbn_10):
     """convert an isbn 10 into an isbn 13"""

bookwyrm/models/readthrough.py

@@ -26,10 +26,14 @@ class ReadThrough(BookWyrmModel):
     )
     start_date = models.DateTimeField(blank=True, null=True)
     finish_date = models.DateTimeField(blank=True, null=True)
+    is_active = models.BooleanField(default=True)
 
     def save(self, *args, **kwargs):
         """update user active time"""
         self.user.update_active_date()
+        # an active readthrough must have an unset finish date
+        if self.finish_date:
+            self.is_active = False
         super().save(*args, **kwargs)
 
     def create_update(self):

bookwyrm/templatetags/bookwyrm_tags.py

@@ -85,7 +85,7 @@ def active_shelf(context, book):
 def latest_read_through(book, user):
     """the most recent read activity"""
     return (
-        models.ReadThrough.objects.filter(user=user, book=book)
+        models.ReadThrough.objects.filter(user=user, book=book, is_active=True)
         .order_by("-start_date")
         .first()
     )

bookwyrm/tests/views/test_reading.py

@@ -113,6 +113,7 @@ class ReadingViews(TestCase):
             {
                 "post-status": True,
                 "privacy": "followers",
+                "start_date": readthrough.start_date,
                 "finish_date": timezone.now().isoformat(),
                 "id": readthrough.id,
             },

bookwyrm/views/reading.py

@@ -5,6 +5,7 @@ import dateutil.tz
 from dateutil.parser import ParserError
 
 from django.contrib.auth.decorators import login_required
+from django.db import transaction
 from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseNotFound
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
@@ -35,7 +36,7 @@ class ReadingStatus(View):
         return TemplateResponse(request, f"reading_progress/{template}", {"book": book})
 
     def post(self, request, status, book_id):
-        """desire a book"""
+        """Change the state of a book by shelving it and adding reading dates"""
         identifier = {
             "want": models.Shelf.TO_READ,
             "start": models.Shelf.READING,
@@ -48,18 +49,21 @@ class ReadingStatus(View):
             identifier=identifier, user=request.user
         ).first()
 
-        book = get_edition(book_id)
+        book = (
-
+            models.Edition.viewer_aware_objects(request.user)
-        current_status_shelfbook = (
+            .prefetch_related("shelfbook_set__shelf")
-            models.ShelfBook.objects.select_related("shelf")
+            .get(id=book_id)
-            .filter(
-                shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS,
-                user=request.user,
-                book=book,
-            )
-            .first()
         )
 
+        # gets the first shelf that indicates a reading status, or None
+        shelves = [
+            s
+            for s in book.current_shelves
+            if s.shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS
+        ]
+        current_status_shelfbook = shelves[0] if shelves else None
+
+        # checking the referer prevents redirecting back to the modal page
         referer = request.headers.get("Referer", "/")
         referer = "/" if "reading-status" in referer else referer
         if current_status_shelfbook is not None:
@@ -72,11 +76,13 @@ class ReadingStatus(View):
             book=book, shelf=desired_shelf, user=request.user
         )
 
-        if desired_shelf.identifier != models.Shelf.TO_READ:
+        update_readthrough_on_shelve(
-            # update or create a readthrough
+            request.user,
-            readthrough = update_readthrough(request, book=book)
+            book,
-            if readthrough:
+            desired_shelf.identifier,
-                readthrough.save()
+            start_date=request.POST.get("start_date"),
+            finish_date=request.POST.get("finish_date"),
+        )
 
         # post about it (if you want)
         if request.POST.get("post-status"):
@@ -97,17 +103,67 @@ class ReadingStatus(View):
         return redirect(referer)
 
 
+@transaction.atomic
+def update_readthrough_on_shelve(
+    user, annotated_book, status, start_date=None, finish_date=None
+):
+    """update the current readthrough for a book when it is re-shelved"""
+    # there *should* only be one of current active readthrough, but it's a list
+    active_readthrough = next(iter(annotated_book.active_readthroughs), None)
+
+    # deactivate all existing active readthroughs
+    for readthrough in annotated_book.active_readthroughs:
+        readthrough.is_active = False
+        readthrough.save()
+
+    # if the state is want-to-read, deactivating existing readthroughs is all we need
+    if status == models.Shelf.TO_READ:
+        return
+
+    # if we're starting a book, we need a fresh clean active readthrough
+    if status == models.Shelf.READING or not active_readthrough:
+        active_readthrough = models.ReadThrough.objects.create(
+            user=user, book=annotated_book
+        )
+    # santiize and set dates
+    active_readthrough.start_date = load_date_in_user_tz_as_utc(start_date, user)
+    # if the finish date is set, the readthrough will be automatically set as inactive
+    active_readthrough.finish_date = load_date_in_user_tz_as_utc(finish_date, user)
+
+    active_readthrough.save()
+
+
 @login_required
 @require_POST
 def edit_readthrough(request):
     """can't use the form because the dates are too finnicky"""
-    readthrough = update_readthrough(request, create=False)
+    readthrough = get_object_or_404(models.ReadThrough, id=request.POST.get("id"))
-    if not readthrough:
-        return HttpResponseNotFound()
 
     # don't let people edit other people's data
     if request.user != readthrough.user:
         return HttpResponseBadRequest()
+
+    readthrough.start_date = load_date_in_user_tz_as_utc(
+        request.POST.get("start_date"), request.user
+    )
+    readthrough.finish_date = load_date_in_user_tz_as_utc(
+        request.POST.get("finish_date"), request.user
+    )
+
+    progress = request.POST.get("progress")
+    try:
+        progress = int(progress)
+        readthrough.progress = progress
+    except (ValueError, TypeError):
+        pass
+
+    progress_mode = request.POST.get("progress_mode")
+    try:
+        progress_mode = models.ProgressMode(progress_mode)
+        readthrough.progress_mode = progress_mode
+    except ValueError:
+        pass
+
     readthrough.save()
 
     # record the progress update individually
@@ -136,73 +192,32 @@ def delete_readthrough(request):
 def create_readthrough(request):
     """can't use the form because the dates are too finnicky"""
     book = get_object_or_404(models.Edition, id=request.POST.get("book"))
-    readthrough = update_readthrough(request, create=True, book=book)
+
-    if not readthrough:
+    start_date = load_date_in_user_tz_as_utc(
-        return redirect(book.local_path)
+        request.POST.get("start_date"), request.user
-    readthrough.save()
+    )
-    return redirect(request.headers.get("Referer", "/"))
+    finish_date = load_date_in_user_tz_as_utc(
+        request.POST.get("finish_date"), request.user
+    )
+    models.ReadThrough.objects.create(
+        user=request.user,
+        book=book,
+        start_date=start_date,
+        finish_date=finish_date,
+    )
+    return redirect("book", book.id)
 
 
 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)
+    if not date_str:
-    start_date = dateutil.parser.parse(date_str, ignoretz=True)
+        return None
-    return start_date.replace(tzinfo=user_tz).astimezone(dateutil.tz.UTC)
+    user_tz = dateutil.tz.gettz(user.preferred_timezone)
-
+    date = dateutil.parser.parse(date_str, ignoretz=True)
-
+    try:
-def update_readthrough(request, book=None, create=True):
+        return date.replace(tzinfo=user_tz).astimezone(dateutil.tz.UTC)
-    """updates but does not save dates on a readthrough"""
+    except ParserError:
-    try:
-        read_id = request.POST.get("id")
-        if not read_id:
-            raise models.ReadThrough.DoesNotExist
-        readthrough = models.ReadThrough.objects.get(id=read_id)
-    except models.ReadThrough.DoesNotExist:
-        if not create or not book:
-            return None
-        readthrough = models.ReadThrough(
-            user=request.user,
-            book=book,
-        )
-
-    start_date = request.POST.get("start_date")
-    if start_date:
-        try:
-            readthrough.start_date = load_date_in_user_tz_as_utc(
-                start_date, request.user
-            )
-        except ParserError:
-            pass
-
-    finish_date = request.POST.get("finish_date")
-    if finish_date:
-        try:
-            readthrough.finish_date = load_date_in_user_tz_as_utc(
-                finish_date, request.user
-            )
-        except ParserError:
-            pass
-
-    progress = request.POST.get("progress")
-    if progress:
-        try:
-            progress = int(progress)
-            readthrough.progress = progress
-        except ValueError:
-            pass
-
-    progress_mode = request.POST.get("progress_mode")
-    if progress_mode:
-        try:
-            progress_mode = models.ProgressMode(progress_mode)
-            readthrough.progress_mode = progress_mode
-        except ValueError:
-            pass
-
-    if not readthrough.start_date and not readthrough.finish_date:
         return None
-
-    return readthrough
 
 
 @login_required

bookwyrm/views/status.py

@@ -5,7 +5,7 @@ from urllib.parse import urlparse
 from django.contrib.auth.decorators import login_required
 from django.core.validators import URLValidator
 from django.core.exceptions import ValidationError
-from django.http import HttpResponse, HttpResponseBadRequest
+from django.http import HttpResponse, HttpResponseBadRequest, Http404
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
@@ -79,7 +79,10 @@ class CreateStatus(View):
         status.save(created=True)
 
         # update a readthorugh, if needed
-        edit_readthrough(request)
+        try:
+            edit_readthrough(request)
+        except Http404:
+            pass
 
         if is_api_request(request):
             return HttpResponse()