From a05ef1a22211e3fc88d3d3a61f1773d23fefa234 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 11:23:56 -0700 Subject: [PATCH 1/7] Adds active field to readthrough model --- .../migrations/0099_readthrough_is_active.py | 18 ++++++++++++++++++ bookwyrm/models/readthrough.py | 1 + 2 files changed, 19 insertions(+) create mode 100644 bookwyrm/migrations/0099_readthrough_is_active.py diff --git a/bookwyrm/migrations/0099_readthrough_is_active.py b/bookwyrm/migrations/0099_readthrough_is_active.py new file mode 100644 index 00000000..a7a2d05f --- /dev/null +++ b/bookwyrm/migrations/0099_readthrough_is_active.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.4 on 2021-09-22 16:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0098_auto_20210918_2238"), + ] + + operations = [ + migrations.AddField( + model_name="readthrough", + name="is_active", + field=models.BooleanField(default=False), + ), + ] diff --git a/bookwyrm/models/readthrough.py b/bookwyrm/models/readthrough.py index 343d3c11..7051deb7 100644 --- a/bookwyrm/models/readthrough.py +++ b/bookwyrm/models/readthrough.py @@ -27,6 +27,7 @@ class ReadThrough(BookWyrmModel): ) start_date = models.DateTimeField(blank=True, null=True) finish_date = models.DateTimeField(blank=True, null=True) + is_active = models.BooleanField(default=False) def save(self, *args, **kwargs): """update user active time""" From 4a3bf1d92dd914d261d5e9495af52fec411da9cc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 11:59:51 -0700 Subject: [PATCH 2/7] Updates active readthrough templatetag --- bookwyrm/templatetags/bookwyrm_tags.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templatetags/bookwyrm_tags.py b/bookwyrm/templatetags/bookwyrm_tags.py index e00a8331..1c915157 100644 --- a/bookwyrm/templatetags/bookwyrm_tags.py +++ b/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() ) From 2160a5c729a8a7e5b444dbfc6fb8ddad4c6f3ef0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 12:33:21 -0700 Subject: [PATCH 3/7] Newly created readthroughs are active by default --- .../migrations/0099_readthrough_is_active.py | 19 +++++++++++++++++++ bookwyrm/models/readthrough.py | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bookwyrm/migrations/0099_readthrough_is_active.py b/bookwyrm/migrations/0099_readthrough_is_active.py index a7a2d05f..e7b177ba 100644 --- a/bookwyrm/migrations/0099_readthrough_is_active.py +++ b/bookwyrm/migrations/0099_readthrough_is_active.py @@ -3,6 +3,19 @@ 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 = [ @@ -15,4 +28,10 @@ class Migration(migrations.Migration): 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), + ), ] diff --git a/bookwyrm/models/readthrough.py b/bookwyrm/models/readthrough.py index 7051deb7..fcff12a8 100644 --- a/bookwyrm/models/readthrough.py +++ b/bookwyrm/models/readthrough.py @@ -27,7 +27,7 @@ class ReadThrough(BookWyrmModel): ) start_date = models.DateTimeField(blank=True, null=True) finish_date = models.DateTimeField(blank=True, null=True) - is_active = models.BooleanField(default=False) + is_active = models.BooleanField(default=True) def save(self, *args, **kwargs): """update user active time""" From c54609d7fd59c7e0e24b7a44e5fedb3c3106d5ee Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 15:59:48 -0700 Subject: [PATCH 4/7] Refactors how readthroughs get updated --- bookwyrm/models/book.py | 25 ++++- bookwyrm/models/readthrough.py | 3 + bookwyrm/views/reading.py | 179 ++++++++++++++++++--------------- 3 files changed, 124 insertions(+), 83 deletions(-) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index ac7c42f6..6f26ef7d 100644 --- a/bookwyrm/models/book.py +++ b/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 transaction +from django.db import models, 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""" diff --git a/bookwyrm/models/readthrough.py b/bookwyrm/models/readthrough.py index 9ce60497..f75918ac 100644 --- a/bookwyrm/models/readthrough.py +++ b/bookwyrm/models/readthrough.py @@ -31,6 +31,9 @@ class ReadThrough(BookWyrmModel): 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): diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index 4c35db00..585d77db 100644 --- a/bookwyrm/views/reading.py +++ b/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,23 @@ class ReadingStatus(View): identifier=identifier, user=request.user ).first() - book = get_edition(book_id) - - current_status_shelfbook = ( - models.ShelfBook.objects.select_related("shelf") - .filter( - shelf__identifier__in=models.Shelf.READ_STATUS_IDENTIFIERS, - user=request.user, - book=book, - ) - .first() + book = ( + models.Edition.viewer_aware_objects(request.user) + .prefetch_related("shelfbook_set__shelf") + .get(id=book_id) ) + # gets the first shelf that indicates a reading status, or None + current_status_shelfbook = next( + iter( + s + for s in book.current_shelves + if s.shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS + ), + 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 +78,13 @@ class ReadingStatus(View): book=book, shelf=desired_shelf, user=request.user ) - if desired_shelf.identifier != models.Shelf.TO_READ: - # update or create a readthrough - readthrough = update_readthrough(request, book=book) - if readthrough: - readthrough.save() + update_readthrough_on_shelve( + request.user, + book, + desired_shelf.identifier, + 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 +105,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) - if not readthrough: - return HttpResponseNotFound() + readthrough = get_object_or_404(models.ReadThrough, id=request.POST.get("id")) # 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 +194,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: - return redirect(book.local_path) - readthrough.save() - return redirect(request.headers.get("Referer", "/")) + + start_date = ( + load_date_in_user_tz_as_utc(request.POST.get("start_date"), request.user), + ) + 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) - start_date = dateutil.parser.parse(date_str, ignoretz=True) - return start_date.replace(tzinfo=user_tz).astimezone(dateutil.tz.UTC) - - -def update_readthrough(request, book=None, create=True): - """updates but does not save dates on a readthrough""" - 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: + if not date_str: + return None + user_tz = dateutil.tz.gettz(user.preferred_timezone) + date = dateutil.parser.parse(date_str, ignoretz=True) + try: + return date.replace(tzinfo=user_tz).astimezone(dateutil.tz.UTC) + except ParserError: return None - - return readthrough @login_required From a96d027cf3699faaff3cfbd961183cf1d6bf5200 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 16:16:53 -0700 Subject: [PATCH 5/7] Easier to read first-item-or-none list logic --- bookwyrm/views/reading.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index 585d77db..99e95ddb 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -56,14 +56,12 @@ class ReadingStatus(View): ) # gets the first shelf that indicates a reading status, or None - current_status_shelfbook = next( - iter( - s - for s in book.current_shelves - if s.shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS - ), - 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", "/") From d510d72b8dbfcbc127cf8d3af290dd6258ea98c2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 16:41:43 -0700 Subject: [PATCH 6/7] Fixes syntax in reading view --- bookwyrm/tests/views/test_reading.py | 1 + bookwyrm/views/reading.py | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/bookwyrm/tests/views/test_reading.py b/bookwyrm/tests/views/test_reading.py index 3f5846d0..52d10aa6 100644 --- a/bookwyrm/tests/views/test_reading.py +++ b/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, }, diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index 99e95ddb..09b4b2bf 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -193,11 +193,11 @@ 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")) - start_date = ( - load_date_in_user_tz_as_utc(request.POST.get("start_date"), request.user), + start_date = load_date_in_user_tz_as_utc( + request.POST.get("start_date"), request.user ) - finish_date = ( - load_date_in_user_tz_as_utc(request.POST.get("finish_date"), request.user), + finish_date = load_date_in_user_tz_as_utc( + request.POST.get("finish_date"), request.user ) models.ReadThrough.objects.create( user=request.user, From c465c70c22f083b8536f8c4640de18e0f81ee505 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 22 Sep 2021 17:06:06 -0700 Subject: [PATCH 7/7] Handles statuses with no readthrough updates --- bookwyrm/views/status.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bookwyrm/views/status.py b/bookwyrm/views/status.py index c8131f65..a8b3ab0a 100644 --- a/bookwyrm/views/status.py +++ b/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()