Skip to content

Commit 1d70696

Browse files
pandafynemesifier
pandafy
and
nemesifier
committed
[fix] Allowed deleting device with "deactivating" config status #949
Fixes #949 --------- Co-authored-by: Federico Capoano <[email protected]> (cherry picked from commit 2e5e0dc)
1 parent 6e6720d commit 1d70696

File tree

12 files changed

+332
-45
lines changed

12 files changed

+332
-45
lines changed

‎docs/user/device-config-status.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,3 +36,11 @@ scheduled to be removed from the device.
3636
The device has been deactivated. The configuration applied through
3737
OpenWISP has been removed, and any other operation to manage the device
3838
will be prevented or rejected.
39+
40+
.. note::
41+
42+
If a device becomes unreachable (e.g., lost, stolen, or
43+
decommissioned) before it can be properly deactivated, you can still
44+
force the deletion from OpenWISP by hitting the delete button in the
45+
device detail page after having deactivated the device or by using the
46+
bulk delete action from the device list page.

‎openwisp_controller/config/admin.py‎

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import json
22
import logging
3+
from collections.abc import Iterable
34

45
import reversion
56
from django import forms
67
from django.conf import settings
78
from django.contrib import admin, messages
89
from django.contrib.admin import helpers
10+
from django.contrib.admin.actions import delete_selected
911
from django.contrib.admin.models import ADDITION, LogEntry
1012
from django.contrib.contenttypes.models import ContentType
1113
from django.core.exceptions import (
@@ -566,8 +568,6 @@ def has_delete_permission(self, request, obj=None):
566568
perm = super().has_delete_permission(request)
567569
if not obj:
568570
return perm
569-
if obj._has_config():
570-
perm = perm and obj.config.is_deactivated()
571571
return perm and obj.is_deactivated()
572572

573573
def save_form(self, request, form, change):
@@ -765,22 +765,42 @@ def deactivate_device(self, request, queryset):
765765
def activate_device(self, request, queryset):
766766
self._change_device_status(request, queryset, 'activate')
767767

768-
def get_deleted_objects(self, objs, request, *args, **kwargs):
769-
# Ensure that all selected devices can be deleted, i.e.
770-
# the device should be flagged as deactivated and if it has
771-
# a config object, it's status should be "deactivated".
772-
active_devices = []
773-
for obj in objs:
774-
if not self.has_delete_permission(request, obj):
775-
active_devices.append(obj)
776-
if active_devices:
777-
return (
778-
active_devices,
779-
{self.model._meta.verbose_name_plural: len(active_devices)},
780-
['active_devices'],
781-
[],
768+
@admin.action(description=delete_selected.short_description, permissions=['delete'])
769+
def delete_selected(self, request, queryset):
770+
response = delete_selected(self, request, queryset)
771+
if not response:
772+
return response
773+
if 'active_devices' in response.context_data.get('perms_lacking', {}):
774+
active_devices = []
775+
for device in queryset.iterator():
776+
if not device.is_deactivated() or (
777+
device._has_config() and not device.config.is_deactivated()
778+
):
779+
active_devices.append(self._get_device_path(device))
780+
response.context_data.update(
781+
{
782+
'active_devices': active_devices,
783+
'perms_lacking': set(),
784+
'title': _('Are you sure?'),
785+
}
782786
)
783-
return super().get_deleted_objects(objs, request, *args, **kwargs)
787+
return response
788+
789+
def get_deleted_objects(self, objs, request, *args, **kwargs):
790+
to_delete, model_count, perms_needed, protected = super().get_deleted_objects(
791+
objs, request, *args, **kwargs
792+
)
793+
if (
794+
isinstance(perms_needed, Iterable)
795+
and len(perms_needed) == 1
796+
and list(perms_needed)[0] == self.model._meta.verbose_name
797+
and objs.filter(_is_deactivated=False).exists()
798+
):
799+
if request.POST.get("post"):
800+
perms_needed = set()
801+
else:
802+
perms_needed = {'active_devices'}
803+
return to_delete, model_count, perms_needed, protected
784804

785805
def get_fields(self, request, obj=None):
786806
"""
@@ -900,6 +920,17 @@ def recover_view(self, request, version_id, extra_context=None):
900920
request._recover_view = True
901921
return super().recover_view(request, version_id, extra_context)
902922

923+
def delete_view(self, request, object_id, extra_context=None):
924+
extra_context = extra_context or {}
925+
obj = self.get_object(request, object_id)
926+
if obj and obj._has_config() and not obj.config.is_deactivated():
927+
extra_context['deactivating_warning'] = True
928+
return super().delete_view(request, object_id, extra_context)
929+
930+
def delete_model(self, request, obj):
931+
force_delete = request.POST.get('force_delete') == 'true'
932+
obj.delete(check_deactivated=not force_delete)
933+
903934
def get_inlines(self, request, obj):
904935
inlines = super().get_inlines(request, obj)
905936
# this only makes sense in existing devices

‎openwisp_controller/config/api/views.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@ class DeviceDetailView(ProtectedAPIMixin, RetrieveUpdateDestroyAPIView):
107107
queryset = Device.objects.select_related('config', 'group', 'organization')
108108
permission_classes = ProtectedAPIMixin.permission_classes + (DevicePermission,)
109109

110+
def perform_destroy(self, instance):
111+
force_deletion = self.request.query_params.get('force', None) == 'true'
112+
instance.delete(check_deactivated=(not force_deletion))
113+
110114

111115
class DeviceActivateView(ProtectedAPIMixin, GenericAPIView):
112116
serializer_class = serializers.Serializer

‎openwisp_controller/config/static/config/css/device-delete-confirmation.css‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#deactivating-warning .warning p {
2+
margin-top: 0px;
3+
}
4+
#main ul.messagelist li.warning ul li {
5+
display: list-item;
6+
padding: 0px;
7+
background: inherit;
8+
}
9+
ul.messagelist li {
10+
font-size: unset;
11+
}

‎openwisp_controller/config/static/config/js/device-delete-confirmation.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
"use strict";
2+
3+
(function ($) {
4+
$(document).ready(function () {
5+
$("#warning-ack").click(function (event) {
6+
event.preventDefault();
7+
$("#deactivating-warning").slideUp("fast");
8+
$("#delete-confirm-container").slideDown("fast");
9+
$('input[name="force_delete"]').val("true");
10+
});
11+
});
12+
})(django.jQuery);

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
{% extends "admin/delete_confirmation.html" %}
2+
{% load i18n static %}
3+
4+
{% block extrastyle %}
5+
{{ block.super }}
6+
<link rel="stylesheet" type="text/css" href="{% static 'config/css/device-delete-confirmation.css' %}" />
7+
{% endblock extrastyle %}
8+
9+
{% block content %}
10+
{% if perms_lacking %}
11+
<p>{% blocktranslate with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktranslate %}</p>
12+
<ul>
13+
{% for obj in perms_lacking %}
14+
<li>{{ obj }}</li>
15+
{% endfor %}
16+
</ul>
17+
{% elif protected %}
18+
<p>{% blocktranslate with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would require deleting the following protected related objects:{% endblocktranslate %}</p>
19+
<ul>
20+
{% for obj in protected %}
21+
<li>{{ obj }}</li>
22+
{% endfor %}
23+
</ul>
24+
{% else %}
25+
{% if deactivating_warning %}
26+
<div id="deactivating-warning">
27+
<ul class="messagelist">
28+
<li class="warning">
29+
<p>
30+
<strong>
31+
{% translate 'Warning: Device is not fully deactivated.' %}
32+
</strong>
33+
</p>
34+
<p>
35+
{% blocktranslate %}
36+
This device is still in the process of being deactivated,
37+
meaning its configuration is still present on the device.
38+
{% endblocktranslate %}
39+
</p>
40+
<p>
41+
{% blocktranslate %}
42+
To ensure its configuration is removed, please
43+
wait until its status changes to
44+
<strong>"deactivated"</strong>.<br>
45+
If you proceed now, the device will be deleted,
46+
but its configuration will remain active.
47+
{% endblocktranslate %}
48+
</p>
49+
<form>
50+
<input type="submit" class="button danger-btn" id="warning-ack"
51+
value="{% translate 'I understand the risks, delete the device' %}">
52+
<a class="button cancel-link">{% translate 'No, take me back' %}</a>
53+
</form>
54+
</li>
55+
</ul>
56+
</div>
57+
{% endif %}
58+
<div id="delete-confirm-container" {% if deactivating_warning %}style="display:none;"{% endif %}>
59+
<p>{% blocktranslate with escaped_object=object %}Are you sure you want to delete the {{ object_name }}
60+
"{{ escaped_object }}"? All of the following related items will be deleted:{% endblocktranslate %}</p>
61+
{% include "admin/includes/object_delete_summary.html" %}
62+
<h2>{% translate "Objects" %}</h2>
63+
<ul id="deleted-objects">{{ deleted_objects|unordered_list }}</ul>
64+
<form method="post">{% csrf_token %}
65+
<div>
66+
<input type="hidden" name="post" value="yes">
67+
{% if is_popup %}<input type="hidden" name="{{ is_popup_var }}" value="1">{% endif %}
68+
{% if to_field %}<input type="hidden" name="{{ to_field_var }}" value="{{ to_field }}">{% endif %}
69+
<input type="submit" value="{% translate 'Yes, I’m sure' %}">
70+
{% if deactivating_warning %}<input type="hidden" name="force_delete" value="false">{% endif %}
71+
<a href="#" class="button cancel-link">{% translate "No, take me back" %}</a>
72+
</div>
73+
</form>
74+
</div>
75+
{% endif %}
76+
{% endblock %}
77+
78+
{% block footer %}
79+
{{ block.super }}
80+
<script type="text/javascript" src="{% static 'config/js/device-delete-confirmation.js' %}"></script>
81+
{% endblock %}

‎openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html‎

Lines changed: 75 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,87 @@
11
{% extends "admin/delete_selected_confirmation.html" %}
22
{% load i18n l10n admin_urls static %}
33

4+
{% block extrastyle %}
5+
{{ block.super }}
6+
<link rel="stylesheet" type="text/css" href="{% static 'config/css/device-delete-confirmation.css' %}" />
7+
{% endblock extrastyle %}
8+
49
{% block content %}
510
{% if perms_lacking %}
6-
{% if perms_lacking|first == 'active_devices' %}
7-
<p>{% blocktranslate %}You have selected the following active device{{ model_count | pluralize }} to delete:{% endblocktranslate %}</p>
8-
<ul>{{ deletable_objects|first|unordered_list }}</ul>
9-
<p>{% blocktrans %}It is required to flag the device as deactivated before deleting the device. If the device has configuration, then wait till the configuration status changes to "deactivated" before deleting the device.{% endblocktrans %}</p>
10-
{% else %}
11-
<p>{% blocktranslate %}Deleting the selected {{ objects_name }} would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktranslate %}</p>
12-
<ul>{{ perms_lacking|unordered_list }}</ul>
13-
{% endif %}
11+
<p>{% blocktranslate %}Deleting the selected {{ objects_name }} would result in deleting related objects, but your account doesn't have permission to delete the following types of objects:{% endblocktranslate %}</p>
12+
<ul>{{ perms_lacking|unordered_list }}</ul>
1413
{% elif protected %}
1514
<p>{% blocktranslate %}Deleting the selected {{ objects_name }} would require deleting the following protected related objects:{% endblocktranslate %}</p>
1615
<ul>{{ protected|unordered_list }}</ul>
1716
{% else %}
18-
<p>{% blocktranslate %}Are you sure you want to delete the selected {{ objects_name }}? All of the following objects and their related items will be deleted:{% endblocktranslate %}</p>
19-
{% include "admin/includes/object_delete_summary.html" %}
20-
<h2>{% translate "Objects" %}</h2>
21-
{% for deletable_object in deletable_objects %}
22-
<ul>{{ deletable_object|unordered_list }}</ul>
23-
{% endfor %}
24-
<form method="post">{% csrf_token %}
25-
<div>
26-
{% for obj in queryset %}
27-
<input type="hidden" name="{{ action_checkbox_name }}" value="{{ obj.pk|unlocalize }}">
28-
{% endfor %}
29-
<input type="hidden" name="action" value="delete_selected">
30-
<input type="hidden" name="post" value="yes">
31-
<input type="submit" value="{% translate 'Yes, I’m sure' %}">
32-
<a href="#" class="button cancel-link">{% translate "No, take me back" %}</a>
17+
{% if active_devices %}
18+
<div id="deactivating-warning">
19+
<ul class="messagelist">
20+
<li class="warning">
21+
<p>
22+
<strong>
23+
{% blocktranslate count counter=active_devices|length %}
24+
Warning: Device is not fully deactivated.
25+
{% plural %}
26+
Warning: Some devices are not fully deactivated.
27+
{% endblocktranslate %}
28+
</strong>
29+
</p>
30+
<p>
31+
{% blocktranslate count counter=active_devices|length %}
32+
The device below is either still active or
33+
in the process of being deactivated:
34+
{% plural %}
35+
The devices listed below are either still active
36+
or in the process of being deactivated:
37+
{% endblocktranslate %}
38+
</p>
39+
<ul>{{ active_devices|unordered_list }}</ul>
40+
<p>
41+
{% blocktranslate count counter=active_devices|length %}
42+
To ensure its configuration is removed, please
43+
wait until its status changes to <strong>"deactivated"</strong>.<br>
44+
If you proceed now, the device will be deleted,
45+
but its configuration will remain active.
46+
{% plural %}
47+
To ensure their configurations are removed, please
48+
wait until their status changes to <strong>"deactivated"</strong>.<br>
49+
If you proceed now, the devices will be deleted,
50+
but their configurations will remain active.
51+
{% endblocktranslate %}
52+
</p>
53+
<form>
54+
<input type="submit" class="button danger-btn" id="warning-ack"
55+
value="{% blocktranslate count counter=active_devices|length %}I understand the risks, delete the device{% plural %}I understand the risks, delete the devices{% endblocktranslate %}">
56+
<a class="button cancel-link">{% translate 'No, take me back' %}</a>
57+
</form>
58+
</li>
59+
</ul>
60+
</div>
61+
{% endif %}
62+
<div id="delete-confirm-container" {% if active_devices %}style="display:none;"{% endif %}>
63+
<p>{% blocktranslate %}Are you sure you want to delete the selected {{ objects_name }}? All of the following objects and their related items will be deleted:{% endblocktranslate %}</p>
64+
{% include "admin/includes/object_delete_summary.html" %}
65+
<h2>{% translate "Objects" %}</h2>
66+
{% for deletable_object in deletable_objects %}
67+
<ul>{{ deletable_object|unordered_list }}</ul>
68+
{% endfor %}
69+
<form method="post">{% csrf_token %}
70+
<div>
71+
{% for obj in queryset %}
72+
<input type="hidden" name="{{ action_checkbox_name }}" value="{{ obj.pk|unlocalize }}">
73+
{% endfor %}
74+
<input type="hidden" name="action" value="delete_selected">
75+
<input type="hidden" name="post" value="yes">
76+
<input type="submit" value="{% translate 'Yes, I’m sure' %}">
77+
<a href="#" class="button cancel-link">{% translate "No, take me back" %}</a>
78+
</div>
79+
</form>
3380
</div>
34-
</form>
3581
{% endif %}
3682
{% endblock %}
83+
84+
{% block footer %}
85+
{{ block.super }}
86+
<script type="text/javascript" src="{% static 'config/js/device-delete-confirmation.js' %}"></script>
87+
{% endblock %}

‎openwisp_controller/config/tests/test_admin.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,9 +2120,8 @@ def test_device_with_config_change_deactivate_deactivate(self):
21202120
)
21212121
# Save buttons are absent on deactivated device
21222122
self.assertNotContains(response, self._save_btn_html)
2123-
# Delete button is not present if config status is deactivating
21242123
self.assertEqual(device.config.status, 'deactivating')
2125-
self.assertNotContains(response, delete_btn_html)
2124+
self.assertContains(response, delete_btn_html)
21262125
self.assertNotContains(response, self._deactivate_btn_html)
21272126
self.assertContains(response, self._activate_btn_html)
21282127
# Verify adding a new DeviceLocation and DeviceConnection is not allowed

‎openwisp_controller/config/tests/test_api.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,22 @@ def test_device_delete_api(self):
539539
self.assertEqual(response.status_code, 204)
540540
self.assertEqual(Device.objects.count(), 0)
541541

542+
def test_deactivating_device_force_deletion(self):
543+
self._create_template(required=True)
544+
device = self._create_device()
545+
config = self._create_config(device=device)
546+
device.deactivate()
547+
path = reverse('config_api:device_detail', args=[device.pk])
548+
549+
with self.subTest(
550+
'Test force deleting device with config in deactivating state'
551+
):
552+
self.assertEqual(device.is_deactivated(), True)
553+
self.assertEqual(config.is_deactivating(), True)
554+
response = self.client.delete(f'{path}?force=true')
555+
self.assertEqual(response.status_code, 204)
556+
self.assertEqual(Device.objects.count(), 0)
557+
542558
def test_template_create_no_org_api(self):
543559
self.assertEqual(Template.objects.count(), 0)
544560
path = reverse('config_api:template_list')

0 commit comments

Comments
 (0)