[fix] Update docs and address coderabbit comments

Author: dee077

docs/user/rest-api.rst

@@ -507,7 +507,6 @@ Parameter        Description
                  (optional)
 ``group``        Device group UUID (optional)
 ``location``     Location UUID (optional)
-``execute_all``  Set to ``true`` to target all devices (optional)
 ================ =========================================================
 
 Execute a Mass Command
@@ -534,7 +533,6 @@ Parameter        Description
 ``devices``      List of device UUIDs (optional)
 ``group``        Device group UUID (optional)
 ``location``     Location UUID (optional)
-``execute_all``  Set to ``true`` to target all devices (optional)
 ================ ========================================================
 
 **Available Command Types:**
@@ -550,8 +548,7 @@ input formats.
         "organization": "org-uuid",
         "type": "custom",
         "input": {"command": "uptime"},
-        "label": "Check uptime",
-        "execute_all": true
+        "label": "Check uptime"
     }
 
 **Example request:**
@@ -566,8 +563,7 @@ input formats.
                 "organization": "org-uuid",
                 "type": "custom",
                 "input": {"command": "uptime"},
-                "label": "Check uptime",
-                "execute_all": true
+                "label": "Check uptime"
             }'
 
 **Response:** ``201 Created`` with the batch command UUID.

docs/user/shell-commands.rst

@@ -201,11 +201,10 @@ You can target devices using any combination of the following:
 - ``devices``: Explicit list of device UUIDs.
 - ``group``: All devices belonging to a device group.
 - ``location``: All devices at a specific location.
-- ``execute_all``: All devices in the organization.
 
-If ``devices`` is provided as an empty list and ``execute_all`` is
-``false``, the request will be rejected. To target all devices, set
-``execute_all`` to ``true``.
+If no targeting options are provided, the command targets all devices in
+the organization. If ``devices`` is provided as an empty list, the request
+is rejected because no devices match.
 
 Refer to the :ref:`Batch Command API <controller_batch_command_api>`
 documentation for the available endpoints, request parameters, and

openwisp_controller/connection/api/serializers.py

@@ -203,7 +203,7 @@ def _redact_skipped_devices(self, data, request):
         user_org_ids = set(str(org) for org in request.user.organizations_managed)
         user_org_device_uuids = set(
             str(pk)
-            for pk, in Device.objects.filter(
+            for pk in Device.objects.filter(
                 pk__in=device_pks, organization_id__in=user_org_ids
             ).values_list("pk", flat=True)
         )
@@ -215,13 +215,19 @@ def _redact_skipped_devices(self, data, request):
                 org_name = device_org_map.get(
                     uuid.UUID(device_pk), "some other organization"
                 )
-                redacted[f"{org_name} device"] = [
-                    f"restricted to {org_name} managers and users"
+                redacted[_("{org_name} device").format(org_name=org_name)] = [
+                    _("restricted to {org_name} managers and users").format(
+                        org_name=org_name
+                    )
                 ]
         data["skipped_devices"] = redacted
 
     def to_representation(self, instance):
         data = super().to_representation(instance)
+        # The raw password is visible in API responses between
+        # when the batch is saved and when the Celery task runs _clean_sensitive_info().
+        if instance.type == "change_password":
+            data["input"] = {"password": "********"}
         request = self.context.get("request")
         if (
             instance.organization_id is None
@@ -263,6 +269,10 @@ class BatchCommandDetailSerializer(BatchCommandSerializer):
 
     def to_representation(self, instance):
         data = super().to_representation(instance)
+        # The raw password is visible in API responses between
+        # when the batch is saved and when the Celery task runs _clean_sensitive_info().
+        if instance.type == "change_password":
+            data["input"] = {"password": "********"}
         request = self.context.get("request")
         if (
             instance.organization_id is None
@@ -274,7 +284,7 @@ def to_representation(self, instance):
             if device_uuids:
                 visible_uuids = set(
                     str(pk)
-                    for pk, in Device.objects.filter(
+                    for pk in Device.objects.filter(
                         pk__in=device_uuids, organization_id__in=user_org_ids
                     ).values_list("pk", flat=True)
                 )

openwisp_controller/connection/api/views.py

@@ -163,7 +163,7 @@ def post(self, request):
                 getattr(e, "message_dict", e.messages),
                 status=status.HTTP_400_BAD_REQUEST,
             )
-        return Response({"batch": str(batch.pk)}, status=201)
+        return Response({"batch": str(batch.pk)}, status=status.HTTP_201_CREATED)
 
     def get(self, request):
         serializer = self.get_serializer(

openwisp_controller/connection/base/models.py

@@ -977,7 +977,7 @@ def create_commands(self):
                 command.full_clean()
                 with transaction.atomic():
                     command.save()
-            except Exception as e:
+            except ValidationError as e:
                 self.skipped_devices[str(device.pk)] = (
                     e.messages if hasattr(e, "messages") else [str(e)]
                 )
@@ -1006,48 +1006,52 @@ def calculate_and_update_status(self):
         - All commands completed successfully: status set to "success".
         - Status unchanged: no database write performed.
         """
-        stats = self.batch_commands.aggregate(
-            total_operations=models.Count("id"),
-            in_progress=models.Count(
-                models.Case(
-                    models.When(status="in-progress", then=1),
-                    output_field=models.IntegerField(),
-                )
-            ),
-            completed=models.Count(
-                models.Case(
-                    models.When(~models.Q(status="in-progress"), then=1),
-                    output_field=models.IntegerField(),
-                )
-            ),
-            successful=models.Count(
-                models.Case(
-                    models.When(status="success", then=1),
-                    output_field=models.IntegerField(),
-                )
-            ),
-            failed=models.Count(
-                models.Case(
-                    models.When(status="failed", then=1),
-                    output_field=models.IntegerField(),
-                )
-            ),
-        )
-        if stats["total_operations"] == 0:
-            if self.skipped_devices:
+        with transaction.atomic():
+            batch = self.__class__.objects.select_for_update().get(pk=self.pk)
+            stats = batch.batch_commands.aggregate(
+                total_operations=models.Count("id"),
+                in_progress=models.Count(
+                    models.Case(
+                        models.When(status="in-progress", then=1),
+                        output_field=models.IntegerField(),
+                    )
+                ),
+                completed=models.Count(
+                    models.Case(
+                        models.When(~models.Q(status="in-progress"), then=1),
+                        output_field=models.IntegerField(),
+                    )
+                ),
+                successful=models.Count(
+                    models.Case(
+                        models.When(status="success", then=1),
+                        output_field=models.IntegerField(),
+                    )
+                ),
+                failed=models.Count(
+                    models.Case(
+                        models.When(status="failed", then=1),
+                        output_field=models.IntegerField(),
+                    )
+                ),
+            )
+            if stats["total_operations"] == 0:
+                if batch.skipped_devices:
+                    new_status = "failed"
+                else:
+                    new_status = "idle"
+            elif stats["in_progress"] > 0:
+                new_status = "in-progress"
+            elif stats["failed"] > 0:
                 new_status = "failed"
+            elif (
+                stats["successful"] > 0
+                and stats["completed"] == stats["total_operations"]
+                and not batch.skipped_devices
+            ):
+                new_status = "success"
             else:
-                new_status = "idle"
-        elif stats["in_progress"] > 0:
-            new_status = "in-progress"
-        elif stats["failed"] > 0:
-            new_status = "failed"
-        elif (
-            stats["successful"] > 0 and stats["completed"] == stats["total_operations"]
-        ):
-            new_status = "success"
-        else:
-            new_status = self.status
-        if self.status != new_status:
-            self.status = new_status
-            self.save(update_fields=["status"])
+                new_status = batch.status
+            if batch.status != new_status:
+                batch.status = new_status
+                batch.save(update_fields=["status"])

openwisp_controller/connection/tasks.py

@@ -108,11 +108,15 @@ def launch_batch_command(self, batch_id):
         return
     try:
         batch.create_commands()
-    except Exception as e:
+    except Exception:
+        batch._clean_sensitive_info()
         batch.status = "failed"
-        batch.save(update_fields=["status"])
+        update_fields = ["status"]
+        if batch.type == "change_password":
+            update_fields.append("input")
+        batch.save(update_fields=update_fields)
         logger.exception(
-            f"An exception was raised while executing batch " f"command {batch_id}: {e}"
+            f"An exception was raised while executing batch command {batch_id}"
         )