Merge pull request #12995 from eileenmcnaughton/activity_extract2

dev/core/454 rationalise activity case permissions & code
civicrm · Dec 19, 2018 · 7a88e04 · 7a88e04
2 parents 3bbe587 + c4937fe
commit 7a88e04
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 34 deletions.
diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php
@@ -1118,21 +1118,33 @@ public static function deprecatedGetActivities($input) {
   }
 
   /**
-   * Get the component id and name if those are enabled and allowed.
+   * Get an array of components that are accessible by the currenct user.
    *
-   * Checks whether logged in user has permission.
-   * To decide whether we are going to include
-   * component related activities with core activity retrieve process.
-   * (what did that just mean?)
+   * This means checking if they are enabled and if the user has appropriate permission.
    *
-   * @return array
+   * For most components the permission is access component (e.g 'access CiviContribute').
+   * Exceptions as CiviCampaign (administer CiviCampaign) and CiviCase
+   * (accesses a case function which enforces edit all cases or edit my cases. Case
+   * permissions are also handled on a per activity basis).
+   *
+   * Checks whether logged in user has permission to the component.
+   *
+   * @param bool $excludeComponentHandledActivities
+   *   Should we exclude components whose display is handled in the components.
+   *   In practice this means should we include CiviCase in the results. Presumbaly
+   *   at the time it was decided case activities should be shown in the case framework and
+   *   that this concept might be extended later. In practice most places that
+   *   call this then re-add CiviCase in some way so it's all a bit... odd.
+   *
+   * @return array Array of component id and name.
    *   Array of component id and name.
    */
-  public static function activityComponents() {
+  public static function activityComponents($excludeComponentHandledActivities = TRUE) {
     $components = array();
     $compInfo = CRM_Core_Component::getEnabledComponents();
     foreach ($compInfo as $compObj) {
-      if (!empty($compObj->info['showActivitiesInCore'])) {
+      $includeComponent = !$excludeComponentHandledActivities || !empty($compObj->info['showActivitiesInCore']);
+      if ($includeComponent) {
         if ($compObj->info['name'] == 'CiviCampaign') {
           $componentPermission = "administer {$compObj->name}";
         }
@@ -2756,7 +2768,7 @@ public static function checkPermission($activityId, $action) {
    */
   protected static function isContactPermittedAccessToCaseActivity($activityId, $action, $activityTypeID) {
     $allow = FALSE;
-    foreach (['administer CiviCase', 'access my cases and activities', 'access all cases and activities'] as $per) {
+    foreach (['access my cases and activities', 'access all cases and activities'] as $per) {
       if (CRM_Core_Permission::check($per)) {
         $allow = TRUE;
         break;
@@ -2783,21 +2795,7 @@ protected static function isContactPermittedAccessToCaseActivity($activityId, $a
    * @return bool
    */
   protected static function hasPermissionForActivityType($activityTypeID) {
-    $compPermissions = [
-      'CiviCase' => [
-        'administer CiviCase',
-        'access my cases and activities',
-        'access all cases and activities',
-      ],
-      'CiviMail' => ['access CiviMail'],
-      'CiviEvent' => ['access CiviEvent'],
-      'CiviGrant' => ['access CiviGrant'],
-      'CiviPledge' => ['access CiviPledge'],
-      'CiviMember' => ['access CiviMember'],
-      'CiviReport' => ['access CiviReport'],
-      'CiviContribute' => ['access CiviContribute'],
-      'CiviCampaign' => ['administer CiviCampaign'],
-    ];
+    $components = self::activityComponents(FALSE);
 
     // First check the component permission.
     $sql = "
@@ -2812,16 +2810,8 @@ protected static function hasPermissionForActivityType($activityTypeID) {
     $componentId = CRM_Core_DAO::singleValueQuery($sql, $params);
 
     if ($componentId) {
-      $componentName = CRM_Core_Component::getComponentName($componentId);
-      $compPermission = CRM_Utils_Array::value($componentName, $compPermissions);
-
-      // Here we are interesting in any single permission.
-      if (is_array($compPermission)) {
-        foreach ($compPermission as $per) {
-          if (CRM_Core_Permission::check($per)) {
-            return TRUE;
-          }
-        }
+      if (!empty($components[$componentId])) {
+        return TRUE;
       }
     }
     else {

diff --git a/tests/phpunit/api/v3/ACLPermissionTest.php b/tests/phpunit/api/v3/ACLPermissionTest.php
@@ -505,6 +505,18 @@ public function testGetActivityCheckPermissionsByComponent() {
     $this->callAPISuccessGetSingle('Activity', ['check_permissions' => 1, 'id' => ['IN' => [$activity['id'], $activity2['id']]]]);
   }
 
+  /**
+   * Check that component related activity filtering works for CiviCase.
+   */
+  public function testGetActivityCheckPermissionsByCaseComponent() {
+    CRM_Core_BAO_ConfigSetting::enableComponent('CiviCase');
+    $activity = $this->activityCreate(['activity_type_id' => 'Open Case']);
+    $activity2 = $this->activityCreate(['activity_type_id' => 'Pledge Reminder']);
+    $this->hookClass->setHook('civicrm_aclWhereClause', array($this, 'aclWhereHookAllResults'));
+    $this->setPermissions(['access CiviCRM', 'access CiviContribute', 'access all cases and activities']);
+    $this->callAPISuccessGetSingle('Activity', ['check_permissions' => 1, 'id' => ['IN' => [$activity['id'], $activity2['id']]]]);
+  }
+
   /**
    * Check that activities can be retrieved by ACL.
    *