From 99827266d29ee49f1fc9f062d574f49149044ba0 Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Wed, 2 Dec 2020 13:25:12 +1100 Subject: [PATCH] Ensure that the is_deleted filter is added correctly if we are not searching for deleted contacts but have access to view deleted contacts Simplify IF down as per feed back from ACD and also comment from Eileen in chat and extend unit test as well --- CRM/Contact/BAO/Query.php | 9 ++--- tests/phpunit/CRM/Contact/BAO/QueryTest.php | 2 +- tests/phpunit/CRM/Contact/SelectorTest.php | 38 ++++++++++++++++----- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index ff2b028c0a81..f34429084e0a 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -5054,15 +5054,10 @@ public function generatePermissionClause($onlyDeleted = FALSE, $count = FALSE) { if (CRM_Core_Permission::check('access deleted contacts')) { if (!$onlyDeleted) { - $this->_permissionWhereClause = str_replace('( 1 )', '(contact_a.is_deleted = 0)', $this->_permissionWhereClause); + $this->_permissionWhereClause .= ' AND (contact_a.is_deleted = 0)'; } else { - if ($this->_permissionWhereClause === '( 1 )') { - $this->_permissionWhereClause = str_replace('( 1 )', '(contact_a.is_deleted)', $this->_permissionWhereClause); - } - else { - $this->_permissionWhereClause .= " AND (contact_a.is_deleted) "; - } + $this->_permissionWhereClause .= " AND (contact_a.is_deleted) "; } } diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index e3d02a668d75..19abb8717fb2 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -405,7 +405,7 @@ public function testSearchProfilePrimaryCityCRM14263($params, $selectClause, $wh '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 ) AND (contact_a.is_deleted = 0) 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 6129ee5ec08a..5316ebbd23e6 100644 --- a/tests/phpunit/CRM/Contact/SelectorTest.php +++ b/tests/phpunit/CRM/Contact/SelectorTest.php @@ -263,7 +263,7 @@ public function querySets() { '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 ) AND (contact_a.is_deleted = 0)", ], ], ], @@ -282,7 +282,7 @@ public function querySets() { '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 ) AND (contact_a.is_deleted = 0)", ], ], ], @@ -301,7 +301,7 @@ public function querySets() { '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 ) AND (contact_a.is_deleted = 0)", ], ], ], @@ -320,7 +320,7 @@ public function querySets() { '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)", + 2 => "WHERE ( civicrm_email.email LIKE 'mickey@mouseville.com%' AND ( ( ( contact_a.sort_name LIKE 'Mouse%' ) OR ( civicrm_email.email LIKE 'Mouse%' ) ) ) ) AND ( 1 ) AND (contact_a.is_deleted)", ], ], ], @@ -344,6 +344,26 @@ public function querySets() { ], ], ], + [ + [ + 'description' => 'Ensure that the Join to the acl contact cache is correct and that if we are not searching in the trash trashed contacts are not returned', + 'class' => 'CRM_Contact_Selector', + 'settings' => [['name' => 'includeWildCardInName', 'value' => FALSE]], + 'form_values' => ['email' => 'mickey@mouseville.com', 'sort_name' => 'Mouse'], + '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 = 0)", + ], + ], + ], [ [ 'description' => 'Use of quotes for exact string', @@ -360,7 +380,7 @@ public function querySets() { '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 ) AND (contact_a.is_deleted = 0)", ], ], ], @@ -383,7 +403,7 @@ public function querySets() { '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 ) AND (contact_a.is_deleted = 0)', ], ], ], @@ -403,7 +423,7 @@ public function querySets() { '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 ) AND (contact_a.is_deleted = 0)', ], ], ], @@ -423,7 +443,7 @@ public function querySets() { 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 ) AND (contact_a.is_deleted = 0)', ], ], ], @@ -499,7 +519,7 @@ public function testSelectorQueryOnNonASCIIlocationType() { $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 ) AND (contact_a.is_deleted = 0)", ]; foreach ($expectedQuery as $index => $queryString) { $this->assertEquals($this->strWrangle($queryString), $this->strWrangle($sql[$index]));