diff --git a/packages/component-faraday-selectors/src/index.js b/packages/component-faraday-selectors/src/index.js index a56105fca0d8e69321c031b9b553cc421a37622d..67590e4c1b0a913ef22aeed2ea008c3301e6b1f9 100644 --- a/packages/component-faraday-selectors/src/index.js +++ b/packages/component-faraday-selectors/src/index.js @@ -1,7 +1,7 @@ import { get, has, last, chain } from 'lodash' import { selectCurrentUser } from 'xpub-selectors' -export const isHEToManuscript = (state, collectionId) => { +export const isHEToManuscript = (state, collectionId = '') => { const { id = '', isAccepted = false } = chain(state) .get('collections', []) .find(c => c.id === collectionId) @@ -37,15 +37,15 @@ const canInviteReviewersStatuses = [ 'underReview', 'reviewCompleted', ] -export const canInviteReviewers = (state, collection) => { +export const canInviteReviewers = (state, collection = {}) => { if (!canInviteReviewersStatuses.includes(get(collection, 'status', 'draft'))) return false - const user = selectCurrentUser(state) + const { id: userId } = selectCurrentUser(state) const isAdmin = currentUserIs(state, 'isAdmin') const { isAccepted, id: heId } = get(collection, 'handlingEditor', {}) - return isAccepted && (user.id === heId || isAdmin) + return isAccepted && (userId === heId || isAdmin) } const cannotViewReviewersDetails = [ @@ -63,7 +63,7 @@ export const canViewReviewersDetails = (state, collection = {}) => { ) { return false } - return canViewReports(state, collection.id) + return canViewReports(state, get(collection, 'id', '')) } const authorCanViewReportsDetailsStatuses = [ @@ -96,7 +96,7 @@ const canHeViewEditorialCommentsStatuses = [ 'reviewCompleted', ] export const canHeViewEditorialComments = (state, collection = {}) => { - const isHE = isHEToManuscript(state, collection.id) + const isHE = isHEToManuscript(state, get(collection, 'id', '')) const status = get(collection, 'status', 'draft') return isHE && canHeViewEditorialCommentsStatuses.includes(status) } @@ -114,6 +114,29 @@ export const canEICViewEditorialComments = (state, collection = {}) => { return isEIC && canEICViewEditorialCommentsStatuses.includes(status) } +const canReviewerViewEditorialCommentsStatuses = [ + 'rejected', + 'accepted', + 'inQA', + 'underReview', + 'reviewCompleted', + 'revisionRequested', +] +export const canReviewerViewEditorialComments = ( + state, + collection = {}, + fragment = {}, +) => { + const isReviewer = currentUserIsReviewer(state, get(fragment, 'id', '')) + const hasRevision = get(fragment, 'revision', false) + const hasRecommendation = get(fragment, 'recommendations', false) + const status = get(collection, 'status', 'draft') + return ( + isReviewer && + (hasRevision || hasRecommendation) && + canReviewerViewEditorialCommentsStatuses.includes(status) + ) +} const cannotAuthorViewEditorialCommentsStatuses = [ 'draft', 'technicalChecks', @@ -140,8 +163,9 @@ export const canAuthorViewEditorialComments = ( export const canViewEditorialComments = ( state, collection = {}, - fragmentId, + fragment = {}, ) => { + const fragmentId = get(fragment, 'id', '') const editorialRecommentations = getFragmentEditorialComments( state, fragmentId, @@ -149,6 +173,7 @@ export const canViewEditorialComments = ( return ( (canHeViewEditorialComments(state, collection) || canEICViewEditorialComments(state, collection) || + canReviewerViewEditorialComments(state, collection, fragment) || canAuthorViewEditorialComments(state, collection, fragmentId)) && editorialRecommentations.length > 0 ) @@ -173,8 +198,9 @@ export const getHERecommendation = (state, collectionId, fragmentId) => { } const canMakeDecisionStatuses = ['submitted', 'pendingApproval'] -export const canMakeDecision = (state, collection, fragment = {}) => { - if (fragment.id !== last(get(collection, 'fragments', []))) return false +export const canMakeDecision = (state, collection = {}, fragment = {}) => { + if (get(fragment, 'id', '') !== last(get(collection, 'fragments', []))) + return false const status = get(collection, 'status', 'draft') const isEIC = currentUserIs(state, 'adminEiC') @@ -182,9 +208,12 @@ export const canMakeDecision = (state, collection, fragment = {}) => { } const canEditManuscriptStatuses = ['draft', 'technicalChecks', 'inQA'] -export const canEditManuscript = (state, collection, fragment = {}) => { +export const canEditManuscript = (state, collection = {}, fragment = {}) => { const isAdmin = currentUserIs(state, 'isAdmin') - if (!isAdmin || fragment.id !== last(get(collection, 'fragments', []))) + if ( + !isAdmin || + get(fragment, 'id', '') !== last(get(collection, 'fragments', [])) + ) return false const status = get(collection, 'status', 'draft') @@ -192,7 +221,7 @@ export const canEditManuscript = (state, collection, fragment = {}) => { } const canOverrideTechnicalChecksStatuses = ['technicalChecks', 'inQA'] -export const canOverrideTechnicalChecks = (state, collection) => { +export const canOverrideTechnicalChecks = (state, collection = {}) => { const isAdmin = currentUserIs(state, 'isAdmin') if (!isAdmin) return false const status = get(collection, 'status', 'draft') @@ -206,12 +235,14 @@ export const canViewReports = (state, collectionId) => { return isHE || isEiC } -export const canMakeRevision = (state, collection, fragment) => { +export const canMakeRevision = (state, collection = {}, fragment = {}) => { const currentUserId = get(state, 'currentUser.user.id') return ( get(fragment, 'revision') && - collection.status === 'revisionRequested' && - fragment.owners.map(o => o.id).includes(currentUserId) + get(collection, 'status', 'draft') === 'revisionRequested' && + get(fragment, 'owners', []) + .map(o => o.id) + .includes(currentUserId) ) } @@ -389,7 +420,7 @@ export const canMakeRecommendation = (state, collection, fragment = {}) => { return isHE && canMakeRecommendationStatuses.includes(status) } -export const canSubmitRevision = (state, collection, fragment) => { +export const canSubmitRevision = (state, fragment = {}) => { const userId = get(state, 'currentUser.user.id') const fragmentAuthors = chain(fragment) .get('authors', []) diff --git a/packages/component-fixture-manager/src/fixtures/fragments.js b/packages/component-fixture-manager/src/fixtures/fragments.js index 3f149caf0e0a148bf47b04fae1076dd58323e36d..c8931dd9bb71a0259f0817a5a0a448ea53e37e42 100644 --- a/packages/component-fixture-manager/src/fixtures/fragments.js +++ b/packages/component-fixture-manager/src/fixtures/fragments.js @@ -46,6 +46,28 @@ const fragments = { updatedOn: chance.timestamp(), submittedOn: chance.timestamp(), }, + { + recommendation: 'reject', + recommendationType: 'review', + comments: [ + { + content: chance.paragraph(), + public: chance.bool(), + files: [ + { + id: chance.guid(), + name: 'file.pdf', + size: chance.natural(), + }, + ], + }, + ], + id: chance.guid(), + userId: '1231njfsdknfkjs23', + createdOn: chance.timestamp(), + updatedOn: chance.timestamp(), + submittedOn: chance.timestamp(), + }, { recommendation: 'minor', recommendationType: 'editorRecommendation', 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 0862779b60d66f4d415632ce421f90fda6290010..6ee20a4684bf3b12a882a095ffa0bda64225095e 100644 --- a/packages/component-manuscript-manager/src/tests/collections/get.test.js +++ b/packages/component-manuscript-manager/src/tests/collections/get.test.js @@ -24,6 +24,7 @@ describe('Get collections route handler', () => { it('should return collections with the latest fragments if the request user is HE', async () => { const { handlingEditor } = testFixtures.users + const { recommendations } = testFixtures.fragments.fragment const res = await requests.sendRequest({ userId: handlingEditor.id, @@ -40,7 +41,9 @@ describe('Get collections route handler', () => { expect(data[0]).toHaveProperty('currentVersion') expect(data[0]).toHaveProperty('visibleStatus') expect(data[0].currentVersion.type).toEqual('fragment') - expect(data[0].currentVersion.recommendations).toHaveLength(3) + expect(data[0].currentVersion.recommendations).toHaveLength( + recommendations.length, + ) }) it('should return collections with the latest fragments if the request user is reviewer', async () => { @@ -58,7 +61,7 @@ describe('Get collections route handler', () => { expect(data).toHaveLength(2) expect(data[0].type).toEqual('collection') - expect(data[0].currentVersion.recommendations).toHaveLength(1) + expect(data[0].currentVersion.recommendations).toHaveLength(3) expect(data[0].currentVersion.authors[0]).not.toHaveProperty('email') }) @@ -77,6 +80,7 @@ describe('Get collections route handler', () => { it('should return all collections with the latest fragments if the request user is admin/EiC', async () => { const { editorInChief } = testFixtures.users + const { recommendations } = testFixtures.fragments.fragment const res = await requests.sendRequest({ userId: editorInChief.id, @@ -91,6 +95,8 @@ describe('Get collections route handler', () => { expect(data).toHaveLength(size(testFixtures.collections)) expect(data[0].type).toEqual('collection') expect(data[0]).toHaveProperty('visibleStatus') - expect(data[0].currentVersion.recommendations).toHaveLength(3) + expect(data[0].currentVersion.recommendations).toHaveLength( + recommendations.length, + ) }) }) diff --git a/packages/component-manuscript/src/components/ManuscriptPage.js b/packages/component-manuscript/src/components/ManuscriptPage.js index cad61f6cb1772696df3435f5a3839395f078bf27..8e67c6d7ef148a2d47928657288a4f516e4937d5 100644 --- a/packages/component-manuscript/src/components/ManuscriptPage.js +++ b/packages/component-manuscript/src/components/ManuscriptPage.js @@ -162,17 +162,17 @@ export default compose( isReviewer: currentUserIsReviewer(state, get(fragment, 'id', '')), isHEToManuscript: isHEToManuscript(state, get(collection, 'id', '')), permissions: { - canSubmitRevision: canSubmitRevision(state, collection, fragment), + canSubmitRevision: canSubmitRevision(state, fragment), canMakeHERecommendation: canMakeHERecommendation(state, { collection, statuses: get(journal, 'statuses', {}), }), - canAssignHE: canAssignHE(state, match.params.project), + canAssignHE: canAssignHE(state, collection), canViewReports: canViewReports(state, match.params.project), canViewEditorialComments: canViewEditorialComments( state, collection, - match.params.version, + fragment, ), canInviteReviewers: canInviteReviewers(state, collection), canMakeRecommendation: !isUndefined(pendingOwnRecommendation), @@ -183,7 +183,7 @@ export default compose( authorCanViewReportsDetails: authorCanViewReportsDetails( state, collection, - fragment.id, + get(fragment, 'id', ''), ), canOverrideTechChecks: canOverrideTechnicalChecks(state, collection), canAuthorViewEditorialComments: canAuthorViewEditorialComments( diff --git a/packages/component-manuscript/src/redux/editors.js b/packages/component-manuscript/src/redux/editors.js index d5f7a8e70c2eee905bc2c7d77999362b816bdf80..ffe5ac1f4fd9382d1899270f2d419b7701528055 100644 --- a/packages/component-manuscript/src/redux/editors.js +++ b/packages/component-manuscript/src/redux/editors.js @@ -24,13 +24,9 @@ export const selectHandlingEditors = state => .value() const canAssignHEStatuses = ['submitted'] -export const canAssignHE = (state, collectionId = '') => { +export const canAssignHE = (state, collection = {}) => { const isEIC = currentUserIs(state, 'adminEiC') - const collection = chain(state) - .get('collections', []) - .find(c => c.id === collectionId) - .value() - const hasHE = get(collection, 'handlingEditor') + const hasHE = get(collection, 'handlingEditor', false) return ( isEIC && diff --git a/packages/xpub-faraday/config/authsome-helpers.js b/packages/xpub-faraday/config/authsome-helpers.js index 0d990961263d0d0ff397c1deb7786c6b2e39b2a5..c962ebd7c50c2c70c4a60e310368f00e089682c2 100644 --- a/packages/xpub-faraday/config/authsome-helpers.js +++ b/packages/xpub-faraday/config/authsome-helpers.js @@ -178,7 +178,16 @@ const stripeFragmentByRole = ({ files: omit(files, ['coverLetter']), authors: authors.map(a => omit(a, ['email'])), recommendations: recommendations - ? recommendations.filter(r => r.userId === user.id) + ? recommendations + .filter( + r => + r.userId === user.id || + r.recommendationType === 'editorRecommendation', + ) + .map(r => ({ + ...r, + comments: r.comments.filter(c => c.public === true), + })) : [], } case 'handlingEditor': diff --git a/packages/xpub-faraday/tests/config/authsome-helpers.test.js b/packages/xpub-faraday/tests/config/authsome-helpers.test.js index 606bb666465c1455e78ce9cc9bb3cfb56cadfddb..ced5b1e955140c07bfa275ae1736d1e34edc909a 100644 --- a/packages/xpub-faraday/tests/config/authsome-helpers.test.js +++ b/packages/xpub-faraday/tests/config/authsome-helpers.test.js @@ -115,11 +115,27 @@ describe('Authsome Helpers', () => { const { files = {} } = result expect(files.coverLetter).toBeFalsy() }) - it('reviewer should not see others reviews', () => { + it('reviewer should not see private comments', () => { const { fragment } = testFixtures.fragments + fragment.recommendations = [ + { + comments: [ + { + content: 'private', + public: false, + }, + { + content: 'public', + public: true, + }, + ], + }, + ] const result = ah.stripeFragmentByRole({ fragment, role: 'reviewer' }) const { recommendations } = result - expect(recommendations).toEqual([]) + expect(recommendations).toHaveLength(1) + expect(recommendations[0].comments).toHaveLength(1) + expect(recommendations[0].comments[0].public).toEqual(true) }) it('author should not see recommendations if a decision has not been made', () => { @@ -170,6 +186,7 @@ describe('Authsome Helpers', () => { const publicComments = get(result, 'recommendations[0].comments') expect(publicComments).toHaveLength(1) }) + it('HE should not see unsubmitted recommendations', () => { const { fragment } = testFixtures.fragments fragment.recommendations = [