diff --git a/awx/api/permissions.py b/awx/api/permissions.py index 09e6f0f1bc..34ee7f76fb 100644 --- a/awx/api/permissions.py +++ b/awx/api/permissions.py @@ -198,8 +198,8 @@ class TaskPermission(ModelAccessPermission): class WorkflowApprovalPermission(ModelAccessPermission): ''' - Permission check used by workflow approval and deny views - to determine who can has access to approve and deny paused workflow nodes + Permission check used by workflow `approval` and `deny` views to determine + who has access to approve and deny paused workflow nodes ''' def check_post_permissions(self, request, view, obj=None): diff --git a/awx/api/serializers.py b/awx/api/serializers.py index e20e191824..158b74c024 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3473,15 +3473,6 @@ class WorkflowApprovalTemplateSerializer(UnifiedJobTemplateSerializer): return res -class WorkflowJobTemplateApprovalSerializer(UnifiedJobTemplateSerializer): - class Meta: - model = WorkflowApprovalTemplate - fields = ('*',) - - def post(self, obj): - return # POST only!!! - - class LaunchConfigurationBaseSerializer(BaseSerializer): scm_branch = serializers.CharField(allow_blank=True, allow_null=True, required=False, default=None) job_type = serializers.ChoiceField(allow_blank=True, allow_null=True, required=False, default=None, diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index cde1ccc6bc..0ce6a6d72f 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -4473,7 +4473,7 @@ class WorkflowApprovalApprove(RetrieveAPIView): if not request.user.can_access(models.WorkflowApproval, 'approve_or_deny', obj): raise PermissionDenied(detail=_("User does not have permission to approve or deny this workflow.")) if obj.status != 'pending': - return Response("This workflow step has already been approved or denied.", status=status.HTTP_400_BAD_REQUEST) + return Response({"error": _("This workflow step has already been approved or denied.")}, status=status.HTTP_400_BAD_REQUEST) obj.approve(request) return Response(status=status.HTTP_204_NO_CONTENT) @@ -4488,6 +4488,6 @@ class WorkflowApprovalDeny(RetrieveAPIView): if not request.user.can_access(models.WorkflowApproval, 'approve_or_deny', obj): raise PermissionDenied(detail=_("User does not have permission to approve or deny this workflow.")) if obj.status != 'pending': - return Response("This workflow step has already been approved or denied.", status=status.HTTP_400_BAD_REQUEST) + return Response({"error": _("This workflow step has already been approved or denied.")}, status=status.HTTP_400_BAD_REQUEST) obj.deny(request) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/awx/main/access.py b/awx/main/access.py index e81b69e16b..e6181afb30 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -2809,8 +2809,6 @@ class WorkflowApprovalAccess(BaseAccess): self.user, 'read_role')) def can_approve_or_deny(self, obj): - if obj.status != 'pending': - return False if self.user in obj.workflow_job_template.approval_role or self.user.is_superuser: return True diff --git a/awx/main/scheduler/task_manager.py b/awx/main/scheduler/task_manager.py index c3d129cf46..df02d6f030 100644 --- a/awx/main/scheduler/task_manager.py +++ b/awx/main/scheduler/task_manager.py @@ -527,10 +527,11 @@ class TaskManager(): if task.timeout == 0: continue if (now - task.created) >= approval_timeout_seconds: - logger.warn("The approval node {} ({}) has expired after {} seconds.".format(task.name, task.pk, task.timeout)) + timeout_message = "The approval node {} ({}) has expired after {} seconds.".format(task.name, task.pk, task.timeout) + logger.warn(timeout_message) task.timed_out = True task.status = 'failed' - task.job_explanation = _("This approval node has timed out.") + task.job_explanation = _(timeout_message) task.save(update_fields=['status', 'job_explanation', 'timed_out']) def calculate_capacity_consumed(self, tasks): diff --git a/awx/main/tests/functional/api/test_workflow_node.py b/awx/main/tests/functional/api/test_workflow_node.py index 04b7780815..64c22898df 100644 --- a/awx/main/tests/functional/api/test_workflow_node.py +++ b/awx/main/tests/functional/api/test_workflow_node.py @@ -152,7 +152,7 @@ class TestApprovalNodes(): node = wfjt.workflow_nodes.create(unified_job_template=job_template) url = reverse('api:workflow_job_template_node_create_approval', kwargs={'pk': node.pk, 'version': 'v2'}) - post(url, {'name': 'Approve/Deny Test1', 'description': '', 'timeout': 0}, + post(url, {'name': 'Approve Test', 'description': '', 'timeout': 0}, user=admin_user, expect=200) post(reverse('api:workflow_job_template_launch', kwargs={'pk': wfjt.pk}), user=admin_user, expect=201) @@ -161,7 +161,7 @@ class TestApprovalNodes(): TaskManager().schedule() wfj_node = wf_job.workflow_nodes.first() approval = wfj_node.job - assert approval.name == 'Approve/Deny Test1' + assert approval.name == 'Approve Test' post(reverse('api:workflow_approval_approve', kwargs={'pk': approval.pk}), user=admin_user, expect=204) # Test that there is an activity stream entry that was created for the "approve" action. @@ -171,7 +171,7 @@ class TestApprovalNodes(): assert WorkflowApproval.objects.get(pk=approval.pk).status == 'successful' assert qs.operation == 'update' post(reverse('api:workflow_approval_approve', kwargs={'pk': approval.pk}), - user=admin_user, expect=403) + user=admin_user, expect=400) @pytest.mark.django_db def test_approval_node_deny(self, post, admin_user, job_template): @@ -182,7 +182,7 @@ class TestApprovalNodes(): node = wfjt.workflow_nodes.create(unified_job_template=job_template) url = reverse('api:workflow_job_template_node_create_approval', kwargs={'pk': node.pk, 'version': 'v2'}) - post(url, {'name': 'Approve/Deny Test2', 'description': '', 'timeout': 0}, + post(url, {'name': 'Deny Test', 'description': '', 'timeout': 0}, user=admin_user, expect=200) post(reverse('api:workflow_job_template_launch', kwargs={'pk': wfjt.pk}), user=admin_user, expect=201) @@ -191,7 +191,7 @@ class TestApprovalNodes(): TaskManager().schedule() wfj_node = wf_job.workflow_nodes.first() approval = wfj_node.job - assert approval.name == 'Approve/Deny Test2' + assert approval.name == 'Deny Test' post(reverse('api:workflow_approval_deny', kwargs={'pk': approval.pk}), user=admin_user, expect=204) # Test that there is an activity stream entry that was created for the "deny" action. @@ -201,7 +201,7 @@ class TestApprovalNodes(): assert WorkflowApproval.objects.get(pk=approval.pk).status == 'failed' assert qs.operation == 'update' post(reverse('api:workflow_approval_deny', kwargs={'pk': approval.pk}), - user=admin_user, expect=403) + user=admin_user, expect=400) def test_approval_node_cleanup(self, post, approval_node, admin_user, get): workflow_job_template = WorkflowJobTemplate.objects.create()