Test, enotice fixes, handling for permissions key for Member_Tasks

Alternate potential approach to the enotice portion of civicrm#20940 This approach adds support for permissions using a syntax like https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_summaryActions/ and would potentially standardise that onto the search_tasks hook https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_searchTasks/ - it's not quite clear what the search hooks supports - https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_searchKitTasks/
eileenmcnaughton · Jul 24, 2021 · 80bc1b7 · 80bc1b7
1 parent 96adc46
commit 80bc1b7
Show file tree

Hide file tree

Showing 3 changed files with 138 additions and 29 deletions.
diff --git a/CRM/Core/Task.php b/CRM/Core/Task.php
@@ -165,6 +165,51 @@ public static function getTask($value) {
     ];
   }
 
+  /**
+   * Filter tasks based on the permission key, if available.
+   *
+   * @param array $tasks
+   * @param bool $hasEditContactPermission
+   *   Does the user have permission to edit the given contact. Required where
+   *   permission to edit the user is required in conjunction with permission
+   *   to do the task.
+   *
+   * @return array
+   */
+  protected static function getFilteredTasks(array $tasks, bool $hasEditContactPermission): array {
+    foreach ($tasks as $index => $task) {
+      // requires_edit_contact_permission is a (hopefully transient way) of denoting which
+      // tasks need 'edit this contact' on top of the membership permission.
+      if (!empty($task['requires_edit_contact_permission']) && !$hasEditContactPermission) {
+        unset($tasks[$index]);
+      }
+      elseif (!empty($task['permissions']) && !CRM_Core_Permission::check($task['permissions'])) {
+        unset($tasks[$index]);
+      }
+    }
+    return $tasks;
+  }
+
+  /**
+   * Get task tiles filtered by any declared permissions.
+   *
+   * @param array $tasks
+   * @param bool $hasEditContactPermission
+   *   Does the user have permission to edit the given contact. Required where
+   *   permission to edit the user is required in conjunction with permission
+   *   to do the task.
+   *
+   * @return array
+   */
+  protected static function getFilteredTiles(array $tasks, bool $hasEditContactPermission): array {
+    $availableTasks = self::getFilteredTasks($tasks, $hasEditContactPermission);
+    $return = [];
+    foreach ($availableTasks as $key => $details) {
+      $return[$key] = $details['title'];
+    }
+    return $return;
+  }
+
   /**
    * Function to return the task information on basis of provided task's form name
    *

diff --git a/CRM/Member/Task.php b/CRM/Member/Task.php
@@ -44,19 +44,28 @@ public static function tasks() {
       self::TASK_DELETE => [
         'title' => ts('Delete memberships'),
         'class' => 'CRM_Member_Form_Task_Delete',
+        'permissions' => ['delete in CiviMember'],
         'result' => FALSE,
+        // Hopefully transitional key - if permission to edit the contact also required.
+        'requires_edit_contact_permission' => FALSE,
       ],
       self::TASK_PRINT => [
         'title' => ts('Print selected rows'),
         'class' => 'CRM_Member_Form_Task_Print',
         'result' => FALSE,
+        'permissions' => [['view memberships', 'edit memberships']],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => FALSE,
       ],
       self::TASK_EXPORT => [
         'title' => ts('Export members'),
         'class' => [
           'CRM_Member_Export_Form_Select',
           'CRM_Member_Export_Form_Map',
         ],
+        'permissions' => [['view memberships', 'edit memberships']],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => FALSE,
         'result' => FALSE,
       ],
       self::TASK_EMAIL => [
@@ -66,50 +75,57 @@ public static function tasks() {
         ]),
         'class' => 'CRM_Member_Form_Task_Email',
         'result' => TRUE,
+        'permissions' => ['edit memberships'],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => TRUE,
       ],
       self::BATCH_UPDATE => [
         'title' => ts('Update multiple memberships'),
         'class' => [
           'CRM_Member_Form_Task_PickProfile',
           'CRM_Member_Form_Task_Batch',
         ],
+        'permissions' => ['edit memberships'],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => TRUE,
         'result' => TRUE,
       ],
       self::LABEL_MEMBERS => [
         'title' => ts('Mailing labels - print'),
         'class' => [
           'CRM_Member_Form_Task_Label',
         ],
+        'permissions' => ['edit memberships'],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => TRUE,
         'result' => TRUE,
       ],
       self::PDF_LETTER => [
         'title' => ts('Print/merge document for memberships'),
         'class' => 'CRM_Member_Form_Task_PDFLetter',
         'result' => FALSE,
+        'permissions' => ['edit memberships'],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => TRUE,
       ],
       self::SAVE_SEARCH => [
         'title' => ts('Group - create smart group'),
         'class' => 'CRM_Contact_Form_Task_SaveSearch',
         'result' => TRUE,
+        'permissions' => ['edit groups'],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => FALSE,
       ],
       self::SAVE_SEARCH_UPDATE => [
         'title' => ts('Group - update smart group'),
         'class' => 'CRM_Contact_Form_Task_SaveSearch_Update',
         'result' => TRUE,
+        'permissions' => ['edit groups'],
+        // Transitional key. May change.
+        'requires_edit_contact_permission' => FALSE,
       ],
     ];
-
-    //CRM-4418, check for delete
-    if (!CRM_Core_Permission::check('delete in CiviMember')) {
-      unset(self::$_tasks[self::TASK_DELETE]);
-    }
-    //CRM-12920 - check for edit permission
-    if (!CRM_Core_Permission::check('edit memberships')) {
-      unset(self::$_tasks[self::BATCH_UPDATE]);
-    }
-
     parent::tasks();
-
     return self::$_tasks;
   }
 
@@ -135,24 +151,8 @@ public static function taskTitles() {
    *   set of tasks that are valid for the user
    */
   public static function permissionedTaskTitles($permission, $params = []) {
-    if (($permission == CRM_Core_Permission::EDIT)
-      || CRM_Core_Permission::check('edit memberships')
-    ) {
-      $tasks = self::taskTitles();
-    }
-    else {
-      $tasks = [
-        self::TASK_EXPORT => self::$_tasks[self::TASK_EXPORT]['title'],
-        self::TASK_EMAIL => self::$_tasks[self::TASK_EMAIL]['title'],
-      ];
-      //CRM-4418,
-      if (CRM_Core_Permission::check('delete in CiviMember')) {
-        $tasks[self::TASK_DELETE] = self::$_tasks[self::TASK_DELETE]['title'];
-      }
-    }
-
-    $tasks = parent::corePermissionedTaskTitles($tasks, $permission, $params);
-    return $tasks;
+    $tasks = self::getFilteredTiles(self::$_tasks, $permission === CRM_Core_Permission::EDIT);
+    return parent::corePermissionedTaskTitles($tasks, $permission, $params);
   }
 
   /**

diff --git a/tests/phpunit/CRM/Member/TaskTest.php b/tests/phpunit/CRM/Member/TaskTest.php
@@ -0,0 +1,64 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ * Class CRM_Member_BAO_MembershipTest
+ * @group headless
+ */
+class CRM_Member_TaskTest extends CiviUnitTestCase {
+
+  use Civi\Test\ACLPermissionTrait;
+
+  /**
+   * Test tiles are correctly filtered on permissions.
+   */
+  public function testPermissionedTiles(): void {
+    $this->createLoggedInUser();
+
+    CRM_Member_Task::tasks();
+    \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['view memberships'];
+    $tasks = CRM_Member_Task::permissionedTaskTitles(CRM_Core_Permission::VIEW);
+    $this->assertEquals([8 => 'Export members', 5 => 'Print selected rows'], $tasks);
+
+    \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['delete in CiviMember', 'view memberships'];
+    $tasks = CRM_Member_Task::permissionedTaskTitles(CRM_Core_Permission::VIEW);
+    $this->assertEquals([8 => 'Export members', 5 => 'Print selected rows', 4 => 'Delete memberships'], $tasks);
+
+    \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['edit memberships'];
+    $tasks = CRM_Member_Task::permissionedTaskTitles(CRM_Core_Permission::VIEW);
+    $this->assertEquals([8 => 'Export members', 5 => 'Print selected rows'], $tasks);
+
+    \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['edit memberships'];
+    $tasks = CRM_Member_Task::permissionedTaskTitles(CRM_Core_Permission::EDIT);
+    $this->assertEquals([
+      8 => 'Export members',
+      5 => 'Print selected rows',
+      9 => 'Email - send now (to 50 or less)',
+      201 => 'Mailing labels - print',
+      3 => 'Print/merge document for memberships',
+      6 => 'Update multiple memberships',
+    ], $tasks);
+
+    \CRM_Core_Config::singleton()->userPermissionClass->permissions = ['edit memberships', 'delete in CiviMember', 'edit groups'];
+    $tasks = CRM_Member_Task::permissionedTaskTitles(CRM_Core_Permission::EDIT);
+    $this->assertEquals([
+      8 => 'Export members',
+      5 => 'Print selected rows',
+      9 => 'Email - send now (to 50 or less)',
+      201 => 'Mailing labels - print',
+      3 => 'Print/merge document for memberships',
+      6 => 'Update multiple memberships',
+      4 => 'Delete memberships',
+      12 => 'Group - create smart group',
+    ], $tasks);
+  }
+
+}