From 0a2a7583cf3a72788872644ab9e04b7c02ef7e0f Mon Sep 17 00:00:00 2001
From: Sebastian Mihalache <sebi.mihalache@gmail.com>
Date: Thu, 29 Nov 2018 15:06:52 +0200
Subject: [PATCH] refactor(recommendations): fully implemented strategy pattern
---
.../src/services/Collection.js | 4 +
.../src/services/Fragment.js | 27 +++-
.../routes/fragmentsRecommendations/post.js | 131 ++++++------------
.../strategies/eicPublish.js | 7 +
.../strategies/eicReject.js | 8 +-
.../strategies/eicReturnToHE.js | 5 +
.../strategies/hePublish.js | 15 +-
.../strategies/heReject.js | 27 ++++
.../strategies/heRequestRevision.js | 30 ++++
.../strategies/reviewerCreateReview.js | 9 ++
.../fragmentsRecommendations/post.test.js | 7 +-
11 files changed, 169 insertions(+), 101 deletions(-)
create mode 100644 packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heReject.js
create mode 100644 packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heRequestRevision.js
create mode 100644 packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/reviewerCreateReview.js
diff --git a/packages/component-helper-service/src/services/Collection.js b/packages/component-helper-service/src/services/Collection.js
index 857f5859a..cd57c0988 100644
--- a/packages/component-helper-service/src/services/Collection.js
+++ b/packages/component-helper-service/src/services/Collection.js
@@ -231,6 +231,10 @@ class Collection {
this.collection.technicalChecks = {}
await this.collection.save()
}
+
+ hasHandlingEditor() {
+ return has(this.collection, 'handlingEditor')
+ }
}
const sendMTSPackage = async ({
diff --git a/packages/component-helper-service/src/services/Fragment.js b/packages/component-helper-service/src/services/Fragment.js
index 83eec9b92..c990c24d7 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, findLast } = require('lodash')
+const { get, remove, findLast, pick, chain } = require('lodash')
const config = require('config')
const User = require('./User')
@@ -230,6 +230,31 @@ class Fragment {
this.fragment.recommendations.push(newRecommendation)
await this.fragment.save()
}
+
+ async addRevision() {
+ this.fragment.revision = pick(this.fragment, [
+ 'authors',
+ 'files',
+ 'metadata',
+ ])
+ await this.fragment.save()
+ }
+
+ hasReviewers() {
+ const { fragment: invitations = [] } = this
+ return invitations.length > 0
+ }
+
+ getLatestUserRecommendation(userId) {
+ return findLast(this.fragment.recommendations, r => r.userId === userId)
+ }
+
+ getLatestRecommendation() {
+ return chain(this.fragment)
+ .get('recommendations', [])
+ .last()
+ .value()
+ }
}
module.exports = Fragment
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js
index f999d4e66..f56995c89 100644
--- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/post.js
@@ -1,4 +1,3 @@
-const { pick, isEmpty, chain, findLast } = require('lodash')
const config = require('config')
const { v4 } = require('uuid')
@@ -11,11 +10,14 @@ const {
const { recommendations } = config
-const Notification = require('../../notifications/notification')
+const rejectAsHE = require('./strategies/heReject')
const publishAsHE = require('./strategies/hePublish')
-const publishAsEiC = require('./strategies/eicPublish')
const rejectAsEiC = require('./strategies/eicReject')
+const publishAsEiC = require('./strategies/eicPublish')
const returnToHE = require('./strategies/eicReturnToHE')
+const Notification = require('../../notifications/notification')
+const createReview = require('./strategies/reviewerCreateReview')
+const requestRevisionAsHE = require('./strategies/heRequestRevision')
module.exports = models => async (req, res) => {
const { recommendation, comments, recommendationType } = req.body
@@ -23,6 +25,8 @@ module.exports = models => async (req, res) => {
return res.status(400).json({ error: 'Recommendation type is required.' })
const reqUser = await models.User.find(req.user)
+ const userId = reqUser.id
+ // console.log('REQ USER', reqUser)
const isEditorInChief = reqUser.editorInChief || reqUser.admin
const { collectionId, fragmentId } = req.params
@@ -78,71 +82,13 @@ module.exports = models => async (req, res) => {
return res.status(400).json({ error })
}
- // const currentUserRecommendations = get(
- // fragment,
- // 'recommendations',
- // [],
- // ).filter(r => r.userId === req.user)
-
- const latestUserRecommendation = findLast(
- fragment.recommendations,
- r => r.userId === req.user,
- )
-
- if (latestUserRecommendation) {
- if (recommendationType === recommendations.type.review) {
- return res
- .status(400)
- .json({ error: 'Cannot write another review on this version.' })
- }
-
- if (
- recommendationType === recommendations.type.editor &&
- !isEditorInChief &&
- !fragmentHelper.canHEMakeAnotherRecommendation(latestUserRecommendation)
- ) {
- return res.status(400).json({
- error: 'Cannot make another recommendation on this version.',
- })
- }
-
- const lastFragmentRecommendation = chain(fragment)
- .get('recommendations', [])
- .last()
- .value()
- 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.',
- })
- }
- }
-
- let role = ''
- switch (recommendationType) {
- case 'review':
- role = 'reviewer'
- break
- case 'editorRecommendation':
- role = isEditorInChief ? 'eic' : 'he'
- break
- default:
- return res.status(400).json({
- error: `Recommendation ${recommendation} is not defined.`,
- })
- }
-
const newRecommendation = {
+ userId,
id: v4(),
- userId: reqUser.id,
+ recommendation,
+ recommendationType,
createdOn: Date.now(),
updatedOn: Date.now(),
- recommendationType,
- recommendation,
comments: comments || [],
}
@@ -156,51 +102,54 @@ module.exports = models => async (req, res) => {
const strategies = {
he: {
+ reject: rejectAsHE,
publish: publishAsHE,
+ major: requestRevisionAsHE,
+ minor: requestRevisionAsHE,
},
eic: {
- publish: publishAsEiC,
reject: rejectAsEiC,
+ publish: publishAsEiC,
'return-to-handling-editor': returnToHE,
},
}
+ let role = ''
+ switch (recommendationType) {
+ case 'review':
+ role = 'reviewer'
+ try {
+ await createReview.execute({
+ userId,
+ fragmentHelper,
+ newRecommendation,
+ })
+ } catch (e) {
+ return res.status(400).json({ error: e.message })
+ }
+ return res.status(200).json(newRecommendation)
+ case 'editorRecommendation':
+ role = isEditorInChief ? 'eic' : 'he'
+ break
+ default:
+ return res.status(400).json({
+ error: `Recommendation ${recommendation} is not defined.`,
+ })
+ }
+
try {
- strategies[role][recommendation].execute({
+ await strategies[role][recommendation].execute({
+ userId,
models,
fragments,
notification,
fragmentHelper,
collectionHelper,
+ newRecommendation,
})
} catch (e) {
return res.status(400).json({ error: e.message })
}
- if (recommendationType === 'editorRecommendation') {
- await collectionHelper.updateStatusOnRecommendation({
- isEditorInChief,
- recommendation,
- })
-
- if (!isEditorInChief && ['minor', 'major'].includes(recommendation)) {
- fragment.revision = pick(fragment, ['authors', 'files', 'metadata'])
- }
-
- const hasPeerReview = !isEmpty(collection.handlingEditor)
-
- if (collection.status === 'revisionRequested') {
- notification.notifySAWhenHERequestsRevision()
- }
-
- if (hasPeerReview) {
- notification.notifyReviewersWhenHEMakesRecommendation()
- notification.notifyEiCWhenHEMakesRecommendation()
- }
- }
-
- fragment.recommendations.push(newRecommendation)
- fragment.save()
-
return res.status(200).json(newRecommendation)
}
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicPublish.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicPublish.js
index 204815c5c..6915c5432 100644
--- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicPublish.js
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicPublish.js
@@ -10,6 +10,13 @@ module.exports = {
collectionHelper,
newRecommendation,
}) => {
+ const latestRecommendation = fragmentHelper.getLatestRecommendation()
+ if (latestRecommendation.recommendation === 'return-to-handling-editor') {
+ throw new Error(
+ 'Cannot make decision to publish after the manuscript has been returned to Handling Editor.',
+ )
+ }
+
await fragmentHelper.addRecommendation(newRecommendation)
let newStatus = ''
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReject.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReject.js
index e7f220191..b10ff0e2a 100644
--- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReject.js
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReject.js
@@ -9,7 +9,11 @@ module.exports = {
await collectionHelper.updateStatus({ newStatus: 'rejected' })
notification.notifyAuthorsWhenEiCMakesDecision()
- notification.notifyHEWhenEiCMakesDecision()
- notification.notifyReviewersWhenEiCMakesDecision()
+ if (collectionHelper.hasHandlingEditor()) {
+ notification.notifyHEWhenEiCMakesDecision()
+ }
+ if (fragmentHelper.hasReviewers()) {
+ notification.notifyReviewersWhenEiCMakesDecision()
+ }
},
}
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReturnToHE.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReturnToHE.js
index 34561326b..633d9c1b8 100644
--- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReturnToHE.js
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/eicReturnToHE.js
@@ -5,6 +5,11 @@ module.exports = {
collectionHelper,
newRecommendation,
}) => {
+ const latestRecommendation = fragmentHelper.getLatestRecommendation()
+ if (latestRecommendation.recommendation === 'return-to-handling-editor') {
+ throw new Error('Cannot return to Handling Editor again.')
+ }
+
if (collectionHelper.hasEQA()) {
await collectionHelper.removeTechnicalChecks()
}
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/hePublish.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/hePublish.js
index e90e1c747..73c88c434 100644
--- a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/hePublish.js
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/hePublish.js
@@ -1,15 +1,26 @@
module.exports = {
execute: async ({
- collectionHelper,
+ userId,
fragments,
- fragmentHelper,
notification,
+ fragmentHelper,
+ collectionHelper,
newRecommendation,
}) => {
if (!collectionHelper.canHEMakeRecommendation(fragments, fragmentHelper)) {
throw new Error('Cannot publish without at least one reviewer report.')
}
+ const latestUserRecommendation = fragmentHelper.getLatestUserRecommendation(
+ userId,
+ )
+ if (
+ latestUserRecommendation &&
+ !fragmentHelper.canHEMakeAnotherRecommendation(latestUserRecommendation)
+ ) {
+ throw new Error('Cannot make another recommendation on this version.')
+ }
+
await fragmentHelper.addRecommendation(newRecommendation)
await collectionHelper.updateStatus({ newStatus: 'pendingApproval' })
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heReject.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heReject.js
new file mode 100644
index 000000000..cd76a8beb
--- /dev/null
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heReject.js
@@ -0,0 +1,27 @@
+module.exports = {
+ execute: async ({
+ userId,
+ notification,
+ fragmentHelper,
+ collectionHelper,
+ newRecommendation,
+ }) => {
+ const latestUserRecommendation = fragmentHelper.getLatestUserRecommendation(
+ userId,
+ )
+ if (
+ latestUserRecommendation &&
+ !fragmentHelper.canHEMakeAnotherRecommendation(latestUserRecommendation)
+ ) {
+ throw new Error('Cannot make another recommendation on this version.')
+ }
+
+ await fragmentHelper.addRecommendation(newRecommendation)
+ await collectionHelper.updateStatus({ newStatus: 'pendingApproval' })
+
+ if (fragmentHelper.hasReviewers()) {
+ notification.notifyReviewersWhenHEMakesRecommendation()
+ }
+ notification.notifyEiCWhenHEMakesRecommendation()
+ },
+}
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heRequestRevision.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heRequestRevision.js
new file mode 100644
index 000000000..7daeb25d9
--- /dev/null
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/heRequestRevision.js
@@ -0,0 +1,30 @@
+module.exports = {
+ execute: async ({
+ userId,
+ notification,
+ fragmentHelper,
+ collectionHelper,
+ newRecommendation,
+ }) => {
+ const latestUserRecommendation = fragmentHelper.getLatestUserRecommendation(
+ userId,
+ )
+ if (
+ latestUserRecommendation &&
+ !fragmentHelper.canHEMakeAnotherRecommendation(latestUserRecommendation)
+ ) {
+ throw new Error('Cannot make another recommendation on this version.')
+ }
+
+ await fragmentHelper.addRevision()
+ await collectionHelper.updateStatus({ newStatus: 'revisionRequested' })
+ await fragmentHelper.addRecommendation(newRecommendation)
+
+ notification.notifySAWhenHERequestsRevision()
+ notification.notifyEiCWhenHEMakesRecommendation()
+
+ if (fragmentHelper.hasReviewers()) {
+ notification.notifyReviewersWhenHEMakesRecommendation()
+ }
+ },
+}
diff --git a/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/reviewerCreateReview.js b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/reviewerCreateReview.js
new file mode 100644
index 000000000..9b8f450e3
--- /dev/null
+++ b/packages/component-manuscript-manager/src/routes/fragmentsRecommendations/strategies/reviewerCreateReview.js
@@ -0,0 +1,9 @@
+module.exports = {
+ execute: async ({ fragmentHelper, newRecommendation, userId }) => {
+ if (fragmentHelper.getLatestUserRecommendation(userId)) {
+ throw new Error('Cannot write another review on this version.')
+ }
+
+ await fragmentHelper.addRecommendation(newRecommendation)
+ },
+}
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 5638eb69f..b45fe772a 100644
--- a/packages/component-manuscript-manager/src/tests/fragmentsRecommendations/post.test.js
+++ b/packages/component-manuscript-manager/src/tests/fragmentsRecommendations/post.test.js
@@ -63,6 +63,7 @@ describe('Post fragments recommendations route handler', () => {
const { reviewer } = testFixtures.users
const { collection } = testFixtures.collections
const { fragment } = testFixtures.fragments
+ body.recommendationType = 'review'
const res = await requests.sendRequest({
body,
@@ -102,7 +103,7 @@ describe('Post fragments recommendations route handler', () => {
expect(data.userId).toEqual(noRecommendationHE.id)
})
- it('should return an error when creating a recommendation with publish as a HE when there is a single version and there are no reviews.', async () => {
+ it('should return an error when recommending to publish as HE when there is a single version and there are no reviews.', async () => {
const { handlingEditor } = testFixtures.users
const { collection } = testFixtures.collections
const { fragment } = testFixtures.fragments
@@ -445,9 +446,6 @@ describe('Post fragments recommendations route handler', () => {
body.recommendationType = 'editorRecommendation'
body.comments = 'This needs more work'
- delete fragment.recommendations
- delete fragment.revision
- delete fragment.invitations
collection.technicalChecks.eqa = false
const res = await requests.sendRequest({
@@ -464,7 +462,6 @@ describe('Post fragments recommendations route handler', () => {
expect(res.statusCode).toBe(200)
const data = JSON.parse(res._getData())
-
expect(collection.status).toBe('reviewCompleted')
expect(collection.technicalChecks).not.toHaveProperty('token')
expect(collection.technicalChecks).not.toHaveProperty('eqa')
--
GitLab