Fixes #23486: Record prechange data for virtual chassis member changes

netbox/dcim/models/devices.py

@@ -1344,9 +1344,11 @@ class VirtualChassis(PrimaryModel):
                 set(interfaces),
             )
 
-        # Clear vc_position and vc_priority on member devices BEFORE calling super().delete()
-        # This must be done here because on_delete=SET_NULL executes before pre_delete signal
+        # Detach member devices BEFORE calling super().delete() so a single snapshotted change record captures the
+        # cleared virtual_chassis, vc_position, and vc_priority (on_delete=SET_NULL would skip the changelog)
         for device in self.members.all():
+            device.snapshot()
+            device.virtual_chassis = None
             device.vc_position = None
             device.vc_priority = None
             device.save()

netbox/dcim/signals.py

@@ -233,6 +233,7 @@ def assign_virtualchassis_master(instance, created, **kwargs):
     """
     if created and instance.master:
         master = Device.objects.get(pk=instance.master.pk)
+        master.snapshot()
         master.virtual_chassis = instance
         master.vc_position = 1
         master.save()

netbox/dcim/tests/test_api.py

@@ -6328,6 +6328,25 @@ class VirtualChassisTestCase(APIViewTestCases.APIViewTestCase):
         member.refresh_from_db()
         self.assertEqual(member.virtual_chassis.name, 'Virtual Chassis 1')
 
+    @tag('regression')  # Ref: #23486
+    def test_create_with_master_records_prechange_data(self):
+        self.add_permissions('dcim.add_virtualchassis')
+        master = Device.objects.get(name='Device 10')
+
+        data = {'name': 'Virtual Chassis 4', 'master': master.pk}
+        response = self.client.post(self._get_list_url(), data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_201_CREATED)
+
+        objectchange = ObjectChange.objects.get(
+            changed_object_type=ContentType.objects.get_for_model(Device),
+            changed_object_id=master.pk,
+        )
+        self.assertEqual(objectchange.action, ObjectChangeActionChoices.ACTION_UPDATE)
+        self.assertIsNone(objectchange.prechange_data['virtual_chassis'])
+        self.assertIsNone(objectchange.prechange_data['vc_position'])
+        self.assertEqual(objectchange.postchange_data['virtual_chassis'], response.data['id'])
+        self.assertEqual(objectchange.postchange_data['vc_position'], 1)
+
 
 class PowerPanelTestCase(APIViewTestCases.APIViewTestCase):
     model = PowerPanel

netbox/dcim/tests/test_views.py

@@ -5748,6 +5748,71 @@ class VirtualChassisTestCase(ViewTestCases.PrimaryObjectViewTestCase):
         self.assertIsNone(member.virtual_chassis)
         self.assertIsNone(member.vc_position)
 
+    @tag('regression')  # Ref: #23486
+    def test_edit_members_records_prechange_data(self):
+        self.add_permissions('dcim.change_virtualchassis')
+        vc = VirtualChassis.objects.get(name='VC1')
+        members = list(vc.members.order_by('vc_position'))
+
+        data = {
+            'name': vc.name,
+            'domain': vc.domain,
+            'master': vc.master.pk,
+            'form-TOTAL_FORMS': 3,
+            'form-INITIAL_FORMS': 3,
+            'form-MIN_NUM_FORMS': 0,
+            'form-MAX_NUM_FORMS': 1000,
+        }
+        for i, member in enumerate(members):
+            data[f'form-{i}-id'] = member.pk
+            data[f'form-{i}-vc_position'] = member.vc_position
+            data[f'form-{i}-vc_priority'] = ''
+        # Change the priority of the first member only
+        data['form-0-vc_priority'] = 20
+
+        url = reverse('dcim:virtualchassis_edit', kwargs={'pk': vc.pk})
+        response = self.client.post(url, data)
+        self.assertHttpStatus(response, 302)
+
+        objectchanges = ObjectChange.objects.filter(
+            changed_object_type=ContentType.objects.get_for_model(Device),
+            changed_object_id=members[0].pk,
+        )
+        self.assertEqual(objectchanges.count(), 1)
+        objectchange = objectchanges.first()
+        self.assertEqual(objectchange.prechange_data['vc_position'], 1)
+        self.assertIsNone(objectchange.prechange_data['vc_priority'])
+        self.assertEqual(objectchange.postchange_data['vc_position'], 1)
+        self.assertEqual(objectchange.postchange_data['vc_priority'], 20)
+
+        # Unchanged members should not be logged
+        self.assertFalse(ObjectChange.objects.filter(
+            changed_object_type=ContentType.objects.get_for_model(Device),
+            changed_object_id__in=[m.pk for m in members[1:]],
+        ).exists())
+
+    @tag('regression')  # Ref: #23486
+    def test_delete_records_member_prechange_data(self):
+        self.add_permissions('dcim.delete_virtualchassis')
+        vc = VirtualChassis.objects.get(name='VC1')
+        members = list(vc.members.order_by('vc_position'))
+
+        url = reverse('dcim:virtualchassis_delete', kwargs={'pk': vc.pk})
+        response = self.client.post(url, {'confirm': True})
+        self.assertHttpStatus(response, 302)
+
+        # Each member should have a single change record capturing its detachment from the VC
+        for member in members:
+            objectchange = ObjectChange.objects.get(
+                changed_object_type=ContentType.objects.get_for_model(Device),
+                changed_object_id=member.pk,
+            )
+            self.assertEqual(objectchange.action, ObjectChangeActionChoices.ACTION_UPDATE)
+            self.assertEqual(objectchange.prechange_data['virtual_chassis'], vc.pk)
+            self.assertEqual(objectchange.prechange_data['vc_position'], member.vc_position)
+            self.assertIsNone(objectchange.postchange_data['virtual_chassis'])
+            self.assertIsNone(objectchange.postchange_data['vc_position'])
+
 
 class PowerPanelTestCase(ViewTestCases.PrimaryObjectViewTestCase):
     model = PowerPanel

netbox/dcim/views.py

@@ -5236,6 +5236,10 @@ class VirtualChassisEditView(ObjectPermissionRequiredMixin, GetReturnURLMixin, V
         vc_form.fields['master'].queryset = members_queryset
         formset = VCMemberFormSet(request.POST, queryset=members_queryset)
 
+        # Snapshot each member before the formset applies the submitted changes
+        for form in formset.initial_forms:
+            form.instance.snapshot()
+
         if vc_form.is_valid() and formset.is_valid():
             virtual_chassis._changelog_message = vc_form.cleaned_data.pop('changelog_message', '')
 
@@ -5244,14 +5248,10 @@ class VirtualChassisEditView(ObjectPermissionRequiredMixin, GetReturnURLMixin, V
                 # Save the VirtualChassis
                 vc_form.save()
 
-                # Nullify the vc_position of each member first to allow reordering without raising an IntegrityError on
-                # duplicate positions. Then save each member instance.
+                # Nullify the vc_position of each member first (without signals) to allow reordering without raising
+                # an IntegrityError on duplicate positions. Then save each member instance.
                 members = formset.save(commit=False)
-                devices = Device.objects.filter(pk__in=[m.pk for m in members])
-                for device in devices:
-                    device.snapshot()
-                    device.vc_position = None
-                    device.save()
+                Device.objects.filter(pk__in=[m.pk for m in members]).update(vc_position=None)
                 for member in members:
                     member.save()