Skip to content

Commit

Permalink
Add ACL hook call and test to financial ACLs
Browse files Browse the repository at this point in the history
If the hook call changes the where cause we will not implement the
core pseudo-hook-code for financial type acls.

This allows us to not conflict with reportfinancialacls extension
Eventually we will not have any of this in core other than the hook.
  • Loading branch information
eileenmcnaughton committed Jan 28, 2019
1 parent 0173576 commit c77f866
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 8 deletions.
20 changes: 13 additions & 7 deletions CRM/Contribute/BAO/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -5590,21 +5590,27 @@ public static function getAnnualQuery($contactIDs) {
$liWhere = " AND i.financial_type_id NOT IN (" . implode(',', array_keys($financialTypes)) . ")";
}
$whereClauses = [
'b.contact_id IN (' . $contactIDs . ')',
'b.contribution_status_id = ' . (int) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'),
'b.is_test = 0',
'b.receive_date >= ' . $startDate,
'b.receive_date < ' . $endDate,
'contact_id' => 'IN (' . $contactIDs . ')',
'contribution_status_id' => '= ' . (int) CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'),
'is_test' => ' = 0',
'receive_date' => ['>=' . $startDate, '< ' . $endDate],
];
CRM_Financial_BAO_FinancialType::buildPermissionedClause($whereClauses, NULL, 'b');
CRM_Financial_BAO_FinancialType::addACLClausesToWhereClauses($whereClauses);

$clauses = [];
foreach ($whereClauses as $key => $clause) {
$clauses[] = 'b.' . $key . " " . implode(' AND b.' . $key, (array) $clause);
}
$whereClauseString = implode(' AND ', $clauses);

$query = "
SELECT COUNT(*) as count,
SUM(total_amount) as amount,
AVG(total_amount) as average,
currency
FROM civicrm_contribution b
LEFT JOIN civicrm_line_item i ON i.contribution_id = b.id AND i.entity_table = 'civicrm_contribution' $liWhere
WHERE " . implode(' AND ', $whereClauses) . "
WHERE " . $whereClauseString . "
GROUP BY currency
";
return $query;
Expand Down
38 changes: 38 additions & 0 deletions CRM/Financial/BAO/FinancialType.php
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,44 @@ public static function getAvailableMembershipTypes(&$membershipTypes = NULL, $ac
return $membershipTypes;
}

/**
* This function adds the Financial ACL clauses to the where clause.
*
* This is currently somewhat mocking the native hook implementation
* for the acls that are in core. If the financialaclreport extension is installed
* core acls are not applied as that would result in them being applied twice.
*
* Long term we should either consolidate the financial acls in core or use only the extension.
* Both require substantial clean up before implementing and by the time the code is clean enough to
* take the final step we should
* be able to implement by removing one half of the other of this function.
*
* @param array $whereClauses
*/
public static function addACLClausesToWhereClauses(&$whereClauses) {
$originalWhereClauses = $whereClauses;
CRM_Utils_Hook::selectWhereClause('Contribution', $whereClauses);
if ($whereClauses !== $originalWhereClauses) {
// In this case permisssions have been applied & we assume the
// financialaclreport is applying these
// https://github.com/JMAConsulting/biz.jmaconsulting.financialaclreport/blob/master/financialaclreport.php#L107
return;
}

if (!self::isACLFinancialTypeStatus()) {
return;
}
$types = self::getAllEnabledAvailableFinancialTypes();
if (empty($types)) {
$whereClauses['financial_type_id'] = 'IN (0)';
}
else {
$whereClauses['financial_type_id'] = [
'IN (' . implode(',', array_keys($types)) . ')'
];
}
}

/**
* Function to build a permissioned sql where clause based on available financial types.
*
Expand Down
26 changes: 26 additions & 0 deletions tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,32 @@ public function testAnnualQueryWithFinancialACLsDisabled() {
CRM_Core_DAO::executeQuery($sql);
}

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

/**
* Add ACL denying values LIKE '0'.
*
* @param string $entity
* @param string $clauses
*/
public function aclIdNoZero($entity, &$clauses) {
if ($entity != 'Contribution') {
return;
}
$clauses['id'] = "NOT IN (0)";
}

/**
* Display sort name during.
* Update multiple contributions
Expand Down
5 changes: 4 additions & 1 deletion tests/phpunit/CRMTraits/Financial/FinancialACLTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,14 @@ protected function disableFinancialACLs() {
* @param array $aclPermissions
* Array of ACL permissions in the format
* [[$action, $financialType], [$action, $financialType])
*
* @return int Contact ID
*/
protected function createLoggedInUserWithFinancialACL($aclPermissions = [['view', 'Donation']]) {
CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM'];
$this->createLoggedInUser();
$contactID = $this->createLoggedInUser();
$this->addFinancialAclPermissions($aclPermissions);
return $contactID;
}

/**
Expand Down

0 comments on commit c77f866

Please sign in to comment.