Modify update_fields in save() when modifying objects

https://docs.djangoproject.com/en/5.0/releases/4.2/#setting-update-fields-in-model-save-may-now-be-required

bookwyrm/models/activitypub_mixin.py

@@ -206,14 +206,10 @@ class ObjectMixin(ActivitypubMixin):
         created: Optional[bool] = None,
         software: Any = None,
         priority: str = BROADCAST,
+        broadcast: bool = True,
         **kwargs: Any,
     ) -> None:
         """broadcast created/updated/deleted objects as appropriate"""
-        broadcast = kwargs.get("broadcast", True)
-        # this bonus kwarg would cause an error in the base save method
-        if "broadcast" in kwargs:
-            del kwargs["broadcast"]
-
         created = created or not bool(self.id)
         # first off, we want to save normally no matter what
         super().save(*args, **kwargs)

bookwyrm/models/author.py

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

bookwyrm/models/book.py

@@ -2,7 +2,7 @@
 
 from itertools import chain
 import re
-from typing import Any, Dict
+from typing import Any, Dict, Optional, Iterable
 from typing_extensions import Self
 
 from django.contrib.postgres.search import SearchVectorField
@@ -27,7 +27,7 @@ from bookwyrm.settings import (
     ENABLE_PREVIEW_IMAGES,
     ENABLE_THUMBNAIL_GENERATION,
 )
-from bookwyrm.utils.db import format_trigger
+from bookwyrm.utils.db import format_trigger, add_update_fields
 
 from .activitypub_mixin import OrderedCollectionPageMixin, ObjectMixin
 from .base_model import BookWyrmModel
@@ -96,14 +96,19 @@ class BookDataModel(ObjectMixin, BookWyrmModel):
 
         abstract = True
 
-    def save(self, *args: Any, **kwargs: Any) -> None:
+    def save(
+        self, *args: Any, update_fields: Optional[Iterable[str]] = None, **kwargs: Any
+    ) -> None:
         """ensure that the remote_id is within this instance"""
         if self.id:
             self.remote_id = self.get_remote_id()
+            update_fields = add_update_fields(update_fields, "remote_id")
         else:
             self.origin_id = self.remote_id
             self.remote_id = None
-        super().save(*args, **kwargs)
+            update_fields = add_update_fields(update_fields, "origin_id", "remote_id")
+
+        super().save(*args, update_fields=update_fields, **kwargs)
 
     # pylint: disable=arguments-differ
     def broadcast(self, activity, sender, software="bookwyrm", **kwargs):
@@ -510,28 +515,39 @@ class Edition(Book):
         # max rank is 9
         return rank
 
-    def save(self, *args: Any, **kwargs: Any) -> None:
+    def save(
+        self, *args: Any, update_fields: Optional[Iterable[str]] = None, **kwargs: Any
+    ) -> None:
         """set some fields on the edition object"""
         # calculate isbn 10/13
-        if self.isbn_13 and self.isbn_13[:3] == "978" and not self.isbn_10:
+        if (
+            self.isbn_10 is None
+            and self.isbn_13 is not None
+            and self.isbn_13[:3] == "978"
+        ):
             self.isbn_10 = isbn_13_to_10(self.isbn_13)
-        if self.isbn_10 and not self.isbn_13:
+            update_fields = add_update_fields(update_fields, "isbn_10")
+        if self.isbn_13 is None and self.isbn_10 is not None:
             self.isbn_13 = isbn_10_to_13(self.isbn_10)
+            update_fields = add_update_fields(update_fields, "isbn_13")
 
         # normalize isbn format
-        if self.isbn_10:
+        if self.isbn_10 is not None:
             self.isbn_10 = normalize_isbn(self.isbn_10)
-        if self.isbn_13:
+        if self.isbn_13 is not None:
             self.isbn_13 = normalize_isbn(self.isbn_13)
 
         # set rank
-        self.edition_rank = self.get_rank()
+        if (new := self.get_rank()) != self.edition_rank:
+            self.edition_rank = new
+            update_fields = add_update_fields(update_fields, "edition_rank")
 
         # Create sort title by removing articles from title
         if self.sort_title in [None, ""]:
             self.sort_title = self.guess_sort_title()
+            update_fields = add_update_fields(update_fields, "sort_title")
 
-        super().save(*args, **kwargs)
+        super().save(*args, update_fields=update_fields, **kwargs)
 
         # clear author cache
         if self.id:

bookwyrm/models/link.py

@@ -1,4 +1,5 @@
 """ outlink data """
+from typing import Optional, Iterable
 from urllib.parse import urlparse
 
 from django.core.exceptions import PermissionDenied
@@ -6,6 +7,7 @@ from django.db import models
 from django.utils.translation import gettext_lazy as _
 
 from bookwyrm import activitypub
+from bookwyrm.utils.db import add_update_fields
 from .activitypub_mixin import ActivitypubMixin
 from .base_model import BookWyrmModel
 from . import fields
@@ -34,17 +36,19 @@ class Link(ActivitypubMixin, BookWyrmModel):
         """link name via the associated domain"""
         return self.domain.name
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """create a link"""
         # get or create the associated domain
         if not self.domain:
             domain = urlparse(self.url).hostname
             self.domain, _ = LinkDomain.objects.get_or_create(domain=domain)
+            update_fields = add_update_fields(update_fields, "domain")
 
         # this is never broadcast, the owning model broadcasts an update
         if "broadcast" in kwargs:
             del kwargs["broadcast"]
-        return super().save(*args, **kwargs)
+
+        super().save(*args, update_fields=update_fields, **kwargs)
 
 
 AvailabilityChoices = [
@@ -88,8 +92,10 @@ class LinkDomain(BookWyrmModel):
             return
         raise PermissionDenied()
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """set a default name"""
         if not self.name:
             self.name = self.domain
-        super().save(*args, **kwargs)
+            update_fields = add_update_fields(update_fields, "name")
+
+        super().save(*args, update_fields=update_fields, **kwargs)

bookwyrm/models/list.py

@@ -1,4 +1,5 @@
 """ make a list of books!! """
+from typing import Optional, Iterable
 import uuid
 
 from django.core.exceptions import PermissionDenied
@@ -8,6 +9,7 @@ from django.utils import timezone
 
 from bookwyrm import activitypub
 from bookwyrm.settings import BASE_URL
+from bookwyrm.utils.db import add_update_fields
 
 from .activitypub_mixin import CollectionItemMixin, OrderedCollectionMixin
 from .base_model import BookWyrmModel
@@ -124,11 +126,13 @@ class List(OrderedCollectionMixin, BookWyrmModel):
             group=None, curation="closed"
         )
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """on save, update embed_key and avoid clash with existing code"""
         if not self.embed_key:
             self.embed_key = uuid.uuid4()
-        super().save(*args, **kwargs)
+            update_fields = add_update_fields(update_fields, "embed_key")
+
+        super().save(*args, update_fields=update_fields, **kwargs)
 
 
 class ListItem(CollectionItemMixin, BookWyrmModel):

bookwyrm/models/move.py

@@ -48,24 +48,21 @@ class MoveUser(Move):
         """update user info and broadcast it"""
 
         # only allow if the source is listed in the target's alsoKnownAs
-        if self.user in self.target.also_known_as.all():
+        if self.user not in self.target.also_known_as.all():
-            self.user.also_known_as.add(self.target.id)
-            self.user.update_active_date()
-            self.user.moved_to = self.target.remote_id
-            self.user.save(update_fields=["moved_to"])
-
-            if self.user.local:
-                kwargs[
-                    "broadcast"
-                ] = True  # Only broadcast if we are initiating the Move
-
-            super().save(*args, **kwargs)
-
-            for follower in self.user.followers.all():
-                if follower.local:
-                    Notification.notify(
-                        follower, self.user, notification_type=NotificationType.MOVE
-                    )
-
-        else:
             raise PermissionDenied()
+
+        self.user.also_known_as.add(self.target.id)
+        self.user.update_active_date()
+        self.user.moved_to = self.target.remote_id
+        self.user.save(update_fields=["moved_to"])
+
+        if self.user.local:
+            kwargs["broadcast"] = True  # Only broadcast if we are initiating the Move
+
+        super().save(*args, **kwargs)
+
+        for follower in self.user.followers.all():
+            if follower.local:
+                Notification.notify(
+                    follower, self.user, notification_type=NotificationType.MOVE
+                )

bookwyrm/models/readthrough.py

@@ -1,9 +1,13 @@
 """ progress in a book """
+from typing import Optional, Iterable
+
 from django.core import validators
 from django.core.cache import cache
 from django.db import models
 from django.db.models import F, Q
 
+from bookwyrm.utils.db import add_update_fields
+
 from .base_model import BookWyrmModel
 
 
@@ -30,13 +34,14 @@ class ReadThrough(BookWyrmModel):
     stopped_date = models.DateTimeField(blank=True, null=True)
     is_active = models.BooleanField(default=True)
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """update user active time"""
         # an active readthrough must have an unset finish date
         if self.finish_date or self.stopped_date:
             self.is_active = False
+            update_fields = add_update_fields(update_fields, "is_active")
 
-        super().save(*args, **kwargs)
+        super().save(*args, update_fields=update_fields, **kwargs)
 
         cache.delete(f"latest_read_through-{self.user_id}-{self.book_id}")
         self.user.update_active_date()

bookwyrm/models/shelf.py

@@ -1,5 +1,6 @@
 """ puttin' books on shelves """
 import re
+from typing import Optional, Iterable
 from django.core.cache import cache
 from django.core.exceptions import PermissionDenied
 from django.db import models
@@ -8,6 +9,7 @@ from django.utils import timezone
 from bookwyrm import activitypub
 from bookwyrm.settings import BASE_URL
 from bookwyrm.tasks import BROADCAST
+from bookwyrm.utils.db import add_update_fields
 from .activitypub_mixin import CollectionItemMixin, OrderedCollectionMixin
 from .base_model import BookWyrmModel
 from . import fields
@@ -46,7 +48,7 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel):
         if not self.identifier:
             # this needs the auto increment ID from the save() above
             self.identifier = self.get_identifier()
-            super().save(*args, **kwargs, broadcast=False)
+            super().save(*args, **kwargs, broadcast=False, update_fields={"identifier"})
 
     def get_identifier(self):
         """custom-shelf-123 for the url"""
@@ -101,12 +103,19 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel):
     activity_serializer = activitypub.ShelfItem
     collection_field = "shelf"
 
-    def save(self, *args, priority=BROADCAST, **kwargs):
+    def save(
+        self,
+        *args,
+        priority=BROADCAST,
+        update_fields: Optional[Iterable[str]] = None,
+        **kwargs,
+    ):
         if not self.user:
             self.user = self.shelf.user
+            update_fields = add_update_fields(update_fields, "user")
 
         is_update = self.id is not None
-        super().save(*args, priority=priority, **kwargs)
+        super().save(*args, priority=priority, update_fields=update_fields, **kwargs)
 
         if is_update and self.user.local:
             # remove all caches related to all editions of this book

bookwyrm/models/site.py

@@ -1,5 +1,6 @@
 """ the particulars for this instance of BookWyrm """
 import datetime
+from typing import Optional, Iterable
 from urllib.parse import urljoin
 import uuid
 
@@ -15,6 +16,7 @@ from bookwyrm.preview_images import generate_site_preview_image_task
 from bookwyrm.settings import BASE_URL, ENABLE_PREVIEW_IMAGES, STATIC_FULL_URL
 from bookwyrm.settings import RELEASE_API
 from bookwyrm.tasks import app, MISC
+from bookwyrm.utils.db import add_update_fields
 from .base_model import BookWyrmModel, new_access_code
 from .user import User
 from .fields import get_absolute_url
@@ -136,13 +138,14 @@ class SiteSettings(SiteModel):
             return get_absolute_url(uploaded)
         return urljoin(STATIC_FULL_URL, default_path)
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """if require_confirm_email is disabled, make sure no users are pending,
         if enabled, make sure invite_question_text is not empty"""
         if not self.invite_question_text:
             self.invite_question_text = "What is your favourite book?"
+            update_fields = add_update_fields(update_fields, "invite_question_text")
 
-        super().save(*args, **kwargs)
+        super().save(*args, update_fields=update_fields, **kwargs)
 
         if not self.require_confirm_email:
             User.objects.filter(is_active=False, deactivation_reason="pending").update(

bookwyrm/models/status.py

@@ -1,6 +1,6 @@
 """ models for storing different kinds of Activities """
 from dataclasses import MISSING
-from typing import Optional
+from typing import Optional, Iterable
 import re
 
 from django.apps import apps
@@ -20,6 +20,7 @@ from model_utils.managers import InheritanceManager
 from bookwyrm import activitypub
 from bookwyrm.preview_images import generate_edition_preview_image_task
 from bookwyrm.settings import ENABLE_PREVIEW_IMAGES
+from bookwyrm.utils.db import add_update_fields
 from .activitypub_mixin import ActivitypubMixin, ActivityMixin
 from .activitypub_mixin import OrderedCollectionPageMixin
 from .base_model import BookWyrmModel
@@ -85,12 +86,13 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
             models.Index(fields=["thread_id"]),
         ]
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """save and notify"""
-        if self.reply_parent:
+        if self.thread_id is None and self.reply_parent:
             self.thread_id = self.reply_parent.thread_id or self.reply_parent_id
+            update_fields = add_update_fields(update_fields, "thread_id")
 
-        super().save(*args, **kwargs)
+        super().save(*args, update_fields=update_fields, **kwargs)
 
         if not self.reply_parent:
             self.thread_id = self.id

bookwyrm/models/user.py

@@ -2,6 +2,7 @@
 import datetime
 import re
 import zoneinfo
+from typing import Optional, Iterable
 from urllib.parse import urlparse
 from uuid import uuid4
 
@@ -24,6 +25,7 @@ from bookwyrm.settings import BASE_URL, ENABLE_PREVIEW_IMAGES, LANGUAGES
 from bookwyrm.signatures import create_key_pair
 from bookwyrm.tasks import app, MISC
 from bookwyrm.utils import regex
+from bookwyrm.utils.db import add_update_fields
 from .activitypub_mixin import OrderedCollectionPageMixin, ActivitypubMixin
 from .base_model import BookWyrmModel, DeactivationReason, new_access_code
 from .federated_server import FederatedServer
@@ -338,13 +340,14 @@ class User(OrderedCollectionPageMixin, AbstractUser):
         ]
         return activity_object
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """populate fields for new local users"""
         created = not bool(self.id)
         if not self.local and not re.match(regex.FULL_USERNAME, self.username):
             # generate a username that uses the domain (webfinger format)
             actor_parts = urlparse(self.remote_id)
             self.username = f"{self.username}@{actor_parts.hostname}"
+            update_fields = add_update_fields(update_fields, "username")
 
         # this user already exists, no need to populate fields
         if not created:
@@ -353,12 +356,12 @@ class User(OrderedCollectionPageMixin, AbstractUser):
             elif not self.deactivation_date:
                 self.deactivation_date = timezone.now()
 
-            super().save(*args, **kwargs)
+            super().save(*args, update_fields=update_fields, **kwargs)
             return
 
         # this is a new remote user, we need to set their remote server field
         if not self.local:
-            super().save(*args, **kwargs)
+            super().save(*args, update_fields=update_fields, **kwargs)
             transaction.on_commit(lambda: set_remote_server(self.id))
             return
 
@@ -370,8 +373,17 @@ class User(OrderedCollectionPageMixin, AbstractUser):
             self.shared_inbox = f"{BASE_URL}/inbox"
             self.outbox = f"{self.remote_id}/outbox"
 
+            update_fields = add_update_fields(
+                update_fields,
+                "remote_id",
+                "followers_url",
+                "inbox",
+                "shared_inbox",
+                "outbox",
+            )
+
             # an id needs to be set before we can proceed with related models
-            super().save(*args, **kwargs)
+            super().save(*args, update_fields=update_fields, **kwargs)
 
             # make users editors by default
             try:
@@ -522,14 +534,19 @@ class KeyPair(ActivitypubMixin, BookWyrmModel):
         # self.owner is set by the OneToOneField on User
         return f"{self.owner.remote_id}/#main-key"
 
-    def save(self, *args, **kwargs):
+    def save(self, *args, update_fields: Optional[Iterable[str]] = None, **kwargs):
         """create a key pair"""
         # no broadcasting happening here
         if "broadcast" in kwargs:
             del kwargs["broadcast"]
+
         if not self.public_key:
             self.private_key, self.public_key = create_key_pair()
-        return super().save(*args, **kwargs)
+            update_fields = add_update_fields(
+                update_fields, "private_key", "public_key"
+            )
+
+        super().save(*args, update_fields=update_fields, **kwargs)
 
 
 @app.task(queue=MISC)

bookwyrm/utils/db.py

@@ -1,6 +1,6 @@
 """ Database utilities """
 
-from typing import cast
+from typing import Optional, Iterable, Set, cast
 import sqlparse  # type: ignore
 
 
@@ -21,3 +21,15 @@ def format_trigger(sql: str) -> str:
             identifier_case="lower",
         ),
     )
+
+
+def add_update_fields(
+    update_fields: Optional[Iterable[str]], *fields: str
+) -> Optional[Set[str]]:
+    """
+    Helper for adding fields to the update_fields kwarg when modifying an object
+    in a model's save() method.
+
+    https://docs.djangoproject.com/en/5.0/releases/4.2/#setting-update-fields-in-model-save-may-now-be-required
+    """
+    return set(fields).union(update_fields) if update_fields is not None else None