From 195973eae14d61ae7d5317575c36387132727670 Mon Sep 17 00:00:00 2001
From: Naz <nazargargol@gmail.com>
Date: Thu, 28 Jul 2022 17:51:59 +0100
Subject: [PATCH] Added successful job run check

refs https://github.com/TryGhost/Toolbox/issues/358

- Allows to check for a **successfull** job run and restart/re-add the job in case it was a failed one off job
---
 .../core/server/services/members/service.js   |  2 +-
 ghost/job-manager/lib/job-manager.js          | 12 ++++--
 ghost/job-manager/test/job-manager.test.js    | 37 ++++++++++++-------
 3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/ghost/core/core/server/services/members/service.js b/ghost/core/core/server/services/members/service.js
index 4eb3abc784..ebe1f1d435 100644
--- a/ghost/core/core/server/services/members/service.js
+++ b/ghost/core/core/server/services/members/service.js
@@ -149,7 +149,7 @@ module.exports = {
         })();
 
         const membersMigrationJobName = 'members-migrations';
-        if (!(await jobsService.hasExecuted(membersMigrationJobName))) {
+        if (!(await jobsService.hasExecutedSuccessfully(membersMigrationJobName))) {
             jobsService.addOneOffJob({
                 name: membersMigrationJobName,
                 offloaded: false,
diff --git a/ghost/job-manager/lib/job-manager.js b/ghost/job-manager/lib/job-manager.js
index 49c4cdb237..027ebf4b78 100644
--- a/ghost/job-manager/lib/job-manager.js
+++ b/ghost/job-manager/lib/job-manager.js
@@ -239,14 +239,18 @@ class JobManager {
     }
 
     /**
-     * Checks if the one-off job has ever been successfully executed
+     * Checks if the one-off job has ever been executed successfully.
      * @param {String} name one-off job name
      */
-    async hasExecuted(name) {
-        // TODO: return false if the job has failed?
+    async hasExecutedSuccessfully(name) {
         if (this._jobsRepository) {
             const persistedJob = await this._jobsRepository.read(name);
-            return !!persistedJob;
+
+            if (!persistedJob) {
+                return false;
+            } else {
+                return (persistedJob.get('status') !== ALL_STATUSES.failed);
+            }
         } else {
             return false;
         }
diff --git a/ghost/job-manager/test/job-manager.test.js b/ghost/job-manager/test/job-manager.test.js
index 76d85dd465..3c4901ec00 100644
--- a/ghost/job-manager/test/job-manager.test.js
+++ b/ghost/job-manager/test/job-manager.test.js
@@ -36,6 +36,8 @@ describe('Job Manager', function () {
         const jobManager = new JobManager({});
 
         should.exist(jobManager.addJob);
+        should.exist(jobManager.hasExecutedSuccessfully);
+        should.exist(jobManager.awaitCompletion);
     });
 
     describe('Add a job', function () {
@@ -513,30 +515,40 @@ describe('Job Manager', function () {
 
     describe('Job execution progress', function () {
         it('checks if job has ever been executed', async function () {
-            const spy = sinon.spy();
             const JobModel = {
                 findOne: sinon.stub()
                     .withArgs('solovei')
                     .onCall(0)
                     .resolves(null)
                     .onCall(1)
-                    .resolves(null)
-                    .resolves({id: 'unique', name: 'solovei'}),
-                add: sinon.stub().resolves()
+                    .resolves({
+                        id: 'unique',
+                        get: (field) => {
+                            if (field === 'status') {
+                                return 'finished';
+                            }
+                        }
+                    })
+                    .onCall(2)
+                    .resolves({
+                        id: 'unique',
+                        get: (field) => {
+                            if (field === 'status') {
+                                return 'failed';
+                            }
+                        }
+                    })
             };
 
             const jobManager = new JobManager({JobModel});
-            let executed = await jobManager.hasExecuted('solovei');
+            let executed = await jobManager.hasExecutedSuccessfully('solovei');
             should.equal(executed, false);
 
-            await jobManager.addOneOffJob({
-                job: spy,
-                name: 'solovei'
-            });
-
-            assert.equal(JobModel.add.called, true);
-            executed = await jobManager.hasExecuted('solovei');
+            executed = await jobManager.hasExecutedSuccessfully('solovei');
             should.equal(executed, true);
+
+            executed = await jobManager.hasExecutedSuccessfully('solovei');
+            should.equal(executed, false);
         });
 
         it('can wait for job completion', async function () {
@@ -556,7 +568,6 @@ describe('Job Manager', function () {
                     .resolves(null)
                     .onCall(1)
                     .resolves(null)
-
                     .resolves({
                         id: 'unique',
                         get: () => status