[feature] Added support for device deactivation #560

- Do not collect device metrics if device is deactivated
- Disable monitoring checks for deactivated devices
- Set DeviceMonitoring status to unknown on device activation
- Documented "deactivated" status

Closes #560

---------

Co-authored-by: Federico Capoano <f.capoano@openwisp.io>

.github/workflows/ci.yml

@@ -65,7 +65,7 @@ jobs:
           pip install -r requirements-test.txt
           pip install -U -I -e .
           pip uninstall -y django
-          pip install ${{ matrix.django-version }}
+          pip install -U ${{ matrix.django-version }}
           sudo npm install -g jshint stylelint
 
           # start influxdb

docs/user/device-health-status.rst

@@ -36,3 +36,8 @@ been crossed).
 
 Example: ping is by default a critical metric which is expected to be
 always 1 (reachable).
+
+``DEACTIVATED``
+---------------
+
+The device is deactivated. All active and passive checks are disabled.

docs/user/settings.rst

@@ -337,7 +337,7 @@ By default, if devices are not reachable by pings they are flagged as
 ============ ==========================================================
 **type**:    ``dict``
 **default**: ``{'unknown': 'unknown', 'ok': 'ok', 'problem': 'problem',
-             'critical': 'critical'}``
+             'critical': 'critical', 'deactivated': 'deactivated'}``
 ============ ==========================================================
 
 This setting allows to change the health status labels, for example, if we

openwisp_monitoring/check/base/models.py

@@ -93,6 +93,9 @@ def check_instance(self):
     def perform_check(self, store=True):
         """Initializes check instance and calls the check method."""
         if (
+            hasattr(self.content_object, 'is_deactivated')
+            and self.content_object.is_deactivated()
+        ) or (
             hasattr(self.content_object, 'organization_id')
             and self.content_object.organization.is_active is False
         ):

openwisp_monitoring/check/tests/test_models.py

@@ -142,7 +142,7 @@ def test_device_deleted(self):
         self.assertEqual(Check.objects.count(), 0)
         d = self._create_device(organization=self._create_org())
         self.assertEqual(Check.objects.count(), 3)
-        d.delete()
+        d.delete(check_deactivated=False)
         self.assertEqual(Check.objects.count(), 0)
 
     def test_config_modified_device_problem(self):
@@ -290,6 +290,16 @@ def test_device_organization_disabled_check_not_performed(self):
             check.perform_check()
         mocked_check.assert_not_called()
 
+    def test_deactivated_device_check_not_performed(self):
+        config = self._create_config(status='modified', organization=self._create_org())
+        self.assertEqual(Check.objects.filter(is_active=True).count(), 3)
+        config.device.deactivate()
+        config.set_status_deactivated()
+        check = Check.objects.filter(check_type=self._CONFIG_APPLIED).first()
+        with patch(f'{self._CONFIG_APPLIED}.check') as mocked_check:
+            check.perform_check()
+        mocked_check.assert_not_called()
+
     def test_config_check_problem_with_interval(self):
         self._create_admin()
         d = self._create_device(organization=self._create_org())

openwisp_monitoring/device/admin.py

@@ -21,6 +21,7 @@
 )
 from swapper import load_model
 
+from openwisp_controller.config.admin import DeactivatedDeviceReadOnlyMixin
 from openwisp_controller.config.admin import DeviceAdmin as BaseDeviceAdmin
 from openwisp_users.multitenancy import MultitenantAdminMixin
 from openwisp_utils.admin import ReadOnlyAdmin
@@ -58,30 +59,68 @@ def full_clean(self):
 
 
 class InlinePermissionMixin:
+    """
+    Manages permissions for the inline objects.
+
+    This mixin class handles the permissions for the inline objects.
+    It checks if the user has permission to modify the main object
+    (e.g., view_model, add_model, change_model, or delete_model),
+    and if not, it checks if the user has permission to modify the
+    inline object (e.g., view_model_inline, add_model_inline,
+    change_model_inline, or delete_model_inline).
+    """
+
     def has_add_permission(self, request, obj=None):
-        # User will be able to add objects from inline even
-        # if it only has permission to add a model object
-        return super().has_add_permission(request, obj) or request.user.has_perm(
+        return super(admin.options.InlineModelAdmin, self).has_add_permission(
+            request
+        ) or request.user.has_perm(
             f'{self.model._meta.app_label}.add_{self.inline_permission_suffix}'
         )
 
     def has_change_permission(self, request, obj=None):
-        return super().has_change_permission(request, obj) or request.user.has_perm(
+        return super(admin.options.InlineModelAdmin, self).has_change_permission(
+            request, obj
+        ) or request.user.has_perm(
             f'{self.model._meta.app_label}.change_{self.inline_permission_suffix}'
         )
 
     def has_view_permission(self, request, obj=None):
-        return super().has_view_permission(request, obj) or request.user.has_perm(
+        return super(admin.options.InlineModelAdmin, self).has_view_permission(
+            request, obj
+        ) or request.user.has_perm(
             f'{self.model._meta.app_label}.view_{self.inline_permission_suffix}'
         )
 
     def has_delete_permission(self, request, obj=None):
-        return super().has_delete_permission(request, obj) or request.user.has_perm(
+        return super(admin.options.InlineModelAdmin, self).has_delete_permission(
+            request, obj
+        ) or request.user.has_perm(
             f'{self.model._meta.app_label}.delete_{self.inline_permission_suffix}'
         )
 
 
-class CheckInline(InlinePermissionMixin, GenericStackedInline):
+class DeactivatedDeviceReadOnlyInlinePermissionMixin(
+    DeactivatedDeviceReadOnlyMixin,
+    InlinePermissionMixin,
+):
+    def has_add_permission(self, request, obj=None):
+        perm = super().has_add_permission(request, obj)
+        return self._has_permission(request, obj, perm)
+
+    def has_change_permission(self, request, obj=None):
+        perm = super().has_change_permission(request, obj)
+        return self._has_permission(request, obj, perm)
+
+    def has_view_permission(self, request, obj=None):
+        perm = super().has_view_permission(request, obj)
+        return self._has_permission(request, obj, perm)
+
+    def has_delete_permission(self, request, obj=None):
+        perm = super().has_delete_permission(request, obj)
+        return self._has_permission(request, obj, perm)
+
+
+class CheckInline(DeactivatedDeviceReadOnlyInlinePermissionMixin, GenericStackedInline):
     model = Check
     extra = 0
     formset = CheckInlineFormSet
@@ -152,7 +191,9 @@ def get_queryset(self, request):
         return super().get_queryset(request).order_by('created')
 
 
-class MetricInline(InlinePermissionMixin, NestedGenericStackedInline):
+class MetricInline(
+    DeactivatedDeviceReadOnlyInlinePermissionMixin, NestedGenericStackedInline
+):
     model = Metric
     extra = 0
     inlines = [AlertSettingsInline]
@@ -205,7 +246,7 @@ def has_delete_permission(self, request, obj=None):
 
 
 class DeviceAdmin(BaseDeviceAdmin, NestedModelAdmin):
-    change_form_template = 'admin/config/device/change_form.html'
+    change_form_template = 'admin/monitoring/device/change_form.html'
     list_filter = ['monitoring__status'] + BaseDeviceAdmin.list_filter
     list_select_related = ['monitoring'] + list(BaseDeviceAdmin.list_select_related)
     list_display = list(BaseDeviceAdmin.list_display)
@@ -302,7 +343,7 @@ def get_fields(self, request, obj=None):
         fields = list(super().get_fields(request, obj))
         if obj and not obj._state.adding:
             fields.insert(fields.index('last_ip'), 'health_status')
-        if not obj or obj.monitoring.status in ['ok', 'unknown']:
+        if not obj or obj.monitoring.status in ['ok', 'unknown', 'deactivated']:
             return fields
         fields.insert(fields.index('health_status') + 1, 'health_checks')
         return fields
@@ -415,7 +456,7 @@ class WiFiSessionInline(WifiSessionAdminHelperMixin, admin.TabularInline):
     readonly_fields = fields
     can_delete = False
     extra = 0
-    template = 'admin/config/device/wifisession_tabular.html'
+    template = 'admin/monitoring/device/wifisession_tabular.html'
 
     class Media:
         css = {'all': ('monitoring/css/wifi-sessions.css',)}

openwisp_monitoring/device/api/views.py

@@ -8,6 +8,7 @@
 from django.core.exceptions import ValidationError
 from django.db.models import Count, Q
 from django.db.models.functions import Round
+from django.http import Http404
 from django.utils.timezone import now
 from django.utils.translation import gettext_lazy as _
 from django_filters.rest_framework import DjangoFilterBackend
@@ -113,6 +114,7 @@ class DeviceMetricView(
     queryset = (
         DeviceData.objects.filter(organization__is_active=True)
         .only(
+            '_is_deactivated',
             'id',
             'key',
         )
@@ -173,6 +175,11 @@ def get_object(self, pk):
 
     def post(self, request, pk):
         self.instance = self.get_object(pk)
+        if self.instance._is_deactivated:
+            # If the device is deactivated, do not accept data.
+            # We don't use "Device.is_deactivated()" to avoid
+            # generating query for the related config.
+            raise Http404
         self.instance.data = request.data
         # validate incoming data
         try:

openwisp_monitoring/device/apps.py

@@ -9,7 +9,12 @@
 from django.utils.translation import gettext_lazy as _
 from swapper import get_model_name, load_model
 
-from openwisp_controller.config.signals import checksum_requested, config_status_changed
+from openwisp_controller.config.signals import (
+    checksum_requested,
+    config_status_changed,
+    device_activated,
+    device_deactivated,
+)
 from openwisp_controller.connection import settings as connection_settings
 from openwisp_controller.connection.signals import is_working_changed
 from openwisp_utils.admin_theme import (
@@ -74,6 +79,7 @@ def connect_device_signals(self):
 
         Device = load_model('config', 'Device')
         DeviceData = load_model('device_monitoring', 'DeviceData')
+        DeviceMonitoring = load_model('device_monitoring', 'DeviceMonitoring')
         DeviceLocation = load_model('geo', 'DeviceLocation')
         Metric = load_model('monitoring', 'Metric')
         Chart = load_model('monitoring', 'Chart')
@@ -145,6 +151,26 @@ def connect_device_signals(self):
             sender=Organization,
             dispatch_uid='post_save_organization_disabled_monitoring',
         )
+        device_deactivated.connect(
+            DeviceMonitoring.handle_deactivated_device,
+            sender=Device,
+            dispatch_uid='device_deactivated_update_devicemonitoring',
+        )
+        device_deactivated.connect(
+            DeviceMetricView.invalidate_get_device_cache,
+            sender=Device,
+            dispatch_uid=('device_deactivated_invalidate_view_device_cache'),
+        )
+        device_activated.connect(
+            DeviceMonitoring.handle_activated_device,
+            sender=Device,
+            dispatch_uid='device_activated_update_devicemonitoring',
+        )
+        device_activated.connect(
+            DeviceMetricView.invalidate_get_device_cache,
+            sender=Device,
+            dispatch_uid=('device_activated_invalidate_view_device_cache'),
+        )
 
     @classmethod
     def device_post_save_receiver(cls, instance, created, **kwargs):
@@ -330,12 +356,14 @@ def register_dashboard_items(self):
                     'problem': '#ffb442',
                     'critical': '#a72d1d',
                     'unknown': '#353c44',
+                    'deactivated': '#000',
                 },
                 'labels': {
                     'ok': app_settings.HEALTH_STATUS_LABELS['ok'],
                     'problem': app_settings.HEALTH_STATUS_LABELS['problem'],
                     'critical': app_settings.HEALTH_STATUS_LABELS['critical'],
                     'unknown': app_settings.HEALTH_STATUS_LABELS['unknown'],
+                    'deactivated': app_settings.HEALTH_STATUS_LABELS['deactivated'],
                 },
             },
         )

openwisp_monitoring/device/base/models.py

@@ -338,6 +338,7 @@ class AbstractDeviceMonitoring(TimeStampedEditableModel):
         ('ok', _(app_settings.HEALTH_STATUS_LABELS['ok'])),
         ('problem', _(app_settings.HEALTH_STATUS_LABELS['problem'])),
         ('critical', _(app_settings.HEALTH_STATUS_LABELS['critical'])),
+        ('deactivated', _(app_settings.HEALTH_STATUS_LABELS['deactivated'])),
     )
     status = StatusField(
         _('health status'),
@@ -346,12 +347,14 @@ class AbstractDeviceMonitoring(TimeStampedEditableModel):
             '"{0}" means the device has been recently added; \n'
             '"{1}" means the device is operating normally; \n'
             '"{2}" means the device is having issues but it\'s still reachable; \n'
-            '"{3}" means the device is not reachable or in critical conditions;'
+            '"{3}" means the device is not reachable or in critical conditions;\n'
+            '"{4}" means the device is deactivated;'
         ).format(
             app_settings.HEALTH_STATUS_LABELS['unknown'],
             app_settings.HEALTH_STATUS_LABELS['ok'],
             app_settings.HEALTH_STATUS_LABELS['problem'],
             app_settings.HEALTH_STATUS_LABELS['critical'],
+            app_settings.HEALTH_STATUS_LABELS['deactivated'],
         ),
     )
 
@@ -444,6 +447,32 @@ def handle_disabled_organization(cls, organization_id):
             status='unknown'
         )
 
+    @classmethod
+    def handle_deactivated_device(cls, instance, **kwargs):
+        """Handles the deactivation of a device
+
+        Sets the device's monitoring status to 'deactivated'
+
+        Parameters: - instance (Device): The device object
+        which is deactivated
+
+        Returns: - None
+        """
+        cls.objects.filter(device_id=instance.id).update(status='deactivated')
+
+    @classmethod
+    def handle_activated_device(cls, instance, **kwargs):
+        """Handles the activation of a deactivated device
+
+        Sets the device's monitoring status to 'unknown'
+
+        Parameters: - instance (Device): The device object
+        which is deactivated
+
+        Returns: - None
+        """
+        cls.objects.filter(device_id=instance.id).update(status='unknown')
+
     @classmethod
     def _get_critical_metric_keys(cls):
         return [metric['key'] for metric in get_critical_device_metrics()]

openwisp_monitoring/device/migrations/0001_squashed_0002_devicemonitoring.py

@@ -88,6 +88,10 @@ class Migration(migrations.Migration):
                                 'critical',
                                 _(app_settings.HEALTH_STATUS_LABELS['critical']),
                             ),
+                            (
+                                'deactivated',
+                                _(app_settings.HEALTH_STATUS_LABELS['deactivated']),
+                            ),
                         ],
                         default='unknown',
                         db_index=True,

openwisp_monitoring/device/settings.py

@@ -27,13 +27,15 @@ def get_health_status_labels():
             'ok': 'ok',
             'problem': 'problem',
             'critical': 'critical',
+            'deactivated': 'deactivated',
         },
     )
     try:
         assert 'unknown' in labels
         assert 'ok' in labels
         assert 'problem' in labels
         assert 'critical' in labels
+        assert 'deactivated' in labels
     except AssertionError as e:  # pragma: no cover
         raise ImproperlyConfigured(
             'OPENWISP_MONITORING_HEALTH_STATUS_LABELS must contain the following '

openwisp_monitoring/device/static/monitoring/css/device-map.css

@@ -1,3 +1,6 @@
+.health-deactivated {
+  color: #000;
+}
 .health-unknown {
   color: grey;
 }

openwisp_monitoring/device/static/monitoring/css/monitoring.css

@@ -38,6 +38,9 @@ td.field-health_status,
   font-weight: bold;
   text-transform: uppercase;
 }
+.health-deactivated {
+  color: #000;
+}
 .health-unknown {
   color: grey;
 }

openwisp_monitoring/device/static/monitoring/js/device-map.js

@@ -5,12 +5,13 @@
     const loadingOverlay = $('#device-map-container .ow-loading-spinner');
     const localStorageKey = 'ow-map-shown';
     const mapContainer = $('#device-map-container');
-    const statuses = ['critical', 'problem', 'ok', 'unknown'];
+    const statuses = ['critical', 'problem', 'ok', 'unknown', 'deactivated'];
     const colors = {
         ok: '#267126',
         problem: '#ffb442',
         critical: '#a72d1d',
         unknown: '#353c44',
+        deactivated: '#0000',
     };
     const getLocationDeviceUrl = function (pk) {
         return window._owGeoMapConfig.locationDeviceUrl.replace('000', pk);

openwisp_monitoring/device/templates/admin/config/device/change_form.html → openwisp_monitoring/device/templates/admin/monitoring/device/change_form.html

@@ -1,4 +1,4 @@
-{% extends "admin/config/change_form.html" %}
+{% extends "admin/config/device/change_form.html" %}
 {% load i18n static %}
 {% block after_field_sets %}
 {% if not add and device_data %}