From 9cab2350852a6d2a932a69ae5a79b909b14be92a Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 11 Jan 2024 16:21:53 +1300 Subject: [PATCH] Standardise implementation of financial type acl in query object This fixes it to call the selectWhere in the BAO_Query, as apiv4 does. There is test cover in the touched test. The primary goal here is to get this logic out of core & to disabled this extension on most sites with a view to moving the whole extension out of core in time --- CRM/Contact/BAO/Query.php | 28 +++++++++---------- ext/financialacls/financialacls.php | 22 +++++++++++++++ .../Civi/Financialacls/LineItemTest.php | 2 +- tests/phpunit/CRM/Contact/BAO/QueryTest.php | 20 +------------ 4 files changed, 38 insertions(+), 34 deletions(-) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index 9249c65c5bc9..bf1f5be69bfe 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -5069,7 +5069,7 @@ public function generatePermissionClause($onlyDeleted = FALSE, $count = FALSE) { if (!$this->_skipPermission) { $permissionClauses = CRM_Contact_BAO_Contact_Permission::cacheClause(); $this->_permissionWhereClause = $permissionClauses[1]; - $this->_permissionFromClause = $permissionClauses[0]; + $this->_permissionFromClause = $permissionClauses[0] ?: ''; if (CRM_Core_Permission::check('access deleted contacts')) { if (!$onlyDeleted) { @@ -5081,13 +5081,19 @@ public function generatePermissionClause($onlyDeleted = FALSE, $count = FALSE) { } // Add ACL permission clause generated by the BAO. This is the same clause used by API::get. - // TODO: Why only activities? + // TODO: Why only activities and contributions?. Not likely to change now as we move away from this. if (isset($this->_tables['civicrm_activity'])) { $clauses = array_filter(CRM_Activity_BAO_Activity::getSelectWhereClause()); if ($clauses) { $this->_permissionWhereClause .= ($this->_permissionWhereClause ? ' AND ' : '') . '(' . implode(' AND ', $clauses) . ')'; } } + if (isset($this->_tables['civicrm_contribution'])) { + $contributionClauses = array_filter(CRM_Contribute_BAO_Contribution::getSelectWhereClause()); + if (!empty($contributionClauses)) { + $this->_permissionWhereClause .= ($this->_permissionWhereClause ? ' AND ' : '') . '(' . implode(' AND ', $contributionClauses) . ')'; + } + } } else { // add delete clause if needed even if we are skipping permission @@ -5112,25 +5118,17 @@ public function setSkipPermission($val) { } /** - * @param null $context + * @param string|null $context * * @return array * @throws \CRM_Core_Exception */ - public function summaryContribution($context = NULL) { - [$innerselect, $from, $where, $having] = $this->query(TRUE); - if (!empty($this->_permissionFromClause) && !stripos($from, 'aclContactCache')) { - $from .= " $this->_permissionFromClause"; - } - if ($this->_permissionWhereClause) { - $where .= " AND " . $this->_permissionWhereClause; - } - if ($context == 'search') { + public function summaryContribution(?string $context = NULL): array { + [, $from, $where] = $this->query(TRUE); + if ($context === 'search') { $where .= " AND contact_a.is_deleted = 0 "; } - $this->appendFinancialTypeWhereAndFromToQueryStrings($where, $from); - $summary = ['total' => [], 'soft_credit' => ['count' => 0, 'avg' => 0, 'amount' => 0]]; $this->addBasicStatsToSummary($summary, $where, $from); @@ -5146,10 +5144,12 @@ public function summaryContribution($context = NULL) { /** * Append financial ACL limits to the query from & where clauses, if applicable. * + * @deprecated since 5.71 will be removed around 5.75 * @param string $where * @param string $from */ public function appendFinancialTypeWhereAndFromToQueryStrings(&$where, &$from) { + CRM_Core_Error::deprecatedFunctionWarning('use selectWhere'); if (!CRM_Financial_BAO_FinancialType::isACLFinancialTypeStatus()) { return; } diff --git a/ext/financialacls/financialacls.php b/ext/financialacls/financialacls.php index 7a84ec2c0df3..a5d5aa863a0b 100644 --- a/ext/financialacls/financialacls.php +++ b/ext/financialacls/financialacls.php @@ -3,6 +3,7 @@ require_once 'financialacls.civix.php'; use Civi\Api4\EntityFinancialAccount; +use Civi\Api4\FinancialType; use Civi\Api4\MembershipType; use CRM_Financialacls_ExtensionUtil as E; @@ -95,6 +96,12 @@ function financialacls_civicrm_selectWhereClause($entity, &$clauses) { case 'Contribution': case 'Product': $clauses['financial_type_id'][] = _financialacls_civicrm_get_type_clause(); + if ($entity === 'Contribution') { + $unavailableTypes = _financialacls_civicrm_get_inaccessible_financial_types(); + if (!empty($unavailableTypes)) { + $clauses['id'][] = 'NOT IN (SELECT contribution_id FROM civicrm_line_item WHERE financial_type_id IN (' . implode(',', $unavailableTypes) . '))'; + } + } break; case 'Membership': @@ -169,6 +176,21 @@ function _financialacls_civicrm_get_accessible_financial_types(): array { return array_keys($types); } +/** + * Get an array of the ids of accessible financial types. + * + * If none then it will be [0] + * + * @return int[] + */ +function _financialacls_civicrm_get_inaccessible_financial_types(): array { + $types = (array) FinancialType::get(FALSE)->addSelect('id')->execute()->indexBy('id'); + foreach (_financialacls_civicrm_get_accessible_financial_types() as $accessibleFinancialType) { + unset($types[$accessibleFinancialType]); + } + return array_keys($types); +} + /** * Get the clause to limit available membership types. * diff --git a/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php b/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php index a12d368eba14..ecc3dabf082e 100644 --- a/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php +++ b/ext/financialacls/tests/phpunit/Civi/Financialacls/LineItemTest.php @@ -64,7 +64,7 @@ public function testLineItemApiPermissions($version): void { $lineItems = $this->callAPISuccess('LineItem', 'get', ['sequential' => TRUE])['values']; $this->assertCount(2, $lineItems); - $this->callAPISuccessGetCount('LineItem', ['check_permissions' => TRUE], 1); + $this->callAPISuccessGetCount('LineItem', ['check_permissions' => TRUE], 0); $this->callAPISuccess('LineItem', 'Delete', ['check_permissions' => ($version == 3), 'id' => $lineItems[0]['id']]); $this->callAPIFailure('LineItem', 'Delete', ['check_permissions' => TRUE, 'id' => $lineItems[1]['id']]); diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index e5a868fbacae..e39181d136be 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -1256,14 +1256,6 @@ public function getSortOptions(): array { public function testGetSummaryQueryWithFinancialACLDisabled(): void { $this->createContributionsForSummaryQueryTests(); - // Test the function directly - $where = $from = NULL; - $queryObject = new CRM_Contact_BAO_Query(); - $queryObject->appendFinancialTypeWhereAndFromToQueryStrings($where, - $from); - $this->assertEquals(NULL, $where); - $this->assertEquals(NULL, $from); - // Test the function in action $queryObject = new CRM_Contact_BAO_Query([['contribution_source', '=', 'SSF', '', '']]); $summary = $queryObject->summaryContribution(); @@ -1292,21 +1284,11 @@ public function testGetSummaryQueryWithFinancialACLDisabled(): void { * @throws \CRM_Core_Exception */ public function testGetSummaryQueryWithFinancialACLEnabled(): void { - $where = $from = NULL; + $this->createContributionsForSummaryQueryTests(); $this->enableFinancialACLs(); $this->createLoggedInUserWithFinancialACL(); - // Test the function directly - $queryObject = new CRM_Contact_BAO_Query(); - $queryObject->appendFinancialTypeWhereAndFromToQueryStrings($where, - $from); - $donationTypeID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'); - $this->assertEquals( - " LEFT JOIN civicrm_line_item li - ON civicrm_contribution.id = li.contribution_id AND - li.entity_table = 'civicrm_contribution' AND li.financial_type_id NOT IN ({$donationTypeID}) ", $from); - // Test the function in action $queryObject = new CRM_Contact_BAO_Query([['contribution_source', '=', 'SSF', '', '']]); $summary = $queryObject->summaryContribution();