From 34ce5544e70c0b7d6df91acf16bc9595eb8fec40 Mon Sep 17 00:00:00 2001 From: Valentin Rigal <rigal@teklia.com> Date: Wed, 27 Mar 2024 11:53:41 +0100 Subject: [PATCH] Suggestions --- arkindex/ponos/api.py | 12 +++++++++--- arkindex/ponos/tests/test_api.py | 27 +++++++++++++++++++++------ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/arkindex/ponos/api.py b/arkindex/ponos/api.py index bb15286ce3..5338c65487 100644 --- a/arkindex/ponos/api.py +++ b/arkindex/ponos/api.py @@ -184,7 +184,7 @@ class TaskUpdate(UpdateAPIView): tags=["ponos"], description=dedent( """ - Restart a task by creating a fresh copy and moves dependent tasks to the new one. + Restart a task by creating a fresh copy and moving dependent tasks to the new one. Scenario restarting `my_worker` task: ``` @@ -196,7 +196,7 @@ class TaskUpdate(UpdateAPIView): my_worker_2 → other worker ``` - Requires an **admin** access to task's process. + Requires an **admin** access to the task's process. The task must be in a final state to be restarted. """ ), @@ -209,7 +209,7 @@ class TaskRestart(ProcessACLMixin, CreateAPIView): def get_task(self): task = get_object_or_404( - Task.objects.prefetch_related("parents").select_related("process"), + Task.objects.prefetch_related("parents").select_related("process__corpus"), pk=self.kwargs["pk"], ) access_level = self.process_access_level(task.process) @@ -223,6 +223,12 @@ class TaskRestart(ProcessACLMixin, CreateAPIView): raise ValidationError( detail="Task's state must be in a final state to be restarted." ) + # TODO Check the original_task_id field directly once it is implemented + # https://gitlab.teklia.com/arkindex/frontend/-/issues/1383 + if task.process.tasks.filter(run=task.run, slug=self.increment(task.slug)).exists(): + raise ValidationError( + detail="This task has already been restarted" + ) return task def increment(self, name): diff --git a/arkindex/ponos/tests/test_api.py b/arkindex/ponos/tests/test_api.py index f4ddb01008..5c05f00140 100644 --- a/arkindex/ponos/tests/test_api.py +++ b/arkindex/ponos/tests/test_api.py @@ -581,13 +581,12 @@ class TestAPI(FixtureAPITestCase): ) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - @expectedFailure - def test_restart_task_forbidden(self): + @patch("arkindex.project.mixins.get_max_level") + def test_restart_task_forbidden(self, get_max_level_mock): """An admin access to the process is required""" + get_max_level_mock.return_value = Role.Guest.value self.client.force_login(self.user) - self.corpus.memberships.filter(user=self.user).update(level=Role.Guest.value) - self.process.save() - with self.assertNumQueries(10): + with self.assertNumQueries(7): response = self.client.post( reverse("api:task-restart", kwargs={"pk": str(self.task1.id)}) ) @@ -599,7 +598,7 @@ class TestAPI(FixtureAPITestCase): def test_restart_task_non_final_state(self): self.client.force_login(self.user) - with self.assertNumQueries(8): + with self.assertNumQueries(7): response = self.client.post( reverse("api:task-restart", kwargs={"pk": str(self.task1.id)}) ) @@ -609,6 +608,22 @@ class TestAPI(FixtureAPITestCase): ["Task's state must be in a final state to be restarted."], ) + def test_restart_task_already_restarted(self): + self.client.force_login(self.user) + self.task2.slug = self.task1.slug + "_restart1" + self.task2.save() + self.task1.state = State.Completed.value + self.task1.save() + with self.assertNumQueries(8): + response = self.client.post( + reverse("api:task-restart", kwargs={"pk": str(self.task1.id)}) + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertListEqual( + response.json(), + ["This task has already been restarted"], + ) + @patch("arkindex.project.aws.s3") def test_restart_task(self, s3_mock): """ -- GitLab