Address PR review feedback for #20054 bulk error correlation

- Use pre-captured `pk` consistently in perform_bulk_destroy error path
- Add comment clarifying the `if results:` sentinel in bulk_update
- Add per-field atomicity assertion to test_bulk_update_objects_validation_error
- Use ID-keyed dict instead of positional index in test_bulk_delete_objects_protected

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

netbox/dcim/tests/test_api.py

@@ -492,13 +492,16 @@ class SiteTestCase(APIViewTestCases.APIViewTestCase):
         self.assertIn('detail', response.data)
         self.assertIn('results', response.data)
         self.assertEqual(len(response.data['results']), 2)
-        # First site (no dependents) would have succeeded
-        self.assertEqual(response.data['results'][0]['id'], site1.pk)
-        self.assertEqual(response.data['results'][0]['status'], 'ok')
-        # Second site (has Device) should have failed
-        self.assertEqual(response.data['results'][1]['id'], site2.pk)
-        self.assertEqual(response.data['results'][1]['status'], 'error')
-        self.assertIn('errors', response.data['results'][1])
+
+        # Index results by ID to avoid relying on queryset ordering
+        results_by_id = {r['id']: r for r in response.data['results']}
+        self.assertIn(site1.pk, results_by_id)
+        self.assertIn(site2.pk, results_by_id)
+        # Site 1 (no dependents) would have succeeded
+        self.assertEqual(results_by_id[site1.pk]['status'], 'ok')
+        # Site 2 (has Device) should have failed
+        self.assertEqual(results_by_id[site2.pk]['status'], 'error')
+        self.assertIn('errors', results_by_id[site2.pk])
 
         # Verify that no sites were actually deleted (transaction rolled back)
         self.assertTrue(Site.objects.filter(pk=site1.pk).exists(), 'Site 1 should not have been deleted')

netbox/netbox/api/viewsets/mixins.py

@@ -247,6 +247,8 @@ class BulkUpdateModelMixin:
 
         object_pks, results = self.perform_bulk_update(qs, update_data, partial=partial)
 
+        # perform_bulk_update returns an empty list on full success; non-empty means at least one
+        # object failed validation and the full results list (with per-object status) is populated.
         if results:
             failed_count = sum(1 for r in results if r['status'] == 'error')
             return Response(
@@ -380,7 +382,7 @@ class BulkDestroyModelMixin:
                     if n > 10:
                         objects_str += f', and {n - 10} more'
                     results.append({
-                        'id': obj.pk,
+                        'id': pk,
                         'status': 'error',
                         'errors': {'detail': f'Unable to delete. {n} dependent object(s): {objects_str}'},
                     })

netbox/utilities/testing/api.py

@@ -568,6 +568,10 @@ class APIViewTestCases:
                 {'id': id_list[0], **self.bulk_update_data},
                 {'id': id_list[1], **self.bulk_update_invalid_data},
             ]
+
+            # Snapshot field values before the request so we can verify atomicity afterward
+            instance0_before = self._get_queryset().get(pk=id_list[0])
+
             response = self.client.patch(self._get_list_url(), data, format='json', **self.header)
 
             self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
@@ -580,6 +584,17 @@ class APIViewTestCases:
             self.assertEqual(response.data['results'][1]['status'], 'error')
             self.assertIn('errors', response.data['results'][1])
 
+            # Verify atomicity: object 0 passed validation but must not have been modified
+            instance0_after = self._get_queryset().get(pk=id_list[0])
+            for field in self.bulk_update_data:
+                if field in ('changelog_message', 'add_tags', 'remove_tags'):
+                    continue
+                self.assertEqual(
+                    getattr(instance0_after, field, None),
+                    getattr(instance0_before, field, None),
+                    f'Field {field!r} of object {id_list[0]} was modified — atomic rollback may be broken',
+                )
+
     class DeleteObjectViewTestCase(APITestCase):
 
         def test_delete_object_without_permission(self):