diff --git a/api/v1/reviewers/suggestions/formRequests/AddReviewerSuggestion.php b/api/v1/reviewers/suggestions/formRequests/AddReviewerSuggestion.php index 29d556e1299..12ca735c20b 100644 --- a/api/v1/reviewers/suggestions/formRequests/AddReviewerSuggestion.php +++ b/api/v1/reviewers/suggestions/formRequests/AddReviewerSuggestion.php @@ -4,10 +4,9 @@ use APP\core\Application; use APP\facades\Repo; -use Illuminate\Support\Arr; use Illuminate\Validation\Rule; -use Illuminate\Validation\Validator; use Illuminate\Foundation\Http\FormRequest; +use PKP\submission\reviewer\suggestion\ReviewerSuggestion; use PKP\validation\traits\HasMultilingualRule; class AddReviewerSuggestion extends FormRequest @@ -16,12 +15,7 @@ class AddReviewerSuggestion extends FormRequest public function multilingualInputs(): array { - return [ - 'familyName', - 'givenName', - 'affiliation', - 'suggestionReason', - ]; + return (new ReviewerSuggestion)->getMultilingualProps(); } public function primaryLocale(): ?string @@ -56,7 +50,7 @@ public function rules(): array ], 'familyName' => [ 'required', - // 'multilingual:en,fr_CA', + // 'multilingual:en,fr_CA', // Alternative way to do multilingual validation ], 'givenName' => [ 'required', @@ -71,20 +65,12 @@ public function rules(): array 'suggestionReason' => [ 'required', ], - ]; - } - - /** - * Get custom attributes for validator errors. - * - * @return array - */ - public function attributes(): array - { - return [ - 'familyName' => __('user.familyName'), - 'givenName' => __('user.givenName'), - 'email' => __('user.email'), + 'orcidId' => [ + 'sometimes', + 'nullable', + 'string', + // TODO; should have a orcid id vaidation rule ? + ], ]; } @@ -98,23 +84,4 @@ protected function prepareForValidation(): void 'submissionId' => $this->route('submissionId'), ]); } - - /** - * Handle a failed validation attempt. - * - * @param \Illuminate\Validation\Validator $validator - * @return void - * - * @throws \Illuminate\Validation\ValidationException - */ - // protected function failedValidation(\Illuminate\Contracts\Validation\Validator $validator) - // { - // $formatted = []; - // foreach ($validator->errors()->getMessages() as $ruleKey => $messages) { - // Arr::set($formatted, $ruleKey, $messages); - // } - - // ray($formatted); - // parent::failedValidation($validator); - // } } diff --git a/api/v1/reviewers/suggestions/formRequests/EditReviewerSuggestion.php b/api/v1/reviewers/suggestions/formRequests/EditReviewerSuggestion.php index 52279cc40f3..55e3f1bb809 100644 --- a/api/v1/reviewers/suggestions/formRequests/EditReviewerSuggestion.php +++ b/api/v1/reviewers/suggestions/formRequests/EditReviewerSuggestion.php @@ -31,6 +31,12 @@ public function rules(): array 'suggestionReason' => [ 'required', ], + 'orcidId' => [ + 'sometimes', + 'nullable', + 'string', + // TODO; should have a orcid id vaidation rule ? + ], ]; } diff --git a/classes/components/forms/publication/ReviewerSuggestionsForm.php b/classes/components/forms/publication/ReviewerSuggestionsForm.php index 9541cf409b6..7b178eb426c 100644 --- a/classes/components/forms/publication/ReviewerSuggestionsForm.php +++ b/classes/components/forms/publication/ReviewerSuggestionsForm.php @@ -73,9 +73,10 @@ public function __construct(string $action, array $locales, Submission $submissi ])); if (OrcidManager::isEnabled()) { - $this->addField(new FieldOrcid('orcid', [ + $this->addField(new FieldText('orcidId', [ 'label' => __('user.orcid'), 'tooltip' => __('orcid.about.orcidExplanation'), + 'isRequired' => false, ]), [FIELD_POSITION_AFTER, 'email']); } } diff --git a/classes/components/listPanels/PKPSelectReviewerListPanel.php b/classes/components/listPanels/PKPSelectReviewerListPanel.php index b23b9db0d4e..bf8065b5f11 100644 --- a/classes/components/listPanels/PKPSelectReviewerListPanel.php +++ b/classes/components/listPanels/PKPSelectReviewerListPanel.php @@ -166,6 +166,7 @@ public function getConfig() $config['submission'] = $this->submission; if ($context->getData('reviewerSuggestionEnabled')) { + $config['suggestionTitle'] = __('editor.submission.findAndSelectReviewerFromSuggestions'); $config['suggestions'] = $this->getReviewerSuggestions(); } @@ -215,6 +216,9 @@ protected function _getCollector(): Collector ->limit($this->count); } + /** + * Get the reviewer suggestions submission + */ protected function getReviewerSuggestions(): array { $map = Repo::submission()->getSchemaMap(); diff --git a/classes/controllers/grid/users/reviewer/PKPReviewerGridHandler.php b/classes/controllers/grid/users/reviewer/PKPReviewerGridHandler.php index 243165f28bf..8342780082a 100644 --- a/classes/controllers/grid/users/reviewer/PKPReviewerGridHandler.php +++ b/classes/controllers/grid/users/reviewer/PKPReviewerGridHandler.php @@ -23,6 +23,7 @@ use APP\notification\NotificationManager; use APP\submission\Submission; use APP\template\TemplateManager; +use Exception; use Illuminate\Support\Facades\Mail; use PKP\controllers\grid\GridColumn; use PKP\controllers\grid\GridHandler; @@ -67,6 +68,7 @@ use PKP\submission\reviewRound\ReviewRoundDAO; use PKP\submission\SubmissionCommentDAO; use PKP\user\User; +use PKP\controllers\grid\users\reviewer\form\ReviewerForm; use PKP\controllers\grid\users\reviewer\form\EnrollExistingReviewerForm; use PKP\controllers\grid\users\reviewer\form\CreateReviewerForm; use PKP\controllers\grid\users\reviewer\form\AdvancedSearchReviewerForm; @@ -409,21 +411,10 @@ public function enrollReviewer($args, $request) public function updateReviewer($args, $request) { $selectionType = $request->getUserVar('selectionType'); - $formClassName = $this->_getReviewerFormClassName($selectionType); - - // Form handling - if ($selectionType == static::REVIEWER_SELECT_CREATE && $request->getUserVar('reviewerSuggestionId')) { - // TODO : probably an exception if an already approved suggestion id passed - $reviewerForm = new $formClassName( - $this->getSubmission(), - $this->getReviewRound(), - ReviewerSuggestion::find($request->getUserVar('reviewerSuggestionId')) - ); - } else { - $reviewerForm = new $formClassName($this->getSubmission(), $this->getReviewRound()); - } + $reviewerForm = $this->getReviewerFrom($selectionType, $request); $reviewerForm->readInputData(); + if ($reviewerForm->validate()) { $reviewAssignment = $reviewerForm->execute(); @@ -1093,21 +1084,10 @@ public function _fetchReviewerForm($args, $request) { $selectionType = $request->getUserVar('selectionType'); assert(!empty($selectionType)); - $formClassName = $this->_getReviewerFormClassName($selectionType); + $userRoles = $this->getAuthorizedContextObject(Application::ASSOC_TYPE_USER_ROLES); - // Form handling. - if ($selectionType == static::REVIEWER_SELECT_CREATE && $request->getUserVar('reviewerSuggestionId')) { - // TODO : probably an exception if an already approved suggestion id passed - $reviewerForm = new $formClassName( - $this->getSubmission(), - $this->getReviewRound(), - ReviewerSuggestion::find($request->getUserVar('reviewerSuggestionId')) - ); - } else { - $reviewerForm = new $formClassName($this->getSubmission(), $this->getReviewRound()); - } - + $reviewerForm = $this->getReviewerFrom($selectionType, $request); $reviewerForm->initData(); $reviewerForm->setUserRoles($userRoles); @@ -1116,12 +1096,8 @@ public function _fetchReviewerForm($args, $request) /** * Get the name of ReviewerForm class for the current selection type. - * - * @param string $selectionType (const) - * - * @return string Form class name */ - public function _getReviewerFormClassName($selectionType) + public function _getReviewerFormClassName(int $selectionType): string { return match ((int)$selectionType) { static::REVIEWER_SELECT_ADVANCED_SEARCH => AdvancedSearchReviewerForm::class, @@ -1251,6 +1227,31 @@ protected function createMail(Mailable $mailable, string $emailBody, EmailTempla return false; } + + /** + * Get the proper reviewer from instance + */ + protected function getReviewerFrom(int $selectionType, Request $request = null): ReviewerForm + { + $request ??= Application::get()->getRequest(); + $formClassName = $this->_getReviewerFormClassName($selectionType); + + if ($selectionType == static::REVIEWER_SELECT_CREATE && $request->getUserVar('reviewerSuggestionId')) { + $reviewerSuggestion = ReviewerSuggestion::find($request->getUserVar('reviewerSuggestionId')); + + if ($reviewerSuggestion?->hasApproved()) { + throw new Exception('Not allowed to add reviewer suggestion as reviewer that has already been approved'); + } + + return new $formClassName( + $this->getSubmission(), + $this->getReviewRound(), + $reviewerSuggestion + ); + } + + return new $formClassName($this->getSubmission(), $this->getReviewRound()); + } } if (!PKP_STRICT_MODE) { diff --git a/classes/migration/install/ReviewerSuggestionsMigration.php b/classes/migration/install/ReviewerSuggestionsMigration.php index 02785cc160a..dc433c5ed9b 100644 --- a/classes/migration/install/ReviewerSuggestionsMigration.php +++ b/classes/migration/install/ReviewerSuggestionsMigration.php @@ -50,11 +50,6 @@ public function up(): void ->timestamp('approved_at') ->nullable() ->comment('If and when the suggestion approved to add/invite suggested_reviewer'); - - $table - ->bigInteger('stage_id') - ->nullable() - ->comment('The stage at which suggestion approved'); $table ->bigInteger('approver_id') diff --git a/classes/submission/reviewer/suggestion/ReviewerSuggestion.php b/classes/submission/reviewer/suggestion/ReviewerSuggestion.php index 24194ef5be3..aa011d1de7f 100644 --- a/classes/submission/reviewer/suggestion/ReviewerSuggestion.php +++ b/classes/submission/reviewer/suggestion/ReviewerSuggestion.php @@ -39,7 +39,6 @@ protected function casts(): array 'email' => 'string', 'orcid_id' => 'string', 'approved_at' => 'datetime', - 'stage_id' => 'integer', 'approver_id' => 'integer', 'reviewer_id' => 'integer', ]; @@ -55,7 +54,6 @@ public static function getSchemaName(): ?string return null; } - // TODO Add instution details as setting public function getSettings(): array { return [ @@ -66,7 +64,6 @@ public function getSettings(): array ]; } - // TODO should the instution details be a multigingual prop public function getMultilingualProps(): array { return [ @@ -98,17 +95,6 @@ protected function fullName(): Attribute ); } - /** - * Accessor and Mutator for primary key => id - */ - protected function id(): Attribute - { - return Attribute::make( - get: fn($value, $attributes) => $attributes[$this->primaryKey] ?? null, - set: fn($value) => [$this->primaryKey => $value], - )->shouldCache(); - } - protected function submission(): Attribute { return Attribute::make( @@ -168,11 +154,6 @@ public function scopeWithSuggestingUserIds(Builder $query, int|array $userIds): return $query->whereIn('suggesting_user_id', Arr::wrap($userIds)); } - public function scopeWithStageIds(Builder $query, int|array $stageIds): Builder - { - return $query->whereIn('stage_id', Arr::wrap($stageIds)); - } - public function scopeWithApproved(Builder $query, bool $hasApproved = true): Builder { return $query->when( diff --git a/classes/validation/MultilingualInput.php b/classes/validation/MultilingualInput.php index 5d5e300370c..e5beeb806f2 100644 --- a/classes/validation/MultilingualInput.php +++ b/classes/validation/MultilingualInput.php @@ -48,7 +48,10 @@ public function setValidator(Validator $validator): static */ public function validate(string $attribute, mixed $value, Closure $fail): void { - $givenLocales = array_keys($value); + // TODO : Should an array_filter needed to be applied ? + // This will cause disallowed locales with null value to get bypassed even though those + // locale with null value have no impact. + $givenLocales = array_keys(array_filter($value)); if ($this->primaryLocale && !empty($this->primaryLocale) && !in_array($this->primaryLocale, $givenLocales)) { $this->passed = false; diff --git a/classes/validation/ValidatorFactory.php b/classes/validation/ValidatorFactory.php index 57995993592..4bd3ae831a0 100644 --- a/classes/validation/ValidatorFactory.php +++ b/classes/validation/ValidatorFactory.php @@ -2,14 +2,12 @@ /** * @file classes/validation/ValidatorFactory.php * - * Copyright (c) 2014-2021 Simon Fraser University - * Copyright (c) 2000-2021 John Willinsky + * Copyright (c) 2014-2024 Simon Fraser University + * Copyright (c) 2000-2024 John Willinsky * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING. * * @class ValidatorFactory * - * @ingroup validation - * * @brief A factory class for creating a Validator from the Laravel framework. */ @@ -17,6 +15,7 @@ use Illuminate\Support\Arr; use Illuminate\Validation\Validator; +use Illuminate\Validation\Factory; use PKP\config\Config; use PKP\facades\Locale; use PKP\file\TemporaryFileManager; @@ -28,6 +27,11 @@ private function __construct() { } + public static function getFactory(): Factory + { + return app()->get('validator'); + } + /** * Create a validator * @@ -41,8 +45,7 @@ private function __construct() */ public static function make(array $props, array $rules, ?array $messages = []): Validator { - /** @var \Illuminate\Validation\Factory $validationFactory */ - $validationFactory = app()->get('validator'); + $validationFactory = static::getFactory(); return $validationFactory->make($props, $rules, self::getMessages($messages)); } diff --git a/locale/en/editor.po b/locale/en/editor.po index bb71149fe06..a29376697a8 100644 --- a/locale/en/editor.po +++ b/locale/en/editor.po @@ -133,6 +133,9 @@ msgstr "" msgid "editor.submission.addReviewer" msgstr "Add Reviewer" +msgid "editor.submission.findAndSelectReviewerFromSuggestions" +msgstr "Select a Reviewer from Reviewer Suggestions" + msgid "editor.submission.findAndSelectReviewer" msgstr "Locate a Reviewer"