Merge pull request #1449 from bookwyrm-social/model-cleanup

Model cleanup

bookwyrm/models/__init__.py

@@ -14,7 +14,6 @@ from .status import Review, ReviewRating
 from .status import Boost
 from .attachment import Image
 from .favorite import Favorite
-from .notification import Notification
 from .readthrough import ReadThrough, ProgressUpdate, ProgressMode
 
 from .user import User, KeyPair, AnnualGoal
@@ -29,6 +28,8 @@ from .site import PasswordReset, InviteRequest
 from .announcement import Announcement
 from .antispam import EmailBlocklist, IPBlocklist
 
+from .notification import Notification
+
 cls_members = inspect.getmembers(sys.modules[__name__], inspect.isclass)
 activity_models = {
     c[1].activity_serializer.__name__: c[1]

bookwyrm/models/favorite.py

@@ -1,7 +1,5 @@
 """ like/fav/star a status """
-from django.apps import apps
 from django.db import models
-from django.utils import timezone
 
 from bookwyrm import activitypub
 from .activitypub_mixin import ActivityMixin
@@ -29,38 +27,9 @@ class Favorite(ActivityMixin, BookWyrmModel):
 
     def save(self, *args, **kwargs):
         """update user active time"""
-        self.user.last_active_date = timezone.now()
+        self.user.update_active_date()
-        self.user.save(broadcast=False, update_fields=["last_active_date"])
         super().save(*args, **kwargs)
 
-        if self.status.user.local and self.status.user != self.user:
-            notification_model = apps.get_model(
-                "bookwyrm.Notification", require_ready=True
-            )
-            notification_model.objects.create(
-                user=self.status.user,
-                notification_type="FAVORITE",
-                related_user=self.user,
-                related_status=self.status,
-            )
-
-    def delete(self, *args, **kwargs):
-        """delete and delete notifications"""
-        # check for notification
-        if self.status.user.local:
-            notification_model = apps.get_model(
-                "bookwyrm.Notification", require_ready=True
-            )
-            notification = notification_model.objects.filter(
-                user=self.status.user,
-                related_user=self.user,
-                related_status=self.status,
-                notification_type="FAVORITE",
-            ).first()
-            if notification:
-                notification.delete()
-        super().delete(*args, **kwargs)
-
     class Meta:
         """can't fav things twice"""
 

bookwyrm/models/import_job.py

@@ -2,7 +2,6 @@
 import re
 import dateutil.parser
 
-from django.apps import apps
 from django.db import models
 from django.utils import timezone
 
@@ -50,19 +49,6 @@ class ImportJob(models.Model):
     )
     retry = models.BooleanField(default=False)
 
-    def save(self, *args, **kwargs):
-        """save and notify"""
-        super().save(*args, **kwargs)
-        if self.complete:
-            notification_model = apps.get_model(
-                "bookwyrm.Notification", require_ready=True
-            )
-            notification_model.objects.create(
-                user=self.user,
-                notification_type="IMPORT",
-                related_import=self,
-            )
-
 
 class ImportItem(models.Model):
     """a single line of a csv being imported"""

bookwyrm/models/notification.py

@@ -1,6 +1,8 @@
 """ alert a user to activity """
 from django.db import models
+from django.dispatch import receiver
 from .base_model import BookWyrmModel
+from . import Boost, Favorite, ImportJob, Report, Status, User
 
 
 NotificationType = models.TextChoices(
@@ -53,3 +55,127 @@ class Notification(BookWyrmModel):
                 name="notification_type_valid",
             )
         ]
+
+
+@receiver(models.signals.post_save, sender=Favorite)
+# pylint: disable=unused-argument
+def notify_on_fav(sender, instance, *args, **kwargs):
+    """someone liked your content, you ARE loved"""
+    if not instance.status.user.local or instance.status.user == instance.user:
+        return
+    Notification.objects.create(
+        user=instance.status.user,
+        notification_type="FAVORITE",
+        related_user=instance.user,
+        related_status=instance.status,
+    )
+
+
+@receiver(models.signals.post_delete, sender=Favorite)
+# pylint: disable=unused-argument
+def notify_on_unfav(sender, instance, *args, **kwargs):
+    """oops, didn't like that after all"""
+    if not instance.status.user.local:
+        return
+    Notification.objects.filter(
+        user=instance.status.user,
+        related_user=instance.user,
+        related_status=instance.status,
+        notification_type="FAVORITE",
+    ).delete()
+
+
+@receiver(models.signals.post_save)
+# pylint: disable=unused-argument
+def notify_user_on_mention(sender, instance, *args, **kwargs):
+    """creating and deleting statuses with @ mentions and replies"""
+    if not issubclass(sender, Status):
+        return
+
+    if instance.deleted:
+        Notification.objects.filter(related_status=instance).delete()
+        return
+
+    if (
+        instance.reply_parent
+        and instance.reply_parent.user != instance.user
+        and instance.reply_parent.user.local
+    ):
+        Notification.objects.create(
+            user=instance.reply_parent.user,
+            notification_type="REPLY",
+            related_user=instance.user,
+            related_status=instance,
+        )
+    for mention_user in instance.mention_users.all():
+        # avoid double-notifying about this status
+        if not mention_user.local or (
+            instance.reply_parent and mention_user == instance.reply_parent.user
+        ):
+            continue
+        Notification.objects.create(
+            user=mention_user,
+            notification_type="MENTION",
+            related_user=instance.user,
+            related_status=instance,
+        )
+
+
+@receiver(models.signals.post_save, sender=Boost)
+# pylint: disable=unused-argument
+def notify_user_on_boost(sender, instance, *args, **kwargs):
+    """boosting a status"""
+    if (
+        not instance.boosted_status.user.local
+        or instance.boosted_status.user == instance.user
+    ):
+        return
+
+    Notification.objects.create(
+        user=instance.boosted_status.user,
+        related_status=instance.boosted_status,
+        related_user=instance.user,
+        notification_type="BOOST",
+    )
+
+
+@receiver(models.signals.post_delete, sender=Boost)
+# pylint: disable=unused-argument
+def notify_user_on_unboost(sender, instance, *args, **kwargs):
+    """unboosting a status"""
+    Notification.objects.filter(
+        user=instance.boosted_status.user,
+        related_status=instance.boosted_status,
+        related_user=instance.user,
+        notification_type="BOOST",
+    ).delete()
+
+
+@receiver(models.signals.post_save, sender=ImportJob)
+# pylint: disable=unused-argument
+def notify_user_on_import_complete(sender, instance, *args, **kwargs):
+    """we imported your books! aren't you proud of us"""
+    if not instance.complete:
+        return
+    Notification.objects.create(
+        user=instance.user,
+        notification_type="IMPORT",
+        related_import=instance,
+    )
+
+
+@receiver(models.signals.post_save, sender=Report)
+# pylint: disable=unused-argument
+def notify_admins_on_report(sender, instance, *args, **kwargs):
+    """something is up, make sure the admins know"""
+    # moderators and superusers should be notified
+    admins = User.objects.filter(
+        models.Q(user_permissions__name__in=["moderate_user", "moderate_post"])
+        | models.Q(is_superuser=True)
+    ).all()
+    for admin in admins:
+        Notification.objects.create(
+            user=admin,
+            related_report=instance,
+            notification_type="REPORT",
+        )

bookwyrm/models/readthrough.py

@@ -2,7 +2,6 @@
 from django.core import validators
 from django.db import models
 from django.db.models import F, Q
-from django.utils import timezone
 
 from .base_model import BookWyrmModel
 
@@ -30,8 +29,7 @@ class ReadThrough(BookWyrmModel):
 
     def save(self, *args, **kwargs):
         """update user active time"""
-        self.user.last_active_date = timezone.now()
+        self.user.update_active_date()
-        self.user.save(broadcast=False, update_fields=["last_active_date"])
         super().save(*args, **kwargs)
 
     def create_update(self):
@@ -65,6 +63,5 @@ class ProgressUpdate(BookWyrmModel):
 
     def save(self, *args, **kwargs):
         """update user active time"""
-        self.user.last_active_date = timezone.now()
+        self.user.update_active_date()
-        self.user.save(broadcast=False, update_fields=["last_active_date"])
         super().save(*args, **kwargs)

bookwyrm/models/report.py

@@ -1,5 +1,4 @@
 """ flagged for moderation """
-from django.apps import apps
 from django.db import models
 from django.db.models import F, Q
 from .base_model import BookWyrmModel
@@ -16,23 +15,6 @@ class Report(BookWyrmModel):
     statuses = models.ManyToManyField("Status", blank=True)
     resolved = models.BooleanField(default=False)
 
-    def save(self, *args, **kwargs):
-        """notify admins when a report is created"""
-        super().save(*args, **kwargs)
-        user_model = apps.get_model("bookwyrm.User", require_ready=True)
-        # moderators and superusers should be notified
-        admins = user_model.objects.filter(
-            Q(user_permissions__name__in=["moderate_user", "moderate_post"])
-            | Q(is_superuser=True)
-        ).all()
-        notification_model = apps.get_model("bookwyrm.Notification", require_ready=True)
-        for admin in admins:
-            notification_model.objects.create(
-                user=admin,
-                related_report=self,
-                notification_type="REPORT",
-            )
-
     class Meta:
         """don't let users report themselves"""
 

bookwyrm/models/status.py

@@ -67,40 +67,6 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
 
         ordering = ("-published_date",)
 
-    def save(self, *args, **kwargs):
-        """save and notify"""
-        super().save(*args, **kwargs)
-
-        notification_model = apps.get_model("bookwyrm.Notification", require_ready=True)
-
-        if self.deleted:
-            notification_model.objects.filter(related_status=self).delete()
-            return
-
-        if (
-            self.reply_parent
-            and self.reply_parent.user != self.user
-            and self.reply_parent.user.local
-        ):
-            notification_model.objects.create(
-                user=self.reply_parent.user,
-                notification_type="REPLY",
-                related_user=self.user,
-                related_status=self,
-            )
-        for mention_user in self.mention_users.all():
-            # avoid double-notifying about this status
-            if not mention_user.local or (
-                self.reply_parent and mention_user == self.reply_parent.user
-            ):
-                continue
-            notification_model.objects.create(
-                user=mention_user,
-                notification_type="MENTION",
-                related_user=self.user,
-                related_status=self,
-            )
-
     def delete(self, *args, **kwargs):  # pylint: disable=unused-argument
         """ "delete" a status"""
         if hasattr(self, "boosted_status"):
@@ -108,6 +74,10 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
             super().delete(*args, **kwargs)
             return
         self.deleted = True
+        # clear user content
+        self.content = None
+        if hasattr(self, "quotation"):
+            self.quotation = None  # pylint: disable=attribute-defined-outside-init
         self.deleted_date = timezone.now()
         self.save()
 
@@ -386,27 +356,6 @@ class Boost(ActivityMixin, Status):
             return
 
         super().save(*args, **kwargs)
-        if not self.boosted_status.user.local or self.boosted_status.user == self.user:
-            return
-
-        notification_model = apps.get_model("bookwyrm.Notification", require_ready=True)
-        notification_model.objects.create(
-            user=self.boosted_status.user,
-            related_status=self.boosted_status,
-            related_user=self.user,
-            notification_type="BOOST",
-        )
-
-    def delete(self, *args, **kwargs):
-        """delete and un-notify"""
-        notification_model = apps.get_model("bookwyrm.Notification", require_ready=True)
-        notification_model.objects.filter(
-            user=self.boosted_status.user,
-            related_status=self.boosted_status,
-            related_user=self.user,
-            notification_type="BOOST",
-        ).delete()
-        super().delete(*args, **kwargs)
 
     def __init__(self, *args, **kwargs):
         """the user field is "actor" here instead of "attributedTo" """
@@ -419,10 +368,6 @@ class Boost(ActivityMixin, Status):
         self.image_fields = []
         self.deserialize_reverse_fields = []
 
-    # This constraint can't work as it would cross tables.
-    # class Meta:
-    #     unique_together = ('user', 'boosted_status')
-
 
 # pylint: disable=unused-argument
 @receiver(models.signals.post_save)

bookwyrm/models/user.py

@@ -195,6 +195,11 @@ class User(OrderedCollectionPageMixin, AbstractUser):
             queryset = queryset.exclude(blocks=viewer)
         return queryset
 
+    def update_active_date(self):
+        """this user is here! they are doing things!"""
+        self.last_active_date = timezone.now()
+        self.save(broadcast=False, update_fields=["last_active_date"])
+
     def to_outbox(self, filter_type=None, **kwargs):
         """an ordered collection of statuses"""
         if filter_type:

bookwyrm/tests/views/test_status.py

@@ -101,7 +101,7 @@ class StatusViews(TestCase):
         """@mention a user in a post"""
         view = views.CreateStatus.as_view()
         user = models.User.objects.create_user(
-            "rat@%s" % DOMAIN,
+            f"rat@{DOMAIN}",
             "rat@rat.com",
             "password",
             local=True,
@@ -124,7 +124,7 @@ class StatusViews(TestCase):
         self.assertEqual(list(status.mention_users.all()), [user])
         self.assertEqual(models.Notification.objects.get().user, user)
         self.assertEqual(
-            status.content, '<p>hi <a href="%s">@rat</a></p>' % user.remote_id
+            status.content, f'<p>hi <a href="{user.remote_id}">@rat</a></p>'
         )
 
     def test_handle_status_reply_with_mentions(self, *_):
@@ -224,13 +224,13 @@ class StatusViews(TestCase):
     def test_find_mentions(self, *_):
         """detect and look up @ mentions of users"""
         user = models.User.objects.create_user(
-            "nutria@%s" % DOMAIN,
+            f"nutria@{DOMAIN}",
             "nutria@nutria.com",
             "password",
             local=True,
             localname="nutria",
         )
-        self.assertEqual(user.username, "nutria@%s" % DOMAIN)
+        self.assertEqual(user.username, f"nutria@{DOMAIN}")
 
         self.assertEqual(
             list(views.status.find_mentions("@nutria"))[0], ("@nutria", user)
@@ -263,19 +263,19 @@ class StatusViews(TestCase):
             self.assertEqual(list(views.status.find_mentions("@beep@beep.com")), [])
 
         self.assertEqual(
-            list(views.status.find_mentions("@nutria@%s" % DOMAIN))[0],
+            list(views.status.find_mentions(f"@nutria@{DOMAIN}"))[0],
-            ("@nutria@%s" % DOMAIN, user),
+            (f"@nutria@{DOMAIN}", user),
         )
 
     def test_format_links_simple_url(self, *_):
         """find and format urls into a tags"""
         url = "http://www.fish.com/"
         self.assertEqual(
-            views.status.format_links(url), '<a href="%s">www.fish.com/</a>' % url
+            views.status.format_links(url), f'<a href="{url}">www.fish.com/</a>'
         )
         self.assertEqual(
-            views.status.format_links("(%s)" % url),
+            views.status.format_links(f"({url})"),
-            '(<a href="%s">www.fish.com/</a>)' % url,
+            f'(<a href="{url}">www.fish.com/</a>)',
         )
 
     def test_format_links_paragraph_break(self, *_):
@@ -292,8 +292,8 @@ http://www.fish.com/"""
         """find and format urls into a tags"""
         url = "http://www.fish.com/"
         self.assertEqual(
-            views.status.format_links("(%s)" % url),
+            views.status.format_links(f"({url})"),
-            '(<a href="%s">www.fish.com/</a>)' % url,
+            f'(<a href="{url}">www.fish.com/</a>)',
         )
 
     def test_format_links_special_chars(self, *_):
@@ -301,27 +301,27 @@ http://www.fish.com/"""
         url = "https://archive.org/details/dli.granth.72113/page/n25/mode/2up"
         self.assertEqual(
             views.status.format_links(url),
-            '<a href="%s">'
+            f'<a href="{url}">'
-            "archive.org/details/dli.granth.72113/page/n25/mode/2up</a>" % url,
+            "archive.org/details/dli.granth.72113/page/n25/mode/2up</a>",
         )
         url = "https://openlibrary.org/search?q=arkady+strugatsky&mode=everything"
         self.assertEqual(
             views.status.format_links(url),
-            '<a href="%s">openlibrary.org/search'
+            f'<a href="{url}">openlibrary.org/search'
-            "?q=arkady+strugatsky&mode=everything</a>" % url,
+            "?q=arkady+strugatsky&mode=everything</a>",
         )
         url = "https://tech.lgbt/@bookwyrm"
         self.assertEqual(
-            views.status.format_links(url), '<a href="%s">tech.lgbt/@bookwyrm</a>' % url
+            views.status.format_links(url), f'<a href="{url}">tech.lgbt/@bookwyrm</a>'
         )
         url = "https://users.speakeasy.net/~lion/nb/book.pdf"
         self.assertEqual(
             views.status.format_links(url),
-            '<a href="%s">users.speakeasy.net/~lion/nb/book.pdf</a>' % url,
+            f'<a href="{url}">users.speakeasy.net/~lion/nb/book.pdf</a>',
         )
-        url = "https://pkm.one/#/page/The%20Book%20which%20launched%20a%201000%20Note%20taking%20apps"
+        url = "https://pkm.one/#/page/The%20Book%20launched%20a%201000%20Note%20apps"
         self.assertEqual(
-            views.status.format_links(url), '<a href="%s">%s</a>' % (url, url[8:])
+            views.status.format_links(url), f'<a href="{url}">{url[8:]}</a>'
         )
 
     def test_to_markdown(self, *_):

bookwyrm/views/login.py

@@ -3,7 +3,6 @@ from django.contrib.auth import authenticate, login, logout
 from django.contrib.auth.decorators import login_required
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
-from django.utils import timezone
 from django.utils.decorators import method_decorator
 from django.utils.translation import gettext_lazy as _
 from django.views import View
@@ -54,8 +53,7 @@ class Login(View):
         if user is not None:
             # successful login
             login(request, user)
-            user.last_active_date = timezone.now()
+            user.update_active_date()
-            user.save(broadcast=False, update_fields=["last_active_date"])
             if request.POST.get("first_login"):
                 return redirect("get-started-profile")
             return redirect(request.GET.get("next", "/"))