From ead4d19e503ad255cacd0079410410e1a9981c31 Mon Sep 17 00:00:00 2001
From: Alexandru Munteanu <alexandru.munt@gmail.com>
Date: Mon, 22 Oct 2018 16:20:39 +0300
Subject: [PATCH] fix(selectors): use lodash get and default parameters for
 safety

---
 .../component-faraday-selectors/src/index.js  | 43 +++++++++++--------
 .../src/components/ManuscriptPage.js          |  6 +--
 .../component-manuscript/src/redux/editors.js |  8 +---
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/packages/component-faraday-selectors/src/index.js b/packages/component-faraday-selectors/src/index.js
index d09bdf1fa..67590e4c1 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)
 }
@@ -127,7 +127,7 @@ export const canReviewerViewEditorialComments = (
   collection = {},
   fragment = {},
 ) => {
-  const isReviewer = currentUserIsReviewer(state, fragment.id)
+  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')
@@ -165,15 +165,16 @@ export const canViewEditorialComments = (
   collection = {},
   fragment = {},
 ) => {
+  const fragmentId = get(fragment, 'id', '')
   const editorialRecommentations = getFragmentEditorialComments(
     state,
-    fragment.id,
+    fragmentId,
   )
   return (
     (canHeViewEditorialComments(state, collection) ||
       canEICViewEditorialComments(state, collection) ||
       canReviewerViewEditorialComments(state, collection, fragment) ||
-      canAuthorViewEditorialComments(state, collection, fragment.id)) &&
+      canAuthorViewEditorialComments(state, collection, fragmentId)) &&
     editorialRecommentations.length > 0
   )
 }
@@ -197,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')
@@ -206,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')
 
@@ -216,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')
@@ -230,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)
   )
 }
 
@@ -413,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-manuscript/src/components/ManuscriptPage.js b/packages/component-manuscript/src/components/ManuscriptPage.js
index c8c59a41e..3c642ff36 100644
--- a/packages/component-manuscript/src/components/ManuscriptPage.js
+++ b/packages/component-manuscript/src/components/ManuscriptPage.js
@@ -165,12 +165,12 @@ 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,
@@ -186,7 +186,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 d5f7a8e70..ffe5ac1f4 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 &&
-- 
GitLab