[chores] Improvements to notifications #191

- added is_new to device_registered signal
- use is_new in device_registered notification
  to specify if it's a new or existing device
- other minor improvements to code and docs

Related to #191

Author: nemesifier

README.rst

@@ -60,9 +60,9 @@ Other popular building blocks that are part of the OpenWISP ecosystem are:
   it can be used in conjunction with openwisp-monitoring to get a better idea
   of the state of the network
 - `openwisp-ipam <https://github.com/openwisp/openwisp-ipam>`_:
-  it allows to manage the IP address space of networks
+  allows to manage the assignment of IP addresses used in the network
 - `openwisp-notifications <https://github.com/openwisp/openwisp-notifications>`_:
-  it allows to create and manage notifications from the network.
+  allows users to be aware of important events happening in the network.
 
 .. image:: https://raw.githubusercontent.com/openwisp/openwisp2-docs/master/assets/design/openwisp-logo-black.svg
   :target: http://openwisp.org
@@ -886,8 +886,12 @@ It is not triggered when the device is created for the first time.
 **Arguments**:
 
 - ``instance``: instance of ``Device`` which got registered.
+- ``is_new``: boolean, will be ``True`` when the device is new,
+  ``False`` when the device already exists
+  (eg: a device which gets a factory reset will register again)
 
-This signal is emitted once the device gets registered automatically through the controller.
+This signal is emitted when a device registers automatically through the controller
+HTTP API.
 
 Setup (Integrate into other Apps)
 ---------------------------------

install-dev.sh

@@ -2,9 +2,9 @@
 set -e
 
 # TODO: remove when openwisp-notifications 0.2 is released
-pip install -U https://github.com/openwisp/openwisp-notifications/tarball/issues/38-disable-object-notification
+pip install -U https://github.com/openwisp/openwisp-notifications/tarball/dev
 
-# TODO: can be remove when openwisp-users 0.4.1 is released
+# TODO: can be removed when openwisp-users 0.4.1 is released
 # Commit needed for failures in tests:
 # https://github.com/openwisp/openwisp-users/commit/ef347927136ddb4676479cb54cdc7cd08049e2e5
 pip install -U --no-deps https://github.com/openwisp/openwisp-users/tarball/master

openwisp_controller/config/apps.py

@@ -106,7 +106,7 @@ def register_notification_types(self):
                     ' {notification.verb}'
                 ),
                 'message': _(
-                    'A new device [{notification.target}]'
+                    '{condition} device [{notification.target}]'
                     '({notification.target_link}) has {notification.verb}.'
                 ),
             },

openwisp_controller/config/controller/views.py

@@ -343,7 +343,7 @@ def post(self, request, *args, **kwargs):
         # add templates specified in tags
         self.add_tagged_templates(config, request)
         # emit device registered signal
-        device_registered.send(sender=config.__class__, instance=device)
+        device_registered.send(sender=device.__class__, instance=device, is_new=new)
         # prepare response
         s = (
             'registration-result: success\n'

openwisp_controller/config/handlers.py

@@ -1,4 +1,5 @@
 from django.dispatch import receiver
+from django.utils.translation import ugettext_lazy as _
 from openwisp_notifications.signals import notify
 from swapper import load_model
 
@@ -22,11 +23,14 @@ def config_status_error_notification(sender, instance, **kwargs):
 
 
 @receiver(
-    device_registered, sender=Config, dispatch_uid='device_registered_notification'
+    device_registered, sender=Device, dispatch_uid='device_registered_notification'
 )
-def device_registered_notification(sender, instance, **kwargs):
+def device_registered_notification(sender, instance, is_new, **kwargs):
     """
     Creates notification when a new device is registered automatically
     through controller.
     """
-    notify.send(sender=instance, type='device_registered', target=instance)
+    condition = _('A new') if is_new else _('The existing')
+    notify.send(
+        sender=instance, type='device_registered', target=instance, condition=condition
+    )

openwisp_controller/config/signals.py

@@ -5,4 +5,4 @@
 config_status_changed = Signal(providing_args=['instance'])
 # device and config args are maintained for backward compatibility
 config_modified = Signal(providing_args=['instance', 'device', 'config'])
-device_registered = Signal(providing_args=['instance'])
+device_registered = Signal(providing_args=['instance', 'is_new'])

openwisp_controller/config/tests/test_controller.py

@@ -1004,5 +1004,5 @@ def test_device_registered_signal(self):
         with catch_signal(device_registered) as handler:
             device = self.test_register()
             handler.assert_called_once_with(
-                sender=Config, signal=device_registered, instance=device,
+                sender=Device, signal=device_registered, instance=device, is_new=True
             )

openwisp_controller/config/tests/test_notifications.py

@@ -8,7 +8,7 @@
 
 from ..signals import device_registered
 
-Config = load_model('config', 'Config')
+Device = load_model('config', 'Device')
 Notification = load_model('openwisp_notifications', 'Notification')
 
 notification_qs = Notification.objects.all()
@@ -40,18 +40,28 @@ def test_device_registered(self):
         # we simulate that "device_registered" signal is emitted
         config = self._create_config()
         device = config.device
-        device_registered.send(sender=Config, instance=config.device)
 
-        self.assertEqual(notification_qs.count(), 1)
-        notification = notification_qs.first()
-        self.assertEqual(notification.actor, device)
-        self.assertEqual(notification.target, device)
-        self.assertEqual(notification.type, 'device_registered')
-        self.assertEqual(
-            notification.email_subject,
-            f'[example.com] SUCCESS: "{device}" registered successfully',
-        )
-        self.assertIn('registered successfully', notification.message)
+        with self.subTest('is_new=True'):
+            device_registered.send(sender=Device, instance=config.device, is_new=True)
+            self.assertEqual(notification_qs.count(), 1)
+            notification = notification_qs.first()
+            self.assertEqual(notification.actor, device)
+            self.assertEqual(notification.target, device)
+            self.assertEqual(notification.type, 'device_registered')
+            self.assertEqual(
+                notification.email_subject,
+                f'[example.com] SUCCESS: "{device}" registered successfully',
+            )
+            self.assertIn('registered successfully', notification.message)
+            self.assertIn('A new device', notification.message)
+
+        Notification.objects.all().delete()
+
+        with self.subTest('is_new=True'):
+            device_registered.send(sender=Device, instance=config.device, is_new=False)
+            self.assertEqual(notification_qs.count(), 1)
+            notification = notification_qs.first()
+            self.assertIn('The existing device', notification.message)
 
     def test_default_notification_type_already_unregistered(self):
         # Simulates if 'default notification type is already unregistered

requirements.txt

@@ -15,5 +15,4 @@ celery>=4.4.3,<4.5.0
 redis>=3.4.1,<4.0.0
 swapper~=1.1.0
 django-flat-json-widget~=0.1
-# TODO: uncomment when openwisp-notifications is released
-# 'openwisp_notifications~=0.1',
+openwisp-notifications~=0.1