diff --git a/packages/component-fixture-manager/src/fixtures/fragments.js b/packages/component-fixture-manager/src/fixtures/fragments.js index 6890cde5142be0ba18d9ef356d9b3624f37580a1..337c0d943c7e6fd87e47fd3e31ff3458c0fb567d 100644 --- a/packages/component-fixture-manager/src/fixtures/fragments.js +++ b/packages/component-fixture-manager/src/fixtures/fragments.js @@ -99,6 +99,69 @@ const fragments = { createdOn: chance.timestamp(), updatedOn: chance.timestamp(), }, + { + recommendation: 'publish', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: handlingEditor.id, + createdOn: 1542361074012, + updatedOn: chance.timestamp(), + }, + { + recommendation: 'return-to-handling-editor', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: admin.id, + createdOn: 1542361115749, + updatedOn: chance.timestamp(), + }, + { + recommendation: 'publish', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: chance.bool(), + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: handlingEditor.id, + createdOn: 1542361115750, + updatedOn: chance.timestamp(), + }, { recommendation: 'publish', recommendationType: 'editorRecommendation', @@ -117,7 +180,7 @@ const fragments = { ], id: chance.guid(), userId: admin.id, - createdOn: chance.timestamp(), + createdOn: 1542361115751, updatedOn: chance.timestamp(), }, ], @@ -458,6 +521,48 @@ const fragments = { updatedOn: chance.timestamp(), submittedOn: chance.timestamp(), }, + { + recommendation: 'publish', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: handlingEditor.id, + createdOn: 1542361074012, + updatedOn: chance.timestamp(), + }, + { + recommendation: 'return-to-handling-editor', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: admin.id, + createdOn: 1542361115749, + updatedOn: chance.timestamp(), + }, ], authors: [ { @@ -491,17 +596,9 @@ fragments.noInvitesFragment = { invites: [], id: chance.guid(), } -fragments.noInvitesFragment = { - ...fragments.fragment1, - recommendations: [], - invites: [], - id: chance.guid(), -} - fragments.noInvitesFragment1 = { ...fragments.fragment, recommendations: [], - invites: [], id: chance.guid(), } fragments.minorRevisionWithoutReview = { diff --git a/packages/component-helper-service/src/services/Collection.js b/packages/component-helper-service/src/services/Collection.js index 5ead71c258d5cad06b646bd6b9639df96b59e42a..0db955323c5e15fe008bf2ef90152788d078df46 100644 --- a/packages/component-helper-service/src/services/Collection.js +++ b/packages/component-helper-service/src/services/Collection.js @@ -10,6 +10,7 @@ class Collection { async updateStatusByRecommendation({ recommendation, isHandlingEditor = false, + fragments, }) { let newStatus if (isHandlingEditor) { @@ -19,7 +20,9 @@ class Collection { } } else { if (recommendation === 'minor') { - newStatus = 'reviewCompleted' + newStatus = this.hasAtLeastOneReviewReport(fragments) + ? 'reviewCompleted' + : 'heAssigned' } if (recommendation === 'major') { @@ -27,7 +30,7 @@ class Collection { } } - this.updateStatus({ newStatus }) + return this.updateStatus({ newStatus }) } async updateFinalStatusByRecommendation({ recommendation }) { @@ -89,18 +92,16 @@ class Collection { async updateStatusOnRecommendation({ isEditorInChief, recommendation }) { if (isEditorInChief) { if (recommendation === 'return-to-handling-editor') { - this.updateStatus({ newStatus: 'reviewCompleted' }) - } else { - this.updateFinalStatusByRecommendation({ - recommendation, - }) + return this.updateStatus({ newStatus: 'reviewCompleted' }) } - } else { - this.updateStatusByRecommendation({ + return this.updateFinalStatusByRecommendation({ recommendation, - isHandlingEditor: true, }) } + return this.updateStatusByRecommendation({ + recommendation, + isHandlingEditor: true, + }) } getHELastName() { @@ -158,6 +159,14 @@ class Collection { return fragmentHelper.hasReviewReport() } } + + async getAllFragments({ FragmentModel }) { + return Promise.all( + this.collection.fragments.map(async fragment => + FragmentModel.find(fragment), + ), + ) + } } module.exports = Collection diff --git a/packages/component-helper-service/src/services/Fragment.js b/packages/component-helper-service/src/services/Fragment.js index ba56f7a41f116f412842fa721f2336cf6430f169..7a0f27ae03d656269c6e5c757c094dd083f1ddb8 100644 --- a/packages/component-helper-service/src/services/Fragment.js +++ b/packages/component-helper-service/src/services/Fragment.js @@ -1,4 +1,4 @@ -const { get, remove } = require('lodash') +const { get, remove, findLast, last } = require('lodash') const config = require('config') const User = require('./User') @@ -145,11 +145,21 @@ class Fragment { hasReviewReport() { const { fragment: { recommendations = [] } } = this - return recommendations.find( + return recommendations.some( rec => rec.recommendationType === 'review' && rec.submittedOn, ) } + canHEMakeAnotherRecommendation(currentUserRecommendations) { + const lastHERecommendation = last(currentUserRecommendations) + const { fragment: { recommendations = [] } } = this + const returnToHERecommendation = findLast( + recommendations, + r => r.recommendation === 'return-to-handling-editor', + ) + if (!returnToHERecommendation) return false + return returnToHERecommendation.createdOn > lastHERecommendation.createdOn + } async getReviewersAndEditorsData({ collection, UserModel }) { const { invitations = [], diff --git a/packages/component-helper-service/src/tests/collection.test.js b/packages/component-helper-service/src/tests/collection.test.js index ebf35e9d666f30daf27cdce4c55c57f6637c3cf1..167153af8c03fc7149c36a9e8c149de351c0f7db 100644 --- a/packages/component-helper-service/src/tests/collection.test.js +++ b/packages/component-helper-service/src/tests/collection.test.js @@ -4,13 +4,16 @@ process.env.SUPPRESS_NO_CONFIG_WARNING = true const { cloneDeep } = require('lodash') const fixturesService = require('pubsweet-component-fixture-service') -const { fixtures } = fixturesService -const { Collection } = require('../Helper') +const { fixtures, Model } = fixturesService +const { Collection, Fragment } = require('../Helper') describe('Collection helper', () => { let testFixtures = {} + let models + beforeEach(() => { testFixtures = cloneDeep(fixtures) + models = Model.build(testFixtures) }) describe('getReviewerNumber', () => { @@ -65,4 +68,167 @@ describe('Collection helper', () => { expect(reviewerNumber).toBe(3) }) }) + + describe('hasAtLeastOneReviewReport', () => { + it('should return true if collection has at least one report from reviewers.', async () => { + const { collection } = testFixtures.collections + const collectionHelper = new Collection({ collection }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const hasReviewReport = await collectionHelper.hasAtLeastOneReviewReport( + fragments, + ) + expect(hasReviewReport).toBe(true) + }) + it('should return false if collection has at least one report from reviewers.', async () => { + const { noInvitesFragment } = testFixtures.fragments + const { collection } = testFixtures.collections + collection.fragments = [noInvitesFragment.id] + const collectionHelper = new Collection({ collection }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const hasReviewReport = await collectionHelper.hasAtLeastOneReviewReport( + fragments, + ) + expect(hasReviewReport).toBe(false) + }) + }) + + describe('canHEMakeRecommendation', () => { + it('should return true when creating a recommendation as a HE when there is a single version with at least one review.', async () => { + const { collection } = testFixtures.collections + const { fragment } = testFixtures.fragments + const collectionHelper = new Collection({ + collection, + }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const fragmentHelper = new Fragment({ fragment }) + const canHEMakeRecommendation = await collectionHelper.canHEMakeRecommendation( + fragments, + fragmentHelper, + ) + expect(canHEMakeRecommendation).toBe(true) + }) + it('should return false when creating a recommendation with publish as a HE when there is a single version and there are no reviews.', async () => { + const { collection } = testFixtures.collections + const { fragment } = testFixtures.fragments + fragment.recommendations = undefined + + const collectionHelper = new Collection({ + collection, + }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const fragmentHelper = new Fragment({ fragment }) + const canHEMakeRecommendation = await collectionHelper.canHEMakeRecommendation( + fragments, + fragmentHelper, + ) + expect(canHEMakeRecommendation).toBe(false) + }) + it('should return true when creating a recommendation as a HE after minor revision and we have at least one review on collection.', async () => { + const { collection } = testFixtures.collections + const { + minorRevisionWithReview, + noInvitesFragment1, + } = testFixtures.fragments + collection.fragments = [minorRevisionWithReview.id, noInvitesFragment1.id] + const collectionHelper = new Collection({ + collection, + }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const fragmentHelper = new Fragment({ fragment: noInvitesFragment1 }) + + const canHEMakeRecommendation = await collectionHelper.canHEMakeRecommendation( + fragments, + fragmentHelper, + ) + expect(canHEMakeRecommendation).toBe(true) + }) + it('should return false when creating a recommendation as a HE after minor revision and there are no reviews.', async () => { + const { collection } = testFixtures.collections + const { + minorRevisionWithoutReview, + noInvitesFragment1, + } = testFixtures.fragments + collection.fragments = [ + minorRevisionWithoutReview.id, + noInvitesFragment1.id, + ] + + const collectionHelper = new Collection({ + collection, + }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const fragmentHelper = new Fragment({ noInvitesFragment1 }) + const canHEMakeRecommendation = await collectionHelper.canHEMakeRecommendation( + fragments, + fragmentHelper, + ) + expect(canHEMakeRecommendation).toBe(false) + }) + it('should return true when creating a recommendation as a HE after major revision and there are least one review on fragment.', async () => { + const { collection } = testFixtures.collections + const { + majorRevisionWithReview, + reviewCompletedFragment, + } = testFixtures.fragments + + reviewCompletedFragment.collectionId = collection.id + collection.fragments = [ + majorRevisionWithReview.id, + reviewCompletedFragment.id, + ] + + const collectionHelper = new Collection({ + collection, + }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const fragmentHelper = new Fragment({ fragment: reviewCompletedFragment }) + const canHEMakeRecommendation = await collectionHelper.canHEMakeRecommendation( + fragments, + fragmentHelper, + ) + expect(canHEMakeRecommendation).toBe(true) + }) + it('should return false when creating a recommendation as a HE after major revision there are no reviews on fragment.', async () => { + const { collection } = testFixtures.collections + const { + majorRevisionWithReview, + noInvitesFragment1, + } = testFixtures.fragments + collection.fragments = [majorRevisionWithReview.id, noInvitesFragment1.id] + const collectionHelper = new Collection({ + collection, + }) + const FragmentModel = models.Fragment + const fragments = await collectionHelper.getAllFragments({ + FragmentModel, + }) + const fragmentHelper = new Fragment({ fragment: noInvitesFragment1 }) + const canHEMakeRecommendation = await collectionHelper.canHEMakeRecommendation( + fragments, + fragmentHelper, + ) + expect(canHEMakeRecommendation).toBe(false) + }) + }) }) diff --git a/packages/component-helper-service/src/tests/fragment.test.js b/packages/component-helper-service/src/tests/fragment.test.js index 26598cfe8bd73fc40a8966a03ab51aa3552f3668..c5623dc18a8d7cf4812d3695189319f48b7e2202 100644 --- a/packages/component-helper-service/src/tests/fragment.test.js +++ b/packages/component-helper-service/src/tests/fragment.test.js @@ -15,6 +15,8 @@ const { recommendations: configRecommendations } = config const acceptedReviewerId = chance.guid() const submittedReviewerId1 = chance.guid() const submittedReviewerId2 = chance.guid() +const handlingEditorId = chance.guid() +const editorInChiefId = chance.guid() const fragment = { invitations: [ { @@ -281,4 +283,93 @@ describe('Fragment helper', () => { } }) }) + describe('canHEMakeAnotherRecommendation', () => { + it('should return true when He makes a recommendation after EIC decision was to return to HE', async () => { + testFragment.recommendations = [ + { + recommendation: 'publish', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: handlingEditorId, + createdOn: 1542361074012, + updatedOn: chance.timestamp(), + }, + { + recommendation: 'return-to-handling-editor', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: editorInChiefId, + createdOn: 1542361115749, + updatedOn: chance.timestamp(), + }, + ] + const currentUserRecommendations = testFragment.recommendations.filter( + r => r.userId === handlingEditorId, + ) + const fragmentHelper = new Fragment({ fragment: testFragment }) + const canHEMakeAnotherRecommendation = await fragmentHelper.canHEMakeAnotherRecommendation( + currentUserRecommendations, + ) + expect(canHEMakeAnotherRecommendation).toBe(true) + }) + it('should return false when He makes another recommendation', async () => { + testFragment.recommendations = [ + { + recommendation: 'publish', + recommendationType: 'editorRecommendation', + comments: [ + { + content: chance.paragraph(), + public: true, + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: handlingEditorId, + createdOn: 1542361074012, + updatedOn: chance.timestamp(), + }, + ] + const currentUserRecommendations = testFragment.recommendations.filter( + r => r.userId === handlingEditorId, + ) + const fragmentHelper = new Fragment({ fragment: testFragment }) + const canHEMakeAnotherRecommendation = await fragmentHelper.canHEMakeAnotherRecommendation( + currentUserRecommendations, + ) + expect(canHEMakeAnotherRecommendation).toBe(false) + }) + }) }) diff --git a/packages/component-invite/src/routes/fragmentsInvitations/post.js b/packages/component-invite/src/routes/fragmentsInvitations/post.js index 1a441531473daef14c295e408fc86e7d57fed96d..0e45b83faac29aef7b05b2ca588cf07b6dc7fb0e 100644 --- a/packages/component-invite/src/routes/fragmentsInvitations/post.js +++ b/packages/component-invite/src/routes/fragmentsInvitations/post.js @@ -4,6 +4,7 @@ const { services, Collection, Invitation, + Fragment, authsome: authsomeHelper, } = require('pubsweet-component-helper-service') @@ -116,7 +117,12 @@ module.exports = models => async (req, res) => { }) } - if (collection.status === 'heAssigned') { + const fragmentHelper = new Fragment({ fragment }) + if ( + collection.status === 'heAssigned' || + (collection.status === 'reviewCompleted' && + !fragmentHelper.hasReviewReport()) + ) { collectionHelper.updateStatus({ newStatus: 'reviewersInvited' }) } diff --git a/packages/component-manuscript-manager/src/routes/fragments/patch.js b/packages/component-manuscript-manager/src/routes/fragments/patch.js index 396672ff07542005238642045ebe226e45d8fa3c..acd037254ddb9ada6417d99980a2a30588799d08 100644 --- a/packages/component-manuscript-manager/src/routes/fragments/patch.js +++ b/packages/component-manuscript-manager/src/routes/fragments/patch.js @@ -103,8 +103,13 @@ module.exports = models => async (req, res) => { await authorsTeam.save() } - collectionHelper.updateStatusByRecommendation({ + const fragments = await collectionHelper.getAllFragments({ + FragmentModel: models.Fragment, + }) + + await collectionHelper.updateStatusByRecommendation({ recommendation: heRecommendation.recommendation, + fragments, }) newFragment.submitted = Date.now() diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/patch.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/patch.js index a973956cfb0e01d956a74168bebe3e81a65b62ca..7cd5b2ae49984d23fff26a53ae1bfb38a19f8244 100644 --- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/patch.js +++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/patch.js @@ -67,7 +67,7 @@ module.exports = models => async (req, res) => { const collectionHelper = new Collection({ collection }) const reviewerNumber = await collectionHelper.getReviewerNumber({ - userId: req.authInfo.id, + userId, }) find(fragment.invitations, [ diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js index db2336e8eebebb019f2eecf603381fa7973a5b6e..b47234c1f1a017149269ba6e415ee49d5af6b159 100644 --- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js +++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js @@ -1,6 +1,5 @@ -/* eslint-disable no-return-await */ const uuid = require('uuid') -const { pick, get, set, has, isEmpty, last } = require('lodash') +const { pick, get, set, has, isEmpty, last, chain } = require('lodash') const config = require('config') const { v4 } = require('uuid') const logger = require('@pubsweet/logger') @@ -45,11 +44,9 @@ module.exports = models => async (req, res) => { const collectionHelper = new Collection({ collection }) try { - fragments = await Promise.all( - collection.fragments.map( - async fragment => await models.Fragment.find(fragment), - ), - ) + fragments = await collectionHelper.getAllFragments({ + FragmentModel: models.Fragment, + }) } catch (e) { const notFoundError = await services.handleNotFoundError(e, 'Item') fragments = [] @@ -57,9 +54,16 @@ module.exports = models => async (req, res) => { error: notFoundError.message, }) } - const currentUserRecommendation = get(fragment, 'recommendations', []).filter( - r => r.userId === req.user, - ) + const currentUserRecommendations = get( + fragment, + 'recommendations', + [], + ).filter(r => r.userId === req.user) + + const lastFragmentRecommendation = chain(fragment) + .get('recommendations', []) + .last() + .value() const authsome = authsomeHelper.getAuthsome(models) const target = { @@ -92,16 +96,32 @@ module.exports = models => async (req, res) => { } if ( last(collection.fragments) === fragmentId && - !isEmpty(currentUserRecommendation) + !isEmpty(currentUserRecommendations) ) { if (recommendationType === recommendations.type.review) { return res .status(400) .json({ error: 'Cannot write another review on this version.' }) } - return res - .status(400) - .json({ error: 'Cannot make another recommendation on this version.' }) + if ( + recommendationType === recommendations.type.editor && + !isEditorInChief && + !fragmentHelper.canHEMakeAnotherRecommendation(currentUserRecommendations) + ) { + return res.status(400).json({ + error: 'Cannot make another recommendation on this version.', + }) + } + if ( + recommendationType === recommendations.type.editor && + isEditorInChief && + recommendation !== recommendations.reject && + lastFragmentRecommendation.recommendation === 'return-to-handling-editor' + ) { + return res.status(400).json({ + error: 'Cannot make another recommendation on this version.', + }) + } } if ( @@ -130,7 +150,7 @@ module.exports = models => async (req, res) => { newRecommendation.comments = comments || undefined if (recommendationType === 'editorRecommendation') { - collectionHelper.updateStatusOnRecommendation({ + await collectionHelper.updateStatusOnRecommendation({ isEditorInChief, recommendation, }) diff --git a/packages/component-manuscript-manager/src/tests/collections/get.test.js b/packages/component-manuscript-manager/src/tests/collections/get.test.js index fb88e5658444b5c3bad2f2cf5ac7760bf9900537..aa4a1cfec4a3e9de92809e07e056a81b2aaede1f 100644 --- a/packages/component-manuscript-manager/src/tests/collections/get.test.js +++ b/packages/component-manuscript-manager/src/tests/collections/get.test.js @@ -61,7 +61,7 @@ describe('Get collections route handler', () => { expect(data).toHaveLength(2) expect(data[0].type).toEqual('collection') - expect(data[0].currentVersion.recommendations).toHaveLength(3) + expect(data[0].currentVersion.recommendations).toHaveLength(6) expect(data[0].currentVersion.authors[0]).not.toHaveProperty('email') }) diff --git a/packages/component-manuscript-manager/src/tests/fragmentsRecommendations/post.test.js b/packages/component-manuscript-manager/src/tests/fragmentsRecommendations/post.test.js index df52effe144daf17b1a9ec16e546c4f189b62187..5638eb69ff3be3a1a164b0226377f6c38d7f0486 100644 --- a/packages/component-manuscript-manager/src/tests/fragmentsRecommendations/post.test.js +++ b/packages/component-manuscript-manager/src/tests/fragmentsRecommendations/post.test.js @@ -571,6 +571,52 @@ describe('Post fragments recommendations route handler', () => { expect(data.error).toEqual('Cannot write another review on this version.') }) + it('should return success when creating another recommendation as a HE on the same version when EiC returned manuscript to He ', async () => { + const { noRecommendationHE } = testFixtures.users + const { noEditorRecomedationCollection } = testFixtures.collections + const { noEditorRecomedationFragment } = testFixtures.fragments + + const res = await requests.sendRequest({ + body, + userId: noRecommendationHE.id, + models, + route, + path, + params: { + collectionId: noEditorRecomedationCollection.id, + fragmentId: noEditorRecomedationFragment.id, + }, + }) + expect(res.statusCode).toBe(200) + const data = JSON.parse(res._getData()) + expect(data.userId).toEqual(noRecommendationHE.id) + }) + + it('should return an error when creating another recommendation as a HE on the same version after EiC made decision to publish', async () => { + const { handlingEditor } = testFixtures.users + const { collection } = testFixtures.collections + const { fragment } = testFixtures.fragments + body.recommendation = 'publish' + body.recommendationType = 'editorRecommendation' + + const res = await requests.sendRequest({ + body, + userId: handlingEditor.id, + models, + route, + path, + params: { + collectionId: collection.id, + fragmentId: fragment.id, + }, + }) + expect(res.statusCode).toBe(400) + const data = JSON.parse(res._getData()) + expect(data.error).toEqual( + 'Cannot make another recommendation on this version.', + ) + }) + it('should return an error when an EiC makes a decision on an older version of a manuscript', async () => { const { editorInChief } = testFixtures.users const { twoVersionsCollection } = testFixtures.collections