From 99a9dbe5f4134b76f5e90da7df25f1b0e7210f16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adeodato=20Sim=C3=B3?= Date: Thu, 9 Nov 2023 21:36:05 -0300 Subject: [PATCH] Create NotificationType as class, not through API This way, we need not list every value again to create the enum. N.B.: enum values are now accessed as `models.NotificationType.FOO`, instead of `models.Notification.FOO`. --- bookwyrm/models/__init__.py | 2 +- bookwyrm/models/antispam.py | 3 +- bookwyrm/models/group.py | 13 +++---- bookwyrm/models/move.py | 5 ++- bookwyrm/models/notification.py | 42 +++++++++++----------- bookwyrm/tests/models/test_notification.py | 24 ++++++------- bookwyrm/views/group.py | 14 +++++--- 7 files changed, 53 insertions(+), 50 deletions(-) diff --git a/bookwyrm/models/__init__.py b/bookwyrm/models/__init__.py index c455c751f..e1c862f2b 100644 --- a/bookwyrm/models/__init__.py +++ b/bookwyrm/models/__init__.py @@ -34,7 +34,7 @@ from .site import PasswordReset, InviteRequest from .announcement import Announcement from .antispam import EmailBlocklist, IPBlocklist, AutoMod, automod_task -from .notification import Notification +from .notification import Notification, NotificationType from .hashtag import Hashtag diff --git a/bookwyrm/models/antispam.py b/bookwyrm/models/antispam.py index 94d978ec4..1067cbf1d 100644 --- a/bookwyrm/models/antispam.py +++ b/bookwyrm/models/antispam.py @@ -10,6 +10,7 @@ from django.utils.translation import gettext_lazy as _ from bookwyrm.tasks import app, MISC from .base_model import BookWyrmModel +from .notification import NotificationType from .user import User @@ -80,7 +81,7 @@ def automod_task(): with transaction.atomic(): for admin in admins: notification, _ = notification_model.objects.get_or_create( - user=admin, notification_type=notification_model.REPORT, read=False + user=admin, notification_type=NotificationType.REPORT, read=False ) notification.related_reports.set(reports) diff --git a/bookwyrm/models/group.py b/bookwyrm/models/group.py index 003b23d02..d02b56ab1 100644 --- a/bookwyrm/models/group.py +++ b/bookwyrm/models/group.py @@ -1,5 +1,4 @@ """ do book related things with other users """ -from django.apps import apps from django.db import models, IntegrityError, transaction from django.db.models import Q from bookwyrm.settings import DOMAIN @@ -143,26 +142,28 @@ class GroupMemberInvitation(models.Model): @transaction.atomic def accept(self): """turn this request into the real deal""" + # pylint: disable-next=import-outside-toplevel + from .notification import Notification, NotificationType # circular dependency + GroupMember.from_request(self) - model = apps.get_model("bookwyrm.Notification", require_ready=True) # tell the group owner - model.notify( + Notification.notify( self.group.user, self.user, related_group=self.group, - notification_type=model.ACCEPT, + notification_type=NotificationType.ACCEPT, ) # let the other members know about it for membership in self.group.memberships.all(): member = membership.user if member not in (self.user, self.group.user): - model.notify( + Notification.notify( member, self.user, related_group=self.group, - notification_type=model.JOIN, + notification_type=NotificationType.JOIN, ) def reject(self): diff --git a/bookwyrm/models/move.py b/bookwyrm/models/move.py index a5bf9d76d..d6d8ef78f 100644 --- a/bookwyrm/models/move.py +++ b/bookwyrm/models/move.py @@ -6,7 +6,7 @@ from bookwyrm import activitypub from .activitypub_mixin import ActivityMixin from .base_model import BookWyrmModel from . import fields -from .notification import Notification +from .notification import Notification, NotificationType class Move(ActivityMixin, BookWyrmModel): @@ -49,7 +49,6 @@ class MoveUser(Move): # only allow if the source is listed in the target's alsoKnownAs if self.user 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 @@ -65,7 +64,7 @@ class MoveUser(Move): for follower in self.user.followers.all(): if follower.local: Notification.notify( - follower, self.user, notification_type=Notification.MOVE + follower, self.user, notification_type=NotificationType.MOVE ) else: diff --git a/bookwyrm/models/notification.py b/bookwyrm/models/notification.py index 093c25c65..46b88f5e5 100644 --- a/bookwyrm/models/notification.py +++ b/bookwyrm/models/notification.py @@ -6,7 +6,7 @@ from . import Boost, Favorite, GroupMemberInvitation, ImportJob, LinkDomain from . import ListItem, Report, Status, User, UserFollowRequest -class Notification(BookWyrmModel): +class NotificationType(models.TextChoices): """you've been tagged, liked, followed, etc""" # Status interactions @@ -43,12 +43,9 @@ class Notification(BookWyrmModel): # Migrations MOVE = "MOVE" - # pylint: disable=line-too-long - NotificationType = models.TextChoices( - # there has got be a better way to do this - "NotificationType", - f"{FAVORITE} {REPLY} {MENTION} {TAG} {FOLLOW} {FOLLOW_REQUEST} {BOOST} {IMPORT} {ADD} {REPORT} {LINK_DOMAIN} {INVITE} {ACCEPT} {JOIN} {LEAVE} {REMOVE} {GROUP_PRIVACY} {GROUP_NAME} {GROUP_DESCRIPTION} {MOVE}", - ) + +class Notification(BookWyrmModel): + """a notification object""" user = models.ForeignKey("User", on_delete=models.CASCADE) read = models.BooleanField(default=False) @@ -93,11 +90,11 @@ class Notification(BookWyrmModel): user=user, related_users=related_user, related_list_items__book_list=list_item.book_list, - notification_type=Notification.ADD, + notification_type=NotificationType.ADD, ).first() if not notification: notification = cls.objects.create( - user=user, notification_type=Notification.ADD + user=user, notification_type=NotificationType.ADD ) notification.related_users.add(related_user) notification.related_list_items.add(list_item) @@ -124,7 +121,7 @@ def notify_on_fav(sender, instance, *args, **kwargs): instance.status.user, instance.user, related_status=instance.status, - notification_type=Notification.FAVORITE, + notification_type=NotificationType.FAVORITE, ) @@ -138,7 +135,7 @@ def notify_on_unfav(sender, instance, *args, **kwargs): instance.status.user, instance.user, related_status=instance.status, - notification_type=Notification.FAVORITE, + notification_type=NotificationType.FAVORITE, ) @@ -163,7 +160,7 @@ def notify_user_on_mention(sender, instance, *args, **kwargs): instance.reply_parent.user, instance.user, related_status=instance, - notification_type=Notification.REPLY, + notification_type=NotificationType.REPLY, ) for mention_user in instance.mention_users.all(): @@ -175,7 +172,7 @@ def notify_user_on_mention(sender, instance, *args, **kwargs): Notification.notify( mention_user, instance.user, - notification_type=Notification.MENTION, + notification_type=NotificationType.MENTION, related_status=instance, ) @@ -194,7 +191,7 @@ def notify_user_on_boost(sender, instance, *args, **kwargs): instance.boosted_status.user, instance.user, related_status=instance.boosted_status, - notification_type=Notification.BOOST, + notification_type=NotificationType.BOOST, ) @@ -206,7 +203,7 @@ def notify_user_on_unboost(sender, instance, *args, **kwargs): instance.boosted_status.user, instance.user, related_status=instance.boosted_status, - notification_type=Notification.BOOST, + notification_type=NotificationType.BOOST, ) @@ -221,7 +218,7 @@ def notify_user_on_import_complete( return Notification.objects.get_or_create( user=instance.user, - notification_type=Notification.IMPORT, + notification_type=NotificationType.IMPORT, related_import=instance, ) @@ -240,7 +237,7 @@ def notify_admins_on_report(sender, instance, created, *args, **kwargs): for admin in admins: notification, _ = Notification.objects.get_or_create( user=admin, - notification_type=Notification.REPORT, + notification_type=NotificationType.REPORT, read=False, ) notification.related_reports.add(instance) @@ -260,7 +257,7 @@ def notify_admins_on_link_domain(sender, instance, created, *args, **kwargs): for admin in admins: notification, _ = Notification.objects.get_or_create( user=admin, - notification_type=Notification.LINK_DOMAIN, + notification_type=NotificationType.LINK_DOMAIN, read=False, ) notification.related_link_domains.add(instance) @@ -274,7 +271,7 @@ def notify_user_on_group_invite(sender, instance, *args, **kwargs): instance.user, instance.group.user, related_group=instance.group, - notification_type=Notification.INVITE, + notification_type=NotificationType.INVITE, ) @@ -312,11 +309,12 @@ def notify_user_on_follow(sender, instance, created, *args, **kwargs): notification = Notification.objects.filter( user=instance.user_object, related_users=instance.user_subject, - notification_type=Notification.FOLLOW_REQUEST, + notification_type=NotificationType.FOLLOW_REQUEST, ).first() if not notification: notification = Notification.objects.create( - user=instance.user_object, notification_type=Notification.FOLLOW_REQUEST + user=instance.user_object, + notification_type=NotificationType.FOLLOW_REQUEST, ) notification.related_users.set([instance.user_subject]) notification.read = False @@ -326,6 +324,6 @@ def notify_user_on_follow(sender, instance, created, *args, **kwargs): Notification.notify( instance.user_object, instance.user_subject, - notification_type=Notification.FOLLOW, + notification_type=NotificationType.FOLLOW, read=False, ) diff --git a/bookwyrm/tests/models/test_notification.py b/bookwyrm/tests/models/test_notification.py index 0e4fe91c7..1c412e1b4 100644 --- a/bookwyrm/tests/models/test_notification.py +++ b/bookwyrm/tests/models/test_notification.py @@ -43,7 +43,7 @@ class Notification(TestCase): def test_notification(self): """New notifications are unread""" notification = models.Notification.objects.create( - user=self.local_user, notification_type=models.Notification.FAVORITE + user=self.local_user, notification_type=models.NotificationType.FAVORITE ) self.assertFalse(notification.read) @@ -52,7 +52,7 @@ class Notification(TestCase): models.Notification.notify( self.local_user, self.remote_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertTrue(models.Notification.objects.exists()) @@ -61,7 +61,7 @@ class Notification(TestCase): models.Notification.notify( self.local_user, self.remote_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertEqual(models.Notification.objects.count(), 1) notification = models.Notification.objects.get() @@ -70,7 +70,7 @@ class Notification(TestCase): models.Notification.notify( self.local_user, self.another_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertEqual(models.Notification.objects.count(), 1) notification.refresh_from_db() @@ -92,7 +92,7 @@ class Notification(TestCase): models.Notification.notify( self.remote_user, self.local_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertFalse(models.Notification.objects.exists()) @@ -101,7 +101,7 @@ class Notification(TestCase): models.Notification.notify( self.local_user, self.local_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertFalse(models.Notification.objects.exists()) @@ -154,14 +154,14 @@ class Notification(TestCase): models.Notification.notify( self.local_user, self.remote_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertTrue(models.Notification.objects.exists()) models.Notification.unnotify( self.local_user, self.remote_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertFalse(models.Notification.objects.exists()) @@ -170,25 +170,25 @@ class Notification(TestCase): models.Notification.notify( self.local_user, self.remote_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) models.Notification.notify( self.local_user, self.another_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertTrue(models.Notification.objects.exists()) models.Notification.unnotify( self.local_user, self.remote_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertTrue(models.Notification.objects.exists()) models.Notification.unnotify( self.local_user, self.another_user, - notification_type=models.Notification.FAVORITE, + notification_type=models.NotificationType.FAVORITE, ) self.assertFalse(models.Notification.objects.exists()) diff --git a/bookwyrm/views/group.py b/bookwyrm/views/group.py index 1ccfd6849..5ac4954e8 100644 --- a/bookwyrm/views/group.py +++ b/bookwyrm/views/group.py @@ -13,9 +13,11 @@ from django.contrib.postgres.search import TrigramSimilarity from django.db.models.functions import Greatest from bookwyrm import forms, models +from bookwyrm.models import NotificationType from bookwyrm.suggested_users import suggested_users from .helpers import get_user_from_username, maybe_redirect_local_path + # pylint: disable=no-self-use class Group(View): """group page""" @@ -59,11 +61,11 @@ class Group(View): model = apps.get_model("bookwyrm.Notification", require_ready=True) for field in form.changed_data: notification_type = ( - model.GROUP_PRIVACY + NotificationType.GROUP_PRIVACY if field == "privacy" - else model.GROUP_NAME + else NotificationType.GROUP_NAME if field == "name" - else model.GROUP_DESCRIPTION + else NotificationType.GROUP_DESCRIPTION if field == "description" else None ) @@ -251,7 +253,9 @@ def remove_member(request): memberships = models.GroupMember.objects.filter(group=group) model = apps.get_model("bookwyrm.Notification", require_ready=True) - notification_type = model.LEAVE if user == request.user else model.REMOVE + notification_type = ( + NotificationType.LEAVE if user == request.user else NotificationType.REMOVE + ) # let the other members know about it for membership in memberships: member = membership.user @@ -264,7 +268,7 @@ def remove_member(request): ) # let the user (now ex-member) know as well, if they were removed - if notification_type == model.REMOVE: + if notification_type == NotificationType.REMOVE: model.notify( user, None, related_group=group, notification_type=notification_type )