Fix dedupe regression whereby deleted contacts are found

This affects api calls where check_permissions = TRUE and getduplicates is called. This can be
done via the api (per this test) or ann easy UI way is with the deduper extension but it should
also affect the 'normal' dedupe screen.

Note that there can be cases where the dedupe results are cached into prevnext cache to hide this

diff --git a/CRM/Dedupe/BAO/RuleGroup.php b/CRM/Dedupe/BAO/RuleGroup.php
index 5269cec1c2c48827e74241b493c0954c28bc60cb..5e1258e775311e520f7601362f1a25ea2c47addf 100644 (file)
--- a/CRM/Dedupe/BAO/RuleGroup.php
+++ b/CRM/Dedupe/BAO/RuleGroup.php
@@ -371,24 +371,23 @@ class CRM_Dedupe_BAO_RuleGroup extends CRM_Dedupe_DAO_RuleGroup {
    */
   public function thresholdQuery($checkPermission = TRUE) {
     $this->_aclFrom = '';
-    // CRM-6603: anonymous dupechecks side-step ACLs
-    $this->_aclWhere = ' AND is_deleted = 0 ';
+    $aclWhere = '';
 
     if ($this->params && !$this->noRules) {
       if ($checkPermission) {
-        list($this->_aclFrom, $this->_aclWhere) = CRM_Contact_BAO_Contact_Permission::cacheClause('civicrm_contact');
-        $this->_aclWhere = $this->_aclWhere ? "AND {$this->_aclWhere}" : '';
+        list($this->_aclFrom, $aclWhere) = CRM_Contact_BAO_Contact_Permission::cacheClause('civicrm_contact');
+        $aclWhere = $aclWhere ? "AND {$aclWhere}" : '';
       }
       $query = "SELECT {$this->temporaryTables['dedupe']}.id1 as id
                 FROM {$this->temporaryTables['dedupe']} JOIN civicrm_contact ON {$this->temporaryTables['dedupe']}.id1 = civicrm_contact.id {$this->_aclFrom}
-                WHERE contact_type = '{$this->contact_type}' {$this->_aclWhere}
+                WHERE contact_type = '{$this->contact_type}' AND is_deleted = 0 $aclWhere
                 AND weight >= {$this->threshold}";
     }
     else {
-      $this->_aclWhere = ' AND c1.is_deleted = 0 AND c2.is_deleted = 0';
+      $aclWhere = '';
       if ($checkPermission) {
-        list($this->_aclFrom, $this->_aclWhere) = CRM_Contact_BAO_Contact_Permission::cacheClause(['c1', 'c2']);
-        $this->_aclWhere = $this->_aclWhere ? "AND {$this->_aclWhere}" : '';
+        list($this->_aclFrom, $aclWhere) = CRM_Contact_BAO_Contact_Permission::cacheClause(['c1', 'c2']);
+        $aclWhere = $aclWhere ? "AND {$aclWhere}" : '';
       }
       $query = "SELECT IF({$this->temporaryTables['dedupe']}.id1 < {$this->temporaryTables['dedupe']}.id2, {$this->temporaryTables['dedupe']}.id1, {$this->temporaryTables['dedupe']}.id2) as id1,
                 IF({$this->temporaryTables['dedupe']}.id1 < {$this->temporaryTables['dedupe']}.id2, {$this->temporaryTables['dedupe']}.id2, {$this->temporaryTables['dedupe']}.id1) as id2, {$this->temporaryTables['dedupe']}.weight
@@ -396,7 +395,9 @@ class CRM_Dedupe_BAO_RuleGroup extends CRM_Dedupe_DAO_RuleGroup {
                             JOIN civicrm_contact c2 ON {$this->temporaryTables['dedupe']}.id2 = c2.id {$this->_aclFrom}
                        LEFT JOIN civicrm_dedupe_exception exc ON {$this->temporaryTables['dedupe']}.id1 = exc.contact_id1 AND {$this->temporaryTables['dedupe']}.id2 = exc.contact_id2
                 WHERE c1.contact_type = '{$this->contact_type}' AND
-                      c2.contact_type = '{$this->contact_type}' {$this->_aclWhere}
+                      c2.contact_type = '{$this->contact_type}'
+                       AND c1.is_deleted = 0 AND c2.is_deleted = 0
+                      {$aclWhere}
                       AND weight >= {$this->threshold} AND exc.contact_id1 IS NULL";
     }
 

diff --git a/tests/phpunit/CRM/Dedupe/MergerTest.php b/tests/phpunit/CRM/Dedupe/MergerTest.php
index c0709f0c5652b00843a42b98133184a85c4f9aca..8b4b57383abfb967ca87e5a5309cf65a67a196b8 100644 (file)
--- a/tests/phpunit/CRM/Dedupe/MergerTest.php
+++ b/tests/phpunit/CRM/Dedupe/MergerTest.php
@@ -339,6 +339,35 @@ class CRM_Dedupe_MergerTest extends CiviUnitTestCase {
     ], $pairs);
   }
 
+  /**
+   * Test function that gets duplicate pairs.
+   *
+   * It turns out there are 2 code paths retrieving this data so my initial
+   * focus is on ensuring they match.
+   *
+   * @dataProvider getBooleanDataProvider
+   *
+   * @param bool $isReverse
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testGetMatchesExcludeDeleted($isReverse) {
+    $this->setupMatchData();
+    $pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
+      'rule_group_id' => 1,
+      'check_permissions' => TRUE,
+      'criteria' => ['Contact' => ['id' => 'IS NOT NULL']],
+    ])['values'];
+    $this->assertCount(2, $pairs);
+    $this->callAPISuccess('Contact', 'delete', ['id' => ($isReverse ? $pairs[0]['dstID'] : $pairs[0]['srcID'])]);
+    $pairs = $this->callAPISuccess('Dedupe', 'getduplicates', [
+      'rule_group_id' => 1,
+      'check_permissions' => TRUE,
+      'criteria' => ['Contact' => ['id' => ['>' => 1]]],
+    ])['values'];
+    $this->assertCount(1, $pairs);
+  }
+
   /**
    * Test that location type is ignored when deduping by postal address.
    *