[feature] Mass command execution from the device changelist selection

docs/user/shell-commands.rst

@@ -214,6 +214,26 @@ devices of the system. Other users must choose at least one target, and
 only see the command types enabled for their organizations (see
 :ref:`openwisp_controller_organization_enabled_commands`).
 
+Sending a Mass Command to Selected Devices
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+A mass command can also be started from the device list: select the
+devices with their checkboxes, choose *Execute mass command* from the
+actions dropdown and click *Go*.
+
+The first step opens with the selection already applied: a message at the
+top of the page states how many devices the command will run on, and the
+organization is filled in and cannot be changed, while device group and
+location are not asked for, since the devices are already known.
+
+The selected devices must belong to the same organization, otherwise the
+action refuses to start. The exception is a superuser selecting every
+device of the system: the command then runs on all of them and no target
+is asked for.
+
+The rest of the workflow is the same as described below: the devices can
+still be reviewed and left out before executing.
+
 Reviewing the Devices
 ~~~~~~~~~~~~~~~~~~~~~
 

openwisp_controller/connection/admin.py

@@ -11,14 +11,20 @@
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
 from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
 from django.db.models import Count, Q
-from django.http import HttpResponseForbidden, HttpResponseNotAllowed, JsonResponse
+from django.http import (
+    HttpResponseForbidden,
+    HttpResponseNotAllowed,
+    HttpResponseRedirect,
+    JsonResponse,
+)
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.urls import path, resolve
 from django.utils.html import format_html, format_html_join
 from django.utils.safestring import mark_safe
 from django.utils.timezone import localtime
 from django.utils.translation import gettext_lazy as _
+from django.utils.translation import ngettext
 
 from openwisp_users.multitenancy import MultitenantOrgFilter
 from openwisp_utils.admin import ReadOnlyAdmin, TimeReadonlyAdminMixin
@@ -61,6 +67,7 @@ class Meta:
 
 class BatchCommandExecutionForm(forms.ModelForm):
     required_css_class = "required"
+    devices = forms.CharField(widget=forms.HiddenInput, required=False)
 
     class Meta:
         model = BatchCommand
@@ -95,9 +102,15 @@ class Media:
             ]
         }
 
-    def __init__(self, *args, request=None, **kwargs):
+    def __init__(self, *args, request=None, device_ids=None, **kwargs):
         super().__init__(*args, **kwargs)
         self.request = request
+        self.device_ids = self._scope_devices(device_ids)
+        if self.device_ids:
+            self.fields["devices"].initial = ",".join(self.device_ids)
+            for field_name in ("organization", "group", "location"):
+                self.fields[field_name].disabled = True
+            self.fields["organization"].initial = self._selected_organization_id()
         if request is None or request.user.is_superuser:
             return
         organization_ids = request.user.organizations_managed
@@ -118,16 +131,44 @@ def __init__(self, *args, request=None, **kwargs):
         ]
         self.fields["type"].choices = empty_choices + list(allowed_commands.items())
 
+    def _scope_devices(self, device_ids):
+        self._organization_ids = set()
+        self._dropped_devices = False
+        if not device_ids:
+            return []
+        devices = Device.objects.filter(pk__in=device_ids)
+        if self.request is not None and not self.request.user.is_superuser:
+            devices = devices.filter(
+                organization_id__in=self.request.user.organizations_managed
+            )
+        rows = list(devices.values_list("pk", "organization_id"))
+        self._organization_ids = {row[1] for row in rows}
+        self._dropped_devices = len(rows) != len(set(device_ids))
+        return [str(row[0]) for row in rows]
+
+    def _selected_organization_id(self):
+        if len(self._organization_ids) != 1:
+            return None
+        return str(next(iter(self._organization_ids)))
+
     def clean(self):
         cleaned_data = super().clean()
+        if self._dropped_devices:
+            raise ValidationError(
+                _("Some of the selected devices are no longer available.")
+            )
+        if len(self._organization_ids) > 1:
+            raise ValidationError(
+                _("All devices must belong to the same organization.")
+            )
         if self.request is None or self.request.user.is_superuser:
             return cleaned_data
         organization = cleaned_data.get("organization")
         group = cleaned_data.get("group")
         location = cleaned_data.get("location")
         # a batch without any target would run on every device of the
         # deployment, which only superusers are allowed to do
-        if not any([organization, group, location]):
+        if not self.device_ids and not any([organization, group, location]):
             raise ValidationError(
                 _(
                     "Please select at least one of: organization, device group,"
@@ -174,6 +215,7 @@ def _pk(value):
             "organization_id": _pk(self.cleaned_data.get("organization")),
             "group_id": _pk(self.cleaned_data.get("group")),
             "location_id": _pk(self.cleaned_data.get("location")),
+            "device_ids": self.device_ids,
         }
 
 
@@ -527,7 +569,11 @@ def execute_command_view(self, request):
             return HttpResponseNotAllowed(["GET", "POST"])
         self._check_add_permission(request)
         if request.method == "POST":
-            form = BatchCommandExecutionForm(request.POST, request=request)
+            form = BatchCommandExecutionForm(
+                request.POST,
+                request=request,
+                device_ids=self._get_pk_list(request.POST, "devices"),
+            )
             if form.is_valid():
                 request.session[self.session_key] = form.to_session()
                 return redirect(
@@ -542,13 +588,31 @@ def execute_command_view(self, request):
             else:
                 request.session.pop(self.session_key, None)
                 form = BatchCommandExecutionForm(request=request)
+        return self._render_execute_page(request, form)
+
+    def _render_execute_page(self, request, form, system_wide=False):
+        device_count = len(form.device_ids)
+        if device_count:
+            messages.warning(
+                request,
+                ngettext(
+                    "The command will run on the device you selected.",
+                    "The command will run on the %(count)d devices you selected.",
+                    device_count,
+                )
+                % {"count": device_count},
+            )
+        elif system_wide:
+            messages.warning(request, _("The command will run on all devices."))
         context = {
             **self.admin_site.each_context(request),
             "title": _("Execute mass command"),
             "opts": self.opts,
             "form": form,
             "media": form.media,
             "has_view_permission": self.has_view_permission(request),
+            "device_count": device_count,
+            "system_wide": system_wide,
         }
         return TemplateResponse(request, self.execute_command_template, context)
 
@@ -617,27 +681,31 @@ def _restart(self, request):
         return redirect(f"admin:{self.opts.app_label}_{self.opts.model_name}_execute")
 
     def _resolve_target_queryset(self, request, wizard):
-        """Devices matched by the organization, group and location chosen.
-        The targeting rule lives on the model so this page and the execution
-        cannot drift apart; the multitenancy scope and the ordering the
-        pagination needs are admin concerns, applied on top.
+        """Devices picked one by one, or matched by the organization, group
+        and location chosen. The targeting rule lives on the model so this
+        page and the execution cannot drift apart; the multitenancy scope and
+        the ordering the pagination needs are admin concerns, applied on top.
         """
-        try:
-            devices = BatchCommand.dry_run(
-                organization_id=wizard.get("organization_id"),
-                group_id=wizard.get("group_id"),
-                location_id=wizard.get("location_id"),
-            )["devices"]
-        except (ObjectDoesNotExist, ValidationError) as error:
-            logger.warning(
-                "Failed to resolve devices for mass command wizard"
-                " (organization_id=%s, group_id=%s, location_id=%s): %s",
-                wizard.get("organization_id"),
-                wizard.get("group_id"),
-                wizard.get("location_id"),
-                error,
-            )
-            return Device.objects.none()
+        device_ids = wizard.get("device_ids")
+        if device_ids:
+            devices = Device.objects.filter(pk__in=device_ids)
+        else:
+            try:
+                devices = BatchCommand.dry_run(
+                    organization_id=wizard.get("organization_id"),
+                    group_id=wizard.get("group_id"),
+                    location_id=wizard.get("location_id"),
+                )["devices"]
+            except (ObjectDoesNotExist, ValidationError) as error:
+                logger.warning(
+                    "Failed to resolve devices for mass command wizard"
+                    " (organization_id=%s, group_id=%s, location_id=%s): %s",
+                    wizard.get("organization_id"),
+                    wizard.get("group_id"),
+                    wizard.get("location_id"),
+                    error,
+                )
+                return Device.objects.none()
         if not request.user.is_superuser:
             devices = devices.filter(
                 organization_id__in=request.user.organizations_managed
@@ -652,6 +720,7 @@ def _devices_digest(self, device_ids):
         return hashlib.sha256(",".join(pks).encode()).hexdigest()
 
     def _confirm_context(self, request, wizard, devices):
+        device_count = devices.count()
         targets = []
         for model, key in (
             (Organization, "organization_id"),
@@ -672,11 +741,18 @@ def _confirm_context(self, request, wizard, devices):
             "wizard": wizard,
             "command_type_display": command_types.get(wizard["type"], wizard["type"]),
             "command_description": self._describe_input(wizard.get("input")),
-            "targets_display": ", ".join(targets) if targets else _("All devices"),
-            "device_count": devices.count(),
+            "targets_display": self._targets_display(wizard, targets, device_count),
+            "device_count": device_count,
             "has_view_permission": self.has_view_permission(request),
         }
 
+    def _targets_display(self, wizard, targets, device_count):
+        if wizard.get("device_ids"):
+            return ngettext(
+                "%(count)d selected device", "%(count)d selected devices", device_count
+            ) % {"count": device_count}
+        return ", ".join(targets) if targets else _("All devices")
+
     def _describe_input(self, command_input):
         """Renders the submitted input for the review step, so that every
         registered command type is shown and not only custom ones.
@@ -1051,5 +1127,36 @@ def change_view(self, request, object_id, form_url="", extra_context=None):
             )
         return super().change_view(request, object_id, extra_context=extra_context)
 
+    @staticmethod
+    @admin.action(description=_("Execute mass command"), permissions=["change"])
+    def execute_mass_command_admin_action(modeladmin, request, queryset):
+        """Second entry point of the mass command workflow: the devices are
+        picked one by one instead of being matched by organization, group or
+        location. The selection travels in the form rather than in the session,
+        so it cannot outlive the wizard it belongs to.
+        """
+        batch_admin = modeladmin.admin_site.get_model_admin(BatchCommand)
+        batch_admin._check_add_permission(request)
+        organization_ids = set(queryset.values_list("organization_id", flat=True))
+        if len(organization_ids) > 1:
+            if request.user.is_superuser and queryset.count() == Device.objects.count():
+                return batch_admin._render_execute_page(
+                    request,
+                    BatchCommandExecutionForm(request=request),
+                    system_wide=True,
+                )
+            modeladmin.message_user(
+                request,
+                _("All devices must belong to the same organization."),
+                messages.ERROR,
+            )
+            return HttpResponseRedirect(request.get_full_path())
+        form = BatchCommandExecutionForm(
+            request=request,
+            device_ids=[str(pk) for pk in queryset.values_list("pk", flat=True)],
+        )
+        return batch_admin._render_execute_page(request, form)
+
 
 admin.site.register(BatchCommand, BatchCommandAdmin)
+DeviceAdmin.actions += [BatchCommandAdmin.execute_mass_command_admin_action]

openwisp_controller/connection/static/connection/js/execute-command.js

@@ -98,7 +98,12 @@ function initCommandInput($, $typeSelect) {
 
 function initOrganizationScope($) {
   const $organization = $("#id_organization");
-  const fields = ["#id_group", "#id_location"];
+  const fields = ["#id_group", "#id_location"].filter(function (selector) {
+    return $(selector).length;
+  });
+  if (!fields.length) {
+    return;
+  }
   fields.forEach(function (selector) {
     $(selector).data("allOptions", $(selector).find("option").clone());
   });

openwisp_controller/connection/templates/admin/connection/batch_command/execute_command.html

@@ -30,8 +30,10 @@
 
 {% block content %}
 <div id="content-main">
-  <form method="post" novalidate class="execute-form">
+  {# the action renders this page from the device changelist, so the target is explicit #}
+  <form method="post" novalidate action="{% url opts|admin_urlname:'execute' %}">
     {% csrf_token %}
+    {{ form.devices }}
     <nav class="stepper" aria-label="{% trans 'Execution progress' %}">
       <div class="stepper-step active" aria-current="step">
         <span class="stepper-badge">1</span>
@@ -64,12 +66,16 @@ <h2>{% trans "Command" %}</h2>
       {% include "admin/connection/batch_command/form_row.html" with field=form.notes %}
     </fieldset>
 
+    {% if not system_wide %}
     <fieldset class="module aligned">
       <h2>{% trans "Targets" %}</h2>
       {% include "admin/connection/batch_command/form_row.html" with field=form.organization %}
+      {% if not device_count %}
       {% include "admin/connection/batch_command/form_row.html" with field=form.location %}
       {% include "admin/connection/batch_command/form_row.html" with field=form.group %}
+      {% endif %}
     </fieldset>
+    {% endif %}
 
     <div class="submit-row">
       <a class="button cancel-link" href="{% url opts|admin_urlname:'changelist' %}">{% trans "Cancel" %}</a>