Skip to content

Commit

Permalink
dev/core#2039 Fix merge code so that deleted contacts are not left wi…
Browse files Browse the repository at this point in the history
…thout a primary address

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
  • Loading branch information
eileenmcnaughton committed Sep 22, 2020
1 parent 7fef2ca commit 98facda
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 7 deletions.
100 changes: 93 additions & 7 deletions CRM/Dedupe/MergeHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,29 +227,115 @@ public function copyDataToNewBlockDAO($otherBlockId, $name, $blkCount) {
}

/**
* Get the DAO object appropriate to the location entity.
* 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_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.
*
* @param string[email|address|phone|website|im] $entity
* @param int|null $locationTypeID
* @param int|null $typeID
*
* @return CRM_Core_DAO_Address|CRM_Core_DAO_Email|CRM_Core_DAO_IM|CRM_Core_DAO_Phone|CRM_Core_DAO_Website
* @throws \CRM_Core_Exception
*/
public function getDAOForLocationEntity($entity) {
public function getDAOForLocationEntity($entity, $locationTypeID = NULL, $typeID = NULL) {
switch ($entity) {
case 'email':
return new CRM_Core_DAO_Email();
$dao = new CRM_Core_DAO_Email();
$dao->location_type_id = $locationTypeID;
return $dao;

case 'address':
return new CRM_Core_DAO_Address();
$dao = new CRM_Core_DAO_Address();
$dao->location_type_id = $locationTypeID;
return $dao;

case 'phone':
return new CRM_Core_DAO_Phone();
$dao = new CRM_Core_DAO_Phone();
$dao->location_type_id = $locationTypeID;
$dao->phone_type_id = $typeID;
return $dao;

case 'website':
return new CRM_Core_DAO_Website();
$dao = new CRM_Core_DAO_Website();
$dao->website_type_id = $typeID;
return $dao;

case 'im':
return new CRM_Core_DAO_IM();
$dao = new CRM_Core_DAO_IM();
$dao->location_type_id = $locationTypeID;
return $dao;

default:
// Mostly here, along with the switch over a more concise format, to help IDEs understand the possibilities.
Expand Down
1 change: 1 addition & 0 deletions CRM/Dedupe/Merger.php
Original file line number Diff line number Diff line change
Expand Up @@ -1864,6 +1864,7 @@ public static function mergeLocations($mergeHandler) {
}
$blocksDAO[$name]['update'][$otherBlockDAO->id] = $otherBlockDAO;
}
$blocksDAO[$name]['update'] = array_merge($mergeHandler->getBlocksToUpdateForDeletedContact($name), $blocksDAO[$name]['update']);
}
}

Expand Down
7 changes: 7 additions & 0 deletions tests/phpunit/api/v3/JobTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ class api_v3_JobTest extends CiviUnitTestCase {
*/
private $report_instance;

/**
* Should location types be checked to ensure primary addresses are correctly assigned after each test.
*
* @var bool
*/
protected $isLocationTypesOnPostAssert = TRUE;

/**
* Set up for tests.
*/
Expand Down

0 comments on commit 98facda

Please sign in to comment.