From c2a377b136ceafa8d6bedb3c1af27423f19b66a2 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 3 Jul 2019 08:56:46 +1200
Subject: [PATCH] dev/core#1098 Add unit test & code comments relating to the
 slow activity search

The issue is the joining in of component filtering - we should fix this but do it correctly....
We probably need to extend the test to reflect contact ACLs first
---
 CRM/Activity/BAO/Activity.php                 |  1 +
 CRM/Activity/Selector/Search.php              | 11 +++-
 .../CRM/Activity/Selector/SearchTest.php      | 50 +++++++++++++++++++
 3 files changed, 60 insertions(+), 2 deletions(-)
 create mode 100644 tests/phpunit/CRM/Activity/Selector/SearchTest.php

diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php
index 313fa5c128..a5e1c898ac 100644
--- a/CRM/Activity/BAO/Activity.php
+++ b/CRM/Activity/BAO/Activity.php
@@ -902,6 +902,7 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity {
    */
   public function addSelectWhereClause() {
     $clauses = [];
+    // @todo - check if $permissedActivityTYpes === all activity types and do not add critieria if so.
     $permittedActivityTypeIDs = self::getPermittedActivityTypes();
     if (empty($permittedActivityTypeIDs)) {
       // This just prevents a mysql fail if they have no access - should be extremely edge case.
diff --git a/CRM/Activity/Selector/Search.php b/CRM/Activity/Selector/Search.php
index abd0bdfb24..77bb0fa9a8 100644
--- a/CRM/Activity/Selector/Search.php
+++ b/CRM/Activity/Selector/Search.php
@@ -129,7 +129,7 @@ class CRM_Activity_Selector_Search extends CRM_Core_Selector_Base implements CRM
   /**
    * The query object.
    *
-   * @var string
+   * @var \CRM_Contact_BAO_Query
    */
   protected $_query;
 
@@ -179,6 +179,13 @@ class CRM_Activity_Selector_Search extends CRM_Core_Selector_Base implements CRM
       // required here rather than "access my cases and activities" to
       // prevent those with only the later permission from seeing a list
       // of all cases which might present a privacy issue.
+      // @todo this is the cause of the current devastatingly bad performance on
+      // activity search - it involves a bad join.
+      // The correct fix is to use the permission infrastrucutre - ie. add in the
+      // clause generated by CRM_Activity_BAO_Query::addSelectWhere
+      // but some testing needs to check that before making the change
+      // see https://github.com/civicrm/civicrm-core/blob/be2fb01f90f5f299dd07402a41fed7c7c7567f00/CRM/Utils/SQL.php#L48
+      // for how it's done in the api kernel.
       if (!CRM_Core_Permission::access($componentName, TRUE, TRUE)) {
         $componentClause[] = " (activity_type.component_id IS NULL OR activity_type.component_id <> {$componentID}) ";
       }
@@ -435,7 +442,7 @@ class CRM_Activity_Selector_Search extends CRM_Core_Selector_Base implements CRM
   }
 
   /**
-   * @return string
+   * @return \CRM_Contact_BAO_Query
    */
   public function &getQuery() {
     return $this->_query;
diff --git a/tests/phpunit/CRM/Activity/Selector/SearchTest.php b/tests/phpunit/CRM/Activity/Selector/SearchTest.php
new file mode 100644
index 0000000000..9370cd56c0
--- /dev/null
+++ b/tests/phpunit/CRM/Activity/Selector/SearchTest.php
@@ -0,0 +1,50 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | CiviCRM version 5                                                  |
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC (c) 2004-2019                                |
+ +--------------------------------------------------------------------+
+ | This file is a part of CiviCRM.                                    |
+ |                                                                    |
+ | CiviCRM is free software; you can copy, modify, and distribute it  |
+ | under the terms of the GNU Affero General Public License           |
+ | Version 3, 19 November 2007 and the CiviCRM Licensing Exception.   |
+ |                                                                    |
+ | CiviCRM is distributed in the hope that it will be useful, but     |
+ | WITHOUT ANY WARRANTY; without even the implied warranty of         |
+ | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.               |
+ | See the GNU Affero General Public License for more details.        |
+ |                                                                    |
+ | You should have received a copy of the GNU Affero General Public   |
+ | License and the CiviCRM Licensing Exception along                  |
+ | with this program; if not, contact CiviCRM LLC                     |
+ | at info[AT]civicrm[DOT]org. If you have questions about the        |
+ | GNU Affero General Public License or the licensing of CiviCRM,     |
+ | see the CiviCRM license FAQ at http://civicrm.org/licensing        |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ *  Class CRM_Activity_Selector_SearchTest
+ *
+ * @package CiviCRM
+ */
+class CRM_Activity_Selector_SearchTest extends CiviUnitTestCase {
+
+  /**
+   * Test activity search applies a permission based component filter.
+   */
+  public function testActivitySearchComponentPermission() {
+    $this->activityCreate(['activity_type_id' => 'Contribution']);
+    $this->activityCreate(['activity_type_id' => 'Pledge Reminder']);
+    $this->activityCreate(['activity_type_id' => 'Meeting']);
+    $this->setPermissions(['access CiviCRM', 'edit all contacts', 'access CiviContribute']);
+    $queryParams = [['activity_location', '=', 'Baker Street', '', '']];
+    $searchSelector = new CRM_Activity_Selector_Search($queryParams, CRM_Core_Action::VIEW);
+    $this->assertEquals(2, $searchSelector->getTotalCount(NULL));
+    $queryObject = $searchSelector->getQuery();
+    $this->assertEquals("civicrm_activity.location = 'Baker Street'", $queryObject->_where[''][0]);
+  }
+
+}
-- 
2.25.1