delete label on last disassociate

awx/api/generics.py

@@ -25,6 +25,7 @@ from rest_framework import views
 
 # AWX
 from awx.main.models import *  # noqa
+from awx.main.models import Label
 from awx.main.utils import * # noqa
 from awx.api.serializers import ResourceAccessListElementSerializer
 
@@ -35,7 +36,8 @@ __all__ = ['APIView', 'GenericAPIView', 'ListAPIView', 'SimpleListAPIView',
            'RetrieveUpdateDestroyAPIView', 'DestroyAPIView',
            'SubDetailAPIView',
            'ResourceAccessList',
-           'ParentMixin',]
+           'ParentMixin',
+           'DeleteLastUnattachLabelMixin',]
 
 logger = logging.getLogger('awx.api.generics')
 
@@ -399,12 +401,15 @@ class SubListCreateAttachDetachAPIView(SubListCreateAPIView):
         else:
             return Response(status=status.HTTP_204_NO_CONTENT)
 
-    def unattach(self, request, *args, **kwargs):
+    def unattach_validate(self, request):
         sub_id = request.data.get('id', None)
+        res = None
         if not sub_id:
             data = dict(msg='"id" is required to disassociate')
-            return Response(data, status=status.HTTP_400_BAD_REQUEST)
+            res = Response(data, status=status.HTTP_400_BAD_REQUEST)
+        return (sub_id, res)
 
+    def unattach_by_id(self, request, sub_id):
         parent = self.get_parent_object()
         parent_key = getattr(self, 'parent_key', None)
         relationship = getattrd(parent, self.relationship)
@@ -421,6 +426,12 @@ class SubListCreateAttachDetachAPIView(SubListCreateAPIView):
 
         return Response(status=status.HTTP_204_NO_CONTENT)
 
+    def unattach(self, request, *args, **kwargs):
+        (sub_id, res) = self.unattach_validate(request)
+        if res:
+            return res
+        return self.unattach_by_id(request, sub_id)
+       
     def post(self, request, *args, **kwargs):
         if not isinstance(request.data, dict):
             return Response('invalid type for post data',
@@ -430,6 +441,19 @@ class SubListCreateAttachDetachAPIView(SubListCreateAPIView):
         else:
             return self.attach(request, *args, **kwargs)
 
+class DeleteLastUnattachLabelMixin(object):
+    def unattach(self, request, *args, **kwargs):
+        (sub_id, res) = super(DeleteLastUnattachLabelMixin, self).unattach_validate(request, *args, **kwargs)
+        if res:
+            return res
+
+        res = super(DeleteLastUnattachLabelMixin, self).unattach_by_id(request, sub_id)
+
+        if Label.is_detached(sub_id):
+            Label.objects.get(id=sub_id).delete()
+
+        return res
+
 class SubDetailAPIView(generics.RetrieveAPIView, GenericAPIView, ParentMixin):
     pass
 

awx/api/views.py

@@ -3359,7 +3359,7 @@ class NotificationDetail(RetrieveAPIView):
     serializer_class = NotificationSerializer
     new_in_300 = True
 
-class LabelList(ListCreateAPIView):
+class LabelList(ListCreateAPIView, DeleteLastUnattachLabelMixin):
 
     model = Label
     serializer_class = LabelSerializer

awx/main/models/label.py

@@ -39,3 +39,13 @@ class Label(CommonModelNameNotUnique):
                 jobtemplate_labels__isnull=True
             )
 
+    @staticmethod
+    def is_detached(id):
+        return bool(
+            Label.objects.filter(
+                id=id, 
+                unifiedjob_labels__isnull=True,
+                unifiedjobtemplate_labels__isnull=True
+            ).count())
+
+

awx/main/tests/unit/api/test_generics.py

@@ -1,13 +1,14 @@
 
 # Python
 import pytest
+import mock
 
 # DRF
 from rest_framework import status
 from rest_framework.response import Response
 
 # AWX
-from awx.api.generics import ParentMixin, SubListCreateAttachDetachAPIView
+from awx.api.generics import ParentMixin, SubListCreateAttachDetachAPIView, DeleteLastUnattachLabelMixin
 
 @pytest.fixture
 def get_object_or_404(mocker):
@@ -37,7 +38,7 @@ def parent_relationship_factory(mocker):
         return (serializer, mock_parent_relationship)
     return rf
 
-# TODO: Test create and associate failure (i.e. id doesn't exist or record already exists)
+# TODO: Test create and associate failure (i.e. id doesn't exist, record already exists, permission denied)
 # TODO: Mock and check return (Response)
 class TestSubListCreateAttachDetachAPIView:
     def test_attach_create_and_associate(self, mocker, get_object_or_400, parent_relationship_factory, mock_response_new):
@@ -65,6 +66,97 @@ class TestSubListCreateAttachDetachAPIView:
         mock_parent_relationship.wife.add.assert_called_with(get_object_or_400.return_value)
         mock_response_new.assert_called_with(Response, status=status.HTTP_204_NO_CONTENT)
 
+    def test_unattach_validate_ok(self, mocker):
+        mock_request = mocker.MagicMock(data=dict(id=1))
+        serializer = SubListCreateAttachDetachAPIView()
+
+        (sub_id, res) = serializer.unattach_validate(mock_request)
+
+        assert sub_id == 1
+        assert res is None
+    
+    def test_unattach_validate_missing_id(self, mocker):
+        mock_request = mocker.MagicMock(data=dict())
+        serializer = SubListCreateAttachDetachAPIView()
+
+        (sub_id, res) = serializer.unattach_validate(mock_request)
+
+        assert sub_id is None
+        assert type(res) is Response
+ 
+    def test_unattach_by_id_ok(self, mocker, parent_relationship_factory, get_object_or_400):
+        (serializer, mock_parent_relationship) = parent_relationship_factory(SubListCreateAttachDetachAPIView, 'wife')
+        mock_request = mocker.MagicMock()
+        mock_sub = mocker.MagicMock(name="object to unattach")
+        get_object_or_400.return_value = mock_sub
+
+        res = serializer.unattach_by_id(mock_request, 1)
+
+        assert type(res) is Response
+        assert res.status_code == status.HTTP_204_NO_CONTENT
+        mock_parent_relationship.wife.remove.assert_called_with(mock_sub)
+
+    def test_unattach_ok(self, mocker):
+        mock_request = mocker.MagicMock()
+        mock_sub_id = mocker.MagicMock()
+        view = SubListCreateAttachDetachAPIView()
+        view.unattach_validate = mocker.MagicMock()
+        view.unattach_by_id = mocker.MagicMock()
+        view.unattach_validate.return_value = (mock_sub_id, None)
+
+        view.unattach(mock_request)
+
+        view.unattach_validate.assert_called_with(mock_request)
+        view.unattach_by_id.assert_called_with(mock_request, mock_sub_id)
+
+    def test_unattach_invalid(self, mocker):
+        mock_request = mocker.MagicMock()
+        mock_res = mocker.MagicMock()
+        view = SubListCreateAttachDetachAPIView()
+        view.unattach_validate = mocker.MagicMock()
+        view.unattach_by_id = mocker.MagicMock()
+        view.unattach_validate.return_value = (None, mock_res)
+
+        view.unattach(mock_request)
+
+        view.unattach_validate.assert_called_with(mock_request)
+        view.unattach_by_id.assert_not_called()
+
+class TestDeleteLastUnattachLabelMixin:
+    @mock.patch('awx.api.generics.super')
+    def test_unattach_ok(self, super, mocker):
+        mock_request = mocker.MagicMock()
+        mock_sub_id = mocker.MagicMock()
+        super.return_value = super
+        super.unattach_validate = mocker.MagicMock(return_value=(mock_sub_id, None))
+        super.unattach_by_id = mocker.MagicMock()
+        mock_label = mocker.patch('awx.api.generics.Label')
+        mock_label.objects.get.return_value = mock_label
+        mock_label.is_detached.return_value = True
+
+        view = DeleteLastUnattachLabelMixin()
+
+        view.unattach(mock_request, None, None)
+
+        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.objects.get.assert_called_with(id=mock_sub_id)
+        mock_label.delete.assert_called_with()
+
+    @mock.patch('awx.api.generics.super')
+    def test_unattach_fail(self, super, mocker):
+        mock_request = mocker.MagicMock()
+        mock_response = mocker.MagicMock()
+        super.return_value = super
+        super.unattach_validate = mocker.MagicMock(return_value=(None, mock_response))
+        view = DeleteLastUnattachLabelMixin()
+
+        res = view.unattach(mock_request, None, None)
+
+        super.unattach_validate.assert_called_with(mock_request, None, None)
+        assert mock_response == res
+
 class TestParentMixin:
     def test_get_parent_object(self, mocker, get_object_or_404):
         parent_mixin = ParentMixin()

awx/main/tests/unit/models/test_label.py

@@ -8,3 +8,25 @@ def test_get_orphaned_labels(mocker):
 
     assert mock_query_set == ret
     Label.objects.filter.assert_called_with(organization=None, jobtemplate_labels__isnull=True)
+
+def test_is_detached(mocker):
+    mock_query_set = mocker.MagicMock()
+    Label.objects.filter = mocker.MagicMock(return_value=mock_query_set)
+    mock_query_set.count.return_value = 1
+
+    ret = Label.is_detached(37)
+
+    assert ret is True
+    Label.objects.filter.assert_called_with(id=37, unifiedjob_labels__isnull=True, unifiedjobtemplate_labels__isnull=True)
+    mock_query_set.count.assert_called_with()
+
+def test_is_detached_not(mocker):
+    mock_query_set = mocker.MagicMock()
+    Label.objects.filter = mocker.MagicMock(return_value=mock_query_set)
+    mock_query_set.count.return_value = 0
+
+    ret = Label.is_detached(37)
+
+    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()