From 9eb1ce7791395f19b6831caef7a8d0de87d8a4ce Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 1 Nov 2023 09:29:31 -0400 Subject: [PATCH] Support bulk removal of parent interfaces via UI if all children are included --- netbox/dcim/tests/test_views.py | 30 +++++++++++++++++++++++ netbox/dcim/views.py | 5 ++-- netbox/netbox/views/generic/bulk_views.py | 11 +++++---- netbox/virtualization/tests/test_views.py | 29 ++++++++++++++++++++++ netbox/virtualization/views.py | 5 ++-- 5 files changed, 71 insertions(+), 9 deletions(-) diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index a6981451f..88e0d44f2 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -2531,6 +2531,36 @@ class InterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase): response = self.client.get(reverse('dcim:interface_trace', kwargs={'pk': interface1.pk})) self.assertHttpStatus(response, 200) + def test_bulk_delete_child_interfaces(self): + interface1 = Interface.objects.get(name='Interface 1') + device = interface1.device + self.add_permissions('dcim.delete_interface') + + # Create a child interface + child = Interface.objects.create( + device=device, + name='Interface 1A', + type=InterfaceTypeChoices.TYPE_VIRTUAL, + parent=interface1 + ) + self.assertEqual(device.interfaces.count(), 6) + + # Attempt to delete only the parent interface + data = { + 'confirm': True, + } + self.client.post(self._get_url('delete', interface1), data) + self.assertEqual(device.interfaces.count(), 6) # Parent was not deleted + + # Attempt to bulk delete parent & child together + data = { + 'pk': [interface1.pk, child.pk], + 'confirm': True, + '_confirm': True, # Form button + } + self.client.post(self._get_url('bulk_delete'), data) + self.assertEqual(device.interfaces.count(), 4) # Child & parent were both deleted + class FrontPortTestCase(ViewTestCases.DeviceComponentViewTestCase): model = FrontPort diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index 0f5768173..be0d6bcbb 100644 --- a/netbox/dcim/views.py +++ b/netbox/dcim/views.py @@ -1,5 +1,4 @@ import traceback -from collections import defaultdict from django.contrib import messages from django.contrib.contenttypes.models import ContentType @@ -26,6 +25,7 @@ from tenancy.views import ObjectContactsView from utilities.forms import ConfirmationForm from utilities.paginator import EnhancedPaginator, get_paginate_count from utilities.permissions import get_permission_for_model +from utilities.query_functions import CollateAsChar from utilities.utils import count_related from utilities.views import GetReturnURLMixin, ObjectPermissionRequiredMixin, ViewTab, register_model_view from virtualization.models import VirtualMachine @@ -2562,7 +2562,8 @@ class InterfaceBulkDisconnectView(BulkDisconnectView): class InterfaceBulkDeleteView(generic.BulkDeleteView): - queryset = Interface.objects.all() + # Ensure child interfaces are deleted prior to their parents + queryset = Interface.objects.order_by('device', 'parent', CollateAsChar('_name')) filterset = filtersets.InterfaceFilterSet table = tables.InterfaceTable diff --git a/netbox/netbox/views/generic/bulk_views.py b/netbox/netbox/views/generic/bulk_views.py index b29465071..fbe3aa2ba 100644 --- a/netbox/netbox/views/generic/bulk_views.py +++ b/netbox/netbox/views/generic/bulk_views.py @@ -798,11 +798,12 @@ class BulkDeleteView(GetReturnURLMixin, BaseMultiObjectView): queryset = self.queryset.filter(pk__in=pk_list) deleted_count = queryset.count() try: - for obj in queryset: - # Take a snapshot of change-logged models - if hasattr(obj, 'snapshot'): - obj.snapshot() - obj.delete() + with transaction.atomic(): + for obj in queryset: + # Take a snapshot of change-logged models + if hasattr(obj, 'snapshot'): + obj.snapshot() + obj.delete() except (ProtectedError, RestrictedError) as e: logger.info(f"Caught {type(e)} while attempting to delete objects") diff --git a/netbox/virtualization/tests/test_views.py b/netbox/virtualization/tests/test_views.py index a5d831d7e..f47c386e9 100644 --- a/netbox/virtualization/tests/test_views.py +++ b/netbox/virtualization/tests/test_views.py @@ -374,3 +374,32 @@ class VMInterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase): 'untagged_vlan': vlans[0].pk, 'tagged_vlans': [v.pk for v in vlans[1:4]], } + + def test_bulk_delete_child_interfaces(self): + interface1 = VMInterface.objects.get(name='Interface 1') + virtual_machine = interface1.virtual_machine + self.add_permissions('virtualization.delete_vminterface') + + # Create a child interface + child = VMInterface.objects.create( + virtual_machine=virtual_machine, + name='Interface 1A', + parent=interface1 + ) + self.assertEqual(virtual_machine.interfaces.count(), 4) + + # Attempt to delete only the parent interface + data = { + 'confirm': True, + } + self.client.post(self._get_url('delete', interface1), data) + self.assertEqual(virtual_machine.interfaces.count(), 4) # Parent was not deleted + + # Attempt to bulk delete parent & child together + data = { + 'pk': [interface1.pk, child.pk], + 'confirm': True, + '_confirm': True, # Form button + } + self.client.post(self._get_url('bulk_delete'), data) + self.assertEqual(virtual_machine.interfaces.count(), 2) # Child & parent were both deleted diff --git a/netbox/virtualization/views.py b/netbox/virtualization/views.py index 798d1fc4d..e8782243f 100644 --- a/netbox/virtualization/views.py +++ b/netbox/virtualization/views.py @@ -1,5 +1,4 @@ import traceback -from collections import defaultdict from django.contrib import messages from django.db import transaction @@ -19,6 +18,7 @@ from ipam.tables import InterfaceVLANTable from netbox.constants import DEFAULT_ACTION_PERMISSIONS from netbox.views import generic from tenancy.views import ObjectContactsView +from utilities.query_functions import CollateAsChar from utilities.utils import count_related from utilities.views import ViewTab, register_model_view from . import filtersets, forms, tables @@ -550,7 +550,8 @@ class VMInterfaceBulkRenameView(generic.BulkRenameView): class VMInterfaceBulkDeleteView(generic.BulkDeleteView): - queryset = VMInterface.objects.all() + # Ensure child interfaces are deleted prior to their parents + queryset = VMInterface.objects.order_by('virtual_machine', 'parent', CollateAsChar('_name')) filterset = filtersets.VMInterfaceFilterSet table = tables.VMInterfaceTable