diff --git a/awx/api/generics.py b/awx/api/generics.py index 08ec1dc356..326b6032d5 100644 --- a/awx/api/generics.py +++ b/awx/api/generics.py @@ -449,8 +449,10 @@ class DeleteLastUnattachLabelMixin(object): res = super(DeleteLastUnattachLabelMixin, self).unattach_by_id(request, sub_id) - if Label.is_detached(sub_id): - Label.objects.get(id=sub_id).delete() + label = Label.objects.get(id=sub_id) + + if label.is_detached(): + label.delete() return res diff --git a/awx/main/models/label.py b/awx/main/models/label.py index 376e5249bf..af9a2241b7 100644 --- a/awx/main/models/label.py +++ b/awx/main/models/label.py @@ -8,6 +8,7 @@ from django.utils.translation import ugettext_lazy as _ # AWX from awx.main.models.base import CommonModelNameNotUnique +from awx.main.models.unified_jobs import UnifiedJobTemplate, UnifiedJob __all__ = ('Label', ) @@ -39,13 +40,19 @@ class Label(CommonModelNameNotUnique): jobtemplate_labels__isnull=True ) - @staticmethod - def is_detached(id): + def is_detached(self): return bool( Label.objects.filter( - id=id, + id=self.id, unifiedjob_labels__isnull=True, unifiedjobtemplate_labels__isnull=True ).count()) + def is_candidate_for_detach(self): + c1 = UnifiedJob.objects.filter(labels__in=[self.id]).count() + c2 = UnifiedJobTemplate.objects.filter(labels__in=[self.id]).count() + if (c1 + c2 - 1) == 0: + return True + else: + return False diff --git a/awx/main/signals.py b/awx/main/signals.py index 068efb4ece..9b574fea65 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -156,6 +156,10 @@ def org_admin_edit_members(instance, action, model, reverse, pk_set, **kwargs): if action == 'pre_remove': instance.content_object.admin_role.children.remove(user.admin_role) +def cleanup_detached_labels_on_deleted_parent(sender, instance, **kwargs): + for l in instance.labels.all(): + if l.is_candidate_for_detach(): + l.delete() post_save.connect(emit_update_inventory_on_created_or_deleted, sender=Host) post_delete.connect(emit_update_inventory_on_created_or_deleted, sender=Host) @@ -175,7 +179,8 @@ m2m_changed.connect(rebuild_role_ancestor_list, Role.parents.through) m2m_changed.connect(org_admin_edit_members, Role.members.through) post_save.connect(sync_superuser_status_to_rbac, sender=User) post_save.connect(create_user_role, sender=User) - +pre_delete.connect(cleanup_detached_labels_on_deleted_parent, sender=UnifiedJob) +pre_delete.connect(cleanup_detached_labels_on_deleted_parent, sender=UnifiedJobTemplate) # Migrate hosts, groups to parent group(s) whenever a group is deleted diff --git a/awx/main/tests/unit/api/test_generics.py b/awx/main/tests/unit/api/test_generics.py index 50ece17556..42d755abcf 100644 --- a/awx/main/tests/unit/api/test_generics.py +++ b/awx/main/tests/unit/api/test_generics.py @@ -140,7 +140,7 @@ class TestDeleteLastUnattachLabelMixin: super.unattach_validate.assert_called_with(mock_request, None, None) super.unattach_by_id.assert_called_with(mock_request, mock_sub_id) - mock_label.is_detached.assert_called_with(mock_sub_id) + mock_label.is_detached.assert_called_with() mock_label.objects.get.assert_called_with(id=mock_sub_id) mock_label.delete.assert_called_with() diff --git a/awx/main/tests/unit/models/test_label.py b/awx/main/tests/unit/models/test_label.py index f96f5f54af..20da73e9ad 100644 --- a/awx/main/tests/unit/models/test_label.py +++ b/awx/main/tests/unit/models/test_label.py @@ -1,4 +1,8 @@ +import pytest + from awx.main.models.label import Label +from awx.main.models.unified_jobs import UnifiedJobTemplate, UnifiedJob + def test_get_orphaned_labels(mocker): mock_query_set = mocker.MagicMock() @@ -14,7 +18,8 @@ def test_is_detached(mocker): Label.objects.filter = mocker.MagicMock(return_value=mock_query_set) mock_query_set.count.return_value = 1 - ret = Label.is_detached(37) + label = Label(id=37) + ret = label.is_detached() assert ret is True Label.objects.filter.assert_called_with(id=37, unifiedjob_labels__isnull=True, unifiedjobtemplate_labels__isnull=True) @@ -25,8 +30,36 @@ def test_is_detached_not(mocker): Label.objects.filter = mocker.MagicMock(return_value=mock_query_set) mock_query_set.count.return_value = 0 - ret = Label.is_detached(37) + label = Label(id=37) + ret = label.is_detached() assert ret is False Label.objects.filter.assert_called_with(id=37, unifiedjob_labels__isnull=True, unifiedjobtemplate_labels__isnull=True) mock_query_set.count.assert_called_with() + +@pytest.mark.parametrize("jt_count,j_count,expected", [ + (1, 0, True), + (0, 1, True), + (1, 1, False), +]) +def test_is_candidate_for_detach(mocker, jt_count, j_count, expected): + mock_job_qs = mocker.MagicMock() + mock_job_qs.count = mocker.MagicMock(return_value=j_count) + UnifiedJob.objects = mocker.MagicMock() + UnifiedJob.objects.filter = mocker.MagicMock(return_value=mock_job_qs) + + mock_jt_qs = mocker.MagicMock() + mock_jt_qs.count = mocker.MagicMock(return_value=jt_count) + UnifiedJobTemplate.objects = mocker.MagicMock() + UnifiedJobTemplate.objects.filter = mocker.MagicMock(return_value=mock_jt_qs) + + label = Label(id=37) + ret = label.is_candidate_for_detach() + + UnifiedJob.objects.filter.assert_called_with(labels__in=[label.id]) + UnifiedJobTemplate.objects.filter.assert_called_with(labels__in=[label.id]) + mock_job_qs.count.assert_called_with() + mock_jt_qs.count.assert_called_with() + + assert ret is expected + diff --git a/awx/main/tests/unit/test_signals.py b/awx/main/tests/unit/test_signals.py new file mode 100644 index 0000000000..c3830ee525 --- /dev/null +++ b/awx/main/tests/unit/test_signals.py @@ -0,0 +1,17 @@ +from awx.main import signals + +class TestCleanupDetachedLabels: + def test_cleanup_detached_labels_on_deleted_parent(self, mocker): + mock_labels = [mocker.MagicMock(), mocker.MagicMock()] + mock_instance = mocker.MagicMock() + mock_instance.labels.all = mocker.MagicMock() + mock_instance.labels.all.return_value = mock_labels + mock_labels[0].is_candidate_for_detach.return_value = True + mock_labels[1].is_candidate_for_detach.return_value = False + + signals.cleanup_detached_labels_on_deleted_parent(None, mock_instance) + + mock_labels[0].is_candidate_for_detach.assert_called_with() + mock_labels[1].is_candidate_for_detach.assert_called_with() + mock_labels[0].delete.assert_called_with() + mock_labels[1].delete.assert_not_called()