Fixes #23596: Snapshot components adopted during module installation for change logging (#23605)

* Fixes #23596: Snapshot components adopted during module installation for change logging

* review fixes

netbox/dcim/models/modules.py

@@ -6,6 +6,7 @@ from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import OperationalError, models, router, transaction
 from django.db.models.signals import post_save
+from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 from jsonschema.exceptions import ValidationError as JSONValidationError
 
@@ -533,6 +534,8 @@ class Module(TrackingModelMixin, PrimaryModel):
         if disable_replication and not adopt_components:
             return
 
+        now = timezone.now()
+
         # Iterate all component types
         for templates, component_attribute, component_model in [
             ("consoleporttemplates", "consoleports", ConsolePort),
@@ -554,7 +557,7 @@ class Module(TrackingModelMixin, PrimaryModel):
                 component.name: component
                 for component in getattr(self.device, component_attribute).db_manager(using).filter(
                     module__isnull=True
-                )
+                ).prefetch_related('tags')
             }
 
             # Get the template for the module type.
@@ -567,8 +570,10 @@ class Module(TrackingModelMixin, PrimaryModel):
 
                     # Check if there's a component with the same name already
                     if existing_item:
-                        # Assign it to the module
+                        # Snapshot for the changelog, then assign it to the module
+                        existing_item.snapshot()
                         existing_item.module = self
+                        existing_item.last_updated = now
                         update_instances.append(existing_item)
                         continue
 
@@ -609,7 +614,7 @@ class Module(TrackingModelMixin, PrimaryModel):
                     update_fields=None
                 )
 
-            update_fields = ['module']
+            update_fields = ['module', 'last_updated']
             # ModuleBay.parent is derived from .module in ModuleBay.save(), and the
             # path/sort_path trigger only fires on parent_id/name changes. A bare
             # bulk_update(['module']) bypasses both, leaving the adopted bay rooted
@@ -618,11 +623,22 @@ class Module(TrackingModelMixin, PrimaryModel):
             if component_model is ModuleBay:
                 for instance in update_instances:
                     instance.parent = self.module_bay
-                update_fields = ['module', 'parent']
+                update_fields = ['module', 'parent', 'last_updated']
 
             component_model.objects.using(using).bulk_update(
                 update_instances, update_fields, batch_size=settings.BULK_UPDATE_CHUNK_SIZE
             )
+            # The triggers rewrote path/sort_path; refresh them so changelog snapshots aren't stale
+            if component_model is ModuleBay and update_instances:
+                refreshed = {
+                    row['pk']: row
+                    for row in ModuleBay.objects.using(using).filter(
+                        pk__in=[bay.pk for bay in update_instances]
+                    ).values('pk', 'path', 'sort_path')
+                }
+                for bay in update_instances:
+                    bay.path = refreshed[bay.pk]['path']
+                    bay.sort_path = refreshed[bay.pk]['sort_path']
             # Emit the post_save signal for each updated object
             for component in update_instances:
                 post_save.send(

netbox/dcim/tests/test_api.py

@@ -3656,6 +3656,90 @@ class ModuleTestCase(APIViewTestCases.APIViewTestCase):
         existing_iface.refresh_from_db()
         self.assertIsNotNone(existing_iface.module)
 
+    def test_adopt_components_changelog(self):
+        """
+        Adopted components should be change-logged with prechange data and have last_updated bumped.
+        """
+        self.add_permissions('dcim.add_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for Adopt Changelog Test')
+        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Adopt Changelog Module Type')
+        InterfaceTemplate.objects.create(module_type=module_type, name='eth0', type='1000base-t')
+        ModuleBayTemplate.objects.create(module_type=module_type, name='Sub Bay')
+        module_bay = ModuleBay.objects.create(device=device, name='Adopt Changelog Bay')
+        # Adopted bays are also re-parented under the module's bay
+        expected_diffs = [
+            (Interface.objects.create(device=device, name='eth0', type='1000base-t'), {'module': None}),
+            (ModuleBay.objects.create(device=device, name='Sub Bay'), {'module': None, 'parent': None}),
+        ]
+        original_last_updated = {component.pk: component.last_updated for component, _ in expected_diffs}
+
+        url = reverse('dcim-api:module-list')
+        data = {
+            'device': device.pk,
+            'module_bay': module_bay.pk,
+            'module_type': module_type.pk,
+            'adopt_components': True,
+            'replicate_components': False,
+        }
+        response = self.client.post(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)
+        module_pk = response.data['id']
+
+        for component, expected_pre in expected_diffs:
+            component.refresh_from_db()
+            self.assertEqual(component.module_id, module_pk)
+            self.assertGreater(component.last_updated, original_last_updated[component.pk])
+            objectchange = ObjectChange.objects.get(
+                changed_object_type=ObjectType.objects.get_for_model(component),
+                changed_object_id=component.pk,
+                action=ObjectChangeActionChoices.ACTION_UPDATE,
+            )
+            self.assertIsNotNone(objectchange.prechange_data)
+            self.assertIsNone(objectchange.prechange_data['module'])
+            self.assertEqual(objectchange.postchange_data['module'], module_pk)
+            self.assertEqual(objectchange.diff()['pre'], expected_pre)
+            # Trigger-maintained ltree columns must reflect the post-adoption location
+            if isinstance(component, ModuleBay):
+                self.assertEqual(objectchange.postchange_data['path'], str(component.path))
+                self.assertEqual(objectchange.postchange_data['sort_path'], component.sort_path)
+
+    def test_install_module_interface_parent_bridge_changelog(self):
+        """
+        Interface parent/bridge assignments made during module installation must record prechange data.
+        """
+        self.add_permissions('dcim.add_module')
+        manufacturer = Manufacturer.objects.get(name='Generic')
+        device = create_test_device('Device for Interface Parent Changelog Test')
+        module_type = ModuleType.objects.create(manufacturer=manufacturer, model='Parent Changelog Module Type')
+        eth0 = InterfaceTemplate.objects.create(module_type=module_type, name='eth0', type='1000base-t')
+        InterfaceTemplate.objects.create(module_type=module_type, name='eth0.100', type='virtual', parent=eth0)
+        InterfaceTemplate.objects.create(module_type=module_type, name='br0', type='bridge')
+        InterfaceTemplate.objects.create(
+            module_type=module_type, name='eth1', type='1000base-t', bridge=InterfaceTemplate.objects.get(name='br0')
+        )
+        module_bay = ModuleBay.objects.create(device=device, name='Parent Changelog Bay')
+
+        url = reverse('dcim-api:module-list')
+        data = {
+            'device': device.pk,
+            'module_bay': module_bay.pk,
+            'module_type': module_type.pk,
+        }
+        response = self.client.post(url, data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)
+
+        for name, field in (('eth0.100', 'parent'), ('eth1', 'bridge')):
+            interface = Interface.objects.get(device=device, name=name)
+            objectchange = ObjectChange.objects.get(
+                changed_object_type=ObjectType.objects.get_for_model(Interface),
+                changed_object_id=interface.pk,
+                action=ObjectChangeActionChoices.ACTION_UPDATE,
+            )
+            self.assertIsNotNone(objectchange.prechange_data)
+            self.assertIsNone(objectchange.prechange_data[field])
+            self.assertEqual(objectchange.postchange_data[field], getattr(interface, f'{field}_id'))
+
     def test_replicate_components_conflict(self):
         """
         Installing a module with replicate_components=True when a component with the same name

netbox/dcim/utils.py

@@ -475,6 +475,7 @@ def update_interface_parents(device, interface_templates, module=None):
 
     for interface_template in interface_templates.exclude(parent=None):
         interface = Interface.objects.get(device=device, name=interface_template.resolve_name(module=module))
+        interface.snapshot()
         interface.parent = Interface.objects.get(
             device=device,
             name=interface_template.parent.resolve_name(module=module)
@@ -497,6 +498,7 @@ def update_interface_bridges(device, interface_templates, module=None):
         )
 
         if interface_template.bridge:
+            interface.snapshot()
             interface.bridge = Interface.objects.get(
                 device=device,
                 name=interface_template.bridge.resolve_name(module=module, device=device)