From cb5d0cde4e716ac18b0cb95aae51e5f6e102121d Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Fri, 21 Aug 2020 15:40:07 +1200
Subject: [PATCH] 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
---
 CRM/Dedupe/BAO/RuleGroup.php            | 19 ++++++++--------
 tests/phpunit/CRM/Dedupe/MergerTest.php | 29 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/CRM/Dedupe/BAO/RuleGroup.php b/CRM/Dedupe/BAO/RuleGroup.php
index 5269cec1c2..5e1258e775 100644
--- 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 c0709f0c56..8b4b57383a 100644
--- 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.
    *
-- 
2.25.1