diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index 57a8c3cbd..1b8ab8498 100644 --- a/netbox/dcim/tests/test_api.py +++ b/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') diff --git a/netbox/netbox/api/viewsets/mixins.py b/netbox/netbox/api/viewsets/mixins.py index cb4061b20..138096449 100644 --- a/netbox/netbox/api/viewsets/mixins.py +++ b/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}'}, }) diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py index 6aaee1f45..99ea69a39 100644 --- a/netbox/utilities/testing/api.py +++ b/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):