Merge lp:~allenap/maas/notifications-dismissal into lp:~maas-committers/maas/trunk

[Gavin Panella]

Finding and dismissing notifications.

[Lee Trager (ltrager)]

LGTM. I noticed that between this branch and its prerequisite you have three seperate migrations. Would it make sense to merge them into one migration for trunk?

review: Approve

[Gavin Panella (allenap)]

> I noticed that between this branch and its prerequisite you have three
> seperate migrations. Would it make sense to merge them into one
> migration for trunk?

I consolidated the first couple that appear in the prerequisite, but
left this one separate. I've also reordered them after Mike landed a
migration in his subnet space branch.

=== added file 'src/maasserver/migrations/builtin/maasserver/0104_notifications_dismissals.py'
--- src/maasserver/migrations/builtin/maasserver/0104_notifications_dismissals.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/migrations/builtin/maasserver/0104_notifications_dismissals.py	2017-01-05 09:51:37 +0000
@@ -0,0 +1,27 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import (
+    migrations,
+    models,
+)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('maasserver', '0103_notifications'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='NotificationDismissal',
+            fields=[
+                ('id', models.AutoField(auto_created=True, verbose_name='ID', serialize=False, primary_key=True)),
+                ('notification', models.ForeignKey(to='maasserver.Notification')),
+                ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL)),
+            ],
+        ),
+    ]

=== modified file 'src/maasserver/models/notification.py'
--- src/maasserver/models/notification.py	2017-01-05 09:51:36 +0000
+++ src/maasserver/models/notification.py	2017-01-05 09:51:37 +0000
@@ -13,6 +13,7 @@
     CharField,
     ForeignKey,
     Manager,
+    Model,
     TextField,
 )
 from maasserver import DefaultMeta
@@ -62,6 +63,26 @@
             ident=ident, user=user, users=users, admins=admins,
             message=message, context={} if context is None else context)
 
+    def find_for_user(self, user):
+        """Find notifications for the given user.
+
+        :return: A `RawQuerySet` of `Notification` instances that haven't been
+            dismissed by `user`.
+        """
+        if user.is_superuser:
+            where = "notification.users OR notification.admins"
+        else:
+            where = "notification.users"
+        return self.raw(self._sql_find_for_user % where, [user.id, user.id])
+
+    _sql_find_for_user = """\
+    SELECT notification.* FROM maasserver_notification AS notification
+    LEFT OUTER JOIN maasserver_notificationdismissal AS dismissal ON
+      (dismissal.notification_id = notification.id AND dismissal.user_id = %%s)
+    WHERE (notification.user_id = %%s OR %s) AND dismissal.id IS NULL
+    ORDER BY notification.updated, notification.id
+    """
+
 
 class Notification(CleanSave, TimestampedModel):
     """A notification message.
@@ -101,6 +122,36 @@
         """Render this notification's message using its context."""
         return self.message.format(**self.context)
 
+    def dismiss(self, user):
+        """Dismiss this notification.
+
+        :param user: The user dismissing this notification.
+        """
+        NotificationDismissal.objects.get_or_create(
+            notification=self, user=user)
+
     def clean(self):
         super(Notification, self).clean()
         self.render()  # Just check it works.
+
+    def __repr__(self):
+        return "<Notification user=%s users=%r admins=%r %r>" % (
+            ("None" if self.user is None else repr(self.user.username)),
+            self.users, self.admins, self.render(),
+        )
+
+
+class NotificationDismissal(Model):
+    """A notification dismissal.
+
+    :ivar notification: The notification which has been dismissed.
+    :ivar user: The user that has dismissed the linked notification.
+    """
+
+    class Meta(DefaultMeta):
+        """Needed for South to recognize this model."""
+
+    objects = Manager()
+
+    notification = ForeignKey(Notification, null=False, blank=False)
+    user = ForeignKey(User, null=False, blank=False)

=== modified file 'src/maasserver/models/tests/test_notification.py'
--- src/maasserver/models/tests/test_notification.py	2017-01-05 09:51:36 +0000
+++ src/maasserver/models/tests/test_notification.py	2017-01-05 09:51:37 +0000
@@ -12,6 +12,7 @@
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils.orm import reload_object
 from testtools.matchers import (
+    AfterPreprocessing,
     Equals,
     HasLength,
     Is,
@@ -150,6 +151,55 @@
             HasLength(1))
 
 
+class TestFindingAndDismissingNotifications(MAASServerTestCase):
+    """Tests for finding and dismissing notifications."""
+
+    def notify(self, user):
+        message = factory.make_name("message")
+        return (
+            Notification.objects.create_for_user(message, user),
+            Notification.objects.create_for_users(message),
+            Notification.objects.create_for_admins(message),
+        )
+
+    def assertNotifications(self, user, notifications):
+        self.assertThat(
+            Notification.objects.find_for_user(user),
+            AfterPreprocessing(list, Equals(notifications)))
+
+    def test_find_and_dismiss_notifications_for_user(self):
+        user = factory.make_User()
+        n_for_user, n_for_users, n_for_admins = self.notify(user)
+        self.assertNotifications(user, [n_for_user, n_for_users])
+        n_for_user.dismiss(user)
+        self.assertNotifications(user, [n_for_users])
+        n_for_users.dismiss(user)
+        self.assertNotifications(user, [])
+
+    def test_find_and_dismiss_notifications_for_users(self):
+        user = factory.make_User("user")
+        user2 = factory.make_User("user2")
+        n_for_user, n_for_users, n_for_admins = self.notify(user)
+        self.assertNotifications(user, [n_for_user, n_for_users])
+        self.assertNotifications(user2, [n_for_users])
+        n_for_users.dismiss(user2)
+        self.assertNotifications(user, [n_for_user, n_for_users])
+        self.assertNotifications(user2, [])
+
+    def test_find_and_dismiss_notifications_for_admins(self):
+        user = factory.make_User("user")
+        admin = factory.make_admin("admin")
+        n_for_user, n_for_users, n_for_admins = self.notify(user)
+        self.assertNotifications(user, [n_for_user, n_for_users])
+        self.assertNotifications(admin, [n_for_users, n_for_admins])
+        n_for_users.dismiss(admin)
+        self.assertNotifications(user, [n_for_user, n_for_users])
+        self.assertNotifications(admin, [n_for_admins])
+        n_for_admins.dismiss(admin)
+        self.assertNotifications(user, [n_for_user, n_for_users])
+        self.assertNotifications(admin, [])
+
+
 class TestNotification(MAASServerTestCase):
     """Tests for the `Notification`."""
 
@@ -171,3 +221,35 @@
         self.assertThat(str(error), Equals(repr("thing")))
         self.assertThat(notification.id, Is(None))
         self.assertThat(Notification.objects.all(), HasLength(0))
+
+
+class TestNotificationRepresentation(MAASServerTestCase):
+    """Tests for the `Notification` representation."""
+
+    def test_for_user(self):
+        notification = Notification(
+            user=factory.make_User("foobar"),
+            message="The cat in the {place}",
+            context=dict(place="bear trap"))
+        self.assertThat(
+            notification, AfterPreprocessing(repr, Equals(
+                "<Notification user='foobar' users=False admins=False "
+                "'The cat in the bear trap'>")))
+
+    def test_for_users(self):
+        notification = Notification(
+            users=True, message="The cat in the {place}",
+            context=dict(place="blender"))
+        self.assertThat(
+            notification, AfterPreprocessing(repr, Equals(
+                "<Notification user=None users=True admins=False "
+                "'The cat in the blender'>")))
+
+    def test_for_admins(self):
+        notification = Notification(
+            admins=True, message="The cat in the {place}",
+            context=dict(place="lava pit"))
+        self.assertThat(
+            notification, AfterPreprocessing(repr, Equals(
+                "<Notification user=None users=False admins=True "
+                "'The cat in the lava pit'>")))