Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkp/pkp-lib#10363 Context setting to control the minimum number of reviews per submission #11080

Merged
merged 1 commit into from
Mar 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions api/v1/_submissions/PKPBackendSubmissionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,18 @@ public function reviews(Request $illuminateRequest): JsonResponse
// limit results depending on a role

if (!$this->canAccessAllSubmissions()) {
$collector->assignedTo([$currentUser->getId()], $queryParams['assignedWithRoles'] ?? null);
$assignedWithRoles = array_map(
intval(...),
paramToArray($queryParams['assignedWithRoles'] ?? [])
);

$collector->assignedTo([$currentUser->getId()], $assignedWithRoles);
}

foreach ($queryParams as $param => $val) {
switch ($param) {
case 'needsReviewers':
$numReviewersPerSubmission = $context->getData('numReviewersPerSubmission');
$collector->filterByReviewersActive(range(0, (int) $numReviewersPerSubmission));
case 'needsReviews':
$collector->filterByNumReviewsConfirmedLimit($context->getNumReviewsPerSubmission());
break;
case 'awaitingReviews':
$collector->filterByAwaitingReviews(true);
Expand Down
16 changes: 8 additions & 8 deletions classes/components/forms/context/PKPReviewSetupForm.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

/**
* @file classes/components/form/context/PKPReviewSetupForm.php
*
Expand All @@ -17,12 +18,11 @@
namespace PKP\components\forms\context;

use PKP\components\forms\FieldHTML;
use PKP\context\Context;
use PKP\components\forms\FieldOptions;
use PKP\components\forms\FieldSlider;
use PKP\components\forms\FieldText;
use PKP\components\forms\FormComponent;
use PKP\services\PKPSchemaService;
use PKP\context\Context;
use PKP\submission\reviewAssignment\ReviewAssignment;

class PKPReviewSetupForm extends FormComponent
Expand Down Expand Up @@ -108,10 +108,10 @@ protected function addDefaultFields(Context $context): static
'size' => 'small',
'groupId' => self::REVIEW_SETTINGS_GROUP,
]))
->addField(new FieldText('numReviewersPerSubmission', [
'label' => __('manager.setup.reviewOptions.numReviewersPerSubmission'),
'description' => __('manager.setup.reviewOptions.numReviewersPerSubmission.description'),
'value' => $context->getData('numReviewersPerSubmission'),
->addField(new FieldText('numReviewsPerSubmission', [
'label' => __('manager.setup.reviewOptions.numReviewsPerSubmission'),
'description' => __('manager.setup.reviewOptions.numReviewsPerSubmission.description'),
'value' => $context->getNumReviewsPerSubmission(),
'size' => 'small',
'groupId' => self::REVIEW_SETTINGS_GROUP,
]));
Expand Down Expand Up @@ -173,7 +173,7 @@ protected function addReminderFields(Context $context): static
'valueLabelMin' => __('manager.setup.reviewOptions.reminders.disbale.label'),
'groupId' => self::REVIEW_REMINDER_GROUP,
]));

return $this;
}

Expand All @@ -184,7 +184,7 @@ protected function addReviewSuggestionControl(Context $context): static
{
$schema = app()->get('schema'); /** @var \PKP\services\PKPSchemaService $schema */

if (!collect($schema->get("context")->properties)->has("reviewerSuggestionEnabled")) {
if (!collect($schema->get('context')->properties)->has('reviewerSuggestionEnabled')) {
return $this;
}

Expand Down
19 changes: 19 additions & 0 deletions classes/context/Context.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ abstract class Context extends \PKP\core\DataObject
public const SUBMISSION_ACKNOWLEDGEMENT_OFF = null;
public const SUBMISSION_ACKNOWLEDGEMENT_SUBMITTING_AUTHOR = 'submittingAuthor';
public const SUBMISSION_ACKNOWLEDGEMENT_ALL_AUTHORS = 'allAuthors';
/*
* The minimum number of completed reviews per submission required to move the submission to the next stage
* can be adjusted by modification of the context setting `numReviewsPerSubmission`
*/
public const REVIEWS_REQUIRED_COUNT = 1;

/**
* Whether DOIs are enabled for this context
Expand Down Expand Up @@ -595,4 +600,18 @@ public function getRequiredMetadata(): array
'type',
])->filter(fn ($prop) => $this->getData($prop) === self::METADATA_REQUIRE)->toArray();
}

/**
* Get the minimal number of reviews required to move the submission to the next stage
*/
public function getNumReviewsPerSubmission(): int
{
$numReviewsPerSubmission = intval($this->getData('numReviewsPerSubmission'));

if ($numReviewsPerSubmission < self::REVIEWS_REQUIRED_COUNT) {
return self::REVIEWS_REQUIRED_COUNT;
}

return $numReviewsPerSubmission;
}
}
81 changes: 29 additions & 52 deletions classes/submission/Collector.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ abstract class Collector implements CollectorInterface, ViewsCount
public ?array $assignedWithRoles = null;

public array|int|null $isReviewedBy = null;
public ?array $reviewersNumber = null;
public ?int $numReviewsConfirmedLimit = null;
public ?bool $awaitingReviews = null;
public ?bool $reviewsSubmitted = null;
public ?bool $reviewsOverdue = null;
Expand Down Expand Up @@ -218,13 +218,12 @@ public function filterByDaysInactive(?int $daysInactive): AppCollector
}

/**
* Limit results to the number of the assigned reviewers.
* Review assignment is considered active after the request is sent by the reviewer
* and it isn't cancelled, declined or overdue
* Limit results by the number of confirmed review assignments
* Returns submissions with the number of confirmed reviews less than this number in the active round
*/
public function filterByReviewersActive(?array $reviewersNumber): AppCollector
public function filterByNumReviewsConfirmedLimit(?int $numReviewsConfirmedLimit): AppCollector
{
$this->reviewersNumber = $reviewersNumber;
$this->numReviewsConfirmedLimit = $numReviewsConfirmedLimit;
return $this;
}

Expand Down Expand Up @@ -760,7 +759,7 @@ protected function buildReviewStageQueries(Builder $q): Builder
{
$reviewFilters = collect([
$this->isReviewedBy,
$this->reviewersNumber,
$this->numReviewsConfirmedLimit,
$this->awaitingReviews,
$this->reviewsSubmitted,
$this->revisionsRequested,
Expand Down Expand Up @@ -814,62 +813,40 @@ protected function buildReviewStageQueries(Builder $q): Builder
)
);

$q->when($this->reviewersNumber !== null, function (Builder $q) use ($currentReviewRound) {
$reviewersNumber = $this->reviewersNumber;
$includeUnassigned = false;
if (in_array(0, $reviewersNumber)) {
$reviewersNumber = array_diff($reviewersNumber, [0]);
$includeUnassigned = true;
}

$q
->when(
$includeUnassigned,
$q->when(
$this->numReviewsConfirmedLimit !== null,
fn (Builder $q) => $q
->whereIn(
's.submission_id',
fn (Builder $q) => $q
->whereNotIn(
's.submission_id',
fn (Builder $q) => $q
->select('s.submission_id')
->from('submissions AS s')
// To retrieve submissions, which require review, we first must calculate the number of confirmed reviews per submission
->leftJoinSub(
DB::table('review_assignments AS ra')
->select('ra.submission_id')
->from('review_assignments AS ra')
->selectRaw('COUNT(ra.submission_id) as reviews_number')
->groupBy('ra.submission_id')
// Retrieve only reviews with the current review round
->joinSub(
$currentReviewRound,
'agrr',
fn (JoinClause $join) =>
$join->on('ra.submission_id', '=', 'agrr.submission_id')
)
->where('ra.declined', 0)
->where('ra.cancelled', 0)
->whereColumn('ra.round', '=', 'agrr.current_round')
->distinct()
->whereNotNull('ra.date_confirmed'),
'rw',
fn (JoinClause $join) => $join->on('s.submission_id', '=', 'rw.submission_id')
)
// Identify submissions with less than the required number of reviews and without assignments
->where(
fn (Builder $q) => $q
->where('rw.reviews_number', '<', $this->numReviewsConfirmedLimit)
->orWhereNull('rw.reviews_number')
)
)
->when(!empty($reviewersNumber), function (Builder $q) use ($reviewersNumber, $currentReviewRound) {
$placeholders = array_fill(0, count($reviewersNumber), '?');

// Aggregate review assignments count per submission
$assignmentsPerSubmission = DB::table('review_assignments', 'ra')
->select('ra.submission_id')
->selectRaw('COUNT(ra.submission_id) as number')
->where('ra.declined', 0)
->where('ra.cancelled', 0)
->groupBy('ra.submission_id')
// Can't replace a single placeholder with array bindings, issue looks similar to laravel/framework#39554
->havingRaw('number IN (' . implode(',', $placeholders) . ')', $reviewersNumber);
$q->whereIn(
's.submission_id',
fn (Builder $q) => $q
// review assignments exist, counting the number of active assignments
->select('agra.submission_id')
->fromSub($assignmentsPerSubmission, 'agra')
->joinSub(
$currentReviewRound,
'agrr',
fn (JoinClause $join) =>
$join->on('agra.submission_id', '=', 'agrr.submission_id')
)
);
});
});
);

$q->when(
$this->awaitingReviews !== null,
Expand Down
2 changes: 1 addition & 1 deletion classes/submission/DashboardView.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class DashboardView
public const TYPE_SUBMISSION = 'initial-review';
public const TYPE_REVIEW_EXTERNAL = 'external-review';
public const TYPE_REVIEW_INTERNAL = 'internal-review';
public const TYPE_NEEDS_REVIEWERS = 'needs-reviewers';
public const TYPE_NEEDS_REVIEWS = 'needs-reviews';
public const TYPE_AWAITING_REVIEWS = 'awaiting-reviews';
public const TYPE_REVIEWS_SUBMITTED = 'reviews-submitted';
public const TYPE_REVIEWS_OVERDUE = 'reviews-overdue';
Expand Down
8 changes: 4 additions & 4 deletions classes/submission/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -934,22 +934,22 @@ protected function mapDashboardViews(Collection $types, Context $context, User $
$canAccessUnassignedSubmission ? null : 'assigned',
['stageIds' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], 'status' => [PKPSubmission::STATUS_QUEUED], 'assignedWithRoles' => $assignedWithRoles]
);
case DashboardView::TYPE_NEEDS_REVIEWERS:
case DashboardView::TYPE_NEEDS_REVIEWS:
$assignedWithRoles = $canAccessUnassignedSubmission ? null : $selectedRoleIds;

$collector = Repo::submission()->getCollector()
->filterByContextIds([$context->getId()])
->filterByReviewersActive(range(0, (int) $context->getData('numReviewersPerSubmission')))
->filterByNumReviewsConfirmedLimit($context->getNumReviewsPerSubmission())
->filterByStatus([PKPSubmission::STATUS_QUEUED]);
return new DashboardView(
$key,
__('submission.dashboard.view.needsReviewer'),
__('submission.dashboard.view.needsReviews'),
[Role::ROLE_ID_SITE_ADMIN, Role::ROLE_ID_MANAGER, Role::ROLE_ID_SUB_EDITOR, Role::ROLE_ID_ASSISTANT],
$canAccessUnassignedSubmission
? $collector
: $collector->assignedTo([$user->getId()], $assignedWithRoles),
'reviews',
['needsReviewers' => true, 'assignedWithRoles' => $assignedWithRoles]
['needsReviews' => true, 'assignedWithRoles' => $assignedWithRoles]
);
case DashboardView::TYPE_AWAITING_REVIEWS:
$assignedWithRoles = $canAccessUnassignedSubmission ? null : $selectedRoleIds;
Expand Down
13 changes: 6 additions & 7 deletions classes/submission/maps/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
use Illuminate\Support\Collection;
use Illuminate\Support\Enumerable;
use Illuminate\Support\LazyCollection;
use PKP\API\v1\reviewers\suggestions\resources\ReviewerSuggestionResource;
use PKP\context\Context;
use PKP\db\DAORegistry;
use PKP\decision\DecisionType;
use PKP\plugins\Hook;
Expand All @@ -31,7 +33,6 @@
use PKP\submission\Genre;
use PKP\submission\reviewAssignment\ReviewAssignment;
use PKP\submission\reviewer\suggestion\ReviewerSuggestion;
use PKP\API\v1\reviewers\suggestions\resources\ReviewerSuggestionResource;
use PKP\submission\reviewRound\ReviewRound;
use PKP\submission\reviewRound\ReviewRoundDAO;
use PKP\submissionFile\SubmissionFile;
Expand Down Expand Up @@ -505,7 +506,7 @@ protected function mapByProperties(array $props, Submission $submission, bool|Co
$output[$prop] = $this->getPropertyParticipants($submission);
break;
case 'reviewersNotAssigned':
$output[$prop] = $currentReviewRound && $this->reviewAssignments->count() >= intval($this->context->getData('numReviewersPerSubmission'));
$output[$prop] = $currentReviewRound && $this->reviewAssignments->count() < $this->context->getNumReviewsPerSubmission();
break;
case 'reviewRounds':
$output[$prop] = $this->getPropertyReviewRounds($reviewRounds);
Expand Down Expand Up @@ -603,7 +604,7 @@ protected function getPropertyReviewerSuggestions(Enumerable $reviewerSuggestion
$reviewerSuggestions = collect(
array_values(
ReviewerSuggestionResource::collection($reviewerSuggestions)
->toArray(app()->get("request"))
->toArray(app()->get('request'))
)
)->map(
fn (array $suggestion): array => array_intersect_key(
Expand Down Expand Up @@ -671,7 +672,7 @@ protected function getPropertyReviewAssignments(Enumerable $reviewAssignments, b
'dateResponseDue' => $dateResponseDue,
'dateConfirmed' => $dateConfirmed,
'dateCompleted' => $dateCompleted,
'dateConsidered' => $dateConsidered,
'dateConsidered' => $dateConsidered,
'dateAssigned' => $dateAssigned,
'competingInterests' => $reviewAssignment->getCompetingInterests(),
'round' => (int) $reviewAssignment->getRound(),
Expand Down Expand Up @@ -740,8 +741,6 @@ protected function getPropertyReviewRounds(Collection $reviewRounds): array
* Get a list of participants assigned to this submission
* and build an array with canLoginAs
*
* @param Submission $submission
* @return array
*/
protected function getPropertyParticipants(Submission $submission): array
{
Expand Down Expand Up @@ -926,7 +925,7 @@ protected function getPropertyStages(Enumerable $stageAssignments, Enumerable $r
!$reviewAssignment->getCancelled()
);
// when being assigned as reviewer to this submission, don't add global roles
if(!$hasCurrentUserReviewAssignment) {
if (!$hasCurrentUserReviewAssignment) {
$globalRoles = array_intersect([Role::ROLE_ID_MANAGER, Role::ROLE_ID_SITE_ADMIN], $this->userRoles);
if (!empty($globalRoles)) {
foreach ($stageIds as $stageId) {
Expand Down
8 changes: 4 additions & 4 deletions locale/en/manager.po
Original file line number Diff line number Diff line change
Expand Up @@ -1338,11 +1338,11 @@ msgstr ""
"Present a link to <button type=\"button\">how to ensure all files are "
"anonymized</button> during upload"

msgid "manager.setup.reviewOptions.numReviewersPerSubmission"
msgstr "Recommended Reviewers Number"
msgid "manager.setup.reviewOptions.numReviewsPerSubmission"
msgstr "Minimum Confirmed Reviews Required"

msgid "manager.setup.reviewOptions.numReviewersPerSubmission.description"
msgstr "Sufficient number of reviewers needed per submission to make recommendation before making a decision"
msgid "manager.setup.reviewOptions.numReviewsPerSubmission.description"
msgstr "Minimum number of confirmed reviews required for a submission"

msgid "manager.setup.reviewOptions.reviewerSuggestionEnabled"
msgstr "Reviewer Suggestion at Submission"
Expand Down
4 changes: 2 additions & 2 deletions locale/en/submission.po
Original file line number Diff line number Diff line change
Expand Up @@ -2422,8 +2422,8 @@ msgstr "All in desk/initial review"
msgid "submission.dashboard.view.reviewExternal"
msgstr "All in peer review"

msgid "submission.dashboard.view.needsReviewer"
msgstr "Needs reviewers"
msgid "submission.dashboard.view.needsReviews"
msgstr "Needs reviews"

msgid "submission.dashboard.view.awaitingReviews"
msgstr "Awaiting reviews"
Expand Down
6 changes: 3 additions & 3 deletions schemas/context.json
Original file line number Diff line number Diff line change
Expand Up @@ -579,12 +579,12 @@
"min:0"
]
},
"numReviewersPerSubmission": {
"numReviewsPerSubmission": {
"type": "integer",
"default": 0,
"default": 1,
"validation": [
"nullable",
"min:0"
"min:1"
]
},
"openAccessPolicy": {
Expand Down
2 changes: 1 addition & 1 deletion schemas/submission.json
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@
},
"reviewersNotAssigned": {
"type": "boolean",
"description": "Whether all needed reviewers are assigned to the submission. Related to the `numReviewersPerSubmission` context setting.",
"description": "Whether all needed reviewers are assigned to the submission. Related to the `numReviewsPerSubmission` context setting.",
"apiSummary": true,
"readOnly": true
},
Expand Down