Просмотр исходного кода

Fixes #23597: Change-log port mappings created from type templates (#23606)

Arthur Hanson 16 часов назад
Родитель
Сommit
56daec7deb

+ 22 - 10
netbox/dcim/models/module_moves.py

@@ -908,19 +908,30 @@ class ModuleMovePlan:
             self._send_post_saves(model, updated, fields)
 
     def _apply_port_mappings(self):
-        # Private model, no changelog or last_updated field; mirrors PortMapping.save()'s device derivation.
+        # Update each mapping's device after moving its ports
         if not self.cross_device:
             return
         moved_front_port_pks = [obj.pk for obj in self.components[FrontPort]]
         moved_rear_port_pks = [obj.pk for obj in self.components[RearPort]]
-        if moved_front_port_pks and moved_rear_port_pks:
-            chunked_update(
-                PortMapping.objects.filter(
-                    front_port_id__in=moved_front_port_pks,
-                    rear_port_id__in=moved_rear_port_pks,
-                ),
-                device_id=self.new_device_id,
-            )
+        if not (moved_front_port_pks and moved_rear_port_pks):
+            return
+        mappings = list(PortMapping.objects.using(router.db_for_write(PortMapping)).filter(
+            front_port_id__in=moved_front_port_pks,
+            rear_port_id__in=moved_rear_port_pks,
+        ).select_related('front_port', 'rear_port'))
+        if not mappings:
+            return
+        for mapping in mappings:
+            mapping.snapshot()
+        chunked_update(
+            PortMapping.objects.filter(pk__in=[mapping.pk for mapping in mappings]),
+            device_id=self.new_device_id,
+            last_updated=self._now,
+        )
+        for mapping in mappings:
+            mapping.device_id = self.new_device_id
+            mapping.last_updated = self._now
+        self._send_post_saves(PortMapping, mappings, ['device', 'last_updated'])
 
     @staticmethod
     def device_counters_by_model(device_model):
@@ -975,7 +986,8 @@ class ModuleMovePlan:
         for instance in instances:
             # Clear tracked counter state so the incremental counter receiver no-ops;
             # counters are recomputed explicitly for cross-device moves.
-            instance.tracker.clear()
+            if hasattr(instance, 'tracker'):
+                instance.tracker.clear()
             post_save.send(
                 sender=model,
                 instance=instance,

+ 4 - 1
netbox/dcim/signals.py

@@ -308,10 +308,13 @@ def retrace_cable_paths(instance, **kwargs):
 
 
 @receiver((post_delete, post_save), sender=PortMapping)
-def update_passthrough_port_paths(instance, **kwargs):
+def update_passthrough_port_paths(instance, update_fields=None, **kwargs):
     """
     When a PortMapping is created or deleted, retrace any CablePaths which traverse its front and/or rear ports.
     """
+    # Moving a module to another device changes only these fields; its connections are unchanged
+    if update_fields and set(update_fields) <= {'device', 'last_updated'}:
+        return
     for cablepath in CablePath.objects.filter(
         Q(_nodes__contains=instance.front_port) | Q(_nodes__contains=instance.rear_port)
     ):

+ 27 - 0
netbox/dcim/tests/test_module_moves.py

@@ -1495,6 +1495,33 @@ class ModuleCrossDeviceMoveTestCase(TestCase):
         mapping.refresh_from_db()
         self.assertEqual(mapping.device, self.device_b)
 
+    @override_settings(BULK_UPDATE_CHUNK_SIZE=1)
+    def test_cross_device_move_changelogs_port_mappings(self):
+        front_port = FrontPort.objects.create(
+            device=self.device_a, module=self.line_card, name='Front 2', type=PortTypeChoices.TYPE_LC
+        )
+        rear_port = RearPort.objects.create(
+            device=self.device_a, module=self.line_card, name='Rear 2', type=PortTypeChoices.TYPE_LC, positions=1
+        )
+        PortMapping.objects.create(
+            front_port=front_port, front_port_position=1, rear_port=rear_port, rear_port_position=1
+        )
+        mappings = PortMapping.objects.filter(front_port__module=self.line_card)
+        self.assertEqual(mappings.count(), 2)
+        request = RequestFactory().get('/')
+        request.id = uuid.uuid4()
+        request.user = User.objects.create_user(username='mapping-mover')
+        with event_tracking(request):
+            self.line_card.snapshot()
+            self._move_to_device_b()
+        for mapping in mappings:
+            self.assertEqual(mapping.device, self.device_b)
+            change = ObjectChange.objects.get(
+                changed_object_type=ContentType.objects.get_for_model(PortMapping), changed_object_id=mapping.pk
+            )
+            self.assertEqual(change.prechange_data['device'], self.device_a.pk)
+            self.assertEqual(change.postchange_data['device'], self.device_b.pk)
+
     def test_cross_device_move_keeps_intra_module_power_link(self):
         outlet = self.line_card.poweroutlets.get(name='Outlet 1')
         self._move_to_device_b()

+ 73 - 0
netbox/dcim/tests/test_port_mappings.py

@@ -15,6 +15,9 @@ from dcim.models import (
     FrontPort,
     FrontPortTemplate,
     Manufacturer,
+    Module,
+    ModuleBayTemplate,
+    ModuleType,
     PortMapping,
     PortTemplateMapping,
     RearPort,
@@ -258,6 +261,76 @@ class ReconcilePortTemplateMappingsTestCase(TestCase):
         self.assertEqual(PortTemplateMapping.objects.get(front_port=self.front_port).pk, original_pk)
 
 
+class InstantiatedPortMappingChangeLoggingTestCase(TestCase):
+    """
+    Port mappings replicated from device/module type templates must be change-logged.
+    """
+    @classmethod
+    def setUpTestData(cls):
+        cls.user = User.objects.create_user(username='testuser', password='pw')
+
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
+        cls.role = DeviceRole.objects.create(name='Device Role 1', slug='device-role-1', color='ff0000')
+        cls.site = Site.objects.create(name='Site 1', slug='site-1')
+
+        cls.device_type = DeviceType.objects.create(
+            manufacturer=manufacturer, model='Device Type 1', slug='device-type-1'
+        )
+        ModuleBayTemplate.objects.create(device_type=cls.device_type, name='Slot 1', position='1')
+        cls.module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Module Type 1')
+        for type_kwargs in ({'device_type': cls.device_type}, {'module_type': cls.module_type}):
+            front_port = FrontPortTemplate.objects.create(
+                name='Front {module}' if 'module_type' in type_kwargs else 'Front',
+                type=PortTypeChoices.TYPE_8P8C, positions=2, **type_kwargs
+            )
+            rear_port = RearPortTemplate.objects.create(
+                name='Rear {module}' if 'module_type' in type_kwargs else 'Rear',
+                type=PortTypeChoices.TYPE_8P8C, positions=2, **type_kwargs
+            )
+            for position in (1, 2):
+                PortTemplateMapping.objects.create(
+                    front_port=front_port, front_port_position=position,
+                    rear_port=rear_port, rear_port_position=position,
+                    **type_kwargs
+                )
+
+    def _mapping_changes(self, request):
+        return ObjectChange.objects.filter(
+            request_id=request.id,
+            changed_object_type=ContentType.objects.get_for_model(PortMapping),
+            action=ObjectChangeActionChoices.ACTION_CREATE,
+        )
+
+    @tag('regression')  # Ref: #23597
+    def test_device_creation_records_mappings(self):
+        request = _build_request(self.user)
+        with event_tracking(request):
+            device = Device.objects.create(
+                device_type=self.device_type, role=self.role, name='Device 1', site=self.site
+            )
+
+        mapping_pks = set(device.port_mappings.values_list('pk', flat=True))
+        self.assertEqual(len(mapping_pks), 2)
+        self.assertEqual(
+            set(self._mapping_changes(request).values_list('changed_object_id', flat=True)), mapping_pks
+        )
+
+    @tag('regression')  # Ref: #23597
+    def test_module_installation_records_mappings(self):
+        device = Device.objects.create(device_type=self.device_type, role=self.role, name='Device 1', site=self.site)
+        request = _build_request(self.user)
+        with event_tracking(request):
+            module = Module.objects.create(
+                device=device, module_bay=device.modulebays.get(name='Slot 1'), module_type=self.module_type
+            )
+
+        mapping_pks = set(PortMapping.objects.filter(front_port__module=module).values_list('pk', flat=True))
+        self.assertEqual(len(mapping_pks), 2)
+        self.assertEqual(
+            set(self._mapping_changes(request).values_list('changed_object_id', flat=True)), mapping_pks
+        )
+
+
 class PortMappingAPITestCase(APITestCase):
     """
     Exercise the reconcile behaviour through PortSerializer.create()/update() over the REST API,

+ 13 - 2
netbox/dcim/utils.py

@@ -5,6 +5,7 @@ from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
 from django.db import router, transaction
 from django.db.models import Q
+from django.db.models.signals import post_save
 from django.utils.translation import gettext as _
 
 from dcim.constants import MODULE_TOKEN
@@ -537,8 +538,18 @@ def create_port_mappings(device, device_or_module_type, module=None):
                 rear_port_position=template.rear_port_position,
             )
         )
-    # Bulk-created (no per-mapping ObjectChange) to match how every other component is instantiated.
-    PortMapping.objects.bulk_create(mappings)
+    # Send post_save for each mapping to record its creation
+    mappings = PortMapping.objects.bulk_create(mappings)
+    using = router.db_for_write(PortMapping)
+    for mapping in mappings:
+        post_save.send(
+            sender=PortMapping,
+            instance=mapping,
+            created=True,
+            raw=False,
+            using=using,
+            update_fields=None
+        )
 
 
 def reconcile_port_mappings(mapping_model, parent_field, parent, desired):