From f307db8fdb3be3d7cc955ba540712cb6a9f2ce83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 1 Jun 2020 16:35:01 +0200
Subject: [PATCH] jobs: restart failed jobs (#42846)

---
 passerelle/base/models.py                     | 17 ++++++
 passerelle/base/views.py                      | 28 ++++++++--
 passerelle/static/js/passerelle.js            | 52 +++++++++---------
 .../includes/resource-jobs-table.html         |  4 +-
 .../templates/passerelle/manage/job.html      | 13 +++++
 .../passerelle/manage/service_jobs.html       |  8 +++
 passerelle/urls.py                            | 17 +++---
 tests/test_manager.py                         | 54 ++++++++++++++++++-
 8 files changed, 155 insertions(+), 38 deletions(-)

diff --git a/passerelle/base/models.py b/passerelle/base/models.py
index 537afc33..de8defce 100644
--- a/passerelle/base/models.py
+++ b/passerelle/base/models.py
@@ -725,6 +725,7 @@ class Job(models.Model):
             choices=(('registered', _('Registered')),
                      ('running', _('Running')),
                      ('failed', _('Failed')),
+                     ('restarted', _('Failed and restarted')),
                      ('completed', _('Completed'))
                     ),
             )
@@ -740,6 +741,22 @@ class Job(models.Model):
         else:
             self.after_timestamp = value
 
+    def restart(self):
+        # clone current job
+        new_job = copy.deepcopy(self)
+        new_job.pk = None
+        # set status
+        new_job.status = 'registered'
+        # reset some fields
+        new_job.done_timestamp = None
+        new_job.status_details = {}
+        new_job.save()
+
+        # change current job status
+        self.status = 'restarted'
+        self.status_details.update({'new_job_pk': new_job.pk})
+        self.save()
+
 
 @six.python_2_unicode_compatible
 class ResourceLog(models.Model):
diff --git a/passerelle/base/views.py b/passerelle/base/views.py
index 4e7c6812..2c74c180 100644
--- a/passerelle/base/views.py
+++ b/passerelle/base/views.py
@@ -20,14 +20,14 @@ import json
 from dateutil import parser as date_parser
 
 from django.contrib.contenttypes.models import ContentType
-from django.contrib import messages
 from django.core.urlresolvers import reverse
 from django.core.exceptions import ObjectDoesNotExist, PermissionDenied
 from django.db.models import Q
 from django.forms import models as model_forms
 from django.views.generic import (
     View, DetailView, ListView, CreateView, UpdateView, DeleteView, FormView)
-from django.http import Http404, HttpResponse
+from django.http import Http404, HttpResponse, HttpResponseRedirect
+from django.shortcuts import get_object_or_404
 from django.utils.timezone import make_aware
 from django.utils.translation import ugettext_lazy as _
 
@@ -225,8 +225,19 @@ class GenericViewJobsConnectorView(GenericConnectorMixin, ListView):
 
     def get_context_data(self, **kwargs):
         context = super(GenericViewJobsConnectorView, self).get_context_data(**kwargs)
-        context['object'] = self.get_object()
+        connector = self.get_object()
+        context['object'] = connector
         context['query'] = self.request.GET.get('q') or ''
+        if self.request.GET.get('job_id'):
+            try:
+                resource_type = ContentType.objects.get_for_model(connector)
+                context['job_target'] = Job.objects.get(
+                    resource_type=resource_type,
+                    resource_pk=connector.pk,
+                    pk=self.request.GET['job_id']
+                )
+            except (ValueError, Job.DoesNotExist):
+                pass
         return context
 
     def get_object(self):
@@ -234,7 +245,6 @@ class GenericViewJobsConnectorView(GenericConnectorMixin, ListView):
 
     def get_queryset(self):
         connector = self.get_object()
-        resource_type = ContentType.objects.get_for_model(connector)
         qs = connector.jobs_set().order_by('-creation_timestamp')
         query = self.request.GET.get('q')
         if query:
@@ -282,6 +292,16 @@ class GenericJobView(GenericConnectorMixin, DetailView):
         return context
 
 
+class GenericRestartJobView(GenericConnectorMixin, View):
+    def get(self, request, *args, **kwargs):
+        connector = get_object_or_404(self.model, slug=kwargs['slug'])
+        resource_type = ContentType.objects.get_for_model(connector)
+        job = get_object_or_404(Job, pk=self.kwargs['job_pk'], resource_type=resource_type, resource_pk=connector.pk, status='failed')
+        job.restart()
+        return HttpResponseRedirect(
+            reverse('view-jobs-connector', kwargs={'connector': kwargs['connector'], 'slug': kwargs['slug']}))
+
+
 class ImportSiteView(FormView):
     template_name = 'passerelle/manage/import_site.html'
     form_class = ImportSiteForm
diff --git a/passerelle/static/js/passerelle.js b/passerelle/static/js/passerelle.js
index b46c5f86..526ffa6b 100644
--- a/passerelle/static/js/passerelle.js
+++ b/passerelle/static/js/passerelle.js
@@ -1,23 +1,31 @@
+open_window = function(base_url, object_pk, object_name) {
+  var url = base_url + object_pk + '/';
+  var current_url = window.location;
+  var object_key = object_name + '_id';
+  if (window.location.href == window.location.origin + base_url + '?'+ object_key + '=' + object_pk) {
+    // remove <object_name>_id from url on modal close if direct access to an object
+    window.history.pushState({}, 'no ' + object_name, base_url);
+  }
+  $.get(url, function(response) {
+    var $dialog = $(response).dialog({
+      modal: true,
+      width: 'auto',
+      open: function(event, ui) {
+        window.history.pushState({object_key: object_pk}, object_name + ' id', base_url + '?' + object_key + '=' + object_pk);
+      },
+      close: function (event, ui) {
+        window.history.back();
+      }
+    });
+  });
+};
+
 open_log_window = function(base_url, log_pk) {
-    var url = base_url + log_pk + '/';
-    var current_url = window.location;
-    if (window.location.href == window.location.origin + base_url + '?log_id=' + log_pk) {
-      // remove log_id from url on modal close if direct access to a log
-      window.history.pushState({}, 'no log', base_url);
-    }
-    $.get(url,
-        function(response) {
-          var $dialog = $(response).dialog({
-            modal: true,
-            width: 'auto',
-            open: function(event, ui) {
-              window.history.pushState({'log_id': log_pk}, 'log id', base_url + '?log_id=' + log_pk);
-            },
-            close: function (event, ui) {
-              window.history.back();
-            }
-          });
-        });
+  open_window(base_url, log_pk, 'log');
+};
+
+open_job_window = function(base_url, job_pk) {
+  open_window(base_url, job_pk, 'job');
 };
 
 $(function() {
@@ -29,11 +37,7 @@ $(function() {
   $('#jobs tbody tr').on('click', function() {
     var base_url = $(this).parents('table.main').data('job-base-url');
     var job_pk = $(this).data('pk');
-    var url = base_url + job_pk + '/';
-    $.get(url,
-        function(response) {
-          var $dialog = $(response).dialog({modal: true, width: 'auto'});
-        });
+    open_job_window(base_url, job_pk);
   });
 
   /* keep title/slug in sync,
diff --git a/passerelle/templates/passerelle/includes/resource-jobs-table.html b/passerelle/templates/passerelle/includes/resource-jobs-table.html
index fe09849a..fd732652 100644
--- a/passerelle/templates/passerelle/includes/resource-jobs-table.html
+++ b/passerelle/templates/passerelle/includes/resource-jobs-table.html
@@ -23,8 +23,8 @@
     </tbody>
 </table>
 
-{% with page_obj=logrecords %}
-  {% include "gadjo/pagination.html" with anchor="#jobs" %}
+{% with page_obj=jobs %}
+  {% include "gadjo/pagination.html" with anchor="#jobs" without_key="job_id" %}
 {% endwith %}
 
 {% endif %}
diff --git a/passerelle/templates/passerelle/manage/job.html b/passerelle/templates/passerelle/manage/job.html
index 4d0f1d5f..199c0d0e 100644
--- a/passerelle/templates/passerelle/manage/job.html
+++ b/passerelle/templates/passerelle/manage/job.html
@@ -6,6 +6,7 @@
   <td>{{ job.parameters }}</td>
 </tr>
 {% for key, value in job.status_details.items %}
+{% if key != 'new_job_pk' %}
 <tr>
   <td>{{key}}</td>
   <td>{% for key2, value2 in value.items %}
@@ -14,6 +15,18 @@
       {{value|linebreaksbr}}
       {% endfor %}</td>
 </tr>
+{% endif %}
 {% endfor %}
 </table>
+
+{% if job.status == 'failed' or job.status == 'restarted' %}
+<div class="buttons">
+{% if job.status == 'failed' %}
+<a class="button" href="{% url 'restart-job' connector=object.get_connector_slug slug=object.slug job_pk=job.pk %}">{% trans "Restart" %}</a>
+{% else %}
+<a class="button" href="{% url 'view-jobs-connector' connector=object.get_connector_slug slug=object.slug %}?job_id={{ job.status_details.new_job_pk }}">{% trans "See new job" %}</a>
+{% endif %}
+</div>
+{% endif %}
+
 </div>
diff --git a/passerelle/templates/passerelle/manage/service_jobs.html b/passerelle/templates/passerelle/manage/service_jobs.html
index 023954b2..62ded2a7 100644
--- a/passerelle/templates/passerelle/manage/service_jobs.html
+++ b/passerelle/templates/passerelle/manage/service_jobs.html
@@ -24,4 +24,12 @@
 {% include "passerelle/includes/resource-jobs-table.html" with jobs=page_obj %}
 </div>
 
+{% if job_target %}
+<script>
+$(function () {
+    open_job_window($('table.main').data('job-base-url'), {{ job_target.pk }});
+});
+</script>
+{% endif %}
+
 {% endblock %}
diff --git a/passerelle/urls.py b/passerelle/urls.py
index 34df2ac0..d8fa5b0e 100644
--- a/passerelle/urls.py
+++ b/passerelle/urls.py
@@ -4,14 +4,16 @@ from django.conf.urls import include, url
 from django.contrib import admin
 
 from django.contrib.auth.decorators import login_required
+from django.contrib.staticfiles.urls import staticfiles_urlpatterns
 from django.views.static import serve as static_serve
 
-from .views import (HomePageView, ManageView, ManageAddView,
-        GenericCreateConnectorView, GenericDeleteConnectorView,
-        GenericEditConnectorView, GenericEndpointView, GenericConnectorView,
-        GenericViewLogsConnectorView, GenericLogView, GenericExportConnectorView,
-        login, logout, menu_json)
-from .base.views import GenericViewJobsConnectorView, GenericJobView
+from .views import (
+    HomePageView, ManageView, ManageAddView,
+    GenericCreateConnectorView, GenericDeleteConnectorView,
+    GenericEditConnectorView, GenericEndpointView, GenericConnectorView,
+    GenericViewLogsConnectorView, GenericLogView, GenericExportConnectorView,
+    login, logout, menu_json)
+from .base.views import GenericViewJobsConnectorView, GenericJobView, GenericRestartJobView
 from .urls_utils import decorated_includes, required, app_enabled, manager_required
 from .base.urls import access_urlpatterns, import_export_urlpatterns
 from .plugins import register_apps_urls
@@ -76,6 +78,8 @@ urlpatterns += [
                 GenericViewJobsConnectorView.as_view(), name='view-jobs-connector'),
             url(r'^(?P<slug>[\w,-]+)/jobs/(?P<job_pk>\d+)/$',
                 GenericJobView.as_view(), name='view-job'),
+            url(r'^(?P<slug>[\w,-]+)/jobs/(?P<job_pk>\d+)/restart/$',
+                GenericRestartJobView.as_view(), name='restart-job'),
             url(r'^(?P<slug>[\w,-]+)/export$',
                 GenericExportConnectorView.as_view(), name='export-connector'),
         ])))
@@ -94,5 +98,4 @@ if settings.DEBUG and 'debug_toolbar' in settings.INSTALLED_APPS:
         url(r'^__debug__/', include(debug_toolbar.urls)),
     ] + urlpatterns
 
-from django.contrib.staticfiles.urls import staticfiles_urlpatterns
 urlpatterns += staticfiles_urlpatterns()
diff --git a/tests/test_manager.py b/tests/test_manager.py
index acb327e9..bda4c9a7 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -7,9 +7,10 @@ from django.contrib.auth.models import User
 from django.contrib.contenttypes.models import ContentType
 from django.core.files import File
 from django.utils.six import StringIO
+from django.utils.timezone import now
 import pytest
 
-from passerelle.base.models import ApiUser, AccessRight, ResourceLog, ResourceStatus
+from passerelle.base.models import ApiUser, AccessRight, ResourceStatus, Job
 from passerelle.apps.csvdatasource.models import CsvDataSource, Query
 
 pytestmark = pytest.mark.django_db
@@ -319,6 +320,7 @@ def test_availability_parameters(app, admin_user, monkeypatch):
     resp = resp.form.submit()
     assert len(resp.pyquery('#id_notification_delays_p .error'))
 
+
 def test_jobs(app, admin_user):
     data = StringIO('1;Foo\n2;Bar\n3;Baz')
     csv = CsvDataSource.objects.create(csv_file=File(data, 't.csv'),
@@ -372,6 +374,56 @@ def test_jobs(app, admin_user):
     resp = app.get(base_url + job_pk + '/')
     resp = app.get(base_url + '12345' + '/', status=404)
 
+    resp = app.get('/manage/csvdatasource/test/jobs/?job_id=%s' % job_pk)
+    assert 'job_target' in resp.context
+
+    # unknown id
+    resp = app.get('/manage/csvdatasource/test/jobs/?job_id=0')
+    assert 'job_target' not in resp.context
+    # bad id
+    resp = app.get('/manage/csvdatasource/test/jobs/?job_id=foo')
+    assert 'job_target' not in resp.context
+
+
+def test_job_restart(app, admin_user):
+    data = StringIO('1;Foo\n2;Bar\n3;Baz')
+    csv = CsvDataSource.objects.create(
+        csv_file=File(data, 't.csv'),
+        columns_keynames='id, text', slug='test', title='a title', description='a description')
+    app = login(app)
+
+    # unknown job
+    app.get('/manage/csvdatasource/test/jobs/0/restart/', status=404)
+    app.get('/manage/csvdatasource/test/jobs/foo/restart/', status=404)
+
+    # create a job, with wrong status
+    job = Job.objects.create(resource=csv, status='registered')
+    app.get('/manage/csvdatasource/test/jobs/%s/restart/' % job.pk, status=404)
+
+    job.status = 'failed'
+    job.done_timestamp = now()
+    job.status_details = {'error_summary': 'foo bar'}
+    job.save()
+
+    # unknown connector
+    app.get('/manage/csvdatasourc/test/jobs/%s/restart/' % job.pk, status=404)
+    app.get('/manage/csvdatasource/teste/jobs/%s/restart/' % job.pk, status=404)
+
+    # ok, restart job
+    resp = app.get('/manage/csvdatasource/test/jobs/%s/restart/' % job.pk, status=302)
+    new_job = Job.objects.latest('pk')
+    assert resp.location.endswith('/manage/csvdatasource/test/jobs/')
+    assert new_job.status == 'registered'
+    assert new_job.done_timestamp is None
+    assert new_job.status_details == {}
+
+    job.refresh_from_db()
+    assert job.status == 'restarted'
+    assert job.status_details == {'error_summary': 'foo bar', 'new_job_pk': new_job.pk}
+
+    resp = app.get('/manage/csvdatasource/test/jobs/%s/' % job.pk)
+    assert '/manage/csvdatasource/test/jobs/?job_id=%s' % new_job.pk in resp
+
 
 def test_manager_import_export(app, admin_user):
     data = StringIO('1;Foo\n2;Bar\n3;Baz')
-- 
2.20.1

