From be08dd3118731e06245ce2dfd494843ea33bcb87 Mon Sep 17 00:00:00 2001 From: Aday Bujeda Date: Thu, 19 Dec 2024 15:25:57 +0000 Subject: [PATCH 1/3] Fixed missing reviewer names in emails when creating a new reviewer in the add reviewer flow --- .../email/PPRFirstNameEmailService.inc.php | 15 +++++++++++++++ .../util/PPRFirstNamesManagementService.inc.php | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/pprOjsPlugin/services/email/PPRFirstNameEmailService.inc.php b/pprOjsPlugin/services/email/PPRFirstNameEmailService.inc.php index e492e2c..8376780 100644 --- a/pprOjsPlugin/services/email/PPRFirstNameEmailService.inc.php +++ b/pprOjsPlugin/services/email/PPRFirstNameEmailService.inc.php @@ -66,6 +66,8 @@ function register() { HookRegistry::register('TemplateManager::fetch', array($this, 'replaceFirstNameInTemplateText')); HookRegistry::register('advancedsearchreviewerform::display', array($this, 'addFirstNameLabelsToAdvancedSearchReviewerForm')); + HookRegistry::register('createreviewerform::display', array($this, 'addFirstNameLabelsToCreateReviewerForm')); + HookRegistry::register('createreviewerform::execute', array($this, 'addCreatedReviewerId')); HookRegistry::register('LoadComponentHandler', array($this, 'addPPRStageParticipantGridHandler')); } @@ -109,6 +111,19 @@ function addFirstNameLabelsToAdvancedSearchReviewerForm($hookName, $arguments) { return false; } + function addFirstNameLabelsToCreateReviewerForm($hookName, $arguments) { + $this->pprObjectFactory->firstNamesManagementService()->addFirstNameLabelsToTemplate('emailVariables'); + + return false; + } + + function addCreatedReviewerId($hookName, $arguments) { + $form = $arguments[0]; + $reviewerId = $form->getData('reviewerId'); + $templateMgr = TemplateManager::getManager(Application::get()->getRequest()); + $templateMgr->assign(['reviewerId' => $reviewerId]); + } + function addFirstNamesToThankReviewerForm($hookName, $arguments) { $thankReviewerForm = $arguments[0]; $review = $thankReviewerForm->getReviewAssignment(); diff --git a/pprOjsPlugin/util/PPRFirstNamesManagementService.inc.php b/pprOjsPlugin/util/PPRFirstNamesManagementService.inc.php index a934be1..d242ef5 100644 --- a/pprOjsPlugin/util/PPRFirstNamesManagementService.inc.php +++ b/pprOjsPlugin/util/PPRFirstNamesManagementService.inc.php @@ -69,12 +69,16 @@ public function replaceFirstNames($originalText, $submission, $reviewerId = null public function getReviewer($reviewerId) { $request = Application::get()->getRequest(); + $templateMgr = TemplateManager::getManager($request); $reviewer = null; if ($reviewerId) { $reviewer = $this->pprSubmissionUtil->getUser($reviewerId); } elseif ($reviewerId = $request->getUserVar('reviewerId')) { // TRY reviewerId REQUEST PARAMETER $reviewer = $this->pprSubmissionUtil->getUser($reviewerId); + } elseif ($reviewerId = $templateMgr->getTemplateVars('reviewerId')) { + // TRY reviewerId IN TEMPLATE MANAGER + $reviewer = $this->pprSubmissionUtil->getUser($reviewerId); } elseif ($reviewId = $request->getUserVar('reviewAssignmentId')) { // TRY reviewAssignment REQUEST PARAMETER $reviewer = $this->pprSubmissionUtil->getReviewer($reviewId); From a13baf56a9bc890cd91752163d4f626a6cf0fda9 Mon Sep 17 00:00:00 2001 From: Aday Bujeda Date: Thu, 19 Dec 2024 15:35:37 +0000 Subject: [PATCH 2/3] Fixed tests after PPRFirstNameEmailService updates --- .../tests/src/services/email/PPRFirstNameEmailServiceTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php b/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php index 6e58f06..fa0d49b 100644 --- a/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php +++ b/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php @@ -36,13 +36,15 @@ public function test_register_should_register_service_hooks_when_firstNameEmailE $target = new PPRFirstNameEmailService($pprPluginMock); $target->register(); - $this->assertEquals(7, $this->countHooks()); + $this->assertEquals(9, $this->countHooks()); $this->assertEquals(1, count($this->getHooks('Mail::send'))); $this->assertEquals(1, count($this->getHooks('reviewreminderform::display'))); $this->assertEquals(1, count($this->getHooks('thankreviewerform::display'))); $this->assertEquals(1, count($this->getHooks('sendreviewsform::display'))); $this->assertEquals(1, count($this->getHooks('TemplateManager::fetch'))); $this->assertEquals(1, count($this->getHooks('advancedsearchreviewerform::display'))); + $this->assertEquals(1, count($this->getHooks('createreviewerform::display'))); + $this->assertEquals(1, count($this->getHooks('createreviewerform::execute'))); $this->assertEquals(1, count($this->getHooks('LoadComponentHandler'))); } From 93a7d49777f3d0d77e7fdea9f77d6f7e12b674a0 Mon Sep 17 00:00:00 2001 From: Aday Bujeda Date: Thu, 19 Dec 2024 19:23:09 +0000 Subject: [PATCH 3/3] Added unit tests for missing reviewer names in emails fix --- .../email/PPRFirstNameEmailServiceTest.php | 24 +++++++++++++++++++ .../PPRFirstNamesManagementServiceTest.php | 18 ++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php b/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php index fa0d49b..5e10725 100644 --- a/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php +++ b/pprOjsPlugin/tests/src/services/email/PPRFirstNameEmailServiceTest.php @@ -6,6 +6,7 @@ import('lib.pkp.controllers.grid.users.reviewer.form.ThankReviewerForm'); import('lib.pkp.controllers.grid.users.reviewer.form.ReviewReminderForm'); +import('lib.pkp.controllers.grid.users.reviewer.form.CreateReviewerForm'); class PPRFirstNameEmailServiceTest extends PPRTestCase { @@ -148,6 +149,29 @@ public function test_addFirstNameLabelsToAdvancedSearchReviewerForm_should_deleg $this->assertEquals(false, $result); } + public function test_addFirstNameLabelsToCreateReviewerForm_should_delegate_to_pprFirstNamesManagementService() { + $objectFactory = $this->getTestUtil()->createObjectFactory(); + + $objectFactory->expects($this->atLeastOnce())->method('firstNamesManagementService'); + $objectFactory->firstNamesManagementService()->expects($this->once())->method('addFirstNameLabelsToTemplate'); + + $target = new PPRFirstNameEmailService($this->defaultPPRPlugin, $objectFactory); + $result = $target->addFirstNameLabelsToCreateReviewerForm('createreviewerform::display', [null]); + $this->assertEquals(false, $result); + } + + public function test_addCreatedReviewerId_should_update_template_manager_with_reviewerId() { + $objectFactory = $this->getTestUtil()->createObjectFactory(); + $form = $this->createMock(CreateReviewerForm::class); + $reviewerId = $this->getRandomId(); + $form->method('getData')->with('reviewerId')->willReturn($reviewerId); + + $target = new PPRFirstNameEmailService($this->defaultPPRPlugin, $objectFactory); + $target->addCreatedReviewerId('createreviewerform::execute', [$form]); + $templateManager = TemplateManager::getManager(); + $this->assertEquals($reviewerId, $templateManager->getTemplateVars('reviewerId')); + } + public function test_addFirstNamesToThankReviewerForm_should_update_form_message_variable_with_firstNamesManagementService_result() { $objectFactory = $this->getTestUtil()->createObjectFactory(); $form = $this->createMock(ThankReviewerForm::class); diff --git a/pprOjsPlugin/tests/src/util/PPRFirstNamesManagementServiceTest.php b/pprOjsPlugin/tests/src/util/PPRFirstNamesManagementServiceTest.php index 87db134..4d49c4e 100644 --- a/pprOjsPlugin/tests/src/util/PPRFirstNamesManagementServiceTest.php +++ b/pprOjsPlugin/tests/src/util/PPRFirstNamesManagementServiceTest.php @@ -5,6 +5,12 @@ class PPRFirstNamesManagementServiceTest extends PPRTestCase { + public function setUp(): void { + parent::setUp(); + # RESET TEMPLATE MANAGER DATA + TemplateManager::getManager()->setData([]); + } + public function test_getReviewer_should_use_reviewerId_if_provided() { $submissionUtil = $this->createMock(PPRSubmissionUtil::class); $reviewerId = $this->getRandomId(); @@ -26,6 +32,18 @@ public function test_getReviewer_should_use_request_reviewerId_reviewerId_is_not $this->assertEquals($reviewer, $result); } + public function test_getReviewer_should_use_template_manager_reviewerId_when_parameter_and_request_reviewerId_not_set() { + $submissionUtil = $this->createMock(PPRSubmissionUtil::class); + $reviewerId = $this->getRandomId(); + $reviewer = $this->addReviewer($submissionUtil, $reviewerId); + $this->getRequestMock()->method('getUserVar')->with('reviewerId')->willReturn(null); + TemplateManager::getManager()->assign('reviewerId', $reviewerId); + + $target = new PPRFirstNamesManagementService($submissionUtil); + $result = $target->getReviewer(null); + $this->assertEquals($reviewer, $result); + } + public function test_getReviewer_should_use_request_reviewAssignmentId_when_parameter_and_request_reviewerId_not_set() { $submissionUtil = $this->createMock(PPRSubmissionUtil::class); $reviewAssignmentId = $this->getRandomId();