Code cleanup

pkp · Nov 20, 2024 · cf0580b · cf0580b
1 parent 34e9b04
commit cf0580b
Show file tree

Hide file tree

Showing 19 changed files with 103 additions and 94 deletions.
diff --git a/classes/components/forms/FieldEmailTemplateUnrestricted.php b/classes/components/forms/FieldEmailTemplateUnrestricted.php
@@ -11,7 +11,7 @@
  *
  * @ingroup classes_controllers_form
  *
- * @brief A component to indicate if an email template is unrestricted, i.e accessible to all user groups within the associated mailable
+ * @brief A component to indicate if an email template is unrestricted, i.e accessible to all user groups.
  */
 
 namespace PKP\components\forms;
@@ -20,13 +20,14 @@ class FieldEmailTemplateUnrestricted extends Field
 {
     /** @copydoc Field::$component */
     public $component = 'field-email-template-unrestricted';
-
+    public string $subNote = '';
     /**
      * @copydoc Field::getConfig()
      */
     public function getConfig()
     {
         $config = parent::getConfig();
+        $config['subNote'] = $this->subNote;
         return $config;
     }
 }
diff --git a/classes/components/forms/FieldEmailTemplateUserGroupSettings.php b/classes/components/forms/FieldEmailTemplateUserGroupSettings.php
@@ -11,7 +11,7 @@
  *
  * @ingroup classes_controllers_form
  *
- * @brief A component managing user groups assignable to an email template
+ * @brief A component managing user groups assigned to an email template
  */
 
 namespace PKP\components\forms;

diff --git a/classes/components/forms/emailTemplate/EmailTemplateForm.php b/classes/components/forms/emailTemplate/EmailTemplateForm.php
@@ -48,16 +48,14 @@ public function __construct(string $action, array $locales)
                 'isMultilingual' => true,
                 'toolbar' => 'bold italic superscript subscript | link | blockquote bullist numlist',
                 'plugins' => 'paste,link,lists',
-            ]))->addField(
-                new FieldEmailTemplateUnrestricted('isUnrestricted'),
-                [
-                    'type' => 'checkbox'
-                ]
-            )
+            ]))->addField(new FieldEmailTemplateUnrestricted('isUnrestricted', [
+                'type' => 'checkbox',
+                'label' => __('admin.workflow.email.userGroup.assign.unrestricted'),
+                'subNote' => __('admin.workflow.email.userGroup.unrestricted.template.note')
+            ]))
             ->addField(new FieldEmailTemplateUserGroupSettings('userGroupIds', [
                 'type' => 'checkbox',
                 'label' => __('admin.workflow.email.userGroup.allowed'),
-
             ]));
     }
 }
diff --git a/classes/controllers/grid/users/reviewer/PKPReviewerGridHandler.php b/classes/controllers/grid/users/reviewer/PKPReviewerGridHandler.php
@@ -1048,7 +1048,7 @@ public function fetchTemplateBody(array $args, PKPRequest $request): ?JSONMessag
         };
         $template = Repo::emailTemplate()->getByKey($context->getId(), $request->getUserVar('template'));
 
-        if (!$template) {
+        if (!$template || ! Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $template, $context->getId())) {
             return null;
         }
 

diff --git a/classes/decision/steps/Email.php b/classes/decision/steps/Email.php
@@ -128,7 +128,7 @@ protected function getEmailTemplates(): array
         $emailTemplates = collect();
         if ($this->mailable::getEmailTemplateKey()) {
             $emailTemplate = Repo::emailTemplate()->getByKey($context->getId(), $this->mailable::getEmailTemplateKey());
-            if (Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $emailTemplate, $context->getId())) {
+            if ($emailTemplate && Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $emailTemplate, $context->getId())) {
                 $emailTemplates->add($emailTemplate);
             }
             Repo::emailTemplate()
@@ -142,7 +142,6 @@ protected function getEmailTemplates(): array
                 });
         }
 
-
         return Repo::emailTemplate()->getSchemaMap()->mapMany($emailTemplates)->toArray();
     }
 

diff --git a/classes/emailTemplate/DAO.php b/classes/emailTemplate/DAO.php
@@ -282,15 +282,16 @@ public function installEmailTemplates(
                 }
             }
 
-            if (isset($attrs['isUnrestricted'])) {
-                if ($attrs['isUnrestricted'] !== '1' && $attrs['isUnrestricted'] !== '0') {
-                    throw new Exception('Invalid value given for the `isUnrestricted` attribute on the ' . $attrs['key'] . ' template');
-                }
+            // Default to true if `isUnrestricted` is not set.
+            $isUnrestricted = $attrs['isUnrestricted'] ?? '1';
 
-                $contextIds = app()->get('context')->getIds();
-                foreach ($contextIds as $contextId) {
-                    Repo::emailTemplate()->markTemplateAsUnrestricted($attrs['key'], (bool)$attrs['isUnrestricted'], $contextId);
-                }
+            if ($isUnrestricted !== '1' && $isUnrestricted !== '0') {
+                throw new Exception('Invalid value given for the `isUnrestricted` attribute on the ' . $attrs['key'] . ' template.');
+            }
+
+            $contextIds = app()->get('context')->getIds();
+            foreach ($contextIds as $contextId) {
+                Repo::emailTemplate()->markTemplateAsUnrestricted($attrs['key'], (bool)$isUnrestricted, $contextId);
             }
         }
         return true;

diff --git a/classes/emailTemplate/Repository.php b/classes/emailTemplate/Repository.php
@@ -130,7 +130,7 @@ public function validate(?EmailTemplate $object, array $props, Context $context)
             });
         }
 
-        //  If groupIds were passed to limit email access, check that groups exists within the context
+        //  If groupIds were passed to limit email access, check that the user groups exists within the context
         if (isset($props['userGroupIds'])) {
             $validator->after(function () use ($validator, $props, $context) {
                 $existingGroupIds = Repo::userGroup()->getCollector()
@@ -141,7 +141,6 @@ public function validate(?EmailTemplate $object, array $props, Context $context)
                     $validator->errors()->add('userGroupIds', __('api.emailTemplates.404.userGroupIds'));
                 }
             });
-
         }
 
         // Check for input from disallowed locales
@@ -231,7 +230,7 @@ public function restoreDefaults($contextId): array
     /***
      * Gets the IDs of the user groups assigned to an email template
      */
-    public function getUserGroupsIdsAssignedToTemplate(string $templateKey, int $contextId): array
+    public function getAssignedGroupsIds(string $templateKey, int $contextId): array
     {
         return EmailTemplateAccessGroup::withEmailKey([$templateKey])
             ->withContextId($contextId)
@@ -248,7 +247,7 @@ public function isTemplateUnrestricted(string $templateKey, int $contextId): boo
     {
         return !!EmailTemplateAccessGroup::withEmailKey([$templateKey])
             ->withContextId($contextId)
-            ->where('user_group_id', null)
+            ->whereNull('user_group_id')
             ->first();
     }
 
@@ -263,7 +262,7 @@ public function isTemplateAccessibleToUser(User $user, EmailTemplate $template,
         }
 
         $userUserGroups = Repo::userGroup()->userUserGroups($user->getId(), $contextId)->all();
-        $templateUserGroups = $this->getUserGroupsIdsAssignedToTemplate($template->getData('key'), $contextId);
+        $templateUserGroups = $this->getAssignedGroupsIds($template->getData('key'), $contextId);
 
         foreach ($userUserGroups as $userGroup) {
             if (in_array($userGroup->getId(), $templateUserGroups)) {
@@ -280,7 +279,7 @@ public function isTemplateAccessibleToUser(User $user, EmailTemplate $template,
      * @param Enumerable $templates List of EmailTemplates to filter.
      * @param User $user The user whose access level is used for filtering.
      *
-     * @return Collection Filtered list of EmailTemplate objects accessible to the user.
+     * @return Collection Filtered list of EmailTemplates accessible to the user.
      */
     public function filterTemplatesByUserAccess(Enumerable $templates, User $user, int $contextId): Collection
     {
@@ -298,7 +297,7 @@ public function filterTemplatesByUserAccess(Enumerable $templates, User $user, i
     /***
      * Internal method used to assign user group IDs to an email template
      */
-    private function _updateTemplateAccessGroups(EmailTemplate $emailTemplate, array $newUserGroupIds, int $contextId): void
+    private function updateTemplateAccessGroups(EmailTemplate $emailTemplate, array $newUserGroupIds, int $contextId): void
     {
         EmailTemplateAccessGroup::withEmailKey([$emailTemplate->getData('key')])
             ->withContextId($contextId)
@@ -307,13 +306,13 @@ private function _updateTemplateAccessGroups(EmailTemplate $emailTemplate, array
         foreach ($newUserGroupIds as $id) {
             EmailTemplateAccessGroup::updateOrCreate(
                 [
-                    // The where conditions (keys that should match)
+                    // The where conditions
                     'email_key' => $emailTemplate->getData('key'),
                     'user_group_id' => $id,
                     'context_id' => $contextId,
                 ],
                 [
-                    // The data to insert or update (values to set)
+                    // The data to insert or update
                     'emailKey' => $emailTemplate->getData('key'),
                     'userGroupId' => $id,
                     'contextId' => $contextId,
@@ -328,7 +327,7 @@ private function _updateTemplateAccessGroups(EmailTemplate $emailTemplate, array
     public function setEmailTemplateAccess(EmailTemplate $emailTemplate, int $contextId, ?array $userGroupIds, ?bool $isUnrestricted): void
     {
         if($userGroupIds !== null) {
-            $this->_updateTemplateAccessGroups($emailTemplate, $userGroupIds, $contextId);
+            $this->updateTemplateAccessGroups($emailTemplate, $userGroupIds, $contextId);
         }
 
         if($isUnrestricted !== null) {
@@ -339,22 +338,21 @@ public function setEmailTemplateAccess(EmailTemplate $emailTemplate, int $contex
 
     /**
      * Mark an email template as unrestricted or not.
-     * An unrestricted email template is available to all user groups associated with the Roles linked to the mailable that the template belongs to.
-     * Mailable roles are stored in the $fromRoleIds property of a mailable
+     * An unrestricted email template is available to all user groups.
      */
     public function markTemplateAsUnrestricted(string $emailKey, bool $isUnrestricted, int $contextId): void
     {
         // Unrestricted emails are represented by an entry with a `null` value for the user group ID
         if ($isUnrestricted) {
             EmailTemplateAccessGroup::updateOrCreate(
                 [
-                    // The where conditions (keys that should match)
+                    // The where conditions
                     'email_key' => $emailKey,
                     'user_group_id' => null,
                     'context_id' => $contextId,
                 ],
                 [
-                    // The data to insert or update (values to set)
+                    // The data to insert or update
                     'emailKey' => $emailKey,
                     'userGroupId' => null,
                     'contextId' => $contextId,

diff --git a/classes/emailTemplate/maps/Schema.php b/classes/emailTemplate/maps/Schema.php
@@ -19,6 +19,7 @@
 use PKP\core\PKPApplication;
 use PKP\emailTemplate\EmailTemplate;
 use PKP\services\PKPSchemaService;
+use PKP\userGroup\UserGroup;
 
 class Schema extends \PKP\core\maps\Schema
 {
@@ -82,8 +83,8 @@ public function summarizeMany(Enumerable $collection, string $mailableClass = nu
     protected function mapByProperties(array $props, EmailTemplate $item, string $mailableClass = null): array
     {
         $output = [];
+        $mailableClass = $mailableClass ?? Repo::mailable()->get($item->getData('key'), Application::get()->getRequest()->getContext());
 
-        $mailableClass = $mailableClass ?? Repo::mailable()->getMailableByEmailTemplate($item);
         foreach ($props as $prop) {
             switch ($prop) {
                 case '_href':
@@ -99,11 +100,27 @@ protected function mapByProperties(array $props, EmailTemplate $item, string $ma
                     break;
                 case 'assignedUserGroupIds':
                     if ($mailableClass && Repo::mailable()->isGroupsAssignableToTemplates($mailableClass)) {
-                        $output['assignedUserGroupIds'] = Repo::emailTemplate()->getUserGroupsIdsAssignedToTemplate($item->getData('key'), Application::get()->getRequest()->getContext()->getId());
+                        $output['assignedUserGroupIds'] = Repo::emailTemplate()->getAssignedGroupsIds($item->getData('key'), Application::get()->getRequest()->getContext()->getId());
                     } else {
                         $output['assignedUserGroupIds'] = [];
                     }
                     break;
+                case 'assignableTemplateUserGroups':
+                    if($mailableClass && Repo::mailable()->isGroupsAssignableToTemplates($mailableClass)) {
+                        $userGroups = collect();
+
+                        Repo::userGroup()->getCollector()
+                            ->filterByContextIds([Application::get()->getRequest()->getContext()->getId()])
+                            ->getMany()->each(fn (UserGroup $group) => $userGroups->add([
+                                'id' => $group->getId(),
+                                'name' => $group->getLocalizedName()
+                            ]));
+
+                        $output['assignableTemplateUserGroups'] = $userGroups;
+                    } else {
+                        $output['assignableTemplateUserGroups'] = [];
+                    }
+                    break;
                 default:
                     $output[$prop] = $item->getData($prop);
                     break;

diff --git a/classes/invitation/sections/Email.php b/classes/invitation/sections/Email.php
@@ -64,7 +64,6 @@ public function getState(): stdClass
 
     /**
      * Get all email recipients for email composer
-     * @return array
      */
     protected function getRecipientOptions(): array
     {
@@ -84,7 +83,6 @@ protected function getRecipientOptions(): array
 
     /**
      * Get all email templates for email composer
-     * @return array
      */
     protected function getEmailTemplates(): array
     {
@@ -93,15 +91,21 @@ protected function getEmailTemplates(): array
 
         $emailTemplates = collect();
         if ($this->mailable::getEmailTemplateKey()) {
+            $user = $request->getUser();
+
             $emailTemplate = Repo::emailTemplate()->getByKey($context->getId(), $this->mailable::getEmailTemplateKey());
-            if ($emailTemplate) {
+            if ($emailTemplate && Repo::emailTemplate()->isTemplateAccessibleToUser($user, $emailTemplate, $context->getId())) {
                 $emailTemplates->add($emailTemplate);
             }
             Repo::emailTemplate()
                 ->getCollector($context->getId())
                 ->alternateTo([$this->mailable::getEmailTemplateKey()])
                 ->getMany()
-                ->each(fn (EmailTemplate $e) => $emailTemplates->add($e));
+                ->each(function (EmailTemplate $e) use ($context, $user, $emailTemplates) {
+                    if(Repo::emailTemplate()->isTemplateAccessibleToUser($user, $e, $context->getId())) {
+                        $emailTemplates->add($e);
+                    }
+                });
         }
 
         return Repo::emailTemplate()->getSchemaMap()->mapMany($emailTemplates)->toArray();

diff --git a/classes/mail/Repository.php b/classes/mail/Repository.php
@@ -18,7 +18,6 @@
 use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
 use PKP\context\Context;
-use PKP\emailTemplate\EmailTemplate;
 use PKP\mail\mailables\DecisionNotifyOtherAuthors;
 use PKP\mail\mailables\EditReviewNotify;
 use PKP\mail\mailables\ReviewCompleteNotifyEditors;
@@ -29,7 +28,6 @@
 use PKP\mail\mailables\SubmissionSavedForLater;
 use PKP\mail\traits\Configurable;
 use PKP\plugins\Hook;
-use PKP\userGroup\UserGroup;
 
 class Repository
 {
@@ -103,23 +101,6 @@ public function summarizeMailable(string $class): array
         $dataDescriptions = $class::getDataDescriptions();
         ksort($dataDescriptions);
 
-        // get roles for mailable
-        $roles = $class::getFromRoleIds();
-        // Get the groups for each role
-        $userGroups = [];
-        $roleNames = Application::get()->getRoleNames();
-
-        $userGroups = collect();
-
-        Repo::userGroup()->getCollector()
-            ->filterByContextIds([Application::get()->getRequest()->getContext()->getId()])
-            ->getMany()->each(fn (UserGroup $group) => $userGroups->add([
-                'id' => $group->getId(),
-                'name' => $group->getLocalizedName(),
-                'roleId' => $group->getRoleId(),
-                'roleName' => $roleNames[$group->getRoleId()]
-            ]));
-
         return [
             '_href' => Application::get()->getRequest()->getDispatcher()->url(
                 Application::get()->getRequest(),
@@ -136,7 +117,6 @@ public function summarizeMailable(string $class): array
             'supportsTemplates' => $class::getSupportsTemplates(),
             'toRoleIds' => $class::getToRoleIds(),
             'canAssignUserGroupToTemplates' => $this->isGroupsAssignableToTemplates($class),
-            'assignableTemplateUserGroups' => $userGroups
         ];
     }
 
@@ -226,10 +206,11 @@ protected function isMailableConfigurable(string $class, Context $context): bool
     }
 
 
-    // Check if the templates of a given mailable can be assigned to specific groups
     /**
+     * Check if the templates of a given mailable can be assigned to specific groups
+     *
      * @param Mailable|string $mailable - Mailable class or qualified string referencing the class
- */
+     */
     public function isGroupsAssignableToTemplates(Mailable|string $mailable): bool
     {
         return !in_array(Mailable::FROM_SYSTEM, $mailable::getFromRoleIds());
@@ -294,19 +275,4 @@ public function map(): Collection
             mailables\ValidateEmailSite::class,
         ]);
     }
-
-    /**
-     * Gets the mailable for a given email template
-     *
-     * @param EmailTemplate $template
-     *
-     * Note: This does not discover/find mailbles defined within plugins
-     *
-     * @return string|null - Fully Qualified Class Name of a mailable
-     */
-    public function getMailableByEmailTemplate(EmailTemplate $template): ?string
-    {
-        $emailKey = $template->getData('alternateTo') ?? $template->getData('key');
-        return $this->map()->first(fn ($class) => $class::getEmailTemplateKey() === $emailKey);
-    }
 }
diff --git a/classes/migration/install/EmailTemplateUserGroupAccessMigration.php b/classes/migration/install/EmailTemplateUserGroupAccessMigration.php
@@ -28,8 +28,8 @@ public function up(): void
         Schema::create('email_template_user_group_access', function (Blueprint $table) use ($contextDao) {
             $table->bigInteger('email_template_user_group_access_id')->autoIncrement()->comment('Primary key');
             $table->string('email_key', 255)->comment("The email template's unique key");
-            $table->bigInteger('context_id')->comment('Identifier for the context for which the user group assignment occurs.');
-            $table->bigInteger('user_group_id')->nullable()->comment('The user group ID.');
+            $table->bigInteger('context_id')->comment('The ID of the context for which the user group assignment is defined.');
+            $table->bigInteger('user_group_id')->nullable()->comment('The user group ID. A null value indicates that the email template is accessible to all user groups.');
 
             $table->foreign('context_id')->references($contextDao->primaryKeyColumn)->on($contextDao->tableName)->onDelete('cascade')->onDelete('cascade');
             $table->foreign('user_group_id')->references('user_group_id')->on('user_groups')->onDelete('cascade')->onDelete('cascade');