Merge pull request #3000 from dato/position_serialization

Minor improvements to Quotation pure content

bookwyrm/models/status.py

@@ -1,5 +1,6 @@
 """ models for storing different kinds of Activities """
 from dataclasses import MISSING
+from typing import Optional
 import re
 
 from django.apps import apps
@@ -269,7 +270,7 @@ class GeneratedNote(Status):
         """indicate the book in question for mastodon (or w/e) users"""
         message = self.content
         books = ", ".join(
-            f'<a href="{book.remote_id}">"{book.title}"</a>'
+            f'<a href="{book.remote_id}"><i>{book.title}</i></a>'
             for book in self.mention_books.all()
         )
         return f"{self.user.display_name} {message} {books}"
@@ -320,17 +321,14 @@ class Comment(BookStatus):
     @property
     def pure_content(self):
         """indicate the book in question for mastodon (or w/e) users"""
-        if self.progress_mode == "PG" and self.progress and (self.progress > 0):
-            return_value = (
-                f'{self.content}<p>(comment on <a href="{self.book.remote_id}">'
-                f'"{self.book.title}"</a>, page {self.progress})</p>'
+        progress = self.progress or 0
+        citation = (
+            f'comment on <a href="{self.book.remote_id}">'
+            f"<i>{self.book.title}</i></a>"
         )
-        else:
-            return_value = (
-                f'{self.content}<p>(comment on <a href="{self.book.remote_id}">'
-                f'"{self.book.title}"</a>)</p>'
-            )
-        return return_value
+        if self.progress_mode == "PG" and progress > 0:
+            citation += f", p. {progress}"
+        return f"{self.content}<p>({citation})</p>"
 
     activity_serializer = activitypub.Comment
 
@@ -354,22 +352,24 @@ class Quotation(BookStatus):
         blank=True,
     )
 
+    def _format_position(self) -> Optional[str]:
+        """serialize page position"""
+        beg = self.position
+        end = self.endposition or 0
+        if self.position_mode != "PG" or not beg:
+            return None
+        return f"pp. {beg}-{end}" if end > beg else f"p. {beg}"
+
     @property
     def pure_content(self):
         """indicate the book in question for mastodon (or w/e) users"""
         quote = re.sub(r"^<p>", '<p>"', self.quote)
         quote = re.sub(r"</p>$", '"</p>', quote)
-        if self.position_mode == "PG" and self.position and (self.position > 0):
-            return_value = (
-                f'{quote} <p>-- <a href="{self.book.remote_id}">'
-                f'"{self.book.title}"</a>, page {self.position}</p>{self.content}'
-            )
-        else:
-            return_value = (
-                f'{quote} <p>-- <a href="{self.book.remote_id}">'
-                f'"{self.book.title}"</a></p>{self.content}'
-            )
-        return return_value
+        title, href = self.book.title, self.book.remote_id
+        citation = f'— <a href="{href}"><i>{title}</i></a>'
+        if position := self._format_position():
+            citation += f", {position}"
+        return f"{quote} <p>{citation}</p>{self.content}"
 
     activity_serializer = activitypub.Quotation
 

bookwyrm/templates/book/editions/editions.html

@@ -5,7 +5,7 @@
 
 {% block content %}
 <div class="block">
-    <h1 class="title">{% blocktrans with work_path=work.local_path work_title=work|book_title %}Editions of <a href="{{ work_path }}">"{{ work_title }}"</a>{% endblocktrans %}</h1>
+    <h1 class="title">{% blocktrans with work_path=work.local_path work_title=work|book_title %}Editions of <a href="{{ work_path }}"><i>{{ work_title }}</i></a>{% endblocktrans %}</h1>
 </div>
 
 {% include 'book/editions/edition_filters.html' %}

bookwyrm/tests/models/test_status_model.py

@@ -212,7 +212,7 @@ class Status(TestCase):
     def test_generated_note_to_pure_activity(self, *_):
         """subclass of the base model version with a "pure" serializer"""
         status = models.GeneratedNote.objects.create(
-            content="test content", user=self.local_user
+            content="reads", user=self.local_user
         )
         status.mention_books.set([self.book])
         status.mention_users.set([self.local_user])
@@ -220,7 +220,7 @@ class Status(TestCase):
         self.assertEqual(activity["id"], status.remote_id)
         self.assertEqual(
             activity["content"],
-            f'mouse test content <a href="{self.book.remote_id}">"Test Edition"</a>',
+            f'mouse reads <a href="{self.book.remote_id}"><i>Test Edition</i></a>',
         )
         self.assertEqual(len(activity["tag"]), 2)
         self.assertEqual(activity["type"], "Note")
@@ -249,14 +249,18 @@ class Status(TestCase):
     def test_comment_to_pure_activity(self, *_):
         """subclass of the base model version with a "pure" serializer"""
         status = models.Comment.objects.create(
-            content="test content", user=self.local_user, book=self.book
+            content="test content", user=self.local_user, book=self.book, progress=27
         )
         activity = status.to_activity(pure=True)
         self.assertEqual(activity["id"], status.remote_id)
         self.assertEqual(activity["type"], "Note")
         self.assertEqual(
             activity["content"],
-            f'test content<p>(comment on <a href="{self.book.remote_id}">"Test Edition"</a>)</p>',
+            (
+                "test content"
+                f'<p>(comment on <a href="{self.book.remote_id}">'
+                "<i>Test Edition</i></a>, p. 27)</p>"
+            ),
         )
         self.assertEqual(activity["attachment"][0]["type"], "Document")
         # self.assertTrue(
@@ -295,7 +299,11 @@ class Status(TestCase):
         self.assertEqual(activity["type"], "Note")
         self.assertEqual(
             activity["content"],
-            f'a sickening sense <p>-- <a href="{self.book.remote_id}">"Test Edition"</a></p>test content',
+            (
+                "a sickening sense "
+                f'<p>— <a href="{self.book.remote_id}">'
+                "<i>Test Edition</i></a></p>test content"
+            ),
         )
         self.assertEqual(activity["attachment"][0]["type"], "Document")
         self.assertTrue(
@@ -306,6 +314,29 @@ class Status(TestCase):
         )
         self.assertEqual(activity["attachment"][0]["name"], "Test Edition")
 
+    def test_quotation_page_serialization(self, *_):
+        """serialization of quotation page position"""
+        tests = [
+            ("single pos", 7, None, "p. 7"),
+            ("page range", 7, 10, "pp. 7-10"),
+        ]
+        for desc, beg, end, pages in tests:
+            with self.subTest(desc):
+                status = models.Quotation.objects.create(
+                    quote="<p>my quote</p>",
+                    content="",
+                    user=self.local_user,
+                    book=self.book,
+                    position=beg,
+                    endposition=end,
+                    position_mode="PG",
+                )
+                activity = status.to_activity(pure=True)
+                self.assertRegex(
+                    activity["content"],
+                    f'^<p>"my quote"</p> <p>— <a .+</a>, {pages}</p>$',
+                )
+
     def test_review_to_activity(self, *_):
         """subclass of the base model version with a "pure" serializer"""
         status = models.Review.objects.create(