From 13d8ccb016ca8c761fe5fa6a4b841db2f0f9a556 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 14:13:36 -0800 Subject: [PATCH 01/11] Moves status notifications into model --- bookwyrm/models/status.py | 25 +++++++++++++++++++++++++ bookwyrm/views/status.py | 23 ++--------------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index edf60281..7091fd0b 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -52,6 +52,31 @@ 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.reply_parent 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 notification_model.objects.filter( + related_status=self, related_user=mention_user).exists(): + 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 ''' diff --git a/bookwyrm/views/status.py b/bookwyrm/views/status.py index 522d1f6d..db924ce8 100644 --- a/bookwyrm/views/status.py +++ b/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'%s\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): From ca08bfa6f516d023d32f8795fb182ec75502a5cb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 14:14:33 -0800 Subject: [PATCH 02/11] Remove duplicate notification generation code in incoming --- bookwyrm/incoming.py | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index 562225e7..ca53d919 100644 --- a/bookwyrm/incoming.py +++ b/bookwyrm/incoming.py @@ -256,27 +256,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): From 106d442a0bc88d1faec9841d23193fde339a486b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 14:18:55 -0800 Subject: [PATCH 03/11] Moves import complete notification to model --- bookwyrm/goodreads_import.py | 2 -- bookwyrm/models/import_job.py | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/bookwyrm/goodreads_import.py b/bookwyrm/goodreads_import.py index 3dcdc2f0..1b2b971c 100644 --- a/bookwyrm/goodreads_import.py +++ b/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() diff --git a/bookwyrm/models/import_job.py b/bookwyrm/models/import_job.py index b10651b9..af8a35dd 100644 --- a/bookwyrm/models/import_job.py +++ b/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,19 @@ 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 ''' From 74d39c3e24629fb39bcc94907a461bc822f1ff98 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 14:31:39 -0800 Subject: [PATCH 04/11] Move fav notifications to model --- bookwyrm/incoming.py | 7 ------- bookwyrm/models/favorite.py | 30 ++++++++++++++++++++++++++++++ bookwyrm/views/interaction.py | 16 ---------------- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index ca53d919..09cda8d9 100644 --- a/bookwyrm/incoming.py +++ b/bookwyrm/incoming.py @@ -288,13 +288,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): diff --git a/bookwyrm/models/favorite.py b/bookwyrm/models/favorite.py index 7d630cf5..3a7249f2 100644 --- a/bookwyrm/models/favorite.py +++ b/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,35 @@ 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) + if notification_model.objects.filter( + user=self.status.user, related_user=self.user, + related_status=self.status, notification_type='FAVORITE' + ).exists(): + return + 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') diff --git a/bookwyrm/views/interaction.py b/bookwyrm/views/interaction.py index ebee4719..88813e60 100644 --- a/bookwyrm/views/interaction.py +++ b/bookwyrm/views/interaction.py @@ -26,13 +26,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 +45,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', '/')) From 6f748a6a2473ce84413616a745aae8d909f9eadc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 15:18:20 -0800 Subject: [PATCH 05/11] Fixes status notifications --- bookwyrm/models/fields.py | 1 + bookwyrm/models/status.py | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index bc10156b..55de1fab 100644 --- a/bookwyrm/models/fields.py +++ b/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: diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 7091fd0b..d2768e28 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -52,13 +52,15 @@ 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.reply_parent and self.reply_parent.user.local: + 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', @@ -68,7 +70,8 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): for mention_user in self.mention_users.all(): # avoid double-notifying about this status if not mention_user.local or notification_model.objects.filter( - related_status=self, related_user=mention_user).exists(): + user=mention_user, related_status=self, + related_user=self.user).exists(): continue notification_model.objects.create( user=mention_user, From ac57db537564eb6a08e2db8f8b519163bdd83b99 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 16:00:02 -0800 Subject: [PATCH 06/11] Boost notifications --- bookwyrm/incoming.py | 10 +--------- bookwyrm/models/status.py | 36 +++++++++++++++++++++++++++++++++++ bookwyrm/views/interaction.py | 18 ------------------ 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index 09cda8d9..a7ada52e 100644 --- a/bookwyrm/incoming.py +++ b/bookwyrm/incoming.py @@ -304,19 +304,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): diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index d2768e28..ce4a18bf 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -59,6 +59,10 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): 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( @@ -264,6 +268,38 @@ 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) + if notification_model.objects.filter( + user=self.boosted_status.user, + related_status=self.boosted_status, + related_user=self.user, notification_type='BOOST').exists(): + return + 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" ''' diff --git a/bookwyrm/views/interaction.py b/bookwyrm/views/interaction.py index 88813e60..a7fcc231 100644 --- a/bookwyrm/views/interaction.py +++ b/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 @@ -68,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', '/')) @@ -90,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', '/')) From d9e65aa3638c4a78c23c34e93359e4c03e58ce7a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 16:06:50 -0800 Subject: [PATCH 07/11] Notifications for follow requests --- bookwyrm/incoming.py | 9 +-------- bookwyrm/models/relationship.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index a7ada52e..a88a748e 100644 --- a/bookwyrm/incoming.py +++ b/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 - - status_builder.create_notification( - relationship.user_object, - 'FOLLOW_REQUEST' if manually_approves else 'FOLLOW', - related_user=relationship.user_subject - ) - if not manually_approves: + if not relationship.user_object.manually_approves_followers: relationship.accept() diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 9f3bf07d..ac8f8286 100644 --- a/bookwyrm/models/relationship.py +++ b/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''' From e0cfb009e40ac0927e656e3c3753f26c9d1d7ae6 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 16:21:29 -0800 Subject: [PATCH 08/11] Deduplicate notifications in notification model --- bookwyrm/models/favorite.py | 5 ----- bookwyrm/models/import_job.py | 1 - bookwyrm/models/notification.py | 15 +++++++++++++++ bookwyrm/models/status.py | 5 ----- bookwyrm/tests/views/test_notifications.py | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/bookwyrm/models/favorite.py b/bookwyrm/models/favorite.py index 3a7249f2..f9019501 100644 --- a/bookwyrm/models/favorite.py +++ b/bookwyrm/models/favorite.py @@ -26,11 +26,6 @@ class Favorite(ActivityMixin, BookWyrmModel): if self.status.user.local and self.status.user != self.user: notification_model = apps.get_model( 'bookwyrm.Notification', require_ready=True) - if notification_model.objects.filter( - user=self.status.user, related_user=self.user, - related_status=self.status, notification_type='FAVORITE' - ).exists(): - return notification_model.objects.create( user=self.status.user, notification_type='FAVORITE', diff --git a/bookwyrm/models/import_job.py b/bookwyrm/models/import_job.py index af8a35dd..407d820b 100644 --- a/bookwyrm/models/import_job.py +++ b/bookwyrm/models/import_job.py @@ -64,7 +64,6 @@ class ImportJob(models.Model): ) - class ImportItem(models.Model): ''' a single line of a csv being imported ''' job = models.ForeignKey( diff --git a/bookwyrm/models/notification.py b/bookwyrm/models/notification.py index b4c91f61..0470b325 100644 --- a/bookwyrm/models/notification.py +++ b/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 = [ diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index ce4a18bf..bda4d3e8 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -276,11 +276,6 @@ class Boost(ActivityMixin, Status): notification_model = apps.get_model( 'bookwyrm.Notification', require_ready=True) - if notification_model.objects.filter( - user=self.boosted_status.user, - related_status=self.boosted_status, - related_user=self.user, notification_type='BOOST').exists(): - return notification_model.objects.create( user=self.boosted_status.user, related_status=self.boosted_status, diff --git a/bookwyrm/tests/views/test_notifications.py b/bookwyrm/tests/views/test_notifications.py index 24fbde1e..55513359 100644 --- a/bookwyrm/tests/views/test_notifications.py +++ b/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) From e6b9985f958e2c79750a981154b47e2c664c71a5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 16:23:49 -0800 Subject: [PATCH 09/11] Don't need a helper function for creating notifications anymore --- bookwyrm/status.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/bookwyrm/status.py b/bookwyrm/status.py index 648f2e7d..9eec1fa2 100644 --- a/bookwyrm/status.py +++ b/bookwyrm/status.py @@ -30,19 +30,3 @@ def create_generated_note(user, content, mention_books=None, privacy='public'): status.mention_books.add(book) 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, - ) From b774e946f3c4a1f4720c642b87049dd0ccda95eb Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 16:28:50 -0800 Subject: [PATCH 10/11] fixes avoiding mention and reply notification --- bookwyrm/models/status.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index bda4d3e8..62effeb8 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -73,9 +73,9 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): ) for mention_user in self.mention_users.all(): # avoid double-notifying about this status - if not mention_user.local or notification_model.objects.filter( - user=mention_user, related_status=self, - related_user=self.user).exists(): + if not mention_user.local or \ + (self.reply_parent and \ + mention_user == self.reply_parent.user): continue notification_model.objects.create( user=mention_user, From 848454c50fab213cab4837dbc0c23ccfd680e0a2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 10 Feb 2021 16:31:41 -0800 Subject: [PATCH 11/11] Fixes templatetag test --- bookwyrm/tests/test_templatetags.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/test_templatetags.py b/bookwyrm/tests/test_templatetags.py index 267587c2..45c99344 100644 --- a/bookwyrm/tests/test_templatetags.py +++ b/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')