From b3df61d82a5070757d0ad378c04e017f4f6f0bb7 Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Fri, 22 May 2020 14:45:33 -0400 Subject: [PATCH] ACL - Don't exclude deleted contacts from ACL cache when user has permission to see them Test failures seem to be due to assumptions of the test. Removed via Hume's guillotine When the user has 'access deleted contacts' permission, deleted contacts should not be excluded from the ACL cache. --- CRM/ACL/API.php | 8 +++++--- tests/phpunit/CRM/Contact/BAO/QueryTest.php | 2 +- tests/phpunit/CRM/Contact/SelectorTest.php | 18 +++++++++--------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/CRM/ACL/API.php b/CRM/ACL/API.php index 95273671bf..ba4991dc2d 100644 --- a/CRM/ACL/API.php +++ b/CRM/ACL/API.php @@ -88,11 +88,13 @@ class CRM_ACL_API { // the default value which is valid for the final AND $deleteClause = ' ( 1 ) '; if (!$skipDeleteClause) { - if (CRM_Core_Permission::check('access deleted contacts') and $onlyDeleted) { - $deleteClause = '(contact_a.is_deleted)'; + if (CRM_Core_Permission::check('access deleted contacts')) { + if ($onlyDeleted) { + $deleteClause = '(contact_a.is_deleted)'; + } } else { - // CRM-6181 + // Exclude deleted contacts due to permissions $deleteClause = '(contact_a.is_deleted = 0)'; } } diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index f6c93b4dbd..aadf33232b 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -400,7 +400,7 @@ class CRM_Contact_BAO_QueryTest extends CiviUnitTestCase { 'contact_sub_type' => 1, 'sort_name' => 1, ]; - $expectedSQL = 'SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, civicrm_address.id as address_id, ' . $selectClause . " FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 ) WHERE ( ( " . $whereClause . " ) ) AND (contact_a.is_deleted = 0) ORDER BY `contact_a`.`sort_name` ASC, `contact_a`.`id` "; + $expectedSQL = 'SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, civicrm_address.id as address_id, ' . $selectClause . " FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 ) WHERE ( ( " . $whereClause . " ) ) AND ( 1 ) ORDER BY `contact_a`.`sort_name` ASC, `contact_a`.`id` "; $queryObj = new CRM_Contact_BAO_Query($params, $returnProperties); try { $this->assertLike($expectedSQL, $queryObj->getSearchSQL()); diff --git a/tests/phpunit/CRM/Contact/SelectorTest.php b/tests/phpunit/CRM/Contact/SelectorTest.php index 6e9740b9cb..8597a60f1f 100644 --- a/tests/phpunit/CRM/Contact/SelectorTest.php +++ b/tests/phpunit/CRM/Contact/SelectorTest.php @@ -176,7 +176,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'expected_query' => [ 0 => 'default', 1 => 'default', - 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' ) AND (contact_a.is_deleted = 0)", + 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' ) AND ( 1 )", ], ]; $params = CRM_Contact_BAO_Query::convertFormValues($dataSet['form_values'], 0, FALSE, NULL, []); @@ -253,7 +253,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'expected_query' => [ 0 => 'default', 1 => 'default', - 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' ) AND (contact_a.is_deleted = 0)", + 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' ) AND ( 1 )", ], ], ], @@ -272,7 +272,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'expected_query' => [ 0 => 'default', 1 => 'default', - 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 = 0)", + 2 => "WHERE ( civicrm_email.email LIKE '%mickey@mouseville.com%' AND ( ( ( contact_a.sort_name LIKE '%Mouse%' ) OR ( civicrm_email.email LIKE '%Mouse%' ) ) ) ) AND ( 1 )", ], ], ], @@ -291,7 +291,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'expected_query' => [ 0 => 'default', 1 => 'default', - 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 = 0)", + 2 => "WHERE ( civicrm_email.email LIKE 'mickey@mouseville.com%' AND ( ( ( contact_a.sort_name LIKE 'Mouse%' ) OR ( civicrm_email.email LIKE 'Mouse%' ) ) ) ) AND ( 1 )", ], ], ], @@ -311,7 +311,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'expected_query' => [ 0 => 'default', 1 => 'default', - 2 => "WHERE ( civicrm_email.email = 'mickey@mouseville.com' AND ( ( ( contact_a.sort_name LIKE 'Mouse%' ) OR ( civicrm_email.email LIKE 'Mouse%' ) ) ) ) AND (contact_a.is_deleted = 0)", + 2 => "WHERE ( civicrm_email.email = 'mickey@mouseville.com' AND ( ( ( contact_a.sort_name LIKE 'Mouse%' ) OR ( civicrm_email.email LIKE 'Mouse%' ) ) ) ) AND ( 1 )", ], ], ], @@ -334,7 +334,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'expected_query' => [ 0 => 'SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, civicrm_address.id as address_id, civicrm_address.country_id as country_id', 1 => ' FROM civicrm_contact contact_a LEFT JOIN civicrm_address ON ( contact_a.id = civicrm_address.contact_id AND civicrm_address.is_primary = 1 )', - 2 => 'WHERE ( contact_a.contact_type IN ("Individual") AND civicrm_address.country_id IS NOT NULL ) AND (contact_a.is_deleted = 0)', + 2 => 'WHERE ( contact_a.contact_type IN ("Individual") AND civicrm_address.country_id IS NOT NULL ) AND ( 1 )', ], ], ], @@ -354,7 +354,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 'searchDescendentGroups' => FALSE, 'expected_query' => [ 0 => 'SELECT contact_a.id as contact_id, source_contact.id as source_contact_id', - 2 => 'WHERE ( source_contact.id IS NOT NULL ) AND (contact_a.is_deleted = 0)', + 2 => 'WHERE ( source_contact.id IS NOT NULL ) AND ( 1 )', ], ], ], @@ -374,7 +374,7 @@ class CRM_Contact_SelectorTest extends CiviUnitTestCase { 0 => 'SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, contact_a.display_name as `display_name`, contact_a.do_not_email as `do_not_email`, contact_a.do_not_phone as `do_not_phone`, contact_a.do_not_mail as `do_not_mail`, contact_a.do_not_sms as `do_not_sms`, contact_a.do_not_trade as `do_not_trade`, contact_a.is_opt_out as `is_opt_out`, contact_a.legal_identifier as `legal_identifier`, contact_a.external_identifier as `external_identifier`, contact_a.nick_name as `nick_name`, contact_a.legal_name as `legal_name`, contact_a.image_URL as `image_URL`, contact_a.preferred_communication_method as `preferred_communication_method`, contact_a.preferred_language as `preferred_language`, contact_a.preferred_mail_format as `preferred_mail_format`, contact_a.first_name as `first_name`, contact_a.middle_name as `middle_name`, contact_a.last_name as `last_name`, contact_a.prefix_id as `prefix_id`, contact_a.suffix_id as `suffix_id`, contact_a.formal_title as `formal_title`, contact_a.communication_style_id as `communication_style_id`, contact_a.job_title as `job_title`, contact_a.gender_id as `gender_id`, contact_a.birth_date as `birth_date`, contact_a.is_deceased as `is_deceased`, contact_a.deceased_date as `deceased_date`, contact_a.household_name as `household_name`, IF ( contact_a.contact_type = \'Individual\', NULL, contact_a.organization_name ) as organization_name, contact_a.sic_code as `sic_code`, contact_a.is_deleted as `contact_is_deleted`, IF ( contact_a.contact_type = \'Individual\', contact_a.organization_name, NULL ) as current_employer, civicrm_address.id as address_id, civicrm_address.street_address as `street_address`, civicrm_address.supplemental_address_1 as `supplemental_address_1`, civicrm_address.supplemental_address_2 as `supplemental_address_2`, civicrm_address.supplemental_address_3 as `supplemental_address_3`, civicrm_address.city as `city`, civicrm_address.postal_code_suffix as `postal_code_suffix`, civicrm_address.postal_code as `postal_code`, civicrm_address.geo_code_1 as `geo_code_1`, civicrm_address.geo_code_2 as `geo_code_2`, civicrm_address.state_province_id as state_province_id, civicrm_address.country_id as country_id, civicrm_phone.id as phone_id, civicrm_phone.phone_type_id as phone_type_id, civicrm_phone.phone as `phone`, civicrm_email.id as email_id, civicrm_email.email as `email`, civicrm_email.on_hold as `on_hold`, civicrm_im.id as im_id, civicrm_im.provider_id as provider_id, civicrm_im.name as `im`, civicrm_worldregion.id as worldregion_id, civicrm_worldregion.name as `world_region`', 2 => 'WHERE displayRelType.relationship_type_id = 1 AND displayRelType.is_active = 1 -AND (contact_a.is_deleted = 0)', +AND ( 1 )', ], ], ], @@ -450,7 +450,7 @@ AND (contact_a.is_deleted = 0)', $expectedQuery = [ 0 => "SELECT contact_a.id as contact_id, contact_a.contact_type as `contact_type`, contact_a.contact_sub_type as `contact_sub_type`, contact_a.sort_name as `sort_name`, `Non_ASCII_Location_Type-location_type`.id as `Non_ASCII_Location_Type-location_type_id`, `Non_ASCII_Location_Type-location_type`.name as `Non_ASCII_Location_Type-location_type`, `Non_ASCII_Location_Type-email`.id as `Non_ASCII_Location_Type-email_id`, `Non_ASCII_Location_Type-email`.email as `Non_ASCII_Location_Type-email`", // @TODO these FROM clause doesn't matches due to extra spaces or special character - 2 => "WHERE ( ( `Non_ASCII_Location_Type-email`.email IS NOT NULL ) ) AND (contact_a.is_deleted = 0)", + 2 => "WHERE ( ( `Non_ASCII_Location_Type-email`.email IS NOT NULL ) ) AND ( 1 )", ]; foreach ($expectedQuery as $index => $queryString) { $this->assertEquals($this->strWrangle($queryString), $this->strWrangle($sql[$index])); -- 2.25.1