From d652cbf9663bfb86d9fac202169e6922937107ef Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Wed, 19 May 2021 12:17:21 +0000
Subject: [PATCH] Raise a 409_CONFLICT when activity update fails

---
 arkindex/dataimport/api.py                     | 18 +++++++++++-------
 .../dataimport/tests/test_workeractivity.py    |  9 ++++++---
 arkindex/project/mixins.py                     |  6 ++++++
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/arkindex/dataimport/api.py b/arkindex/dataimport/api.py
index edbeed5ced..5b536c049a 100644
--- a/arkindex/dataimport/api.py
+++ b/arkindex/dataimport/api.py
@@ -72,6 +72,7 @@ from arkindex.dataimport.serializers.workers import (
 from arkindex.documents.models import Corpus, Element
 from arkindex.project.fields import ArrayRemove
 from arkindex.project.mixins import (
+    ConflictAPIException,
     CorpusACLMixin,
     CustomPaginationViewMixin,
     DeprecatedMixin,
@@ -1252,6 +1253,7 @@ class UpdateWorkerActivity(GenericAPIView):
     """
     permission_classes = (IsInternalUser, )
     serializer_class = WorkerActivitySerializer
+    queryset = WorkerActivity.objects.none()
 
     @cached_property
     def allowed_transitions(self):
@@ -1298,13 +1300,15 @@ class UpdateWorkerActivity(GenericAPIView):
         update_count = activity.update(state=state, process_id=process_id)
 
         if not update_count:
-            # As no row has been updated the provided data was incorrect
-            raise ValidationError({
-                '__all__': [
-                    'Either this worker activity does not exists '
-                    f"or updating the state to '{state}' is forbidden."
-                ]
-            })
+            # As no row has been updated the provided data was in conflict with the actual state
+            raise ConflictAPIException(
+                {
+                    '__all__': [
+                        'Either this worker activity does not exists '
+                        f"or updating the state to '{state}' is forbidden."
+                    ]
+                }
+            )
 
         return Response(serializer.data)
 
diff --git a/arkindex/dataimport/tests/test_workeractivity.py b/arkindex/dataimport/tests/test_workeractivity.py
index 994b975119..626d203553 100644
--- a/arkindex/dataimport/tests/test_workeractivity.py
+++ b/arkindex/dataimport/tests/test_workeractivity.py
@@ -144,6 +144,7 @@ class TestWorkerActivity(FixtureTestCase):
     def test_put_activity_wrong_worker_version(self):
         """
         Raises a generic error in case the worker version does not exists because a single SQL request is performed
+        The response is a HTTP_409_CONFLICT
         """
         self.client.force_login(self.internal_user)
         with self.assertNumQueries(4):
@@ -156,7 +157,7 @@ class TestWorkerActivity(FixtureTestCase):
                 },
                 content_type='application/json',
             )
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.status_code, status.HTTP_409_CONFLICT)
         self.assertDictEqual(response.json(), {
             '__all__': [
                 'Either this worker activity does not exists or '
@@ -167,6 +168,7 @@ class TestWorkerActivity(FixtureTestCase):
     def test_put_activity_element_unexisting(self):
         """
         Raises a generic error in case no activity exists for this element
+        The response is a HTTP_409_CONFLICT
         """
         self.client.force_login(self.internal_user)
         with self.assertNumQueries(4):
@@ -179,7 +181,7 @@ class TestWorkerActivity(FixtureTestCase):
                 },
                 content_type='application/json',
             )
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.status_code, status.HTTP_409_CONFLICT)
         self.assertDictEqual(response.json(), {
             '__all__': [
                 'Either this worker activity does not exists or '
@@ -244,6 +246,7 @@ class TestWorkerActivity(FixtureTestCase):
     def test_put_activity_forbidden_states(self):
         """
         Check state update is forbidden for some non consistant cases
+        The response is a HTTP_409_CONFLICT
         """
         queued, started, error, processed = (
             WorkerActivityState[state].value
@@ -269,7 +272,7 @@ class TestWorkerActivity(FixtureTestCase):
                     },
                     content_type='application/json',
                 )
-            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+            self.assertEqual(response.status_code, status.HTTP_409_CONFLICT)
             self.assertDictEqual(response.json(), {
                 '__all__': [
                     'Either this worker activity does not exists or '
diff --git a/arkindex/project/mixins.py b/arkindex/project/mixins.py
index 7fd28b510d..560e4633ef 100644
--- a/arkindex/project/mixins.py
+++ b/arkindex/project/mixins.py
@@ -3,6 +3,7 @@ from django.db.models import Q
 from django.shortcuts import get_object_or_404
 from django.views.decorators.cache import cache_page
 from drf_spectacular.utils import extend_schema, extend_schema_view
+from rest_framework import status
 from rest_framework.exceptions import APIException, PermissionDenied, ValidationError
 from rest_framework.serializers import CharField, Serializer
 
@@ -242,6 +243,11 @@ class DeprecatedExceptionSerializer(Serializer):
     detail = CharField()
 
 
+class ConflictAPIException(APIException):
+    status_code = status.HTTP_409_CONFLICT
+    default_code = 'conflict'
+
+
 class DeprecatedAPIException(APIException):
     status_code = 410
     default_detail = 'This endpoint has been deprecated.'
-- 
GitLab