Move financial acls for membership types to the extension

CRM/Contact/BAO/Query.php

@@ -582,10 +582,9 @@ public function initialize($apiEntity = NULL) {
     if (isset($component) && !$this->_skipPermission) {
       // Unit test coverage in api_v3_FinancialTypeACLTest::testGetACLContribution.
       $clauses = CRM_Financial_BAO_FinancialType::buildPermissionedClause($component);
-      if (!empty($this->_whereClause) && !empty($clauses)) {
-        $this->_whereClause .= ' AND ';
+      if ($clauses) {
+        $this->_whereClause .= ' AND ' . $clauses;
       }
-      $this->_whereClause .= $clauses;
     }
 
     $this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode, $apiEntity);
@@ -2112,7 +2111,7 @@ public function whereClause($isForcePrimaryEmailOnly = NULL): string {
       }
     }
 
-    return implode(' AND ', $andClauses);
+    return $andClauses ? implode(' AND ', $andClauses) : ' 1 ';
   }
 
   /**

CRM/Contribute/BAO/Contribution.php

@@ -4371,30 +4371,29 @@ public static function getAnnualQuery($contactIDs) {
     }
     $startDate = "$year$monthDay";
     $endDate = "$nextYear$monthDay";
-
-    $whereClauses = [
-      'contact_id' => 'IN (' . $contactIDs . ')',
-      'is_test' => ' = 0',
-      'receive_date' => ['>=' . $startDate, '<  ' . $endDate],
-    ];
     $havingClause = 'contribution_status_id = ' . (int) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed');
-    CRM_Financial_BAO_FinancialType::addACLClausesToWhereClauses($whereClauses);
+
+    $contributionBAO = new CRM_Contribute_BAO_Contribution();
+    $whereClauses = $contributionBAO->addSelectWhereClause();
 
     $clauses = [];
     foreach ($whereClauses as $key => $clause) {
-      $clauses[] = 'b.' . $key . " " . implode(' AND b.' . $key, (array) $clause);
+      $clauses[] = 'b.' . $key . ' ' . implode(' AND b.' . $key, (array) $clause);
     }
+    $clauses[] = 'b.contact_id IN (' . $contactIDs . ')';
+    $clauses[] = 'b.is_test = 0';
+    $clauses[] = 'b.receive_date >=' . $startDate . ' AND b.receive_date < ' . $endDate;
     $whereClauseString = implode(' AND ', $clauses);
 
     // See https://github.com/civicrm/civicrm-core/pull/13512 for discussion of how
     // this group by + having on contribution_status_id improves performance
-    $query = "
+    $query = '
       SELECT COUNT(*) as count,
              SUM(total_amount) as amount,
              AVG(total_amount) as average,
              currency
       FROM civicrm_contribution b
-      WHERE " . $whereClauseString . "
+      WHERE ' . $whereClauseString . "
       GROUP BY currency, contribution_status_id
       HAVING $havingClause
       ";

CRM/Core/DAO.php

@@ -3082,8 +3082,11 @@ public function addSelectWhereClause() {
     $fields = $this->fields();
     foreach ($fields as $fieldName => $field) {
       // Clause for contact-related entities like Email, Relationship, etc.
-      if (strpos($fieldName, 'contact_id') === 0 && CRM_Utils_Array::value('FKClassName', $field) == 'CRM_Contact_DAO_Contact') {
-        $clauses[$fieldName] = CRM_Utils_SQL::mergeSubquery('Contact');
+      if (strpos($field['name'], 'contact_id') === 0 && CRM_Utils_Array::value('FKClassName', $field) == 'CRM_Contact_DAO_Contact') {
+        $contactClause = CRM_Utils_SQL::mergeSubquery('Contact');
+        if (!empty($contactClause)) {
+          $clauses[$field['name']] = CRM_Utils_SQL::mergeSubquery('Contact');
+        }
       }
       // Clause for an entity_table/entity_id combo
       if ($fieldName === 'entity_id' && isset($fields['entity_table'])) {

CRM/Financial/BAO/FinancialType.php

@@ -349,22 +349,11 @@ public static function addACLClausesToWhereClauses(&$whereClauses) {
    * @return string $clauses
    */
   public static function buildPermissionedClause(string $component): string {
-    $clauses = [];
-    // @todo the relevant addSelectWhere clause should be called.
-    if (!self::isACLFinancialTypeStatus()) {
-      return '';
-    }
     if ($component === 'contribution') {
       $clauses = CRM_Contribute_BAO_Contribution::getSelectWhereClause();
     }
     if ($component === 'membership') {
-      self::getAvailableMembershipTypes($types, CRM_Core_Action::VIEW);
-      $types = array_keys($types);
-      if (empty($types)) {
-        $types = [0];
-      }
-      $clauses[] = ' civicrm_membership.membership_type_id IN (' . implode(',', $types) . ')';
-
+      $clauses = CRM_Member_BAO_Membership::getSelectWhereClause();
     }
     return implode(' AND ', $clauses);
   }

ext/financialacls/financialacls.php

@@ -3,6 +3,7 @@
 require_once 'financialacls.civix.php';
 // phpcs:disable
 use Civi\Api4\EntityFinancialAccount;
+use Civi\Api4\MembershipType;
 use CRM_Financialacls_ExtensionUtil as E;
 // phpcs:enable
 
@@ -143,6 +144,10 @@ function financialacls_civicrm_selectWhereClause($entity, &$clauses) {
       $clauses['financial_type_id'] = _financialacls_civicrm_get_type_clause();
       break;
 
+    case 'Membership':
+      $clauses['membership_type_id'] = _financialacls_civicrm_get_membership_type_clause();
+      break;
+
     case 'FinancialType':
       $clauses['id'] = _financialacls_civicrm_get_type_clause();
       break;
@@ -192,12 +197,40 @@ function _financialacls_civicrm_get_accounts_clause(): string {
  * @return string
  */
 function _financialacls_civicrm_get_type_clause(): string {
+  return 'IN (' . implode(',', _financialacls_civicrm_get_accessible_financial_types()) . ')';
+}
+
+/**
+ * Get an array of the ids of accessible financial types.
+ *
+ * If none then it will be [0]
+ *
+ * @return int[]
+ */
+function _financialacls_civicrm_get_accessible_financial_types(): array {
   $types = [];
   CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($types);
-  if ($types) {
-    return 'IN (' . implode(',', array_keys($types)) . ')';
+  if (empty($types)) {
+    $types = [0];
+  }
+  return array_keys($types);
+}
+
+/**
+ * Get the clause to limit available membership types.
+ *
+ * @return string
+ *
+ * @throws \API_Exception
+ */
+function _financialacls_civicrm_get_membership_type_clause(): string {
+  $financialTypes = _financialacls_civicrm_get_accessible_financial_types();
+  if ($financialTypes === [0]) {
+    return 0;
   }
-  return '= 0';
+  $membershipTypes = (array) MembershipType::get(FALSE)
+    ->addWhere('financial_type_id', 'IN', $financialTypes)->execute()->indexBy('id');
+  return empty($membershipTypes) ? '= 0' : ('IN (' . implode(',', array_keys($membershipTypes)) . ')');
 }
 
 /**

ext/financialacls/tests/phpunit/Civi/Financialacls/FinancialTypeTest.php

@@ -86,25 +86,30 @@ public function testBuildPermissionedClause(): void {
       'period_type' => 'fixed',
     ])->execute()->first()['id'];
     $this->setPermissions([
+      'view all contacts',
+      'access deleted contacts',
+      'access CiviMember',
       'view contributions of type Donation',
       'view contributions of type Member Dues',
     ]);
     $whereClause = \CRM_Financial_BAO_FinancialType::buildPermissionedClause('contribution');
-    $this->assertEquals('(`civicrm_contribution`.`financial_type_id` IS NULL OR (`civicrm_contribution`.`financial_type_id` IN (1,2)))', $whereClause);
+    $this->assertEquals(' AND (`civicrm_contribution`.`financial_type_id` IS NULL OR (`civicrm_contribution`.`financial_type_id` IN (1,2)))', $whereClause);
     $whereClause = \CRM_Financial_BAO_FinancialType::buildPermissionedClause('membership');
-    $this->assertEquals(' civicrm_membership.membership_type_id IN (0)', $whereClause);
+    $this->assertEquals(' AND (`civicrm_membership`.`membership_type_id` IS NULL OR (`civicrm_membership`.`membership_type_id` = 0))', $whereClause);
 
     $this->setPermissions([
+      'view all contacts',
+      'access deleted contacts',
+      'access CiviMember',
       'view contributions of type Donation',
       'view contributions of type Member Dues',
       'view contributions of type Event Fee',
     ]);
 
     $whereClause = \CRM_Financial_BAO_FinancialType::buildPermissionedClause('contribution');
-    $this->assertEquals('(`civicrm_contribution`.`financial_type_id` IS NULL OR (`civicrm_contribution`.`financial_type_id` IN (1,4,2)))', $whereClause);
+    $this->assertEquals(' AND (`civicrm_contribution`.`financial_type_id` IS NULL OR (`civicrm_contribution`.`financial_type_id` IN (1,4,2)))', $whereClause);
     $whereClause = \CRM_Financial_BAO_FinancialType::buildPermissionedClause('membership');
-    $this->assertEquals(' civicrm_membership.membership_type_id IN (' . $membershipTypeID . ')', $whereClause);
-
+    $this->assertEquals(' AND (`civicrm_membership`.`membership_type_id` IS NULL OR (`civicrm_membership`.`membership_type_id` IN (' . $membershipTypeID . ')))', $whereClause);
   }
 
 }

tests/phpunit/CRM/Contribute/BAO/ContributionTest.php

@@ -343,7 +343,7 @@ public function testAnnualWithMultipleLineItems(): void {
   /**
    * Test that financial type data is not added to the annual query if acls not enabled.
    */
-  public function testAnnualQueryWithFinancialACLsDisabled() {
+  public function testAnnualQueryWithFinancialACLsDisabled(): void {
     $sql = CRM_Contribute_BAO_Contribution::getAnnualQuery([1, 2, 3]);
     $this->assertStringContainsString('SUM(total_amount) as amount,', $sql);
     $this->assertStringContainsString('WHERE b.contact_id IN (1,2,3)', $sql);
@@ -356,12 +356,12 @@ public function testAnnualQueryWithFinancialACLsDisabled() {
   /**
    * Test that financial type data is not added to the annual query if acls not enabled.
    */
-  public function testAnnualQueryWithFinancialHook() {
+  public function testAnnualQueryWithFinancialHook(): void {
     $this->hookClass->setHook('civicrm_selectWhereClause', [$this, 'aclIdNoZero']);
     $sql = CRM_Contribute_BAO_Contribution::getAnnualQuery([1, 2, 3]);
     $this->assertStringContainsString('SUM(total_amount) as amount,', $sql);
-    $this->assertStringContainsString('WHERE b.contact_id IN (1,2,3)', $sql);
-    $this->assertStringContainsString('b.id NOT IN (0)', $sql);
+    $this->assertStringContainsString('b.contact_id IN (1,2,3)', $sql);
+    $this->assertStringContainsString('WHERE b.id NOT IN (0)', $sql);
     $this->assertStringNotContainsString('b.financial_type_id', $sql);
     CRM_Core_DAO::executeQuery($sql);
   }