Merge branch 'master' into issues/471-snmp-connections

README.rst

@@ -2283,17 +2283,17 @@ Configure timeout for the TCP connect when establishing a SSH connection.
 ``OPENWISP_CONNECTORS``
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-+--------------+-------------------------------------------------------------------------------------------+
-| **type**:    | ``tuple``                                                                                 |
-+--------------+-------------------------------------------------------------------------------------------+
-| **default**: | .. code-block:: python                                                                    |
-|              |                                                                                           |
-|              |   (                                                                                       |
-|              |     ('openwisp_controller.connection.connectors.ssh.Ssh', 'SSH'),                         |
-|              |     ('openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp', 'OpenWRT SNMP'),              |
++--------------+------------------------------------------------------------------------------------------------+
+| **type**:    | ``tuple``                                                                                      |
++--------------+------------------------------------------------------------------------------------------------+
+| **default**: | .. code-block:: python                                                                         |
+|              |                                                                                                |
+|              |   (                                                                                            |
+|              |     ('openwisp_controller.connection.connectors.ssh.Ssh', 'SSH'),                              |
+|              |     ('openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp', 'OpenWRT SNMP'),    |
 |              |     ('openwisp_controller.connection.connectors.airos.snmp.AirOsSnmp', 'Ubiquiti AirOS SNMP'), |
-|              |   )                                                                                       |
-+--------------+-------------------------------------------------------------------------------------------+
+|              |   )                                                                                            |
++--------------+------------------------------------------------------------------------------------------------+
 
 Available connector classes. Connectors are python classes that specify ways
 in which OpenWISP can connect to devices in order to launch commands.
@@ -2823,6 +2823,23 @@ The value of this setting decides whether to use DSA syntax
 (OpenWrt >=21 configuration syntax) if openwisp-controller fails
 to make that decision automatically.
 
+``OPENWISP_CONTROLLER_GROUP_PIE_CHART``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
++--------------+-----------+
+| **type**:    |  ``bool`` |
++--------------+-----------+
+| **default**: | ``False`` |
++--------------+-----------+
+
+Allows to show a pie chart like the one in the screenshot.
+
+.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/devicegroups-piechart.png
+   :alt: device groups piechart
+
+Active groups are groups which have at least one device in them,
+while emtpy groups do not have any device assigned.
+
 Signals
 -------
 

openwisp_controller/config/admin.py

@@ -832,6 +832,22 @@ class Meta(BaseForm.Meta):
         }
 
 
+class DeviceGroupFilter(admin.SimpleListFilter):
+    title = _('has devices?')
+    parameter_name = 'empty'
+
+    def lookups(self, request, model_admin):
+        return (
+            ('true', _('No')),
+            ('false', _('Yes')),
+        )
+
+    def queryset(self, request, queryset):
+        if self.value():
+            return queryset.filter(device__isnull=self.value() == 'true').distinct()
+        return queryset
+
+
 class DeviceGroupAdmin(MultitenantAdminMixin, BaseAdmin):
     form = DeviceGroupForm
     list_display = [
@@ -849,9 +865,7 @@ class DeviceGroupAdmin(MultitenantAdminMixin, BaseAdmin):
         'modified',
     ]
     search_fields = ['name', 'description', 'meta_data']
-    list_filter = [
-        ('organization', MultitenantOrgFilter),
-    ]
+    list_filter = [('organization', MultitenantOrgFilter), DeviceGroupFilter]
 
     class Media:
         css = {'all': (f'{prefix}css/admin.css',)}

openwisp_controller/config/apps.py

@@ -1,6 +1,7 @@
 from django.apps import AppConfig
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
+from django.db.models import Case, Count, When
 from django.db.models.signals import m2m_changed, post_delete, post_save, pre_delete
 from django.utils.translation import gettext_lazy as _
 from openwisp_notifications.types import (
@@ -320,6 +321,52 @@ def register_dashboard_charts(self):
                 'labels': {'': _('undefined')},
             },
         )
+        if app_settings.GROUP_PIE_CHART:
+            register_dashboard_chart(
+                position=13,
+                config={
+                    'name': _('Groups'),
+                    'query_params': {
+                        'app_label': 'config',
+                        'model': 'devicegroup',
+                        'annotate': {
+                            'active_count': Count(
+                                Case(
+                                    When(
+                                        device__isnull=False,
+                                        then=1,
+                                    )
+                                )
+                            ),
+                            'empty_count': Count(
+                                Case(
+                                    When(
+                                        device__isnull=True,
+                                        then=1,
+                                    )
+                                )
+                            ),
+                        },
+                        'aggregate': {
+                            'active': Count(Case(When(active_count__gt=0, then=1))),
+                            'empty': Count(Case(When(empty_count__gt=0, then=1))),
+                        },
+                    },
+                    'colors': {
+                        'active': '#2277b4',
+                        'empty': '#EF7D2D',
+                    },
+                    'labels': {
+                        'active': _('Active groups'),
+                        'empty': _('Empty groups'),
+                    },
+                    'filters': {
+                        'key': 'empty',
+                        'active': 'false',
+                        'empty': 'true',
+                    },
+                },
+            )
 
     def notification_cache_update(self):
         from openwisp_notifications.handlers import register_notification_cache_update

openwisp_controller/config/base/device.py

@@ -192,8 +192,13 @@ def save(self, *args, **kwargs):
                 self.key = KeyField.default_callable()
             else:
                 self.key = self.generate_key(shared_secret)
+        state_adding = self._state.adding
         super().save(*args, **kwargs)
-        self._check_changed_fields()
+        # The value of "self._state.adding" will always be "False"
+        # after performing the save operation. Hence, the actual value
+        # is stored in the "state_adding" variable.
+        if not state_adding:
+            self._check_changed_fields()
 
     def _check_changed_fields(self):
         self._get_initial_values_for_checked_fields()

openwisp_controller/config/settings.py

@@ -78,3 +78,4 @@ def get_settings_value(option, default):
 )
 DSA_OS_MAPPING = get_settings_value('DSA_OS_MAPPING', {})
 DSA_DEFAULT_FALLBACK = get_settings_value('DSA_DEFAULT_FALLBACK', True)
+GROUP_PIE_CHART = get_settings_value('GROUP_PIE_CHART', False)

openwisp_controller/config/tests/test_admin.py

@@ -10,11 +10,18 @@
 from swapper import load_model
 
 from openwisp_users.tests.utils import TestOrganizationMixin
+from openwisp_utils.tests import catch_signal
 
 from ...geo.tests.utils import TestGeoMixin
 from ...tests.utils import TestAdminMixin
 from .. import settings as app_settings
-from .utils import CreateConfigTemplateMixin, CreateDeviceGroupMixin, TestVpnX509Mixin
+from ..signals import device_group_changed, device_name_changed, management_ip_changed
+from .utils import (
+    CreateConfigTemplateMixin,
+    CreateDeviceGroupMixin,
+    CreateDeviceMixin,
+    TestVpnX509Mixin,
+)
 
 devnull = open(os.devnull, 'w')
 Config = load_model('config', 'Config')
@@ -31,6 +38,7 @@
 
 class TestAdmin(
     TestGeoMixin,
+    CreateDeviceGroupMixin,
     CreateConfigTemplateMixin,
     TestVpnX509Mixin,
     TestAdminMixin,
@@ -135,6 +143,25 @@ def test_add_device(self):
             device.config.templates.filter(name__in=['t1', 't2']).count(), 2
         )
 
+    def test_add_device_does_not_emit_changed_signals(self):
+        org1 = self._get_org()
+        path = reverse(f'admin:{self.app_label}_device_add')
+        data = self._get_device_params(org=org1)
+        data.update({'group': str(self._create_device_group().pk)})
+        self._login()
+        with catch_signal(
+            device_group_changed
+        ) as mocked_device_group_changed, catch_signal(
+            device_name_changed
+        ) as mocked_device_name_changed, catch_signal(
+            management_ip_changed
+        ) as mocked_management_ip_changed:
+            self.client.post(path, data)
+
+        mocked_device_group_changed.assert_not_called()
+        mocked_device_name_changed.assert_not_called()
+        mocked_management_ip_changed.assert_not_called()
+
     def test_preview_device(self):
         org = self._get_org()
         self._create_template(organization=org)
@@ -1270,7 +1297,11 @@ def tearDownClass(cls):
 
 
 class TestDeviceGroupAdmin(
-    CreateDeviceGroupMixin, TestOrganizationMixin, TestAdminMixin, TestCase
+    CreateDeviceGroupMixin,
+    CreateDeviceMixin,
+    TestOrganizationMixin,
+    TestAdminMixin,
+    TestCase,
 ):
     app_label = 'config'
 
@@ -1311,6 +1342,19 @@ def test_organization_filter(self):
         self.assertContains(response, 'Org1 APs')
         self.assertNotContains(response, 'Org2 APs')
 
+    def test_has_devices_filter(self):
+        org1 = self._create_org(name='org1')
+        dg1 = self._create_device_group(name='Device Group 1', organization=org1)
+        self._create_device_group(name='Device Group 2', organization=org1)
+        self._create_device(name='d1', group=dg1, organization=org1)
+        url = reverse(f'admin:{self.app_label}_devicegroup_changelist') + '?empty='
+        response = self.client.get(url + 'true')
+        self.assertNotContains(response, 'Device Group 1')
+        self.assertContains(response, 'Device Group 2')
+        response = self.client.get(url + 'false')
+        self.assertContains(response, 'Device Group 1')
+        self.assertNotContains(response, 'Device Group 2')
+
     def test_admin_menu_groups(self):
         # Test menu group (openwisp-utils menu group) for Device Group, Template
         # and Vpn models

openwisp_controller/config/tests/test_apps.py

@@ -51,3 +51,23 @@ def test_system_type_chart_registered(self):
         query_params = chart_config['query_params']
         self.assertIn('group_by', query_params)
         self.assertEqual(query_params['group_by'], 'system')
+
+    def test_device_group_chart_registered(self):
+        chart_config = DASHBOARD_CHARTS.get(13, None)
+        self.assertIsNotNone(chart_config)
+        self.assertEqual(chart_config['name'], 'Groups')
+        self.assertIn('labels', chart_config)
+        self.assertDictEqual(
+            chart_config['labels'],
+            {'active': 'Active groups', 'empty': 'Empty groups'},
+        )
+        self.assertIn('filters', chart_config)
+        query_params = chart_config['query_params']
+        self.assertNotIn('group_by', query_params)
+        self.assertIn('annotate', query_params)
+        self.assertIn('aggregate', query_params)
+        self.assertIn('filters', chart_config)
+        filters = chart_config['filters']
+        self.assertIn('key', filters)
+        self.assertIn('active', chart_config['filters'])
+        self.assertIn('empty', chart_config['filters'])

openwisp_controller/config/tests/test_device.py

@@ -389,8 +389,10 @@ def test_device_group_changed_emitted(self):
             )
 
     def test_device_group_changed_not_emitted_on_creation(self):
+        org = self._get_org()
+        device_group = self._create_device_group(organization=org)
         with catch_signal(device_group_changed) as handler:
-            self._create_device(organization=self._get_org())
+            self._create_device(name='test', organization=org, group=device_group)
         handler.assert_not_called()
 
     def test_device_field_changed_checks(self):

openwisp_controller/connection/settings.py

@@ -2,7 +2,10 @@
 
 DEFAULT_CONNECTORS = (
     ('openwisp_controller.connection.connectors.ssh.Ssh', 'SSH'),
-    ('openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp', 'OpenWRT SNMP'),
+    (
+        'openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp',
+        'OpenWRT SNMP',
+    ),
     (
         'openwisp_controller.connection.connectors.airos.snmp.AirOsSnmp',
         'Ubiquiti AirOS SNMP',

openwisp_controller/connection/tests/test_snmp.py

@@ -22,7 +22,8 @@ def test_create_snmp_connector(self):
             self.assertEqual(params, obj.params)
             self.assertEqual(Credentials.objects.count(), init_credentials_count + 1)
             self.assertEqual(
-                obj.connector, 'openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp'
+                obj.connector,
+                'openwisp_controller.connection.connectors.openwrt.snmp.OpenWRTSnmp',
             )
 
         with self.subTest('test airos'):

tests/openwisp2/settings.py

@@ -151,7 +151,7 @@
 ACCOUNT_LOGOUT_REDIRECT_URL = LOGIN_REDIRECT_URL
 OPENWISP_ORGANIZATION_USER_ADMIN = True  # tests will fail without this setting
 OPENWISP_ADMIN_DASHBOARD_ENABLED = True
-
+OPENWISP_CONTROLLER_GROUP_PIE_CHART = True
 # during development only
 EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend'