#10571 Remove unnecessary email form components and clean up code

pkp · Nov 25, 2024 · efecaab · efecaab
1 parent 098ea45
commit efecaab
Show file tree

Hide file tree

Showing 16 changed files with 101 additions and 157 deletions.
diff --git a/api/v1/emailTemplates/PKPEmailTemplateController.php b/api/v1/emailTemplates/PKPEmailTemplateController.php
@@ -205,7 +205,7 @@ public function add(Request $illuminateRequest): JsonResponse
         $emailTemplate = Repo::emailTemplate()->newDataObject($params);
         Repo::emailTemplate()->add($emailTemplate);
 
-        Repo::emailTemplate()->setEmailTemplateAccess($emailTemplate, $requestContext->getId(), $params['userGroupIds'], $params['isUnrestricted']);
+        Repo::emailTemplate()->setEmailTemplateAccess($emailTemplate, $requestContext->getId(), $params['assignedUserGroupIds'], $params['isUnrestricted']);
 
         $emailTemplate = Repo::emailTemplate()->getByKey($emailTemplate->getData('contextId'), $emailTemplate->getData('key'));
 
@@ -247,7 +247,7 @@ public function edit(Request $illuminateRequest): JsonResponse
 
         // If the user submitted an empty list (meaning all user groups were unchecked), the empty array is converted to null in the request's data.
         // Convert back to an empty array.
-        $params['userGroupIds'] = $params['userGroupIds'] ?: [];
+        $params['assignedUserGroupIds'] = $params['assignedUserGroupIds'] ?: [];
 
         $errors = Repo::emailTemplate()->validate(
             $emailTemplate,
@@ -260,7 +260,7 @@ public function edit(Request $illuminateRequest): JsonResponse
         }
 
         Repo::emailTemplate()->edit($emailTemplate, $params);
-        Repo::emailTemplate()->setEmailTemplateAccess($emailTemplate, $requestContext->getId(), $params['userGroupIds'], $params['isUnrestricted']);
+        Repo::emailTemplate()->setEmailTemplateAccess($emailTemplate, $requestContext->getId(), $params['assignedUserGroupIds'], $params['isUnrestricted']);
 
         $emailTemplate = Repo::emailTemplate()->getByKey(
             // context ID is null if edited for the first time

diff --git a/classes/components/forms/FieldEmailTemplateUnrestricted.php b/classes/components/forms/FieldEmailTemplateUnrestricted.php
diff --git a/classes/components/forms/FieldEmailTemplateUserGroupSettings.php b/classes/components/forms/FieldEmailTemplateUserGroupSettings.php
diff --git a/classes/components/forms/emailTemplate/EmailTemplateForm.php b/classes/components/forms/emailTemplate/EmailTemplateForm.php
@@ -15,11 +15,13 @@
 
 namespace PKP\components\forms\emailTemplate;
 
-use PKP\components\forms\FieldEmailTemplateUnrestricted;
-use PKP\components\forms\FieldEmailTemplateUserGroupSettings;
+use APP\core\Application;
+use APP\facades\Repo;
+use PKP\components\forms\FieldOptions;
 use PKP\components\forms\FieldPreparedContent;
 use PKP\components\forms\FieldText;
 use PKP\components\forms\FormComponent;
+use PKP\userGroup\UserGroup;
 
 class EmailTemplateForm extends FormComponent
 {
@@ -32,6 +34,14 @@ public function __construct(string $action, array $locales)
         $this->method = 'POST';
         $this->locales = $locales;
 
+        $userGroups = collect();
+        Repo::userGroup()->getCollector()
+            ->filterByContextIds([Application::get()->getRequest()->getContext()->getId()])
+            ->getMany()->each(fn (UserGroup $group) => $userGroups->add([
+                'value' => $group->getId(),
+                'label' => $group->getLocalizedName()
+            ]));
+
         $this->addField(new FieldText('name', [
             'label' => __('common.name'),
             'description' => __('manager.emailTemplate.name.description'),
@@ -48,14 +58,22 @@ 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',
-                'label' => __('admin.workflow.email.userGroup.assign.unrestricted'),
-                'subNote' => __('admin.workflow.email.userGroup.unrestricted.template.note')
             ]))
-            ->addField(new FieldEmailTemplateUserGroupSettings('userGroupIds', [
+            ->addField(new FieldOptions('isUnrestricted', [
+                'label' => __('admin.workflow.email.userGroup.assign.unrestricted'),
+                'groupId' => 'isUnrestricted',
+                'description' => __('admin.workflow.email.userGroup.unrestricted.template.note'),
                 'type' => 'checkbox',
+                'options' => [
+                    ['value' => true, 'label' => __('admin.workflow.email.userGroup.assign.unrestricted')],
+                ],
+                'value' => true
+            ]))
+            ->addField(new FieldOptions('assignedUserGroupIds', [
                 'label' => __('admin.workflow.email.userGroup.allowed'),
+                'type' => 'checkbox',
+                'value' => [],
+                'options' => $userGroups
             ]));
     }
 }
diff --git a/classes/decision/steps/Email.php b/classes/decision/steps/Email.php
@@ -135,9 +135,9 @@ protected function getEmailTemplates(): array
                 ->getCollector($context->getId())
                 ->alternateTo([$this->mailable::getEmailTemplateKey()])
                 ->getMany()
-                ->each(function (EmailTemplate $e) use ($context, $request, $emailTemplates) {
-                    if (Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $e, $context->getId())) {
-                        $emailTemplates->add($e);
+                ->each(function (EmailTemplate $template) use ($context, $request, $emailTemplates) {
+                    if (Repo::emailTemplate()->isTemplateAccessibleToUser($request->getUser(), $template, $context->getId())) {
+                        $emailTemplates->add($template);
                     }
                 });
         }

diff --git a/classes/emailTemplate/DAO.php b/classes/emailTemplate/DAO.php
@@ -235,7 +235,7 @@ public function getMainEmailTemplatesFilename()
      * skipping others
      * @param bool $skipExisting If true, do not install email templates
      * that already exist in the database
-     * @param bool $recordTemplateGroupAccess - If true, records the templates as unrestricted.
+     * @param bool $recordTemplateGroupAccess - If true, records the templates as unrestricted. For versions 3.6 or higher, this value should be set to true when calling `installEmailTemplates`.
      * By default, it is set to false to ensure compatibility with older processes (e.g., migrations)
      * where the `email_template_user_group_access` table may not exist at the time of execution.
      *

diff --git a/classes/emailTemplate/EmailTemplateAccessGroup.php b/classes/emailTemplate/EmailTemplateAccessGroup.php
@@ -1,32 +1,56 @@
 <?php
 
+/**
+ * @file classes/emailTemplate/EmailTemplateAccessGroup.php
+ *
+ * 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 EmailTemplateAccessGroup
+ *
+ * @ingroup emailTemplate
+ *
+ * @brief Eloquent model for email template user group access
+ */
+
 namespace PKP\emailTemplate;
 
 use Eloquence\Behaviours\HasCamelCasing;
-use Eloquence\Database\Model;
 use Illuminate\Contracts\Database\Eloquent\Builder;
+use Illuminate\Database\Eloquent\Model;
 
 class EmailTemplateAccessGroup extends Model
 {
     use HasCamelCasing;
+
     public $timestamps = false;
     protected $primaryKey = 'email_template_user_group_access_id';
     protected $table = 'email_template_user_group_access';
-    protected $fillable = ['userGroupId', 'contextId','emailKey'];
+    protected $fillable = ['userGroupId', 'contextId', 'emailKey'];
 
 
+    /**
+     * Scope a query to only include email template access records for email templates with specific keys
+     */
     public function scopeWithEmailKey(Builder $query, ?array $keys): Builder
     {
         return $query->when(!empty($keys), function ($query) use ($keys) {
             return $query->whereIn('email_key', $keys);
         });
     }
 
+    /**
+     * Scope a query to only include email template access records that are related to a specific context ID
+     */
     public function scopeWithContextId(Builder $query, int $contextId): Builder
     {
         return $query->where('context_id', $contextId);
     }
 
+    /**
+     * Scope a query to only include email template access records for specific user group IDs
+     */
     public function scopeWithGroupIds(Builder $query, array $ids): Builder
     {
         return $query->whereIn('user_group_id', $ids);

diff --git a/classes/emailTemplate/Repository.php b/classes/emailTemplate/Repository.php
@@ -131,15 +131,15 @@ public function validate(?EmailTemplate $object, array $props, Context $context)
             });
         }
 
-        //  If groupIds were passed to limit email access, check that the user groups exists within the context
-        if (isset($props['userGroupIds'])) {
+        //  If assignedUserGroupIds were passed to limit email access, check that the user groups exists within the context
+        if (isset($props['assignedUserGroupIds'])) {
             $validator->after(function () use ($validator, $props, $context) {
                 $existingGroupIds = Repo::userGroup()->getCollector()
                     ->filterByContextIds([$context->getId()])
-                    ->filterByUserGroupIds($props['userGroupIds'])->getIds()->toArray();
+                    ->filterByUserGroupIds($props['assignedUserGroupIds'])->getIds()->toArray();
 
-                if (!empty(array_diff($existingGroupIds, $props['userGroupIds']))) {
-                    $validator->errors()->add('userGroupIds', __('api.emailTemplates.404.userGroupIds'));
+                if (!empty(array_diff($existingGroupIds, $props['assignedUserGroupIds']))) {
+                    $validator->errors()->add('assignedUserGroupIds', __('api.emailTemplates.404.userGroupIds'));
                 }
             });
         }
@@ -231,7 +231,6 @@ public function restoreDefaults($contextId): array
         return $deletedKeys;
     }
 
-
     /***
      * Gets the IDs of the user groups assigned to an email template
      */
@@ -256,7 +255,6 @@ public function isTemplateUnrestricted(string $templateKey, int $contextId): boo
             ->first();
     }
 
-
     /**
      * Checks if an email template is accessible to a user. A template is accessible if it is assigned to a user group that the user belongs to or if the template is unrestricted
      */
@@ -327,6 +325,7 @@ private function updateTemplateAccessGroups(EmailTemplate $emailTemplate, array
     }
 
     /**
+     * Sets the restrictions for an email template
      * Pass empty array in $userGroupIds to delete all existing user groups for a template
      */
     public function setEmailTemplateAccess(EmailTemplate $emailTemplate, int $contextId, ?array $userGroupIds, ?bool $isUnrestricted): void
@@ -340,7 +339,6 @@ 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.
@@ -372,7 +370,6 @@ public function markTemplateAsUnrestricted(string $emailKey, bool $isUnrestricte
         }
     }
 
-
     /**
      * Deletes all user group access for an email
      */

diff --git a/classes/emailTemplate/maps/Schema.php b/classes/emailTemplate/maps/Schema.php
@@ -19,7 +19,6 @@
 use PKP\core\PKPApplication;
 use PKP\emailTemplate\EmailTemplate;
 use PKP\services\PKPSchemaService;
-use PKP\userGroup\UserGroup;
 
 class Schema extends \PKP\core\maps\Schema
 {
@@ -44,9 +43,8 @@ public function map(EmailTemplate $item): array
      *
      * Includes properties with the apiSummary flag in the email template schema.
      *
-     * @param null|mixed $mailableClass
      */
-    public function summarize(EmailTemplate $item, $mailableClass = null): array
+    public function summarize(EmailTemplate $item, ?string $mailableClass = null): array
     {
         return $this->mapByProperties($this->getSummaryProps(), $item, $mailableClass);
     }
@@ -69,7 +67,7 @@ public function mapMany(Enumerable $collection): Enumerable
      *
      * @see self::summarize
      */
-    public function summarizeMany(Enumerable $collection, string $mailableClass = null): Enumerable
+    public function summarizeMany(Enumerable $collection, ?string $mailableClass = null): Enumerable
     {
         $this->collection = $collection;
         return $collection->map(function ($item) use ($mailableClass) {
@@ -80,10 +78,12 @@ public function summarizeMany(Enumerable $collection, string $mailableClass = nu
     /**
      * Map schema properties of an Email Template to an assoc array
      */
-    protected function mapByProperties(array $props, EmailTemplate $item, string $mailableClass = null): array
+    protected function mapByProperties(array $props, EmailTemplate $item, ?string $mailableClass = null): array
     {
         $output = [];
-        $mailableClass = $mailableClass ?? Repo::mailable()->get($item->getData('key'), Application::get()->getRequest()->getContext());
+        $context = Application::get()->getRequest()->getContext();
+        $contextId = $context->getId();
+        $mailableClass = $mailableClass ?? Repo::mailable()->get($item->getData('key'), $context);
 
         foreach ($props as $prop) {
             switch ($prop) {
@@ -96,31 +96,15 @@ protected function mapByProperties(array $props, EmailTemplate $item, string $ma
                     );
                     break;
                 case 'isUnrestricted':
-                    $output['isUnrestricted'] = Repo::emailTemplate()->isTemplateUnrestricted($item->getData('key'), Application::get()->getRequest()->getContext()->getId());
+                    $output['isUnrestricted'] = Repo::emailTemplate()->isTemplateUnrestricted($item->getData('key'), $contextId);
                     break;
                 case 'assignedUserGroupIds':
                     if ($mailableClass && Repo::mailable()->isGroupsAssignableToTemplates($mailableClass)) {
-                        $output['assignedUserGroupIds'] = Repo::emailTemplate()->getAssignedGroupsIds($item->getData('key'), Application::get()->getRequest()->getContext()->getId());
+                        $output['assignedUserGroupIds'] = Repo::emailTemplate()->getAssignedGroupsIds($item->getData('key'), $contextId);
                     } 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;