Skip to content

Commit

Permalink
Merge pull request #17707 from seamuslee001/ref_permission_clause_search
Browse files Browse the repository at this point in the history 
[REF] Use Standard function cacheClause to re-use contact acl cache t…
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton authored Jul 29, 2020
2 parents 6c73585 + 21b8bcb commit 93317f7
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 30 deletions.
16 changes: 9 additions & 7 deletions CRM/Contact/BAO/Contact/Permission.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,17 +307,16 @@ public static function cacheClause($contactAlias = 'contact_a') {
if (CRM_Core_Permission::check('view all contacts') ||
CRM_Core_Permission::check('edit all contacts')
) {
if (is_array($contactAlias)) {
if (!CRM_Core_Permission::check('access deleted contacts')) {
$wheres = [];
foreach ($contactAlias as $alias) {
foreach ((array) $contactAlias as $alias) {
// CRM-6181
$wheres[] = "$alias.is_deleted = 0";
}
return [NULL, '(' . implode(' AND ', $wheres) . ')'];
}
else {
// CRM-6181
return [NULL, "$contactAlias.is_deleted = 0"];
return [NULL, '( 1 )'];
}
}

Expand All @@ -332,14 +331,17 @@ public static function cacheClause($contactAlias = 'contact_a') {
}

$fromClause = implode(" ", $clauses);
$whereClase = NULL;
$whereClause = NULL;
}
else {
$fromClause = " INNER JOIN civicrm_acl_contact_cache aclContactCache ON {$contactAlias}.id = aclContactCache.contact_id ";
$whereClase = " aclContactCache.user_id = $contactID AND $contactAlias.is_deleted = 0";
$whereClause = " aclContactCache.user_id = $contactID";
if (!CRM_Core_Permission::check('access deleted contacts')) {
$whereClause .= " AND $contactAlias.is_deleted = 0";
}
}

return [$fromClause, $whereClase];
return [$fromClause, $whereClause];
}

/**
Expand Down
50 changes: 27 additions & 23 deletions CRM/Contact/BAO/Query.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,9 @@ public function query($count = FALSE, $sortByChar = FALSE, $groupContacts = FALS
}

if (!empty($this->_permissionWhereClause) && empty($this->_displayRelationshipType)) {
if (!empty($this->_permissionFromClause)) {
$from .= " $this->_permissionFromClause";
}
if (empty($where)) {
$where = "WHERE $this->_permissionWhereClause";
}
Expand Down Expand Up @@ -4603,6 +4606,9 @@ public static function apiQuery(

$options = $query->_options;
if (!empty($query->_permissionWhereClause)) {
if (!empty($query->_permissionFromClause) && !stripos($from, 'aclContactCache')) {
$from .= " $query->_permissionFromClause";
}
if (empty($where)) {
$where = "WHERE $query->_permissionWhereClause";
}
Expand Down Expand Up @@ -5051,17 +5057,22 @@ private function createSqlCase($idCol, $cids) {
*/
public function generatePermissionClause($onlyDeleted = FALSE, $count = FALSE) {
if (!$this->_skipPermission) {
$this->_permissionWhereClause = CRM_ACL_API::whereClause(
CRM_Core_Permission::VIEW,
$this->_tables,
$this->_whereTables,
NULL,
$onlyDeleted,
$this->_skipDeleteClause
);
$permissionClauses = CRM_Contact_BAO_Contact_Permission::cacheClause();
$this->_permissionWhereClause = $permissionClauses[1];
$this->_permissionFromClause = $permissionClauses[0];

if (!$onlyDeleted && CRM_Core_Permission::check('access deleted contacts')) {
$this->_permissionWhereClause = str_replace(' ( 1 ) ', '(contact_a.is_deleted = 0)', $this->_permissionWhereClause);
if (CRM_Core_Permission::check('access deleted contacts')) {
if (!$onlyDeleted) {
$this->_permissionWhereClause = str_replace('( 1 )', '(contact_a.is_deleted = 0)', $this->_permissionWhereClause);
}
else {
if ($this->_permissionWhereClause === '( 1 )') {
$this->_permissionWhereClause = str_replace('( 1 )', '(contact_a.is_deleted)', $this->_permissionWhereClause);
}
else {
$this->_permissionWhereClause .= " AND (contact_a.is_deleted) ";
}
}
}

if (isset($this->_tables['civicrm_activity'])) {
Expand All @@ -5085,19 +5096,6 @@ public function generatePermissionClause($onlyDeleted = FALSE, $count = FALSE) {
$this->_permissionWhereClause .= '(' . implode(' AND ', $clauses) . ')';
}
}

// regenerate fromClause since permission might have added tables
if ($this->_permissionWhereClause) {
//fix for row count in qill (in contribute/membership find)
if (!$count) {
$this->_useDistinct = TRUE;
}
//CRM-15231
$this->_fromClause = self::fromClause($this->_tables, NULL, NULL, $this->_primaryLocation, $this->_mode);
$this->_simpleFromClause = self::fromClause($this->_whereTables, NULL, NULL, $this->_primaryLocation, $this->_mode);
// note : this modifies _fromClause and _simpleFromClause
$this->includePseudoFieldsJoin($this->_sort);
}
}
else {
// add delete clause if needed even if we are skipping permission
Expand Down Expand Up @@ -5129,6 +5127,9 @@ public function setSkipPermission($val) {
*/
public function summaryContribution($context = NULL) {
list($innerselect, $from, $where, $having) = $this->query(TRUE);
if (!empty($this->_permissionFromClause) && !stripos($from, 'aclContactCache')) {
$from .= " $this->_permissionFromClause";
}
if ($this->_permissionWhereClause) {
$where .= " AND " . $this->_permissionWhereClause;
}
Expand Down Expand Up @@ -5832,6 +5833,9 @@ public function filterRelatedContacts(&$from, &$where, &$having) {
$from = str_replace("INNER JOIN", "LEFT JOIN", $from);
$from .= $qcache['from'];
$where = $qcache['where'];
if (!empty($this->_permissionFromClause) && !stripos($from, 'aclContactCache')) {
$from .= " $this->_permissionFromClause";
}
if (!empty($this->_permissionWhereClause)) {
$where .= "AND $this->_permissionWhereClause";
}
Expand Down
31 changes: 31 additions & 0 deletions tests/phpunit/CRM/Contact/SelectorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase {
* @throws \Exception
*/
public function testSelectorQuery($dataSet) {
if (!empty($dataSet['limitedPermissions'])) {
CRM_Core_Config::singleton()->userPermissionClass->permissions = [
'access CiviCRM',
'access deleted contacts',
];
}
$params = CRM_Contact_BAO_Query::convertFormValues($dataSet['form_values'], 0, FALSE, NULL, []);
$isDeleted = in_array(['deleted_contacts', '=', 1, 0, 0], $params);
foreach ($dataSet['settings'] as $setting) {
Expand Down Expand Up @@ -90,6 +96,9 @@ public function testSelectorQuery($dataSet) {
$selector->getQueryObject()->getCachedContacts([$contactID], FALSE);
}
}
if (!empty($dataSet['limitedPermissions'])) {
$this->cleanUpAfterACLs();
}
}

/**
Expand Down Expand Up @@ -314,6 +323,28 @@ public function querySets() {
2 => "WHERE ( civicrm_email.email LIKE 'mickey@mouseville.com%' AND ( ( ( contact_a.sort_name LIKE 'Mouse%' ) OR ( civicrm_email.email LIKE 'Mouse%' ) ) ) ) AND (contact_a.is_deleted)",
],
],
],
[
[
'description' => 'Ensure that the Join to the acl contact cache is correct and that if we are searching in deleted contacts appropriate where clause is added',
'class' => 'CRM_Contact_Selector',
'settings' => [['name' => 'includeWildCardInName', 'value' => FALSE]],
'form_values' => ['email' => 'mickey@mouseville.com', 'sort_name' => 'Mouse', 'deleted_contacts' => 1],
'params' => [],
'return_properties' => NULL,
'context' => 'advanced',
'action' => CRM_Core_Action::ADVANCED,
'includeContactIds' => NULL,
'searchDescendentGroups' => FALSE,
'limitedPermissions' => TRUE,
'expected_query' => [
0 => 'default',
1 => 'FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 ) LEFT JOIN civicrm_country ON ( civicrm_address.country_id = civicrm_country.id ) LEFT JOIN civicrm_email ON (contact_a.id = civicrm_email.contact_id AND civicrm_email.is_primary = 1) LEFT JOIN civicrm_phone ON (contact_a.id = civicrm_phone.contact_id AND civicrm_phone.is_primary = 1) LEFT JOIN civicrm_im ON (contact_a.id = civicrm_im.contact_id AND civicrm_im.is_primary = 1) LEFT JOIN civicrm_worldregion ON civicrm_country.region_id = civicrm_worldregion.id INNER JOIN civicrm_acl_contact_cache aclContactCache ON contact_a.id = aclContactCache.contact_id',
2 => "WHERE ( civicrm_email.email LIKE 'mickey@mouseville.com%' AND ( ( ( contact_a.sort_name LIKE 'Mouse%' ) OR ( civicrm_email.email LIKE 'Mouse%' ) ) ) ) AND aclContactCache.user_id = 0 AND (contact_a.is_deleted)",
],
],
],
[
[
'description' => 'Use of quotes for exact string',
'use_case_comments' => 'This is something that was in the code but seemingly not working. No UI info on it though!',
Expand Down

0 comments on commit 93317f7

Please sign in to comment.