delete orphaned labels when jt or j deleted

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 = Label.objects.get(id=sub_id)
-            Label.objects.get(id=sub_id).delete()
+
+        if label.is_detached():
+            label.delete()
 
         return res
 

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(self):
-    def is_detached(id):
         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
 

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
 

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()
 

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
+

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()