From 2f18a9f2c142728639c63f2dfc42568766529fb0 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Mon, 25 Apr 2016 17:14:09 -0400 Subject: [PATCH] delete label on last disassociate --- awx/api/generics.py | 30 +++++++- awx/api/views.py | 2 +- awx/main/models/label.py | 10 +++ awx/main/tests/unit/api/test_generics.py | 96 +++++++++++++++++++++++- awx/main/tests/unit/models/test_label.py | 22 ++++++ 5 files changed, 154 insertions(+), 6 deletions(-) diff --git a/awx/api/generics.py b/awx/api/generics.py index cdd10e497d..08ec1dc356 100644 --- a/awx/api/generics.py +++ b/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 diff --git a/awx/api/views.py b/awx/api/views.py index 3e70cb8099..fa4743887b 100644 --- a/awx/api/views.py +++ b/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 diff --git a/awx/main/models/label.py b/awx/main/models/label.py index 8458e4cb23..376e5249bf 100644 --- a/awx/main/models/label.py +++ b/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()) + + diff --git a/awx/main/tests/unit/api/test_generics.py b/awx/main/tests/unit/api/test_generics.py index 42fe141c49..50ece17556 100644 --- a/awx/main/tests/unit/api/test_generics.py +++ b/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() diff --git a/awx/main/tests/unit/models/test_label.py b/awx/main/tests/unit/models/test_label.py index c5c6e50b91..f96f5f54af 100644 --- a/awx/main/tests/unit/models/test_label.py +++ b/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()