From 2521bebc3db529eaa1a1d5ebefaf4984bc00229c Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Tue, 22 Feb 2022 11:59:25 +0000
Subject: [PATCH] Add the workflow completion date to ListDataImports

---
 arkindex/dataimport/api.py                 |  7 +--
 arkindex/dataimport/serializers/imports.py |  5 ++-
 arkindex/dataimport/tests/test_imports.py  |  9 +++-
 arkindex/project/tests/test_ponos_view.py  | 51 ++++++++++++----------
 4 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/arkindex/dataimport/api.py b/arkindex/dataimport/api.py
index 3b8044fa90..06d5043b57 100644
--- a/arkindex/dataimport/api.py
+++ b/arkindex/dataimport/api.py
@@ -7,7 +7,7 @@ from django.conf import settings
 from django.core.mail import send_mail
 from django.db import transaction
 from django.db.models import CharField, Count, F, Max, Q, Value
-from django.db.models.functions import Cast, Concat, Greatest, Now, NullIf
+from django.db.models.functions import Cast, Coalesce, Concat, Greatest, Now, NullIf
 from django.db.models.query import Prefetch
 from django.shortcuts import get_object_or_404
 from django.template.loader import render_to_string
@@ -195,9 +195,10 @@ class DataImportsList(ProcessACLMixin, ListAPIView):
         qs = self.readable_processes \
             .filter(filters) \
             .prefetch_related('workflow__tasks')
-        # Order workflow by date of last updated task in workflow
+        # Order workflow by completion date when available, or by date of last updated task in workflow
         qs = qs.annotate(
             last_date=Greatest(Max('workflow__tasks__updated'), 'updated'),
+            date_order=Coalesce('workflow__finished', 'last_date'),
             last_run=Max('workflow__tasks__run')
         )
 
@@ -220,7 +221,7 @@ class DataImportsList(ProcessACLMixin, ListAPIView):
                 state_query |= Q(workflow__tasks__isnull=True)
             qs = qs.filter(state_query).exclude(id__in=excluded_imports.values('id')).distinct()
 
-        return qs.order_by('-last_date')
+        return qs.order_by('-date_order')
 
 
 @extend_schema(tags=['imports'])
diff --git a/arkindex/dataimport/serializers/imports.py b/arkindex/dataimport/serializers/imports.py
index e9a3f06c43..7e289c6646 100644
--- a/arkindex/dataimport/serializers/imports.py
+++ b/arkindex/dataimport/serializers/imports.py
@@ -135,10 +135,11 @@ class DataImportSerializer(DataImportLightSerializer):
 class DataImportListSerializer(DataImportLightSerializer):
     created = serializers.DateTimeField(read_only=True)
     updated = serializers.DateTimeField(source='last_date', read_only=True)
+    finished = serializers.DateTimeField(source='workflow.finished', read_only=True, default=None)
 
     class Meta(DataImportLightSerializer.Meta):
-        fields = DataImportLightSerializer.Meta.fields + ('created', 'updated')
-        read_only_fields = DataImportLightSerializer.Meta.read_only_fields + ('created', 'updated')
+        fields = DataImportLightSerializer.Meta.fields + ('created', 'updated', 'finished')
+        read_only_fields = DataImportLightSerializer.Meta.read_only_fields + ('created', 'updated', 'finished')
 
 
 class DataImportFromFilesSerializer(serializers.Serializer):
diff --git a/arkindex/dataimport/tests/test_imports.py b/arkindex/dataimport/tests/test_imports.py
index 638a6e92a6..566c09068e 100644
--- a/arkindex/dataimport/tests/test_imports.py
+++ b/arkindex/dataimport/tests/test_imports.py
@@ -100,7 +100,12 @@ class TestImports(FixtureAPITestCase):
             'workflow': process.workflow and f'http://testserver/ponos/v1/workflow/{process.workflow.id}/',
             'activity_state': process.activity_state.value,
             'created': process.created.isoformat().replace('+00:00', 'Z'),
-            'updated': updated.isoformat().replace('+00:00', 'Z')
+            'updated': updated.isoformat().replace('+00:00', 'Z'),
+            'finished': (
+                process.workflow.finished.isoformat().replace('+00:00', 'Z')
+                if process.workflow and process.workflow.finished
+                else None
+            ),
         }
 
     def build_task(self, workflow_id, run, state, depth=1):
@@ -865,7 +870,7 @@ class TestImports(FixtureAPITestCase):
         self.elts_process.start()
         self.elts_process.workflow.tasks.all().update(state=State.Error)
         self.assertEqual(self.elts_process.state, State.Error)
-        with self.assertNumQueries(15):
+        with self.assertNumQueries(16):
             response = self.client.post(reverse('api:import-retry', kwargs={'pk': self.elts_process.id}))
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.elts_process.refresh_from_db()
diff --git a/arkindex/project/tests/test_ponos_view.py b/arkindex/project/tests/test_ponos_view.py
index 00e01c3ebc..5470e37901 100644
--- a/arkindex/project/tests/test_ponos_view.py
+++ b/arkindex/project/tests/test_ponos_view.py
@@ -75,9 +75,10 @@ class TestPonosView(FixtureAPITestCase):
             {'email': 'anonymous'}
         ))
         for user in non_internal_users:
-            self.client.force_login(user)
-            response = self.client.get(reverse('secret-details', kwargs={"name": "secret/name"}))
-            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+            with self.subTest(user=user):
+                self.client.force_login(user)
+                response = self.client.get(reverse('secret-details', kwargs={"name": "secret/name"}))
+                self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
 
     def test_list_agents_requires_login(self):
         """
@@ -129,20 +130,21 @@ class TestPonosView(FixtureAPITestCase):
         """
         test_cases = (
             (None, status.HTTP_403_FORBIDDEN, 1),
-            (self.creator, status.HTTP_200_OK, 9),
+            (self.creator, status.HTTP_200_OK, 11),
             (self.user, status.HTTP_403_FORBIDDEN, 9),
-            (self.superuser, status.HTTP_200_OK, 7),
-            (self.corpus_admin, status.HTTP_200_OK, 11),
+            (self.superuser, status.HTTP_200_OK, 10),
+            (self.corpus_admin, status.HTTP_200_OK, 15),
         )
         for user, status_code, requests_count in test_cases:
-            if user:
-                self.client.force_login(user)
-            with self.assertNumQueries(requests_count):
-                response = self.client.patch(
-                    reverse('ponos-task-update', kwargs={'pk': str(self.task.id)}),
-                    json={'state': 'stopping'}
-                )
-                self.assertEqual(response.status_code, status_code)
+            with self.subTest(user=user):
+                if user:
+                    self.client.force_login(user)
+                with self.assertNumQueries(requests_count):
+                    response = self.client.patch(
+                        reverse('ponos-task-update', kwargs={'pk': str(self.task.id)}),
+                        json={'state': 'stopping'}
+                    )
+                    self.assertEqual(response.status_code, status_code)
 
     def test_download_artifacts(self):
         """
@@ -152,19 +154,20 @@ class TestPonosView(FixtureAPITestCase):
         test_cases = (
             (None, status.HTTP_403_FORBIDDEN, 2),
             (self.creator, status.HTTP_302_FOUND, 6),
-            (self.user, status.HTTP_403_FORBIDDEN, 9),
+            (self.user, status.HTTP_403_FORBIDDEN, 10),
             (self.superuser, status.HTTP_302_FOUND, 4),
-            (self.corpus_admin, status.HTTP_302_FOUND, 8),
+            (self.corpus_admin, status.HTTP_302_FOUND, 10),
         )
         for user, status_code, requests_count in test_cases:
-            if user:
-                self.client.force_login(user)
-            with self.assertNumQueries(requests_count):
-                response = self.client.get(
-                    reverse('ponos-artifact-dl', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}),
-                    follow=False
-                )
-                self.assertEqual(response.status_code, status_code)
+            with self.subTest(user=user):
+                if user:
+                    self.client.force_login(user)
+                with self.assertNumQueries(requests_count):
+                    response = self.client.get(
+                        reverse('ponos-artifact-dl', kwargs={'pk': str(self.task.id), 'path': self.artifact.path}),
+                        follow=False
+                    )
+                    self.assertEqual(response.status_code, status_code)
 
     def test_download_artifacts_by_agent(self):
         """
-- 
GitLab