Merge pull request #1483 from bookwyrm-social/threads

Threads

bookwyrm/migrations/0104_auto_20211001_2012.py

@@ -0,0 +1,53 @@
+# Generated by Django 3.2.5 on 2021-10-01 20:12
+
+from django.db import migrations, models
+
+
+def set_thread_id(app_registry, schema_editor):
+    """set thread ids"""
+    db_alias = schema_editor.connection.alias
+    # set the thread id on parent nodes
+    model = app_registry.get_model("bookwyrm", "Status")
+    model.objects.using(db_alias).filter(reply_parent__isnull=True).update(
+        thread_id=models.F("id")
+    )
+
+    queryset = model.objects.using(db_alias).filter(
+        reply_parent__isnull=False,
+        reply_parent__thread_id__isnull=False,
+        thread_id__isnull=True,
+    )
+    iters = 0
+    while queryset.exists():
+        queryset.update(
+            thread_id=models.Subquery(
+                model.objects.filter(id=models.OuterRef("reply_parent")).values_list(
+                    "thread_id"
+                )[:1]
+            )
+        )
+        print(iters)
+        iters += 1
+        if iters > 50:
+            print("exceeded query depth")
+            break
+
+
+def reverse(*_):
+    """do nothing"""
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0103_remove_connector_local"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="status",
+            name="thread_id",
+            field=models.IntegerField(blank=True, null=True),
+        ),
+        migrations.RunPython(set_thread_id, reverse),
+    ]

bookwyrm/models/status.py

@@ -57,6 +57,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
         on_delete=models.PROTECT,
         activitypub_field="inReplyTo",
     )
+    thread_id = models.IntegerField(blank=True, null=True)
     objects = InheritanceManager()
 
     activity_serializer = activitypub.Note
@@ -68,6 +69,17 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
 
         ordering = ("-published_date",)
 
+    def save(self, *args, **kwargs):
+        """save and notify"""
+        if self.reply_parent:
+            self.thread_id = self.reply_parent.thread_id or self.reply_parent.id
+
+        super().save(*args, **kwargs)
+
+        if not self.reply_parent:
+            self.thread_id = self.id
+        super().save(broadcast=False, update_fields=["thread_id"])
+
     def delete(self, *args, **kwargs):  # pylint: disable=unused-argument
         """ "delete" a status"""
         if hasattr(self, "boosted_status"):

bookwyrm/settings.py

@@ -13,7 +13,7 @@ VERSION = "0.0.1"
 PAGE_LENGTH = env("PAGE_LENGTH", 15)
 DEFAULT_LANGUAGE = env("DEFAULT_LANGUAGE", "English")
 
-JS_CACHE = "e2bc0653"
+JS_CACHE = "c02929b1"
 
 # email
 EMAIL_BACKEND = env("EMAIL_BACKEND", "django.core.mail.backends.smtp.EmailBackend")

bookwyrm/static/css/bookwyrm.css

@@ -492,6 +492,23 @@ ol.ordered-list li::before {
     }
 }
 
+/* Threads
+ ******************************************************************************/
+
+.thread .is-main .card {
+    box-shadow: 0 0.5em 1em -0.125em rgb(50 115 220 / 35%), 0 0 0 1px rgb(50 115 220 / 2%);
+}
+
+.thread::after {
+    content: "";
+    position: absolute;
+    z-index: -1;
+    top: 0;
+    bottom: 0;
+    left: 2.5em;
+    border-left: 2px solid #e0e0e0;
+}
+
 /* Dimensions
  * @todo These could be in rem.
  ******************************************************************************/

bookwyrm/templates/feed/thread.html

@@ -1,6 +1,7 @@
 {% load status_display %}
-<div class="block">
 
+<div class="thread-parent is-relative block">
+<div class="thread">
 {% with depth=depth|add:1 %}
     {% if depth <= max_depth and status.reply_parent and direction <= 0 %}
         {% with direction=-1 %}
@@ -8,7 +9,9 @@
         {% endwith %}
     {% endif %}
 
-{% include 'snippets/status/status.html' with status=status main=is_root %}
+<div{% if is_root %} class="block mt-5 is-main"{% endif %}>
+    {% include 'snippets/status/status.html' with status=status main=is_root %}
+</div>
 
 {% if depth <= max_depth and direction >= 0 %}
     {% for reply in status|replies  %}
@@ -18,5 +21,5 @@
     {% endfor %}
 {% endif %}
 {% endwith %}
-
+</div>
 </div>

bookwyrm/templates/snippets/status/header.html

@@ -32,7 +32,7 @@
             {% include "snippets/status/header_content.html" %}
         </h3>
         <p class="is-size-7 is-flex is-align-items-center">
-            <a href="{{ status.remote_id }}">{{ status.published_date|published_date }}</a>
+            <a href="{{ status.remote_id }}{% if status.user.local %}#anchor-{{ status.id }}{% endif %}">{{ status.published_date|published_date }}</a>
             {% if status.progress %}
             <span class="ml-1">
             {% if status.progress_mode == 'PG' %}

bookwyrm/templates/snippets/status/layout.html

@@ -3,7 +3,10 @@
 {% load utilities %}
 
 {% block card-header %}
-<div class="card-header-title has-background-white-ter is-block">
+<div
+    class="card-header-title has-background-white-ter is-block"
+    {% if main %}id="anchor-{{ status.id }}"{% endif %}
+>
     {% include 'snippets/status/header.html' with status=status %}
 </div>
 {% endblock %}

bookwyrm/tests/models/test_status_model.py

@@ -14,6 +14,7 @@ from bookwyrm import activitypub, models, settings
 
 
 # pylint: disable=too-many-public-methods
+# pylint: disable=line-too-long
 @patch("bookwyrm.models.Status.broadcast")
 @patch("bookwyrm.activitystreams.add_status_task.delay")
 @patch("bookwyrm.activitystreams.remove_status_task.delay")
@@ -52,22 +53,26 @@ class Status(TestCase):
     def test_status_generated_fields(self, *_):
         """setting remote id"""
         status = models.Status.objects.create(content="bleh", user=self.local_user)
-        expected_id = "https://%s/user/mouse/status/%d" % (settings.DOMAIN, status.id)
+        expected_id = f"https://{settings.DOMAIN}/user/mouse/status/{status.id}"
         self.assertEqual(status.remote_id, expected_id)
         self.assertEqual(status.privacy, "public")
 
     def test_replies(self, *_):
         """get a list of replies"""
-        parent = models.Status.objects.create(content="hi", user=self.local_user)
+        parent = models.Status(content="hi", user=self.local_user)
-        child = models.Status.objects.create(
+        parent.save(broadcast=False)
+        child = models.Status(
             content="hello", reply_parent=parent, user=self.local_user
         )
-        models.Review.objects.create(
+        child.save(broadcast=False)
+        sibling = models.Review(
             content="hey", reply_parent=parent, user=self.local_user, book=self.book
         )
-        models.Status.objects.create(
+        sibling.save(broadcast=False)
+        grandchild = models.Status(
             content="hi hello", reply_parent=child, user=self.local_user
         )
+        grandchild.save(broadcast=False)
 
         replies = models.Status.replies(parent)
         self.assertEqual(replies.count(), 2)
@@ -75,6 +80,11 @@ class Status(TestCase):
         # should select subclasses
         self.assertIsInstance(replies.last(), models.Review)
 
+        self.assertEqual(parent.thread_id, parent.id)
+        self.assertEqual(child.thread_id, parent.id)
+        self.assertEqual(sibling.thread_id, parent.id)
+        self.assertEqual(grandchild.thread_id, parent.id)
+
     def test_status_type(self, *_):
         """class name"""
         self.assertEqual(models.Status().status_type, "Note")
@@ -104,7 +114,7 @@ class Status(TestCase):
         )
 
         replies = parent.to_replies()
-        self.assertEqual(replies["id"], "%s/replies" % parent.remote_id)
+        self.assertEqual(replies["id"], f"{parent.remote_id}/replies")
         self.assertEqual(replies["totalItems"], 2)
 
     def test_status_to_activity(self, *_):
@@ -168,7 +178,7 @@ class Status(TestCase):
         self.assertEqual(activity["id"], status.remote_id)
         self.assertEqual(
             activity["content"],
-            'mouse test content <a href="%s">"Test Edition"</a>' % self.book.remote_id,
+            f'mouse test content <a href="{self.book.remote_id}">"Test Edition"</a>',
         )
         self.assertEqual(len(activity["tag"]), 2)
         self.assertEqual(activity["type"], "Note")
@@ -177,7 +187,7 @@ class Status(TestCase):
         self.assertEqual(activity["attachment"][0].type, "Document")
         self.assertEqual(
             activity["attachment"][0].url,
-            "https://%s%s" % (settings.DOMAIN, self.book.cover.url),
+            f"https://{settings.DOMAIN}{self.book.cover.url}",
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -202,13 +212,12 @@ class Status(TestCase):
         self.assertEqual(activity["type"], "Note")
         self.assertEqual(
             activity["content"],
-            'test content<p>(comment on <a href="%s">"Test Edition"</a>)</p>'
+            f'test content<p>(comment on <a href="{self.book.remote_id}">"Test Edition"</a>)</p>',
-            % self.book.remote_id,
         )
         self.assertEqual(activity["attachment"][0].type, "Document")
         self.assertEqual(
             activity["attachment"][0].url,
-            "https://%s%s" % (settings.DOMAIN, self.book.cover.url),
+            f"https://{settings.DOMAIN}{self.book.cover.url}",
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -240,13 +249,12 @@ class Status(TestCase):
         self.assertEqual(activity["type"], "Note")
         self.assertEqual(
             activity["content"],
-            'a sickening sense <p>-- <a href="%s">"Test Edition"</a></p>'
+            f'a sickening sense <p>-- <a href="{self.book.remote_id}">"Test Edition"</a></p>test content',
-            "test content" % self.book.remote_id,
         )
         self.assertEqual(activity["attachment"][0].type, "Document")
         self.assertEqual(
             activity["attachment"][0].url,
-            "https://%s%s" % (settings.DOMAIN, self.book.cover.url),
+            f"https://{settings.DOMAIN}{self.book.cover.url}",
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -281,13 +289,13 @@ class Status(TestCase):
         self.assertEqual(activity["type"], "Article")
         self.assertEqual(
             activity["name"],
-            'Review of "%s" (3 stars): Review\'s name' % self.book.title,
+            f'Review of "{self.book.title}" (3 stars): Review\'s name',
         )
         self.assertEqual(activity["content"], "test content")
         self.assertEqual(activity["attachment"][0].type, "Document")
         self.assertEqual(
             activity["attachment"][0].url,
-            "https://%s%s" % (settings.DOMAIN, self.book.cover.url),
+            f"https://{settings.DOMAIN}{self.book.cover.url}",
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -303,13 +311,13 @@ class Status(TestCase):
         self.assertEqual(activity["id"], status.remote_id)
         self.assertEqual(activity["type"], "Article")
         self.assertEqual(
-            activity["name"], 'Review of "%s": Review name' % self.book.title
+            activity["name"], f'Review of "{self.book.title}": Review name'
         )
         self.assertEqual(activity["content"], "test content")
         self.assertEqual(activity["attachment"][0].type, "Document")
         self.assertEqual(
             activity["attachment"][0].url,
-            "https://%s%s" % (settings.DOMAIN, self.book.cover.url),
+            f"https://{settings.DOMAIN}{self.book.cover.url}",
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 
@@ -325,13 +333,12 @@ class Status(TestCase):
         self.assertEqual(activity["type"], "Note")
         self.assertEqual(
             activity["content"],
-            'rated <em><a href="%s">%s</a></em>: 3 stars'
+            f'rated <em><a href="{self.book.remote_id}">{self.book.title}</a></em>: 3 stars',
-            % (self.book.remote_id, self.book.title),
         )
         self.assertEqual(activity["attachment"][0].type, "Document")
         self.assertEqual(
             activity["attachment"][0].url,
-            "https://%s%s" % (settings.DOMAIN, self.book.cover.url),
+            f"https://{settings.DOMAIN}{self.book.cover.url}",
         )
         self.assertEqual(activity["attachment"][0].name, "Test Edition")
 

bookwyrm/tests/views/inbox/test_inbox_create.py

@@ -72,6 +72,7 @@ class InboxCreate(TestCase):
         self.assertEqual(status.quote, "quote body")
         self.assertEqual(status.content, "commentary")
         self.assertEqual(status.user, self.local_user)
+        self.assertEqual(status.thread_id, status.id)
 
         # while we're here, lets ensure we avoid dupes
         views.inbox.activity_task(activity)
@@ -144,6 +145,7 @@ class InboxCreate(TestCase):
         status = models.Status.objects.last()
         self.assertEqual(status.content, "test content in note")
         self.assertEqual(status.reply_parent, parent_status)
+        self.assertEqual(status.thread_id, parent_status.id)
         self.assertTrue(models.Notification.objects.filter(user=self.local_user))
         self.assertEqual(models.Notification.objects.get().notification_type, "REPLY")