Merge pull request #1221 from cincodenada/shelved-date

Add shelved date, populate from Goodreads import

bookwyrm/importers/importer.py

@@ -2,6 +2,8 @@
 import csv
 import logging
 
+from django.utils import timezone
+
 from bookwyrm import models
 from bookwyrm.models import ImportJob, ImportItem
 from bookwyrm.tasks import app
@@ -100,7 +102,10 @@ def handle_imported_book(source, user, item, include_reviews, privacy):
     # shelve the book if it hasn't been shelved already
     if item.shelf and not existing_shelf:
         desired_shelf = models.Shelf.objects.get(identifier=item.shelf, user=user)
-        models.ShelfBook.objects.create(book=item.book, shelf=desired_shelf, user=user)
+        shelved_date = item.date_added or timezone.now()
+        models.ShelfBook.objects.create(
+            book=item.book, shelf=desired_shelf, user=user, shelved_date=shelved_date
+        )
 
     for read in item.reads:
         # check for an existing readthrough with the same dates

bookwyrm/migrations/0078_add_shelved_date.py

@@ -0,0 +1,34 @@
+# Generated by Django 3.2.4 on 2021-07-03 08:25
+
+from django.db import migrations, models
+import django.utils.timezone
+
+
+def copy_created_date(app_registry, schema_editor):
+    db_alias = schema_editor.connection.alias
+    ShelfBook = app_registry.get_model("bookwyrm", "ShelfBook")
+    ShelfBook.objects.all().update(shelved_date=models.F("created_date"))
+
+
+def do_nothing(app_registry, schema_editor):
+    pass
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0077_auto_20210623_2155"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="shelfbook",
+            options={"ordering": ("-shelved_date",)},
+        ),
+        migrations.AddField(
+            model_name="shelfbook",
+            name="shelved_date",
+            field=models.DateTimeField(default=django.utils.timezone.now),
+        ),
+        migrations.RunPython(copy_created_date, reverse_code=do_nothing),
+    ]

bookwyrm/models/shelf.py

@@ -1,6 +1,7 @@
 """ puttin' books on shelves """
 import re
 from django.db import models
+from django.utils import timezone
 
 from bookwyrm import activitypub
 from .activitypub_mixin import CollectionItemMixin, OrderedCollectionMixin
@@ -69,6 +70,7 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
         "Edition", on_delete=models.PROTECT, activitypub_field="book"
     )
     shelf = models.ForeignKey("Shelf", on_delete=models.PROTECT)
+    shelved_date = models.DateTimeField(default=timezone.now)
     user = fields.ForeignKey(
         "User", on_delete=models.PROTECT, activitypub_field="actor"
     )
@@ -86,4 +88,4 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
         you can't put a book on shelf twice"""
 
         unique_together = ("book", "shelf")
-        ordering = ("-created_date",)
+        ordering = ("-shelved_date", "-created_date", "-updated_date")

bookwyrm/templates/user/shelf/shelf.html

@@ -103,7 +103,7 @@
                         {% include 'snippets/authors.html' %}
                     </td>
                     <td data-title="{% trans "Shelved" %}">
-                        {{ book.created_date|naturalday }}
+                        {{ book.shelved_date|naturalday }}
                     </td>
                     {% latest_read_through book user as read_through %}
                     <td data-title="{% trans "Started" %}">

bookwyrm/tests/importers/test_goodreads_import.py

@@ -3,6 +3,8 @@ from collections import namedtuple
 import csv
 import pathlib
 from unittest.mock import patch
+import datetime
+import pytz
 
 from django.test import TestCase
 import responses
@@ -13,6 +15,10 @@ from bookwyrm.importers.importer import import_data, handle_imported_book
 from bookwyrm.settings import DOMAIN
 
 
+def make_date(*args):
+    return datetime.datetime(*args, tzinfo=pytz.UTC)
+
+
 class GoodreadsImport(TestCase):
     """importing from goodreads csv"""
 
@@ -130,22 +136,25 @@ class GoodreadsImport(TestCase):
 
         shelf.refresh_from_db()
         self.assertEqual(shelf.books.first(), self.book)
+        self.assertEqual(
+            shelf.shelfbook_set.first().shelved_date, make_date(2020, 10, 21)
+        )
 
         readthrough = models.ReadThrough.objects.get(user=self.user)
         self.assertEqual(readthrough.book, self.book)
-        # I can't remember how to create dates and I don't want to look it up.
+        self.assertEqual(readthrough.start_date, make_date(2020, 10, 21))
-        self.assertEqual(readthrough.start_date.year, 2020)
+        self.assertEqual(readthrough.finish_date, make_date(2020, 10, 25))
-        self.assertEqual(readthrough.start_date.month, 10)
-        self.assertEqual(readthrough.start_date.day, 21)
-        self.assertEqual(readthrough.finish_date.year, 2020)
-        self.assertEqual(readthrough.finish_date.month, 10)
-        self.assertEqual(readthrough.finish_date.day, 25)
 
     def test_handle_imported_book_already_shelved(self):
         """goodreads import added a book, this adds related connections"""
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             shelf = self.user.shelf_set.filter(identifier="to-read").first()
-            models.ShelfBook.objects.create(shelf=shelf, user=self.user, book=self.book)
+            models.ShelfBook.objects.create(
+                shelf=shelf,
+                user=self.user,
+                book=self.book,
+                shelved_date=make_date(2020, 2, 2),
+            )
 
         import_job = models.ImportJob.objects.create(user=self.user)
         datafile = pathlib.Path(__file__).parent.joinpath("../data/goodreads.csv")
@@ -164,15 +173,15 @@ class GoodreadsImport(TestCase):
 
         shelf.refresh_from_db()
         self.assertEqual(shelf.books.first(), self.book)
+        self.assertEqual(
+            shelf.shelfbook_set.first().shelved_date, make_date(2020, 2, 2)
+        )
         self.assertIsNone(self.user.shelf_set.get(identifier="read").books.first())
+
         readthrough = models.ReadThrough.objects.get(user=self.user)
         self.assertEqual(readthrough.book, self.book)
-        self.assertEqual(readthrough.start_date.year, 2020)
+        self.assertEqual(readthrough.start_date, make_date(2020, 10, 21))
-        self.assertEqual(readthrough.start_date.month, 10)
+        self.assertEqual(readthrough.finish_date, make_date(2020, 10, 25))
-        self.assertEqual(readthrough.start_date.day, 21)
-        self.assertEqual(readthrough.finish_date.year, 2020)
-        self.assertEqual(readthrough.finish_date.month, 10)
-        self.assertEqual(readthrough.finish_date.day, 25)
 
     def test_handle_import_twice(self):
         """re-importing books"""
@@ -198,16 +207,14 @@ class GoodreadsImport(TestCase):
 
         shelf.refresh_from_db()
         self.assertEqual(shelf.books.first(), self.book)
+        self.assertEqual(
+            shelf.shelfbook_set.first().shelved_date, make_date(2020, 10, 21)
+        )
 
         readthrough = models.ReadThrough.objects.get(user=self.user)
         self.assertEqual(readthrough.book, self.book)
-        # I can't remember how to create dates and I don't want to look it up.
+        self.assertEqual(readthrough.start_date, make_date(2020, 10, 21))
-        self.assertEqual(readthrough.start_date.year, 2020)
+        self.assertEqual(readthrough.finish_date, make_date(2020, 10, 25))
-        self.assertEqual(readthrough.start_date.month, 10)
-        self.assertEqual(readthrough.start_date.day, 21)
-        self.assertEqual(readthrough.finish_date.year, 2020)
-        self.assertEqual(readthrough.finish_date.month, 10)
-        self.assertEqual(readthrough.finish_date.day, 25)
 
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_handle_imported_book_review(self, _):
@@ -229,9 +236,7 @@ class GoodreadsImport(TestCase):
         review = models.Review.objects.get(book=self.book, user=self.user)
         self.assertEqual(review.content, "mixed feelings")
         self.assertEqual(review.rating, 2)
-        self.assertEqual(review.published_date.year, 2019)
+        self.assertEqual(review.published_date, make_date(2019, 7, 8))
-        self.assertEqual(review.published_date.month, 7)
-        self.assertEqual(review.published_date.day, 8)
         self.assertEqual(review.privacy, "unlisted")
 
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
@@ -256,9 +261,7 @@ class GoodreadsImport(TestCase):
         review = models.ReviewRating.objects.get(book=self.book, user=self.user)
         self.assertIsInstance(review, models.ReviewRating)
         self.assertEqual(review.rating, 2)
-        self.assertEqual(review.published_date.year, 2019)
+        self.assertEqual(review.published_date, make_date(2019, 7, 8))
-        self.assertEqual(review.published_date.month, 7)
-        self.assertEqual(review.published_date.day, 8)
         self.assertEqual(review.privacy, "unlisted")
 
     def test_handle_imported_book_reviews_disabled(self):

bookwyrm/tests/importers/test_librarything_import.py

@@ -2,6 +2,8 @@
 import csv
 import pathlib
 from unittest.mock import patch
+import datetime
+import pytz
 
 from django.test import TestCase
 import responses
@@ -12,6 +14,10 @@ from bookwyrm.importers.importer import import_data, handle_imported_book
 from bookwyrm.settings import DOMAIN
 
 
+def make_date(*args):
+    return datetime.datetime(*args, tzinfo=pytz.UTC)
+
+
 class LibrarythingImport(TestCase):
     """importing from librarything tsv"""
 
@@ -125,13 +131,8 @@ class LibrarythingImport(TestCase):
 
         readthrough = models.ReadThrough.objects.get(user=self.user)
         self.assertEqual(readthrough.book, self.book)
-        # I can't remember how to create dates and I don't want to look it up.
+        self.assertEqual(readthrough.start_date, make_date(2007, 4, 16))
-        self.assertEqual(readthrough.start_date.year, 2007)
+        self.assertEqual(readthrough.finish_date, make_date(2007, 5, 8))
-        self.assertEqual(readthrough.start_date.month, 4)
-        self.assertEqual(readthrough.start_date.day, 16)
-        self.assertEqual(readthrough.finish_date.year, 2007)
-        self.assertEqual(readthrough.finish_date.month, 5)
-        self.assertEqual(readthrough.finish_date.day, 8)
 
     def test_handle_imported_book_already_shelved(self):
         """librarything import added a book, this adds related connections"""
@@ -160,14 +161,11 @@ class LibrarythingImport(TestCase):
         shelf.refresh_from_db()
         self.assertEqual(shelf.books.first(), self.book)
         self.assertIsNone(self.user.shelf_set.get(identifier="read").books.first())
+
         readthrough = models.ReadThrough.objects.get(user=self.user)
         self.assertEqual(readthrough.book, self.book)
-        self.assertEqual(readthrough.start_date.year, 2007)
+        self.assertEqual(readthrough.start_date, make_date(2007, 4, 16))
-        self.assertEqual(readthrough.start_date.month, 4)
+        self.assertEqual(readthrough.finish_date, make_date(2007, 5, 8))
-        self.assertEqual(readthrough.start_date.day, 16)
-        self.assertEqual(readthrough.finish_date.year, 2007)
-        self.assertEqual(readthrough.finish_date.month, 5)
-        self.assertEqual(readthrough.finish_date.day, 8)
 
     def test_handle_import_twice(self):
         """re-importing books"""
@@ -198,13 +196,8 @@ class LibrarythingImport(TestCase):
 
         readthrough = models.ReadThrough.objects.get(user=self.user)
         self.assertEqual(readthrough.book, self.book)
-        # I can't remember how to create dates and I don't want to look it up.
+        self.assertEqual(readthrough.start_date, make_date(2007, 4, 16))
-        self.assertEqual(readthrough.start_date.year, 2007)
+        self.assertEqual(readthrough.finish_date, make_date(2007, 5, 8))
-        self.assertEqual(readthrough.start_date.month, 4)
-        self.assertEqual(readthrough.start_date.day, 16)
-        self.assertEqual(readthrough.finish_date.year, 2007)
-        self.assertEqual(readthrough.finish_date.month, 5)
-        self.assertEqual(readthrough.finish_date.day, 8)
 
     @patch("bookwyrm.activitystreams.ActivityStream.add_status")
     def test_handle_imported_book_review(self, _):
@@ -226,9 +219,7 @@ class LibrarythingImport(TestCase):
         review = models.Review.objects.get(book=self.book, user=self.user)
         self.assertEqual(review.content, "chef d'oeuvre")
         self.assertEqual(review.rating, 5)
-        self.assertEqual(review.published_date.year, 2007)
+        self.assertEqual(review.published_date, make_date(2007, 5, 8))
-        self.assertEqual(review.published_date.month, 5)
-        self.assertEqual(review.published_date.day, 8)
         self.assertEqual(review.privacy, "unlisted")
 
     def test_handle_imported_book_reviews_disabled(self):

bookwyrm/views/shelf.py

@@ -2,7 +2,7 @@
 from collections import namedtuple
 
 from django.db import IntegrityError
-from django.db.models import OuterRef, Subquery
+from django.db.models import OuterRef, Subquery, F
 from django.contrib.auth.decorators import login_required
 from django.core.paginator import Paginator
 from django.http import HttpResponseBadRequest, HttpResponseNotFound
@@ -69,7 +69,8 @@ class Shelf(View):
             reviews = privacy_filter(request.user, reviews)
 
         books = books.annotate(
-            rating=Subquery(reviews.values("rating")[:1])
+            rating=Subquery(reviews.values("rating")[:1]),
+            shelved_date=F("shelfbook__shelved_date"),
         ).prefetch_related("authors")
 
         paginated = Paginator(