From 21935368bf6b4fb3e539befb43b84c877938ef6d Mon Sep 17 00:00:00 2001 From: Bozana Bokan Date: Tue, 19 Jan 2021 20:15:28 +0100 Subject: [PATCH] pkp/pkp-lib#4915 display error if several blind participants are selected for a review discussion --- .../ReviewAssignmentDAO.inc.php | 19 +++- .../grid/queries/form/QueryForm.inc.php | 89 ++++++++++++++++++- locale/en_US/editor.po | 6 ++ 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/classes/submission/reviewAssignment/ReviewAssignmentDAO.inc.php b/classes/submission/reviewAssignment/ReviewAssignmentDAO.inc.php index ec18c6e354d..259ba01a830 100644 --- a/classes/submission/reviewAssignment/ReviewAssignmentDAO.inc.php +++ b/classes/submission/reviewAssignment/ReviewAssignmentDAO.inc.php @@ -103,7 +103,6 @@ function getReviewAssignment($reviewRoundId, $reviewerId) { return $row ? $this->_fromRow((array) $row) : null; } - /** * Retrieve a review assignment by review assignment id. * @param $reviewId int @@ -188,6 +187,24 @@ function getBySubmissionId($submissionId, $reviewRoundId = null, $stageId = null return $this->_getReviewAssignmentsArray($query, $queryParams); } + /** + * Retrieve all review assignments by submission and reviewer. + * @param $submissionId int + * @param $reviewerId int + * @param $stageId int optional + * @return array + */ + function getBySubmissionReviewer($submissionId, $reviewerId, $stageId = null) { + $query = $this->_getSelectQuery() . + ' WHERE r.submission_id = ? AND r.reviewer_id = ?'; + $queryParams = [(int) $submissionId, (int) $reviewerId]; + if ($stageId != null) { + $query .= ' AND r.stage_id = ?'; + $queryParams[] = (int) $stageId; + } + return $this->_getReviewAssignmentsArray($query, $queryParams); + } + /** * Get all review assignments for a reviewer. * @param $userId int diff --git a/controllers/grid/queries/form/QueryForm.inc.php b/controllers/grid/queries/form/QueryForm.inc.php index 32e256be0be..a83332149d6 100644 --- a/controllers/grid/queries/form/QueryForm.inc.php +++ b/controllers/grid/queries/form/QueryForm.inc.php @@ -231,7 +231,7 @@ function fetch($request, $template = null, $display = false, $actionArgs = array } // if current user is editor, add all reviewers - if ($user->hasRole([ROLE_ID_SITE_ADMIN], CONTEXT_SITE) || + if ($user->hasRole([ROLE_ID_SITE_ADMIN], CONTEXT_SITE) || $user->hasRole([ROLE_ID_MANAGER], $context->getId()) || array_intersect([ROLE_ID_MANAGER, ROLE_ID_SUB_EDITOR], $assignedRoles)) { foreach ($reviewAssignments as $reviewAssignment) { $includeUsers[] = $reviewAssignment->getReviewerId(); @@ -323,6 +323,93 @@ function readInputData() { )); } + /** + * @copydoc Form::validate() + */ + function validate($callHooks = true) { + // Display error if anonymity is impacted in a review stage: + // 1) several blind reviewers are selected, or + // 2) a blind reviewer and another participant (other than editor or assistant) are selected. + // Editors and assistants are ignored, they can see everything. + // Also admin and manager, if they are creating the discussion, are ignored -- they can see everything. + // In other stages validate that participants are assigned to that stage. + $query = $this->getQuery(); + // Queryies only support ASSOC_TYPE_SUBMISSION so far (see above) + if ($query->getAssocType() == ASSOC_TYPE_SUBMISSION) { + $request = Application::get()->getRequest(); + $user = $request->getUser(); + $context = $request->getContext(); + $submissionId = $query->getAssocId(); + $stageId = $query->getStageId(); + + $reviewAssignmentDao = DAORegistry::getDAO('ReviewAssignmentDAO'); /* @var $reviewAssignmentDao ReviewAssignmentDAO */ + $stageAssignmentDao = DAORegistry::getDAO('StageAssignmentDAO'); /* @var $stageAssignmentDao StageAssignmentDAO */ + $userGroupDao = DAORegistry::getDAO('UserGroupDAO'); /* @var $userGroupDao UserGroupDAO */ + + // get the selected participants + $newParticipantIds = (array) $this->getData('users'); + $participantsToConsider = $blindReviewerCount = 0; + foreach ($newParticipantIds as $participantId) { + // get participant roles in this workflow stage + $assignedRoles = []; + $usersAssignments = $stageAssignmentDao->getBySubmissionAndStageId($submissionId, $stageId, null, $participantId); + while ($usersAssignment = $usersAssignments->next()) { + $userGroup = $userGroupDao->getById($usersAssignment->getUserGroupId()); + $assignedRoles[] = $userGroup->getRoleId(); + } + + if ($stageId == WORKFLOW_STAGE_ID_EXTERNAL_REVIEW || $stageId == WORKFLOW_STAGE_ID_INTERNAL_REVIEW) { + // validate the anonymity + // get participant review assignemnts + $reviewAssignments = $reviewAssignmentDao->getBySubmissionReviewer($submissionId, $participantId, $stageId); + // if participant has no role in this stage and is not a reviewer + if (empty($assignedRoles) && empty($reviewAssignments)) { + // if participant is current user and the user has admin or manager role, ignore participant + if (($participantId == $user->getId()) && ($user->hasRole([ROLE_ID_SITE_ADMIN], CONTEXT_SITE) || $user->hasRole([ROLE_ID_MANAGER], $context->getId()))) { + continue; + } else { + $this->addError('users', __('editor.discussion.errorNotStageParticipant')); + $this->addErrorField('users'); + break; + } + } + // is participant a blind reviewer + $blindReviewer = false; + foreach($reviewAssignments as $reviewAssignment) { + if ($reviewAssignment->getReviewMethod() != SUBMISSION_REVIEW_METHOD_OPEN) { + $blindReviewerCount++; + $blindReviewer = true; + break; + } + } + // if participant is not a blind reviewer and has a role different than editor or assistant + if (!$blindReviewer && !array_intersect([ROLE_ID_MANAGER, ROLE_ID_SUB_EDITOR, ROLE_ID_ASSISTANT], $assignedRoles)) { + $participantsToConsider++; + } + // if anonymity is impacted, display error + if (($blindReviewerCount > 1) || ($blindReviewerCount > 0 && $participantsToConsider > 0)) { + $this->addError('users', __('editor.discussion.errorAnonymousParticipants')); + $this->addErrorField('users'); + break; + } + } else { + // if participant has no role/assignment in the current stage + if (empty($assignedRoles)) { + // if participant is current user and the user has admin or manager role, ignore participant + if (($participantId == $user->getId()) && ($user->hasRole([ROLE_ID_SITE_ADMIN], CONTEXT_SITE) || $user->hasRole([ROLE_ID_MANAGER], $context->getId()))) { + continue; + } else { + $this->addError('users', __('editor.discussion.errorNotStageParticipant')); + $this->addErrorField('users'); + break; + } + } + } + } + } + return parent::validate($callHooks); + } + /** * @copydoc Form::execute() */ diff --git a/locale/en_US/editor.po b/locale/en_US/editor.po index 11b6f43a3f8..95403798007 100644 --- a/locale/en_US/editor.po +++ b/locale/en_US/editor.po @@ -272,6 +272,12 @@ msgstr "Unconsider this Review" msgid "editor.review.readConfirmation" msgstr "Once this review has been read, press \"Confirm\" to indicate that the review process may proceed. If the reviewer has submitted their review elsewhere, you may upload the file below and then press \"Confirm\" to proceed." +msgid "editor.discussion.errorAnonymousParticipants" +msgstr "A discussion can not be created with the selected participants because it would impact the anonymity of the review process." + +msgid "editor.discussion.errorNotStageParticipant" +msgstr "A selected participant is not assigned to this stage." + msgid "editor.submission.schedulePublication" msgstr "Schedule For Publication"