Merge pull request #609 from mouse-reeve/model-notifications

Refactors generating notifications

bookwyrm/goodreads_import.py

@@ -4,7 +4,6 @@ import logging
 
 from bookwyrm import models
 from bookwyrm.models import ImportJob, ImportItem
-from bookwyrm.status import create_notification
 from bookwyrm.tasks import app
 
 logger = logging.getLogger(__name__)
@@ -68,7 +67,6 @@ def import_data(job_id):
                 item.fail_reason = 'Could not find a match for book'
                 item.save()
     finally:
-        create_notification(job.user, 'IMPORT', related_import=job)
         job.complete = True
         job.save()
 

bookwyrm/incoming.py

@@ -136,14 +136,7 @@ def handle_follow(activity):
         )
         # send the accept normally for a duplicate request
 
-    manually_approves = relationship.user_object.manually_approves_followers
+    if not relationship.user_object.manually_approves_followers:
-
-    status_builder.create_notification(
-        relationship.user_object,
-        'FOLLOW_REQUEST' if manually_approves else 'FOLLOW',
-        related_user=relationship.user_subject
-    )
-    if not manually_approves:
         relationship.accept()
 
 
@@ -256,27 +249,6 @@ def handle_create_status(activity):
         # it was discarded because it's not a bookwyrm type
         return
 
-    # create a notification if this is a reply
-    notified = []
-    if status.reply_parent and status.reply_parent.user.local:
-        notified.append(status.reply_parent.user)
-        status_builder.create_notification(
-            status.reply_parent.user,
-            'REPLY',
-            related_user=status.user,
-            related_status=status,
-        )
-    if status.mention_users.exists():
-        for mentioned_user in status.mention_users.all():
-            if not mentioned_user.local or mentioned_user in notified:
-                continue
-            status_builder.create_notification(
-                mentioned_user,
-                'MENTION',
-                related_user=status.user,
-                related_status=status,
-            )
-
 
 @app.task
 def handle_delete_status(activity):
@@ -309,13 +281,6 @@ def handle_favorite(activity):
     if fav.user.local:
         return
 
-    status_builder.create_notification(
-        fav.status.user,
-        'FAVORITE',
-        related_user=fav.user,
-        related_status=fav.status,
-    )
-
 
 @app.task
 def handle_unfavorite(activity):
@@ -332,19 +297,11 @@ def handle_unfavorite(activity):
 def handle_boost(activity):
     ''' someone gave us a boost! '''
     try:
-        boost = activitypub.Boost(**activity).to_model(models.Boost)
+        activitypub.Boost(**activity).to_model(models.Boost)
     except activitypub.ActivitySerializerError:
         # this probably just means we tried to boost an unknown status
         return
 
-    if not boost.user.local:
-        status_builder.create_notification(
-            boost.boosted_status.user,
-            'BOOST',
-            related_user=boost.user,
-            related_status=boost.boosted_status,
-        )
-
 
 @app.task
 def handle_unboost(activity):

bookwyrm/models/favorite.py

@@ -1,4 +1,5 @@
 ''' like/fav/star a status '''
+from django.apps import apps
 from django.db import models
 from django.utils import timezone
 
@@ -22,6 +23,30 @@ class Favorite(ActivityMixin, BookWyrmModel):
         self.user.save(broadcast=False)
         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 '''
         unique_together = ('user', 'status')

bookwyrm/models/fields.py

@@ -263,6 +263,7 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField):
         if formatted is None or formatted is MISSING:
             return
         getattr(instance, self.name).set(formatted)
+        instance.save(broadcast=False)
 
     def field_to_activity(self, value):
         if self.link_only:

bookwyrm/models/import_job.py

@@ -2,6 +2,7 @@
 import re
 import dateutil.parser
 
+from django.apps import apps
 from django.contrib.postgres.fields import JSONField
 from django.db import models
 from django.utils import timezone
@@ -50,6 +51,18 @@ 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

@@ -25,6 +25,21 @@ class Notification(BookWyrmModel):
     notification_type = models.CharField(
         max_length=255, choices=NotificationType.choices)
 
+    def save(self, *args, **kwargs):
+        ''' save, but don't make dupes '''
+        # there's probably a better way to do this
+        if self.__class__.objects.filter(
+                user=self.user,
+                related_book=self.related_book,
+                related_user=self.related_user,
+                related_status=self.related_status,
+                related_import=self.related_import,
+                related_list_item=self.related_list_item,
+                notification_type=self.notification_type,
+            ).exists():
+            return
+        super().save(*args, **kwargs)
+
     class Meta:
         ''' checks if notifcation is in enum list for valid types '''
         constraints = [

bookwyrm/models/relationship.py

@@ -1,4 +1,5 @@
 ''' defines relationships between users '''
+from django.apps import apps
 from django.db import models, transaction
 from django.db.models import Q
 from django.dispatch import receiver
@@ -90,9 +91,20 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship):
             return None
         except (UserFollows.DoesNotExist, UserBlocks.DoesNotExist):
             super().save(*args, **kwargs)
+
         if broadcast and self.user_subject.local and not self.user_object.local:
             self.broadcast(self.to_activity(), self.user_subject)
 
+        if self.user_object.local:
+            model = apps.get_model('bookwyrm.Notification', require_ready=True)
+            notification_type = 'FOLLOW_REQUEST' \
+                if self.user_object.manually_approves_followers else 'FOLLOW'
+            model.objects.create(
+                user=self.user_object,
+                related_user=self.user_subject,
+                notification_type=notification_type,
+            )
+
 
     def accept(self):
         ''' turn this request into the real deal'''

bookwyrm/models/status.py

@@ -52,6 +52,38 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel):
     serialize_reverse_fields = [('attachments', 'attachment', 'id')]
     deserialize_reverse_fields = [('attachments', 'attachment')]
 
+
+    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()
+
+        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,
+            )
+
     @property
     def recipients(self):
         ''' tagged users who definitely need to get this status in broadcast '''
@@ -236,6 +268,33 @@ class Boost(ActivityMixin, Status):
     )
     activity_serializer = activitypub.Boost
 
+    def save(self, *args, **kwargs):
+        ''' save and notify '''
+        super().save(*args, **kwargs)
+        if not self.boosted_status.user.local:
+            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" '''

bookwyrm/status.py

@@ -35,19 +35,3 @@ def create_generated_note(user, content, mention_books=None, privacy='public'):
             status.mention_books.set(mention_books)
         status.save(created=True)
     return status
-
-
-def create_notification(user, notification_type, related_user=None, \
-        related_book=None, related_status=None, related_import=None):
-    ''' let a user know when someone interacts with their content '''
-    if user == related_user:
-        # don't create notification when you interact with your own stuff
-        return
-    models.Notification.objects.create(
-        user=user,
-        related_book=related_book,
-        related_user=related_user,
-        related_status=related_status,
-        related_import=related_import,
-        notification_type=notification_type,
-    )

bookwyrm/tests/test_templatetags.py

@@ -65,9 +65,9 @@ class TemplateTags(TestCase):
         self.assertEqual(bookwyrm_tags.get_notification_count(self.user), 0)
 
         models.Notification.objects.create(
-            user=self.user, notification_type='FOLLOW')
+            user=self.user, notification_type='FAVORITE')
         models.Notification.objects.create(
-            user=self.user, notification_type='FOLLOW')
+            user=self.user, notification_type='MENTION')
 
         models.Notification.objects.create(
             user=self.remote_user, notification_type='FOLLOW')

bookwyrm/tests/views/test_notifications.py

@@ -30,7 +30,7 @@ class NotificationViews(TestCase):
     def test_clear_notifications(self):
         ''' erase notifications '''
         models.Notification.objects.create(
-            user=self.local_user, notification_type='MENTION')
+            user=self.local_user, notification_type='FAVORITE')
         models.Notification.objects.create(
             user=self.local_user, notification_type='MENTION', read=True)
         self.assertEqual(models.Notification.objects.count(), 2)

bookwyrm/views/interaction.py

@@ -7,7 +7,6 @@ from django.utils.decorators import method_decorator
 from django.views import View
 
 from bookwyrm import models
-from bookwyrm.status import create_notification
 
 
 # pylint: disable= no-self-use
@@ -26,13 +25,6 @@ class Favorite(View):
             # you already fav'ed that
             return HttpResponseBadRequest()
 
-        if status.user.local:
-            create_notification(
-                status.user,
-                'FAVORITE',
-                related_user=request.user,
-                related_status=status
-            )
         return redirect(request.headers.get('Referer', '/'))
 
 
@@ -52,15 +44,6 @@ class Unfavorite(View):
             return HttpResponseNotFound()
 
         favorite.delete()
-
-        # check for notification
-        if status.user.local:
-            notification = models.Notification.objects.filter(
-                user=status.user, related_user=request.user,
-                related_status=status, notification_type='FAVORITE'
-            ).first()
-            if notification:
-                notification.delete()
         return redirect(request.headers.get('Referer', '/'))
 
 
@@ -84,14 +67,6 @@ class Boost(View):
             privacy=status.privacy,
             user=request.user,
         )
-
-        if status.user.local:
-            create_notification(
-                status.user,
-                'BOOST',
-                related_user=request.user,
-                related_status=status
-            )
         return redirect(request.headers.get('Referer', '/'))
 
 
@@ -106,13 +81,4 @@ class Unboost(View):
         ).first()
 
         boost.delete()
-
-        # delete related notification
-        if status.user.local:
-            notification = models.Notification.objects.filter(
-                user=status.user, related_user=request.user,
-                related_status=status, notification_type='BOOST'
-            ).first()
-            if notification:
-                notification.delete()
         return redirect(request.headers.get('Referer', '/'))

bookwyrm/views/status.py

@@ -10,7 +10,7 @@ from markdown import markdown
 from bookwyrm import forms, models
 from bookwyrm.sanitize_html import InputHtmlParser
 from bookwyrm.settings import DOMAIN
-from bookwyrm.status import create_notification, delete_status
+from bookwyrm.status import delete_status
 from bookwyrm.utils import regex
 from .helpers import handle_remote_webfinger
 
@@ -48,31 +48,12 @@ class CreateStatus(View):
                 r'<a href="%s">%s</a>\g<1>' % \
                     (mention_user.remote_id, mention_text),
                 content)
-        # add reply parent to mentions and notify
+        # add reply parent to mentions
         if status.reply_parent:
             status.mention_users.add(status.reply_parent.user)
 
-            if status.reply_parent.user.local:
-                create_notification(
-                    status.reply_parent.user,
-                    'REPLY',
-                    related_user=request.user,
-                    related_status=status
-                )
-
         # deduplicate mentions
         status.mention_users.set(set(status.mention_users.all()))
-        # create mention notifications
-        for mention_user in status.mention_users.all():
-            if status.reply_parent and mention_user == status.reply_parent.user:
-                continue
-            if mention_user.local:
-                create_notification(
-                    mention_user,
-                    'MENTION',
-                    related_user=request.user,
-                    related_status=status
-                )
 
         # don't apply formatting to generated notes
         if not isinstance(status, models.GeneratedNote):