[fix] Fixed user defined commands that do not require input

Bugs: - The "Send Command" operation was failing if the user defined command does not require an input. - The command.js was relying on presence of "reboot" default command. The JS would run into error in absence of the default "reboot" command and the loading overlay would not be hidden. This patch fixes the above mentioned bugs.
openwisp · Jun 13, 2024 · 7aceba7 · 7aceba7
1 parent 84dde10
commit 7aceba7
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 5 deletions.
diff --git a/openwisp_controller/connection/base/models.py b/openwisp_controller/connection/base/models.py
@@ -545,7 +545,8 @@ def _exec_command(self):
             command = self.get_type_display()
         # user registered command
         else:
-            command = self._callable(**self.input)
+            input = self.input or {}
+            command = self._callable(**input)
             output, exit_code = self.connection.connector_instance.exec_command(
                 command, raise_unexpected_exit=False
             )

diff --git a/openwisp_controller/connection/commands.py b/openwisp_controller/connection/commands.py
@@ -25,12 +25,20 @@
                     },
                     'message': _('Command cannot be empty.'),
                     'required': ['command'],
+                    'additionalProperties': False,
                 },
             },
         ),
         (
             'reboot',
-            {'label': _('Reboot'), 'schema': {'title': _('Reboot'), 'type': 'null'}},
+            {
+                'label': _('Reboot'),
+                'schema': {
+                    'title': _('Reboot'),
+                    'type': 'null',
+                    'additionalProperties': False,
+                },
+            },
         ),
         (
             'change_password',

diff --git a/openwisp_controller/connection/static/connection/js/commands.js b/openwisp_controller/connection/static/connection/js/commands.js
@@ -397,8 +397,9 @@ function initCommandOverlay($) {
     });
 
     function closeOverlay() {
-        $('#id_command_set-0-type').val('reboot');
-        $('#id_command_set-0-type').trigger('change');
+        // The user may close the form without submitting it.
+        // The following line ensures that all input fields are cleared.
+        $('#command_set-0 .jsoneditor-wrapper input').val('');
         $('#command_set-group').css('display', 'none');
         $('.ow-command-overlay-errornote').addClass('ow-hide');
         commandConfirmationDialog.hide();

diff --git a/openwisp_controller/connection/tests/test_command_utilities.py b/openwisp_controller/connection/tests/test_command_utilities.py
@@ -5,7 +5,7 @@
 
 
 class TestCommandUtilities(TestCase):
-    REBOOT_SCHEMA = {'title': 'Reboot', 'type': 'null'}
+    REBOOT_SCHEMA = {'title': 'Reboot', 'type': 'null', 'additionalProperties': False}
 
     def test_get_command_schema(self):
         with self.subTest('Test existing command option'):

diff --git a/openwisp_controller/connection/tests/test_models.py b/openwisp_controller/connection/tests/test_models.py
@@ -786,6 +786,47 @@ def _command_assertions(destination_address, mocked_exec_command):
         unregister_command('callable_ping')
         unregister_command('path_ping')
 
+    @mock.patch(_connect_path)
+    @mock.patch.dict(COMMANDS, {})
+    @mock.patch.dict(ORGANIZATION_ENABLED_COMMANDS, {'__all__': ('restart_network')})
+    @mock.patch(_exec_command_path)
+    def test_execute_user_registered_command_without_input(
+        self, mocked_exec_command, connect_mocked
+    ):
+        restart_network_schema = {
+            'label': 'Restart Network',
+            'schema': {
+                'title': 'Restart Network',
+                'type': 'null',
+                'additionalProperties': False,
+            },
+            'callable': 'openwisp_controller.connection.tests.utils'
+            '._restart_network_command_callable',
+        }
+        dc = self._create_device_connection()
+        register_command('restart_network', restart_network_schema)
+        command = Command(
+            device=dc.device,
+            connection=dc,
+            type='restart_network',
+        )
+        command.full_clean()
+        mocked_exec_command.return_value = self._exec_command_return_value(
+            stdout='Network restarted'
+        )
+        command.save()
+        # must call this explicitly because lack of transactions in this test case
+        command.execute()
+        connect_mocked.assert_called()
+        mocked_exec_command.assert_called_once()
+        mocked_exec_command.assert_called_with(
+            '/etc/init.d/networking restart',
+            timeout=app_settings.SSH_COMMAND_TIMEOUT,
+        )
+        command.refresh_from_db()
+        self.assertEqual(command.status, 'success')
+        self.assertEqual(command.output, 'Network restarted\n')
+
     def test_command_permissions(self):
         ct = ContentType.objects.get_by_natural_key(
             app_label=self.app_label, model='command'

diff --git a/openwisp_controller/connection/tests/utils.py b/openwisp_controller/connection/tests/utils.py
@@ -138,3 +138,7 @@ def _ping_command_callable(destination_address, interface_name=None):
     if interface_name:
         command += f' -I {interface_name}'
     return command
+
+
+def _restart_network_command_callable():
+    return '/etc/init.d/networking restart'