From c345299e164071b528da0bad16a43da1fd2a8cf6 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 2 Jan 2025 22:36:25 +0530 Subject: [PATCH 01/10] [fix] Allowed deleting device with "deactivating" config status #949 Fixes #949 --- openwisp_controller/config/admin.py | 13 +++- openwisp_controller/config/api/views.py | 4 ++ .../config/device/delete_confirmation.html | 61 +++++++++++++++++++ .../config/tests/test_admin.py | 3 +- openwisp_controller/config/tests/test_api.py | 16 +++++ 5 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 openwisp_controller/config/templates/admin/config/device/delete_confirmation.html diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index ddc3c84b5..0ada5c859 100644 --- a/openwisp_controller/config/admin.py +++ b/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 diff --git a/openwisp_controller/config/api/views.py b/openwisp_controller/config/api/views.py index c56f1437c..5f6f205c0 100644 --- a/openwisp_controller/config/api/views.py +++ b/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 diff --git a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html new file mode 100644 index 000000000..e5fe5dd3e --- /dev/null +++ b/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 }} + +{% endblock extrastyle %} + +{% block delete_confirm %} +{% if deactivating_warning %} +
+ +
+{% endif %} +
+

{% 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 %}

+ {% include "admin/includes/object_delete_summary.html" %} +

{% translate "Objects" %}

+ +
{% csrf_token %} +
+ + + {% if is_popup %}{% endif %} + {% if to_field %}{% endif %} + + {% translate "No, take me back" %} +
+
+
+{% endblock %} + +{% block footer %} +{{ block.super }} + +{% endblock %} diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index f2da37829..62c28b313 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/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 diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 13ae381b4..a5a508d1f 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/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') From 82ff62dad127fca24e941bdb3b06fe57e546bbb8 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 6 Jan 2025 21:48:58 +0530 Subject: [PATCH 02/10] [fix] Show warning message on deleting multiple active devices --- openwisp_controller/config/admin.py | 52 +++++++---- .../config/css/device-delete-confirmation.css | 8 ++ .../config/js/device-delete-confirmation.js | 12 +++ .../config/device/delete_confirmation.html | 27 ++---- .../device/delete_selected_confirmation.html | 88 ++++++++++++++----- 5 files changed, 127 insertions(+), 60 deletions(-) create mode 100644 openwisp_controller/config/static/config/css/device-delete-confirmation.css create mode 100644 openwisp_controller/config/static/config/js/device-delete-confirmation.js diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 0ada5c859..339560b8c 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -1,11 +1,13 @@ import json import logging +from collections.abc import Iterable import reversion from django import forms from django.conf import settings from django.contrib import admin, messages from django.contrib.admin import helpers +from django.contrib.admin.actions import delete_selected from django.contrib.admin.models import ADDITION, LogEntry from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ( @@ -763,22 +765,42 @@ def deactivate_device(self, request, queryset): def activate_device(self, request, queryset): self._change_device_status(request, queryset, 'activate') - def get_deleted_objects(self, objs, request, *args, **kwargs): - # Ensure that all selected devices can be deleted, i.e. - # the device should be flagged as deactivated and if it has - # a config object, it's status should be "deactivated". - active_devices = [] - for obj in objs: - if not self.has_delete_permission(request, obj): - active_devices.append(obj) - if active_devices: - return ( - active_devices, - {self.model._meta.verbose_name_plural: len(active_devices)}, - ['active_devices'], - [], + @admin.action(description=delete_selected.short_description, permissions=['delete']) + def delete_selected(self, request, queryset): + response = delete_selected(self, request, queryset) + if not response: + return response + if 'active_devices' in response.context_data.get('perms_lacking', {}): + active_devices = [] + for device in queryset.iterator(): + if not device.is_deactivated() or ( + device._has_config() and not device.config.is_deactivated() + ): + active_devices.append(self._get_device_path(device)) + response.context_data.update( + { + 'active_devices': active_devices, + 'perms_lacking': set(), + 'title': _('Are you sure?'), + } ) - return super().get_deleted_objects(objs, request, *args, **kwargs) + return response + + def get_deleted_objects(self, objs, request, *args, **kwargs): + to_delete, model_count, perms_needed, protected = super().get_deleted_objects( + objs, request, *args, **kwargs + ) + if ( + isinstance(perms_needed, Iterable) + and len(perms_needed) == 1 + and list(perms_needed)[0] == self.model._meta.verbose_name + and objs.filter(_is_deactivated=False).exists() + ): + if request.POST.get("post"): + perms_needed = set() + else: + perms_needed = {'active_devices'} + return to_delete, model_count, perms_needed, protected def get_fields(self, request, obj=None): """ diff --git a/openwisp_controller/config/static/config/css/device-delete-confirmation.css b/openwisp_controller/config/static/config/css/device-delete-confirmation.css new file mode 100644 index 000000000..3f23394b6 --- /dev/null +++ b/openwisp_controller/config/static/config/css/device-delete-confirmation.css @@ -0,0 +1,8 @@ +#deactivating-warning .warning p { + margin-top: 0px; +} +#main ul.messagelist li.warning ul li { + display: list-item; + padding: 0px; + background: inherit; +} diff --git a/openwisp_controller/config/static/config/js/device-delete-confirmation.js b/openwisp_controller/config/static/config/js/device-delete-confirmation.js new file mode 100644 index 000000000..7918a64d3 --- /dev/null +++ b/openwisp_controller/config/static/config/js/device-delete-confirmation.js @@ -0,0 +1,12 @@ +'use strict'; + +(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); diff --git a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html index e5fe5dd3e..6830ae019 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html @@ -1,13 +1,9 @@ {% extends "admin/delete_confirmation.html" %} -{% load i18n %} +{% load i18n static %} {% block extrastyle %} {{ block.super }} - + {% endblock extrastyle %} {% block delete_confirm %} @@ -15,11 +11,11 @@
  • -

    {% 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." %}

    +

    {% 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.' %}

    - No, take me back + value="{% trans 'I understand the risks, delete the device' %}"> + {% trans 'No, take me back' %}
@@ -46,16 +42,5 @@

{% translate "Objects" %}

{% block footer %} {{ block.super }} - + {% endblock %} diff --git a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html index a66130f86..382c862e5 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html @@ -1,36 +1,76 @@ {% extends "admin/delete_selected_confirmation.html" %} {% load i18n l10n admin_urls static %} +{% block extrastyle %} +{{ block.super }} + +{% endblock extrastyle %} + {% block content %} {% if perms_lacking %} - {% if perms_lacking|first == 'active_devices' %} -

{% blocktranslate %}You have selected the following active device{{ model_count | pluralize }} to delete:{% endblocktranslate %}

-
    {{ deletable_objects|first|unordered_list }}
-

{% 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 %}

- {% else %} -

{% 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 %}

-
    {{ perms_lacking|unordered_list }}
- {% endif %} +

{% 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 %}

+
    {{ perms_lacking|unordered_list }}
{% elif protected %}

{% blocktranslate %}Deleting the selected {{ objects_name }} would require deleting the following protected related objects:{% endblocktranslate %}

    {{ protected|unordered_list }}
{% else %} -

{% 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 %}

- {% include "admin/includes/object_delete_summary.html" %} -

{% translate "Objects" %}

- {% for deletable_object in deletable_objects %} -
    {{ deletable_object|unordered_list }}
- {% endfor %} -
{% csrf_token %} -
- {% for obj in queryset %} - - {% endfor %} - - - - {% translate "No, take me back" %} + {% if active_devices %} +
+
    +
  • +

    + {% blocktrans count counter=active_devices|length %} + The following device you selected for deletion is not deactivated + (either it is active or its configuration status is still "deactivating"): + {% plural %} + The following devices you selected for deletion are not deactivated + (either they are active or their configuration status is still "deactivating"): + {% endblocktrans %} +

    +
      {{ active_devices|unordered_list }}
    +

    + {% blocktranslate count counter=active_devices|length %} + If you wish to remove the configuration from the device, please wait until its + configuration status changes to "deactivated". Proceeding will delete the device + from OpenWISP without ensuring its configuration has been removed. + {% plural %} + If you wish to remove the configurations from the devices, please wait until their + configuration status change to "deactivated." Proceeding will delete the devices + from OpenWISP without ensuring their configurations have been removed. + {% endblocktranslate %} +

    + + + No, take me back +
    • + +
+
+ {% endif %} +
+

{% 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 %}

+ {% include "admin/includes/object_delete_summary.html" %} +

{% translate "Objects" %}

+ {% for deletable_object in deletable_objects %} +
    {{ deletable_object|unordered_list }}
+ {% endfor %} +
{% csrf_token %} +
+ {% for obj in queryset %} + + {% endfor %} + + + + {% translate "No, take me back" %} +
+
- {% endif %} {% endblock %} + +{% block footer %} +{{ block.super }} + +{% endblock %} From 4b6ecd226a3d33891c254bd7ad4b6f73caf84c5c Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 7 Jan 2025 14:26:03 +0530 Subject: [PATCH 03/10] [tests] Added tests --- .../config/device/delete_confirmation.html | 2 +- .../config/tests/test_selenium.py | 74 +++++++++++++++++++ openwisp_controller/tests/test_selenium.py | 2 +- 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html index 6830ae019..54a6b7890 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html @@ -30,10 +30,10 @@

{% translate "Objects" %}

{% csrf_token %}
- {% if is_popup %}{% endif %} {% if to_field %}{% endif %} + {% if deactivating_warning %}{% endif %} {% translate "No, take me back" %}
diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index becb85339..dfce1b8e2 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -12,11 +12,14 @@ from selenium.webdriver.common.keys import Keys from selenium.webdriver.support import expected_conditions as EC from selenium.webdriver.support.ui import Select, WebDriverWait +from swapper import load_model from openwisp_utils.test_selenium_mixins import SeleniumTestMixin from .utils import CreateConfigTemplateMixin, TestWireguardVpnMixin +Device = load_model('config', 'Device') + class SeleniumBaseMixin(CreateConfigTemplateMixin, SeleniumTestMixin): def setUp(self): @@ -321,6 +324,77 @@ def test_template_context_variables(self): alert.accept() self.fail('Unsaved changes alert displayed without any change') + def test_force_delete_device_with_deactivating_config(self): + self._create_template(default=True) + config = self._create_config(organization=self._get_org()) + device = config.device + self.assertEqual(device.is_deactivated(), False) + self.assertEqual(config.status, 'modified') + + self.login() + self.open(reverse('admin:config_device_change', args=[device.id])) + self.web_driver.find_elements( + by=By.CSS_SELECTOR, value='input.deletelink[type="submit"]' + )[-1].click() + device.refresh_from_db() + config.refresh_from_db() + self.assertEqual(device.is_deactivated(), True) + self.assertEqual(config.is_deactivating(), True) + + self.open(reverse('admin:config_device_change', args=[device.id])) + self.web_driver.find_elements(by=By.CSS_SELECTOR, value='a.deletelink')[ + -1 + ].click() + WebDriverWait(self.web_driver, 5).until( + EC.visibility_of_element_located( + (By.CSS_SELECTOR, '#deactivating-warning .messagelist .warning p') + ) + ) + self.web_driver.find_element(by=By.CSS_SELECTOR, value='#warning-ack').click() + delete_confirm = WebDriverWait(self.web_driver, 2).until( + EC.visibility_of_element_located( + (By.CSS_SELECTOR, 'form[method="post"] input[type="submit"]') + ) + ) + delete_confirm.click() + self.assertEqual(Device.objects.count(), 0) + + def test_force_delete_multiple_devices_with_deactivating_config(self): + self._create_template(default=True) + org = self._get_org() + device1 = self._create_device(organization=org) + config1 = self._create_config(device=device1) + device2 = self._create_device( + organization=org, name='test2', mac_address='22:22:22:22:22:22' + ) + config2 = self._create_config(device=device2) + self.assertEqual(device1.is_deactivated(), False) + self.assertEqual(config1.status, 'modified') + self.assertEqual(device2.is_deactivated(), False) + self.assertEqual(config2.status, 'modified') + + self.login() + self.open(reverse('admin:config_device_changelist')) + self.web_driver.find_element(by=By.CSS_SELECTOR, value='#action-toggle').click() + select = Select(self.web_driver.find_element(by=By.NAME, value='action')) + select.select_by_value('delete_selected') + self.web_driver.find_element( + by=By.CSS_SELECTOR, value='button[type="submit"][name="index"][value="0"]' + ).click() + WebDriverWait(self.web_driver, 5).until( + EC.visibility_of_element_located( + (By.CSS_SELECTOR, '#deactivating-warning .messagelist .warning p') + ) + ) + self.web_driver.find_element(by=By.CSS_SELECTOR, value='#warning-ack').click() + delete_confirm = WebDriverWait(self.web_driver, 2).until( + EC.visibility_of_element_located( + (By.CSS_SELECTOR, 'form[method="post"] input[type="submit"]') + ) + ) + delete_confirm.click() + self.assertEqual(Device.objects.count(), 0) + class TestVpnAdmin(SeleniumBaseMixin, TestWireguardVpnMixin, StaticLiveServerTestCase): def test_vpn_edit(self): diff --git a/openwisp_controller/tests/test_selenium.py b/openwisp_controller/tests/test_selenium.py index 13b52f5de..5a16bc789 100644 --- a/openwisp_controller/tests/test_selenium.py +++ b/openwisp_controller/tests/test_selenium.py @@ -63,7 +63,7 @@ def test_restoring_deleted_device(self, *args): reverse(f'admin:{self.config_app_label}_device_delete', args=[device.id]) ) self.web_driver.find_element( - by=By.XPATH, value='//*[@id="content"]/form/div/input[2]' + by=By.CSS_SELECTOR, value='#content form input[type="submit"]' ).click() # Delete location object location.delete() From 1dcd4e24e0b52ed0194a67ecd20911dc57cdbab7 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 7 Jan 2025 15:00:48 +0530 Subject: [PATCH 04/10] [tests] Fixed tests for Django < 4.2 --- .../config/device/delete_confirmation.html | 76 +++++++++++-------- .../device/delete_selected_confirmation.html | 4 +- 2 files changed, 48 insertions(+), 32 deletions(-) diff --git a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html index 54a6b7890..b05e739ed 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html @@ -6,38 +6,54 @@ {% endblock extrastyle %} -{% block delete_confirm %} -{% if deactivating_warning %} -
-
    -
  • -

    {% 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.' %}

    -
    - - {% trans 'No, take me back' %} -
    -
    • +{% block content %} +{% if perms_lacking %} +

    {% 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 %}

    +
      + {% for obj in perms_lacking %} +
    • {{ obj }}
      • + {% endfor %}
    -
+{% elif protected %} +

{% blocktranslate with escaped_object=object %}Deleting the {{ object_name }} '{{ escaped_object }}' would require deleting the following protected related objects:{% endblocktranslate %}

+
    + {% for obj in protected %} +
  • {{ obj }}
    • + {% endfor %} +
+{% else %} + {% if deactivating_warning %} +
+
    +
  • +

    {% translate '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.' %}

    +
    + + {% translate 'No, take me back' %} +
    +
    • +
+
+ {% endif %} +
+

{% 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 %}

+ {% include "admin/includes/object_delete_summary.html" %} +

{% translate "Objects" %}

+
    {{ deleted_objects|unordered_list }}
+
{% csrf_token %} +
+ + {% if is_popup %}{% endif %} + {% if to_field %}{% endif %} + + {% if deactivating_warning %}{% endif %} + {% translate "No, take me back" %} +
+
+
{% endif %} -
-

{% 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 %}

- {% include "admin/includes/object_delete_summary.html" %} -

{% translate "Objects" %}

-
    {{ deleted_objects|unordered_list }}
-
{% csrf_token %} -
- - {% if is_popup %}{% endif %} - {% if to_field %}{% endif %} - - {% if deactivating_warning %}{% endif %} - {% translate "No, take me back" %} -
-
-
{% endblock %} {% block footer %} diff --git a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html index 382c862e5..540f030b2 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html @@ -19,13 +19,13 @@

  • - {% blocktrans count counter=active_devices|length %} + {% blocktranslate count counter=active_devices|length %} The following device you selected for deletion is not deactivated (either it is active or its configuration status is still "deactivating"): {% plural %} The following devices you selected for deletion are not deactivated (either they are active or their configuration status is still "deactivating"): - {% endblocktrans %} + {% endblocktranslate %}

      {{ active_devices|unordered_list }}

    From 5d68fd2d98c98b4de489ef2f779bd4fac058e4c2 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 21 Jan 2025 18:39:52 +0530 Subject: [PATCH 05/10] [chores] Fixed coverage reporting --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c1f5a1ad4..23aa7c344 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,7 +1,7 @@ [tool.coverage.run] source = ["openwisp_controller"] parallel = true -concurrency = ["multiprocessing"] +concurrency = ["multiprocessing", "thread"] omit = [ "openwisp_controller/__init__.py", "*/tests/*", From 5f52b96875b086cee9275837c05e615187d1a537 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 27 Jan 2025 20:42:55 +0530 Subject: [PATCH 06/10] [qa] Formatted by prettier --- .../config/css/device-delete-confirmation.css | 8 ++++---- .../config/js/device-delete-confirmation.js | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/openwisp_controller/config/static/config/css/device-delete-confirmation.css b/openwisp_controller/config/static/config/css/device-delete-confirmation.css index 3f23394b6..581414912 100644 --- a/openwisp_controller/config/static/config/css/device-delete-confirmation.css +++ b/openwisp_controller/config/static/config/css/device-delete-confirmation.css @@ -1,8 +1,8 @@ #deactivating-warning .warning p { - margin-top: 0px; + margin-top: 0px; } #main ul.messagelist li.warning ul li { - display: list-item; - padding: 0px; - background: inherit; + display: list-item; + padding: 0px; + background: inherit; } diff --git a/openwisp_controller/config/static/config/js/device-delete-confirmation.js b/openwisp_controller/config/static/config/js/device-delete-confirmation.js index 7918a64d3..8960d5ea9 100644 --- a/openwisp_controller/config/static/config/js/device-delete-confirmation.js +++ b/openwisp_controller/config/static/config/js/device-delete-confirmation.js @@ -1,12 +1,12 @@ -'use strict'; +"use strict"; (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'); - }); + $(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); From 2652cbb997b3855706b75157fcb9f84f5b02fe60 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Mon, 27 Jan 2025 20:51:44 +0530 Subject: [PATCH 07/10] [skip ci][docs] Added note for deleting the device --- docs/user/device-config-status.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/user/device-config-status.rst b/docs/user/device-config-status.rst index 68d6c540b..a612e36ea 100644 --- a/docs/user/device-config-status.rst +++ b/docs/user/device-config-status.rst @@ -36,3 +36,9 @@ scheduled to be removed from the device. The device has been deactivated. The configuration applied through OpenWISP has been removed, and any other operation to manage the device will be prevented or rejected. + +.. note:: + + If a device becomes unreachable (e.g., lost, stolen, or decommissioned) + before it can be properly deactivated, you can remove it from OpenWISP + by deleting the device from the system. From 62aacd531e408339d3e760737fa21481ada0fbba Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 28 Jan 2025 15:48:19 +0530 Subject: [PATCH 08/10] [req-changes] Improved button text and fixed font size --- docs/user/device-config-status.rst | 8 +++++--- .../static/config/css/device-delete-confirmation.css | 3 +++ .../admin/config/device/delete_selected_confirmation.html | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/user/device-config-status.rst b/docs/user/device-config-status.rst index a612e36ea..58c64882b 100644 --- a/docs/user/device-config-status.rst +++ b/docs/user/device-config-status.rst @@ -39,6 +39,8 @@ will be prevented or rejected. .. note:: - If a device becomes unreachable (e.g., lost, stolen, or decommissioned) - before it can be properly deactivated, you can remove it from OpenWISP - by deleting the device from the system. + If a device becomes unreachable (e.g., lost, stolen, or + decommissioned) before it can be properly deactivated, you can still + force the deletion from OpenWISP by hitting the delete button in the + device detail page after having deactivated the device or by using the + bulk delete action from the device list page. diff --git a/openwisp_controller/config/static/config/css/device-delete-confirmation.css b/openwisp_controller/config/static/config/css/device-delete-confirmation.css index 581414912..52065d658 100644 --- a/openwisp_controller/config/static/config/css/device-delete-confirmation.css +++ b/openwisp_controller/config/static/config/css/device-delete-confirmation.css @@ -6,3 +6,6 @@ padding: 0px; background: inherit; } +ul.messagelist li { + font-size: unset; +} diff --git a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html index 540f030b2..5f263e3b3 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html @@ -41,7 +41,7 @@

    + value="{% blocktranslate count counter=active_devices|length %}I understand the risks, delete the device{% plural %}I understand the risks, delete the devices{% endblocktranslate %}"> No, take me back
    • From 4892967565ae3a3e36e6d50d6485a25bdac390aa Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 28 Jan 2025 16:43:05 -0300 Subject: [PATCH 09/10] [chores] Improved intermediate page text --- .../device/delete_selected_confirmation.html | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html index 5f263e3b3..8e2ef86ee 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html @@ -19,30 +19,41 @@

    • + {% blocktranslate count counter=active_devices|length %} - The following device you selected for deletion is not deactivated - (either it is active or its configuration status is still "deactivating"): + Warning: Device is not fully deactivated. {% plural %} - The following devices you selected for deletion are not deactivated - (either they are active or their configuration status is still "deactivating"): + Warning: Some devices are not fully deactivated. + {% endblocktranslate %} + +

      +

      + {% blocktranslate count counter=active_devices|length %} + The device below is either still active or + in the process of being deactivated: + {% plural %} + The devices listed below are either still active + or in the process of being deactivated: {% endblocktranslate %}

        {{ active_devices|unordered_list }}

      {% blocktranslate count counter=active_devices|length %} - If you wish to remove the configuration from the device, please wait until its - configuration status changes to "deactivated". Proceeding will delete the device - from OpenWISP without ensuring its configuration has been removed. + To ensure its configuration is removed, please + wait until its status changes to "deactivated".
      + If you proceed now, the device will be deleted, + but its configuration will remain active. {% plural %} - If you wish to remove the configurations from the devices, please wait until their - configuration status change to "deactivated." Proceeding will delete the devices - from OpenWISP without ensuring their configurations have been removed. + To ensure their configurations are removed, please + wait until their status changes to "deactivated".
      + If you proceed now, the devices will be deleted, + but their configurations will remain active. {% endblocktranslate %}

      - No, take me back + {% translate "No, take me back" %}
    From 6e5fd68a3703e7a12a80eaed9b814f04dc04bddf Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Tue, 28 Jan 2025 16:55:37 -0300 Subject: [PATCH 10/10] [chores] Improved wording of delete_confirmation.html template --- .../config/device/delete_confirmation.html | 21 ++++++++++++++++++- .../device/delete_selected_confirmation.html | 2 +- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html index b05e739ed..6917f224f 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_confirmation.html @@ -26,7 +26,26 @@
    • -

      {% translate '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.' %}

      +

      + + {% translate 'Warning: Device is not fully deactivated.' %} + +

      +

      + {% blocktranslate %} + This device is still in the process of being deactivated, + meaning its configuration is still present on the device. + {% endblocktranslate %} +

      +

      + {% blocktranslate %} + To ensure its configuration is removed, please + wait until its status changes to + "deactivated".
      + If you proceed now, the device will be deleted, + but its configuration will remain active. + {% endblocktranslate %} +

      diff --git a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html index 8e2ef86ee..e88f8cd33 100644 --- a/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html +++ b/openwisp_controller/config/templates/admin/config/device/delete_selected_confirmation.html @@ -53,7 +53,7 @@ - {% translate "No, take me back" %} + {% translate 'No, take me back' %}