dev/core#2039 Fix merge code so that deleted contacts are not left without a primary...
authoreileen <emcnaughton@wikimedia.org>
Tue, 22 Sep 2020 02:43:29 +0000 (14:43 +1200)
committereileen <emcnaughton@wikimedia.org>
Mon, 12 Oct 2020 10:05:40 +0000 (23:05 +1300)
This was picked up in tests to ensure that removing a line of code creating excessing queries
would not cause regressions.

https://lab.civicrm.org/dev/core/-/issues/2039

I considered altering the test to exclude deleted contacts but we run the risk that if a contact
is undeleted they will have no primary address so I figured the integrity makes sense.

Note there are a couple of queries in this code that can go (retrieving stuff
we already have) - depending how I go on getting review on this & related tidy up I'll remove them
in a later PR

CRM/Dedupe/MergeHandler.php
CRM/Dedupe/Merger.php
tests/phpunit/api/v3/JobTest.php

index 654e87c20930b6cd6aed0960a4e92fea3417244c..cb0d64ebacdc9c8dbe4d5e615a590e23aa663be9 100644 (file)
@@ -213,6 +213,79 @@ class CRM_Dedupe_MergeHandler {
     return $otherBlockDAO;
   }
 
+  /**
+   * Get blocks, if any, to update for the deleted contact.
+   *
+   * If the deleted contact no longer has a primary address but still has
+   * one or more blocks we want to ensure the remaining block is updated
+   * to have is_primary = 1 in case the contact is ever undeleted.
+   *
+   * @param string $entity
+   *
+   * @return array
+   * @throws \CRM_Core_Exception
+   */
+  public function getBlocksToUpdateForDeletedContact($entity) {
+    $movedBlocks = $this->getLocationBlocksToMerge()[$entity];
+    $deletedContactsBlocks = $this->getLocationBlocksForContactToRemove()[$entity];
+    $unMovedBlocks = array_values(array_diff_key($deletedContactsBlocks, $movedBlocks));
+    if (empty($unMovedBlocks) || empty($movedBlocks)) {
+      return [];
+    }
+    foreach (array_keys($movedBlocks) as $index) {
+      if ($deletedContactsBlocks[$index]['is_primary']) {
+        // We have moved the primary - change any other block to be primary.
+        $newPrimaryBlock = $this->getDAOForLocationEntity($entity);
+        $newPrimaryBlock->id = $unMovedBlocks[0]['id'];
+        $newPrimaryBlock->is_primary = 1;
+        return [$newPrimaryBlock->id => $newPrimaryBlock];
+      }
+    }
+    return [];
+  }
+
+  /**
+   * Get the details of the blocks to be transferred over for the given entity.
+   *
+   * @param string $entity
+   *
+   * @return array
+   */
+  protected function getLocationBlocksToMoveForEntity($entity) {
+    $movedBlocks = $this->getLocationBlocksToMerge()[$entity];
+    $blockDetails = $this->getLocationBlocksForContactToRemove()[$entity];
+    return array_intersect_key($blockDetails, $movedBlocks);
+  }
+
+  /**
+   * Does the contact to keep have location blocks for the given entity.
+   *
+   * @param string $entity
+   *
+   * @return bool
+   */
+  public function contactToKeepHasLocationBlocksForEntity($entity) {
+    return !empty($this->getLocationBlocksForContactToKeep()[$entity]);
+  }
+
+  /**
+   * Get the location blocks for the contact to be kept.
+   *
+   * @return array
+   */
+  public function getLocationBlocksForContactToKeep() {
+    return $this->getMigrationInfo()['main_details']['location_blocks'];
+  }
+
+  /**
+   * Get the location blocks for the contact to be deleted.
+   *
+   * @return array
+   */
+  public function getLocationBlocksForContactToRemove() {
+    return $this->getMigrationInfo()['other_details']['location_blocks'];
+  }
+
   /**
    * Get the DAO object appropriate to the location entity.
    *
index b3898072216a75a775f272e7b47d4cf49d1eabb2..7fe56cdb923e586377e972eb866fb48eead0836c 100644 (file)
@@ -1861,6 +1861,7 @@ INNER JOIN  civicrm_membership membership2 ON membership1.membership_type_id = m
           }
           $blocksDAO[$name]['update'][$otherBlockDAO->id] = $otherBlockDAO;
         }
+        $blocksDAO[$name]['update'] += $mergeHandler->getBlocksToUpdateForDeletedContact($name);
       }
     }
 
index 492b324c49337c5f2c623925bb4c338cf457774f..5bc0ccb8fddb19c9184ffb6469378eb0eed36df4 100644 (file)
@@ -44,15 +44,6 @@ class api_v3_JobTest extends CiviUnitTestCase {
    */
   private $report_instance;
 
-  /**
-   * Should location types be checked to ensure primary addresses are correctly assigned after each test.
-   *
-   * We cannot enable this until https://github.com/civicrm/civicrm-core/pull/18555 is merged
-   *
-   * @var bool
-   */
-  protected $isLocationTypesOnPostAssert = FALSE;
-
   /**
    * Set up for tests.
    */