Merge branch 'main' into installable-pwa

bookwyrm/activitypub/__init__.py

@@ -4,7 +4,11 @@ import sys
 
 from .base_activity import ActivityEncoder, Signature, naive_parse
 from .base_activity import Link, Mention, Hashtag
-from .base_activity import ActivitySerializerError, resolve_remote_id
+from .base_activity import (
+    ActivitySerializerError,
+    resolve_remote_id,
+    get_representative,
+)
 from .image import Document, Image
 from .note import Note, GeneratedNote, Article, Comment, Quotation
 from .note import Review, Rating

bookwyrm/activitypub/base_activity.py

@@ -1,4 +1,5 @@
 """ basics for an activitypub serializer """
+from __future__ import annotations
 from dataclasses import dataclass, fields, MISSING
 from json import JSONEncoder
 import logging
@@ -72,8 +73,10 @@ class ActivityObject:
 
     def __init__(
         self,
-        activity_objects: Optional[list[str, base_model.BookWyrmModel]] = None,
-        **kwargs: dict[str, Any],
+        activity_objects: Optional[
+            dict[str, Union[str, list[str], ActivityObject, base_model.BookWyrmModel]]
+        ] = None,
+        **kwargs: Any,
     ):
         """this lets you pass in an object with fields that aren't in the
         dataclass, which it ignores. Any field in the dataclass is required or

bookwyrm/models/author.py

@@ -1,5 +1,7 @@
 """ database schema for info about authors """
 import re
+from typing import Tuple, Any
+
 from django.contrib.postgres.indexes import GinIndex
 from django.db import models
 
@@ -38,7 +40,7 @@ class Author(BookDataModel):
     )
     bio = fields.HtmlField(null=True, blank=True)
 
-    def save(self, *args, **kwargs):
+    def save(self, *args: Tuple[Any, ...], **kwargs: dict[str, Any]) -> None:
         """normalize isni format"""
         if self.isni:
             self.isni = re.sub(r"\s", "", self.isni)

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>'
-            )
-        else:
-            return_value = (
-                f'{self.content}<p>(comment on <a href="{self.book.remote_id}">'
-                f'"{self.book.title}"</a>)</p>'
-            )
-        return return_value
+        progress = self.progress or 0
+        citation = (
+            f'comment on <a href="{self.book.remote_id}">'
+            f"<i>{self.book.title}</i></a>"
+        )
+        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/settings.py

@@ -14,7 +14,7 @@ from django.core.exceptions import ImproperlyConfigured
 env = Env()
 env.read_env()
 DOMAIN = env("DOMAIN")
-VERSION = "0.6.5"
+VERSION = "0.6.6"
 
 RELEASE_API = env(
     "RELEASE_API",
@@ -24,7 +24,7 @@ RELEASE_API = env(
 PAGE_LENGTH = env.int("PAGE_LENGTH", 15)
 DEFAULT_LANGUAGE = env("DEFAULT_LANGUAGE", "English")
 
-JS_CACHE = "b972a43c"
+JS_CACHE = "ac315a3b"
 
 # email
 EMAIL_BACKEND = env("EMAIL_BACKEND", "django.core.mail.backends.smtp.EmailBackend")
@@ -317,6 +317,7 @@ LANGUAGES = [
 
 LANGUAGE_ARTICLES = {
     "English": {"the", "a", "an"},
+    "Español (Spanish)": {"un", "una", "unos", "unas", "el", "la", "los", "las"},
 }
 
 TIME_ZONE = "UTC"

bookwyrm/static/js/autocomplete.js

@@ -106,7 +106,7 @@ const tries = {
         e: {
             p: {
                 u: {
-                    b: "ePub",
+                    b: "EPUB",
                 },
             },
         },

bookwyrm/templates/book/edit/edit_book_form.html

@@ -10,7 +10,7 @@
 {% csrf_token %}
 
 <input type="hidden" name="last_edited_by" value="{{ request.user.id }}">
-{% if form.parent_work %}
+{% if book.parent_work.id or form.parent_work %}
 <input type="hidden" name="parent_work" value="{% firstof book.parent_work.id form.parent_work %}">
 {% endif %}
 

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/templates/book/file_links/add_link_modal.html

@@ -35,7 +35,7 @@
             required=""
             id="id_filetype"
             value="{% firstof file_link_form.filetype.value '' %}"
-            placeholder="ePub"
+            placeholder="EPUB"
             list="mimetypes-list"
             data-autocomplete="mimetype"
         >

bookwyrm/templates/import/import.html

@@ -18,7 +18,7 @@
         {% if import_size_limit and import_limit_reset %}
             <div class="notification">
                 <p>
-                    {% blocktrans count days=import_limit_reset with display_size=import_size_limit|intcomma %}
+                    {% blocktrans trimmed count days=import_limit_reset with display_size=import_size_limit|intcomma %}
                         Currently, you are allowed to import {{ display_size }} books every {{ import_limit_reset }} day.
                     {% plural %}
                         Currently, you are allowed to import {{ import_size_limit }} books every {{ import_limit_reset }} days.

bookwyrm/templates/settings/registration.html

@@ -75,13 +75,13 @@
             {% include 'snippets/form_errors.html' with errors_list=form.invite_request_text.errors id="desc_invite_request_text" %}
         </div>
         <div class="field">
-            <label class="label" for="id_invite_requests_question">
+            <label class="label">
                 {{ form.invite_request_question }}
                 {% trans "Set a question for invite requests" %}
             </label>
         </div>
         <div class="field">
-            <label class="label" for="id_invite_question_text">
+            <label class="label">
                 {% trans "Question:" %}
                 {{ form.invite_question_text }}
             </label>

bookwyrm/templates/settings/registration_limited.html

@@ -45,7 +45,7 @@
             {% include 'snippets/form_errors.html' with errors_list=form.invite_request_text.errors id="desc_invite_request_text" %}
         </div>
         <div class="field">
-            <label class="label" for="id_invite_requests_question">
+            <label class="label">
                 {{ form.invite_request_question }}
                 {% trans "Set a question for invite requests" %}
             </label>

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(

bookwyrm/tests/views/test_search.py

@@ -156,7 +156,7 @@ class Views(TestCase):
         response = view(request)
 
         validate_html(response.render())
-        self.assertFalse("results" in response.context_data)
+        self.assertTrue("results" in response.context_data)
 
     def test_search_lists(self):
         """searches remote connectors"""

bookwyrm/tests/views/test_setup.py

@@ -72,7 +72,7 @@ class SetupViews(TestCase):
         self.site.refresh_from_db()
         self.assertFalse(self.site.install_mode)
 
-        user = models.User.objects.get()
+        user = models.User.objects.first()
         self.assertTrue(user.is_active)
         self.assertTrue(user.is_superuser)
         self.assertTrue(user.is_staff)

bookwyrm/urls.py

@@ -1,6 +1,7 @@
 """ url routing for the app and api """
 from django.conf.urls.static import static
 from django.contrib import admin
+from django.contrib.staticfiles.urls import staticfiles_urlpatterns
 from django.urls import path, re_path
 from django.views.generic.base import TemplateView
 
@@ -780,5 +781,8 @@ urlpatterns = [
     path("guided-tour/<tour>", views.toggle_guided_tour),
 ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)
 
+# Serves /static when DEBUG is true.
+urlpatterns.extend(staticfiles_urlpatterns())
+
 # pylint: disable=invalid-name
 handler500 = "bookwyrm.views.server_error"

bookwyrm/utils/cache.py

@@ -1,8 +1,15 @@
 """ Custom handler for caching """
+from typing import Any, Callable, Tuple, Union
+
 from django.core.cache import cache
 
 
-def get_or_set(cache_key, function, *args, timeout=None):
+def get_or_set(
+    cache_key: str,
+    function: Callable[..., Any],
+    *args: Tuple[Any, ...],
+    timeout: Union[float, None] = None
+) -> Any:
     """Django's built-in get_or_set isn't cutting it"""
     value = cache.get(cache_key)
     if value is None:

bookwyrm/utils/isni.py

@@ -1,15 +1,24 @@
 """ISNI author checking utilities"""
 import xml.etree.ElementTree as ET
+from typing import Union, Optional
+
 import requests
 
 from bookwyrm import activitypub, models
 
 
-def request_isni_data(search_index, search_term, max_records=5):
+def get_element_text(element: Optional[ET.Element]) -> str:
+    """If the element is not None and there is a text attribute return this"""
+    if element is not None and element.text is not None:
+        return element.text
+    return ""
+
+
+def request_isni_data(search_index: str, search_term: str, max_records: int = 5) -> str:
     """Request data from the ISNI API"""
 
     search_string = f'{search_index}="{search_term}"'
-    query_params = {
+    query_params: dict[str, Union[str, int]] = {
         "query": search_string,
         "version": "1.1",
         "operation": "searchRetrieve",
@@ -26,41 +35,52 @@ def request_isni_data(search_index, search_term, max_records=5):
     return result.text
 
 
-def make_name_string(element):
+def make_name_string(element: ET.Element) -> str:
     """create a string of form 'personal_name surname'"""
 
     # NOTE: this will often be incorrect, many naming systems
     # list "surname" before personal name
     forename = element.find(".//forename")
     surname = element.find(".//surname")
-    if forename is not None:
-        return "".join([forename.text, " ", surname.text])
-    return surname.text
+
+    forename_text = get_element_text(forename)
+    surname_text = get_element_text(surname)
+
+    return "".join(
+        [forename_text, " " if forename_text and surname_text else "", surname_text]
+    )
 
 
-def get_other_identifier(element, code):
+def get_other_identifier(element: ET.Element, code: str) -> str:
     """Get other identifiers associated with an author from their ISNI record"""
 
     identifiers = element.findall(".//otherIdentifierOfIdentity")
     for section_head in identifiers:
         if (
-            section_head.find(".//type") is not None
-            and section_head.find(".//type").text == code
-            and section_head.find(".//identifier") is not None
+            (section_type := section_head.find(".//type")) is not None
+            and section_type.text is not None
+            and section_type.text == code
+            and (identifier := section_head.find(".//identifier")) is not None
+            and identifier.text is not None
         ):
-            return section_head.find(".//identifier").text
+            return identifier.text
 
     # if we can't find it in otherIdentifierOfIdentity,
     # try sources
     for source in element.findall(".//sources"):
-        code_of_source = source.find(".//codeOfSource")
-        if code_of_source is not None and code_of_source.text.lower() == code.lower():
-            return source.find(".//sourceIdentifier").text
+        if (
+            (code_of_source := source.find(".//codeOfSource")) is not None
+            and code_of_source.text is not None
+            and code_of_source.text.lower() == code.lower()
+            and (source_identifier := source.find(".//sourceIdentifier")) is not None
+            and source_identifier.text is not None
+        ):
+            return source_identifier.text
 
     return ""
 
 
-def get_external_information_uri(element, match_string):
+def get_external_information_uri(element: ET.Element, match_string: str) -> str:
     """Get URLs associated with an author from their ISNI record"""
 
     sources = element.findall(".//externalInformation")
@@ -69,14 +89,18 @@ def get_external_information_uri(element, match_string):
         uri = source.find(".//URI")
         if (
             uri is not None
+            and uri.text is not None
             and information is not None
+            and information.text is not None
             and information.text.lower() == match_string.lower()
         ):
             return uri.text
     return ""
 
 
-def find_authors_by_name(name_string, description=False):
+def find_authors_by_name(
+    name_string: str, description: bool = False
+) -> list[activitypub.Author]:
     """Query the ISNI database for possible author matches by name"""
 
     payload = request_isni_data("pica.na", name_string)
@@ -92,7 +116,11 @@ def find_authors_by_name(name_string, description=False):
         if not personal_name:
             continue
 
-        author = get_author_from_isni(element.find(".//isniUnformatted").text)
+        author = get_author_from_isni(
+            get_element_text(element.find(".//isniUnformatted"))
+        )
+        if author is None:
+            continue
 
         if bool(description):
 
@@ -111,22 +139,23 @@ def find_authors_by_name(name_string, description=False):
                 # some of the "titles" in ISNI are a little ...iffy
                 # @ is used by ISNI/OCLC to index the starting point ignoring stop words
                 # (e.g. "The @Government of no one")
-                title_elements = [
-                    e
-                    for e in titles
-                    if hasattr(e, "text") and not e.text.replace("@", "").isnumeric()
-                ]
-                if len(title_elements):
-                    author.bio = title_elements[0].text.replace("@", "")
-                else:
-                    author.bio = None
+                author.bio = ""
+                for title in titles:
+                    if (
+                        title is not None
+                        and hasattr(title, "text")
+                        and title.text is not None
+                        and not title.text.replace("@", "").isnumeric()
+                    ):
+                        author.bio = title.text.replace("@", "")
+                        break
 
         possible_authors.append(author)
 
     return possible_authors
 
 
-def get_author_from_isni(isni):
+def get_author_from_isni(isni: str) -> Optional[activitypub.Author]:
     """Find data to populate a new author record from their ISNI"""
 
     payload = request_isni_data("pica.isn", isni)
@@ -135,25 +164,30 @@ def get_author_from_isni(isni):
     # there should only be a single responseRecord
     # but let's use the first one just in case
     element = root.find(".//responseRecord")
-    name = make_name_string(element.find(".//forename/.."))
+    if element is None:
+        return None
+
+    name = (
+        make_name_string(forename)
+        if (forename := element.find(".//forename/..")) is not None
+        else ""
+    )
     viaf = get_other_identifier(element, "viaf")
     # use a set to dedupe aliases in ISNI
     aliases = set()
     aliases_element = element.findall(".//personalNameVariant")
     for entry in aliases_element:
         aliases.add(make_name_string(entry))
-    # aliases needs to be list not set
-    aliases = list(aliases)
-    bio = element.find(".//nameTitle")
-    bio = bio.text if bio is not None else ""
+    bio = get_element_text(element.find(".//nameTitle"))
     wikipedia = get_external_information_uri(element, "Wikipedia")
 
     author = activitypub.Author(
-        id=element.find(".//isniURI").text,
+        id=get_element_text(element.find(".//isniURI")),
         name=name,
         isni=isni,
         viafId=viaf,
-        aliases=aliases,
+        # aliases needs to be list not set
+        aliases=list(aliases),
         bio=bio,
         wikipediaLink=wikipedia,
     )
@@ -161,21 +195,26 @@ def get_author_from_isni(isni):
     return author
 
 
-def build_author_from_isni(match_value):
+def build_author_from_isni(match_value: str) -> dict[str, activitypub.Author]:
     """Build basic author class object from ISNI URL"""
 
     # if it is an isni value get the data
     if match_value.startswith("https://isni.org/isni/"):
         isni = match_value.replace("https://isni.org/isni/", "")
-        return {"author": get_author_from_isni(isni)}
+        author = get_author_from_isni(isni)
+        if author is not None:
+            return {"author": author}
     # otherwise it's a name string
     return {}
 
 
-def augment_author_metadata(author, isni):
+def augment_author_metadata(author: models.Author, isni: str) -> None:
     """Update any missing author fields from ISNI data"""
 
     isni_author = get_author_from_isni(isni)
+    if isni_author is None:
+        return
+
     isni_author.to_model(model=models.Author, instance=author, overwrite=False)
 
     # we DO want to overwrite aliases because we're adding them to the

bookwyrm/utils/log.py

@@ -10,7 +10,7 @@ class IgnoreVariableDoesNotExist(logging.Filter):
     these errors are not useful to us.
     """
 
-    def filter(self, record):
+    def filter(self, record: logging.LogRecord) -> bool:
         if record.exc_info:
             (_, err_value, _) = record.exc_info
             while err_value:

bookwyrm/utils/validate.py

@@ -1,8 +1,10 @@
 """Validations"""
+from typing import Optional
+
 from bookwyrm.settings import DOMAIN, USE_HTTPS
 
 
-def validate_url_domain(url):
+def validate_url_domain(url: str) -> Optional[str]:
     """Basic check that the URL starts with the instance domain name"""
     if not url:
         return None

bookwyrm/views/search.py

@@ -91,18 +91,15 @@ def book_search(request):
 
 
 def user_search(request):
-    """cool kids members only user search"""
+    """user search: search for a user"""
     viewer = request.user
     query = request.GET.get("q")
     query = query.strip()
     data = {"type": "user", "query": query}
-    # logged out viewers can't search users
-    if not viewer.is_authenticated:
-        return TemplateResponse(request, "search/user.html", data)
 
     # use webfinger for mastodon style account@domain.com username to load the user if
     # they don't exist locally (handle_remote_webfinger will check the db)
-    if re.match(regex.FULL_USERNAME, query):
+    if re.match(regex.FULL_USERNAME, query) and viewer.is_authenticated:
         handle_remote_webfinger(query)
 
     results = (
@@ -118,6 +115,11 @@ def user_search(request):
         )
         .order_by("-similarity")
     )
+
+    # don't expose remote users
+    if not viewer.is_authenticated:
+        results = results.filter(local=True)
+
     paginated = Paginator(results, PAGE_LENGTH)
     page = paginated.get_page(request.GET.get("page"))
     data["results"] = page

bookwyrm/views/setup.py

@@ -9,6 +9,7 @@ from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.views import View
 
+from bookwyrm.activitypub import get_representative
 from bookwyrm import forms, models
 from bookwyrm import settings
 from bookwyrm.utils import regex
@@ -96,4 +97,5 @@ class CreateAdmin(View):
         login(request, user)
         site.install_mode = False
         site.save()
+        get_representative()  # create the instance user
         return redirect("settings-site")