[fix] Allowed deleting device with "deactivating" config status #949

Fixes #949

Author: pandafy

openwisp_controller/config/admin.py

@@ -566,8 +566,6 @@ def has_delete_permission(self, request, obj=None):
         perm = super().has_delete_permission(request)
         if not obj:
             return perm
-        if obj._has_config():
-            perm = perm and obj.config.is_deactivated()
         return perm and obj.is_deactivated()
 
     def save_form(self, request, form, change):
@@ -900,6 +898,17 @@ def recover_view(self, request, version_id, extra_context=None):
         request._recover_view = True
         return super().recover_view(request, version_id, extra_context)
 
+    def delete_view(self, request, object_id, extra_context=None):
+        extra_context = extra_context or {}
+        obj = self.get_object(request, object_id)
+        if obj and obj._has_config() and not obj.config.is_deactivated():
+            extra_context['deactivating_warning'] = True
+        return super().delete_view(request, object_id, extra_context)
+
+    def delete_model(self, request, obj):
+        force_delete = request.POST.get('force_delete') == 'true'
+        obj.delete(check_deactivated=not force_delete)
+
     def get_inlines(self, request, obj):
         inlines = super().get_inlines(request, obj)
         # this only makes sense in existing devices

openwisp_controller/config/api/views.py

@@ -107,6 +107,10 @@ class DeviceDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView):
     queryset = Device.objects.select_related('config', 'group', 'organization')
     permission_classes = ProtectedAPIMixin.permission_classes + (DevicePermission,)
 
+    def perform_destroy(self, instance):
+        force_deletion = self.request.query_params.get('force', None) == 'true'
+        instance.delete(check_deactivated=(not force_deletion))
+
 
 class DeviceActivateView(ProtectedAPIMixin, GenericAPIView):
     serializer_class = serializers.Serializer

openwisp_controller/config/templates/admin/config/device/delete_confirmation.html

@@ -0,0 +1,61 @@
+{% extends "admin/delete_confirmation.html" %}
+{% load i18n %}
+
+{% block extrastyle %}
+{{ block.super }}
+<style>
+#deactivating-warning .warning p {
+    margin-top: 0px;
+}
+</style>
+{% endblock extrastyle %}
+
+{% block delete_confirm %}
+{% if deactivating_warning %}
+<div id="deactivating-warning">
+    <ul class="messagelist">
+        <li class="warning">
+            <p>{% trans "The device is still in the deactivating state, meaning its configuration is still present on the device. If you wish to remove the configuration from the device, please wait until the config status changes to deactivated. Proceeding will delete the device from OpenWISP without ensuring its configuration has been removed." %}</p>
+            <form>
+                <input type="submit" class="button danger-btn" id="warning-ack"
+                    value="{% trans "I understand the risks, delete the device" %}">
+                <a class="button cancel-link">No, take me back</a>
+            </form>
+        </li>
+    </ul>
+</div>
+{% endif %}
+<div id="delete-confirm-container" {% if deactivating_warning %}style="display:none;"{% endif %}>
+    <p>{% blocktranslate with escaped_object=object %}Are you sure you want to delete the {{ object_name }}
+        "{{ escaped_object }}"? All of the following related items will be deleted:{% endblocktranslate %}</p>
+    {% include "admin/includes/object_delete_summary.html" %}
+    <h2>{% translate "Objects" %}</h2>
+    <ul id="deleted-objects">{{ deleted_objects|unordered_list }}</ul>
+    <form method="post">{% csrf_token %}
+        <div>
+            <input type="hidden" name="post" value="yes">
+            <input type="hidden" name="force_delete" value="false">
+            {% if is_popup %}<input type="hidden" name="{{ is_popup_var }}" value="1">{% endif %}
+            {% if to_field %}<input type="hidden" name="{{ to_field_var }}" value="{{ to_field }}">{% endif %}
+            <input type="submit" value="{% translate 'Yes, I’m sure' %}">
+            <a href="#" class="button cancel-link">{% translate "No, take me back" %}</a>
+        </div>
+    </form>
+</div>
+{% endblock %}
+
+{% block footer %}
+{{ block.super }}
+<script>
+    (function ($) {
+        $(document).ready(function () {
+            $('#warning-ack').click(function (event) {
+                event.preventDefault();
+                $('#deactivating-warning').slideUp('fast');
+                $('#delete-confirm-container').slideDown('fast');
+                $('input[name="force_delete"]').val('true');
+            });
+        })
+    })(django.jQuery);
+</script>
+{% endblock %}

openwisp_controller/config/tests/test_admin.py

@@ -2120,9 +2120,8 @@ def test_device_with_config_change_deactivate_deactivate(self):
         )
         # Save buttons are absent on deactivated device
         self.assertNotContains(response, self._save_btn_html)
-        # Delete button is not present if config status is deactivating
         self.assertEqual(device.config.status, 'deactivating')
-        self.assertNotContains(response, delete_btn_html)
+        self.assertContains(response, delete_btn_html)
         self.assertNotContains(response, self._deactivate_btn_html)
         self.assertContains(response, self._activate_btn_html)
         # Verify adding a new DeviceLocation and DeviceConnection is not allowed

openwisp_controller/config/tests/test_api.py

@@ -539,6 +539,22 @@ def test_device_delete_api(self):
             self.assertEqual(response.status_code, 204)
             self.assertEqual(Device.objects.count(), 0)
 
+    def test_deactivating_device_force_deletion(self):
+        self._create_template(required=True)
+        device = self._create_device()
+        config = self._create_config(device=device)
+        device.deactivate()
+        path = reverse('config_api:device_detail', args=[device.pk])
+
+        with self.subTest(
+            'Test force deleting device with config in deactivating state'
+        ):
+            self.assertEqual(device.is_deactivated(), True)
+            self.assertEqual(config.is_deactivating(), True)
+            response = self.client.delete(f'{path}?force=true')
+            self.assertEqual(response.status_code, 204)
+            self.assertEqual(Device.objects.count(), 0)
+
     def test_template_create_no_org_api(self):
         self.assertEqual(Template.objects.count(), 0)
         path = reverse('config_api:template_list')