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