From: Seamus Lee <seamuslee001@gmail.com>
Date: Wed, 8 Feb 2023 02:40:19 +0000 (+1100)
Subject: dev/core#4068 Prevent adding in addSelectWhereClause for civicrm_group when filtering... 
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=0db220583bd9307a3b4a42f42a89f3bef9897078;p=civicrm-core.git

dev/core#4068 Prevent adding in addSelectWhereClause for civicrm_group when filtering by a group as unneded for reports

Add in failing test to demonstrate issue with Group add SelectWhereClause on reports
---

diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php
index e659a50357..0c401cae78 100644
--- a/CRM/Report/Form.php
+++ b/CRM/Report/Form.php
@@ -3770,14 +3770,18 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND
     }
 
     CRM_Contact_BAO_GroupContactCache::check($smartGroups);
-
+    $aclFilter = NULL;
+    $selectWhereClauses = array_filter(CRM_Contact_BAO_Group::getSelectWhereClause('group'));
+    $aclFilter = implode(' AND ', $selectWhereClauses);
+    $aclFilter = !empty($aclFilter) ? ' AND ' . $aclFilter : '';
     $smartGroupQuery = '';
     if (!empty($smartGroups)) {
       $smartGroups = implode(',', $smartGroups);
       $smartGroupQuery = " UNION DISTINCT
                   SELECT DISTINCT smartgroup_contact.contact_id
                   FROM civicrm_group_contact_cache smartgroup_contact
-                  WHERE smartgroup_contact.group_id IN ({$smartGroups}) ";
+                  INNER JOIN civicrm_group group ON group.id = smartgroup_contact.group_id
+                  WHERE smartgroup_contact.group_id IN ({$smartGroups}) {$aclFilter}";
     }
 
     $sqlOp = $this->getSQLOperator($op);
@@ -3796,7 +3800,8 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND
     return " {$contactAlias}.id {$sqlOp} (
                           SELECT DISTINCT {$this->_aliases['civicrm_group']}.contact_id
                           FROM civicrm_group_contact {$this->_aliases['civicrm_group']}
-                          WHERE {$clause} AND {$this->_aliases['civicrm_group']}.status = 'Added'
+                          INNER JOIN civicrm_group group ON group.id = smartgroup_contact.group_id
+                          WHERE {$clause} AND {$this->_aliases['civicrm_group']}.status = 'Added' {$aclFilter}
                           {$smartGroupQuery} ) ";
   }
 
@@ -3950,6 +3955,10 @@ WHERE cg.extends IN ('" . implode("','", $this->_customGroupExtends) . "') AND
     $ret = [];
     foreach ($this->selectedTables() as $tableName) {
       $baoName = str_replace('_DAO_', '_BAO_', (CRM_Core_DAO_AllCoreTables::getClassForTable($tableName) ?? ''));
+      // Do not include CiviCRM group add Select Where clause because we don't necessarily join here for reports with optimisedGroupFilters
+      if ($baoName === 'CRM_Contact_BAO_Group') {
+        continue;
+      }
       if ($baoName && class_exists($baoName) && !empty($this->_columns[$tableName]['alias'])) {
         $tableAlias = $this->_columns[$tableName]['alias'];
         $clauses = array_filter($baoName::getSelectWhereClause($tableAlias));
diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php
index 03bfc6636a..73eb27a5e7 100644
--- a/tests/phpunit/api/v3/ReportTemplateTest.php
+++ b/tests/phpunit/api/v3/ReportTemplateTest.php
@@ -24,6 +24,8 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
 
   protected $contactIDs = [];
 
+  protected $aclGroupId = NULL;
+
   /**
    * Our group reports use an alter so transaction cleanup won't work.
    *
@@ -33,6 +35,9 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     $this->quickCleanUpFinancialEntities();
     $this->quickCleanup(['civicrm_group', 'civicrm_saved_search', 'civicrm_group_contact', 'civicrm_group_contact_cache', 'civicrm_group'], TRUE);
     (new CRM_Logging_Schema())->dropAllLogTables();
+    CRM_Utils_Hook::singleton()->reset();
+    $config = CRM_Core_Config::singleton();
+    unset($config->userPermissionClass->permissions);
     parent::tearDown();
   }
 
@@ -684,6 +689,33 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     $this->assertNumberOfContactsInResult(1, $rows, $template);
   }
 
+  /**
+   * Test the group filter works on various reports when ACLed user is in play
+   *
+   * @dataProvider getMembershipAndContributionReportTemplatesForGroupTests
+   *
+   * @param string $template
+   *   Report template unique identifier.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testReportsWithNonSmartGroupFilterWithACL($template) {
+    $this->aclGroupId = $this->setUpPopulatedGroup();
+    $this->createLoggedInUser();
+    CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM'];
+    $this->callAPISuccessGetCount('Group', ['check_permissions' => 1], 0);
+    $this->hookClass->setHook('civicrm_aclGroup', [$this, 'aclGroupOnly']);
+    $this->hookClass->setHook('civicrm_aclWhereClause', [$this, 'aclGroupContactsOnly']);
+    unset(Civi::$statics['CRM_ACL_API']['group_permission']);
+    $rows = $this->callAPISuccess('report_template', 'getrows', [
+      'report_id' => $template,
+      'gid_value' => [$this->aclGroupId],
+      'gid_op' => 'in',
+      'options' => ['metadata' => ['sql']],
+    ]);
+    $this->assertNumberOfContactsInResult(1, $rows, $template);
+  }
+
   /**
    * Assert the included results match the expected.
    *
@@ -1774,4 +1806,33 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     \Civi::service('sql_triggers')->rebuild(NULL, TRUE);
   }
 
+  /**
+   * Implement hook to restrict to test group 1.
+   *
+   * @param string $type
+   * @param int $contactID
+   * @param string $tableName
+   * @param array $allGroups
+   * @param array $ids
+   */
+  public function aclGroupOnly($type, $contactID, $tableName, $allGroups, &$ids) {
+    $ids = [$this->aclGroupId];
+  }
+
+  /**
+   * Implements hook to limit to contacts only in the aclGroup
+   *
+   * @param string $type
+   * @param array $tables
+   * @param array $whereTables
+   * @param int|null $contactID
+   * @param string $where
+   */
+  public function aclGroupContactsOnly($type, &$tables, &$whereTables, &$contactID, &$where) {
+    if (!empty($where)) {
+      $where .= ' AND ';
+    }
+    $where .= 'contact_a.id IN (SELECT contact_id FROM civicrm_group_contact WHERE status = \'Added\' AND group_id = ' . $this->aclGroupId . ')';
+  }
+
 }