diff --git a/docs/user/shell-commands.rst b/docs/user/shell-commands.rst index 7e57b298c..86dd436af 100644 --- a/docs/user/shell-commands.rst +++ b/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 ~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_controller/connection/admin.py b/openwisp_controller/connection/admin.py index a39c4c9ac..4b3577777 100644 --- a/openwisp_controller/connection/admin.py +++ b/openwisp_controller/connection/admin.py @@ -11,7 +11,12 @@ 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 @@ -19,6 +24,7 @@ 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,8 +131,36 @@ 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") @@ -127,7 +168,7 @@ def clean(self): 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,6 +588,22 @@ 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"), @@ -549,6 +611,8 @@ def execute_command_view(self, request): "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] diff --git a/openwisp_controller/connection/static/connection/js/execute-command.js b/openwisp_controller/connection/static/connection/js/execute-command.js index bc5863766..12ed0368e 100644 --- a/openwisp_controller/connection/static/connection/js/execute-command.js +++ b/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()); }); diff --git a/openwisp_controller/connection/templates/admin/connection/batch_command/execute_command.html b/openwisp_controller/connection/templates/admin/connection/batch_command/execute_command.html index 3dae5f60d..852a4a70d 100644 --- a/openwisp_controller/connection/templates/admin/connection/batch_command/execute_command.html +++ b/openwisp_controller/connection/templates/admin/connection/batch_command/execute_command.html @@ -30,8 +30,10 @@ {% block content %}
-
+ {# the action renders this page from the device changelist, so the target is explicit #} + {% csrf_token %} + {{ form.devices }}