Merge branch 'improve_notifications' into 'master'

Improve notifications

See merge request allianceauth/allianceauth!1324

allianceauth/notifications/admin.py

@@ -1,4 +1,35 @@
 from django.contrib import admin
 from .models import Notification
 
-admin.site.register(Notification)
+
+@admin.register(Notification)
+class NotificationAdmin(admin.ModelAdmin):        
+    list_display = ("timestamp", "_main", "_state", "title", "level", "viewed")
+    list_select_related = ("user", "user__profile__main_character", "user__profile__state")
+    list_filter = (        
+        "level", 
+        "timestamp",
+        "user__profile__state",
+        ('user__profile__main_character', admin.RelatedOnlyFieldListFilter),
+    )
+    ordering = ("-timestamp", )
+    search_fields = ["user__username", "user__profile__main_character__character_name"]
+
+    def _main(self, obj):
+        try:
+            return obj.user.profile.main_character
+        except AttributeError:
+            return obj.user
+    
+    _main.admin_order_field = "user__profile__main_character__character_name"
+
+    def _state(self, obj):
+        return obj.user.profile.state
+    
+    _state.admin_order_field = "user__profile__state__name"
+
+    def has_change_permission(self, request, obj=None):
+        return False
+
+    def has_add_permission(self, request) -> bool:
+        return False

allianceauth/notifications/managers.py

@@ -12,7 +12,7 @@ class NotificationQuerySet(models.QuerySet):
     """Custom QuerySet for Notification model"""
 
     def update(self, *args, **kwargs):
-        # overriden update to ensure cache is invaidated on very call
+        """Override update to ensure cache is invalidated on very call."""
         super().update(*args, **kwargs)
         user_pks = set(self.select_related("user").values_list('user__pk', flat=True))
         for user_pk in user_pks:
@@ -43,6 +43,8 @@ class NotificationManager(models.Manager):
         if not message:
             message = title
         
+        if level not in self.model.Level:
+            level = self.model.Level.INFO
         obj = self.create(user=user, title=title, message=message, level=level)
         logger.info("Created notification %s", obj)
         return obj

allianceauth/notifications/migrations/0005_fix_level_choices.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.1.12 on 2021-07-01 21:22
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('notifications', '0004_performance_tuning'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='notification',
+            name='level',
+            field=models.CharField(choices=[('danger', 'danger'), ('warning', 'warning'), ('info', 'info'), ('success', 'success')], default='info', max_length=10),
+        ),
+    ]

allianceauth/notifications/models.py

@@ -2,6 +2,7 @@ import logging
 
 from django.db import models
 from django.contrib.auth.models import User
+from django.utils.translation import gettext_lazy as _
 
 from .managers import NotificationManager
 
@@ -14,16 +15,42 @@ class Notification(models.Model):
     NOTIFICATIONS_MAX_PER_USER_DEFAULT = 50
     NOTIFICATIONS_REFRESH_TIME_DEFAULT = 30
 
-    LEVEL_CHOICES = (
-        ('danger', 'CRITICAL'),
-        ('danger', 'ERROR'),
-        ('warning', 'WARN'),
-        ('info', 'INFO'),
-        ('success', 'DEBUG'),
-    )
+    class Level(models.TextChoices):
+        """A notification level."""
+
+        DANGER = 'danger', _('danger')  #: 
+        WARNING = 'warning', _('warning')  #:
+        INFO = 'info', _('info')  #:
+        SUCCESS = 'success', _('success')  #:
+
+        @classmethod
+        def from_old_name(cls, name: str) -> object:
+            """Map old name to enum.
+            
+            Raises ValueError for invalid names.
+            """            
+            name_map = {
+                "CRITICAL": cls.DANGER,
+                "ERROR": cls.DANGER,
+                "WARN": cls.WARNING,
+                "INFO": cls.INFO,
+                "DEBUG": cls.SUCCESS,
+            }
+            try:
+                return name_map[name]
+            except KeyError:
+                raise ValueError(f"Unknown name: {name}") from None
+
+    # LEVEL_CHOICES = (
+    #     ('danger', 'CRITICAL'),
+    #     ('danger', 'ERROR'),
+    #     ('warning', 'WARN'),
+    #     ('info', 'INFO'),
+    #     ('success', 'DEBUG'),
+    # )
 
     user = models.ForeignKey(User, on_delete=models.CASCADE)
-    level = models.CharField(choices=LEVEL_CHOICES, max_length=10)
+    level = models.CharField(choices=Level.choices, max_length=10, default=Level.INFO)
     title = models.CharField(max_length=254)
     message = models.TextField()
     timestamp = models.DateTimeField(auto_now_add=True, db_index=True)
@@ -45,22 +72,15 @@ class Notification(models.Model):
         Notification.objects.invalidate_user_notification_cache(self.user.pk)
 
     def mark_viewed(self) -> None:
-        """mark notification as viewed"""
+        """Mark notification as viewed."""
         logger.info("Marking notification as viewed: %s" % self)
         self.viewed = True
         self.save()
 
     def set_level(self, level_name: str) -> None:
-        """set notification level according to level name, e.g. 'CRITICAL'
+        """Set notification level according to old level name, e.g. 'CRITICAL'.
         
-        raised exception on invalid level names
+        Raises ValueError on invalid level names.
         """        
-        try:
-            new_level = [
-                item[0] for item in self.LEVEL_CHOICES if item[1] == level_name
-            ][0]
-        except IndexError:
-            raise ValueError('Invalid level name: %s' % level_name)
-
-        self.level = new_level
+        self.level = self.Level.from_old_name(level_name)
         self.save()

allianceauth/notifications/tests/test_managers.py

@@ -65,6 +65,35 @@ class TestUserNotify(TestCase):
         self.assertEqual(obj.title, title)
         self.assertEqual(obj.message, title)       
 
+    def test_should_use_default_level_when_not_specified(self):
+        # given
+        title = 'dummy_title'
+        message = 'dummy message'        
+        # when
+        Notification.objects.notify_user(self.user, title, message)
+        # then
+        self.assertEqual(Notification.objects.filter(user=self.user).count(), 1)
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, title)
+        self.assertEqual(obj.message, message)
+        self.assertEqual(obj.level, Notification.Level.INFO)
+
+    def test_should_use_default_level_when_invalid_level_given(self):
+        # given
+        title = 'dummy_title'
+        message = 'dummy message'        
+        level = "invalid"
+        # when
+        Notification.objects.notify_user(self.user, title, message, level)
+        # then
+        self.assertEqual(Notification.objects.filter(user=self.user).count(), 1)
+        obj = Notification.objects.first()
+        self.assertEqual(obj.user, self.user)
+        self.assertEqual(obj.title, title)
+        self.assertEqual(obj.message, message)
+        self.assertEqual(obj.level, Notification.Level.INFO)
+            
     @override_settings(NOTIFICATIONS_MAX_PER_USER=3)
     def test_remove_when_too_many_notifications(self):
         Notification.objects.notify_user(self.user, 'dummy')

docs/development/tech_docu/api/notifications.rst

@@ -8,18 +8,15 @@ Location: ``allianceauth.notifications``
 
 .. automodule:: allianceauth.notifications.__init__
     :members: notify    
-    :undoc-members:
 
 models
 ===========
 
 .. autoclass:: allianceauth.notifications.models.Notification
-    :members: LEVEL_CHOICES, mark_viewed, set_level
-    :undoc-members:
+    :members: Level, mark_viewed, set_level    
 
 managers
 ===========
 
 .. autoclass:: allianceauth.notifications.managers.NotificationManager
     :members: notify_user, user_unread_count
-    :undoc-members: