[chores] Refactoring in test cases and model checks

Modified checks in model for better readability.
Choices labels are now being shown in browseable
API form using `COMMAND_CHOICES`.
`test_available_commands` is modified to check
using browseable api form instead of serializer.
Also, added additional test case for Connection
model.

Signed-off-by: DragnEmperor <dragnemperor@gmail.com>

Author: DragnEmperor

openwisp_controller/connection/api/serializers.py

@@ -5,6 +5,7 @@
 from openwisp_utils.api.serializers import ValidatedModelSerializer
 
 from ...serializers import BaseSerializer
+from ..commands import COMMAND_CHOICES
 
 Command = load_model('connection', 'Command')
 DeviceConnection = load_model('connection', 'DeviceConnection')
@@ -34,17 +35,18 @@ class CommandSerializer(ValidatedDeviceFieldSerializer):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         # show only connections and command types available for the device
-        if (
-            device := Device.objects.filter(pk=self.context.get('device_id'))
-            .only('organization_id', 'id')
-            .first()
-        ):
-            self.fields['connection'].queryset = DeviceConnection.objects.filter(
-                device=device
-            )
-            self.fields['type'].choices = Command.get_org_choices(
-                device.organization_id
-            )
+        if device_id := self.context.get('device_id'):
+            self.fields['connection'].queryset = self.fields[
+                'connection'
+            ].queryset.filter(device_id=device_id)
+            device = Device.objects.only('organization_id', 'id').get(pk=device_id)
+            # filter command types based on the device's organization
+            allowed_commands = Command.get_org_choices(device.organization_id)
+            # this translates the commands to their labels
+            commands_map = dict(COMMAND_CHOICES)
+            self.fields['type'].choices = [
+                (i, commands_map[i]) for i in commands_map if i in allowed_commands
+            ]
 
     def to_representation(self, instance):
         repr = super().to_representation(instance)

openwisp_controller/connection/base/models.py

@@ -467,12 +467,7 @@ def clean(self):
 
     def _verify_connection(self):
         """Raises validation error if device has no connection and credentials."""
-        DeviceConnection = load_model('connection', 'DeviceConnection')
-
-        if not DeviceConnection.objects.filter(
-            device=self.device,
-            credentials__isnull=False,
-        ).exists():
+        if self.device and not self.device.deviceconnection_set.exists():
             raise ValidationError({'device': _('Device has no credentials assigned.')})
 
     def _verify_command_type_allowed(self):

openwisp_controller/connection/tests/test_api.py

@@ -10,7 +10,6 @@
 from rest_framework.exceptions import ErrorDetail
 from swapper import load_model
 
-from openwisp_controller.connection.api.serializers import CommandSerializer
 from openwisp_controller.tests.utils import TestAdminMixin
 from openwisp_users.tests.test_api import AuthenticationMixin
 
@@ -178,12 +177,6 @@ def test_command_attributes(self, payload):
 
     # for ensuring that only related connections are shown
     def test_available_connections(self):
-        url = self._get_path('device_command_list', self.device_id)
-        response = self.client.get(url)
-        serializer = CommandSerializer(
-            instance=self.device_conn.device,
-            context={'device_id': self.device_id, 'request': response.wsgi_request},
-        )
         device = self._create_device(
             name='default.test.device2', mac_address='12:23:34:45:56:67'
         )
@@ -192,11 +185,11 @@ def test_available_connections(self):
         device_conn2 = self._create_device_connection(
             device=device, credentials=credentials_2
         )
-        connections = serializer.fields['connection']
-        queryset = list(connections.get_queryset().values_list('id', flat=True))
-        queryset = [str(i) for i in queryset]
-        self.assertIn(str(self.device_conn.id), queryset)
-        self.assertNotIn(device_conn2.id, queryset)
+        url = self._get_path('device_command_list', self.device_id)
+        response = self.client.get(url, {'format': 'api'})
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, str(self.device_conn.id))
+        self.assertNotContains(response, device_conn2.id)
 
     def test_command_details_api(self):
         command_obj = self._create_command(device_conn=self.device_conn)

openwisp_controller/connection/tests/test_models.py

@@ -563,6 +563,22 @@ def test_command_validation(self):
                     ],
                 )
 
+        with self.subTest('Test command creation without device connection'):
+            device = self._create_device(
+                name='default.test.device2', mac_address='11:22:33:44:55:66'
+            )
+            new_command = Command(
+                device=device, type='custom', input={'command': 'echo test'}
+            )
+            with self.assertRaises(ValidationError) as context_manager:
+                new_command.full_clean()
+            exception = context_manager.exception
+            self.assertIn('device', exception.message_dict)
+            self.assertEqual(
+                exception.message_dict['device'],
+                ['Device has no credentials assigned.'],
+            )
+
     @tag('skip_prod')
     def test_enabled_command(self):
         self.assertEqual(