From 8669d9a7adf0e3623df5db2205270e210d27c3fb Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 24 Jan 2020 18:25:14 +1300 Subject: [PATCH] dev/core#1364 Merge all addresses on export should INCLUDE merging households This is an alternate to https://github.com/civicrm/civicrm-core/pull/15725 that still achieves the goal in https://github.com/civicrm/civicrm-core/pull/15725#issuecomment-572408173 of making is so that the mergeSharedAddress option mergesHouseholds first & then merges sharedAddresses. In #15725 @monishdeb was fixing the code that differentiated between how master_id was treated versus otherwise merged addresses. However that code 'broke' a long time ago and the arguably broken behaviour is locked in by unit tests. Fixing it has not been requested - in this issue or any other in the past year or 2 when it has been broken so I opted to remove the broken code entirely & to open up the next query such that it does not exclude household addresses. This didn't break any tests (woot) and also allowed me to enable the test rows @monishdeb wrote to test this issue --- CRM/Export/BAO/ExportProcessor.php | 38 +++++---------------- tests/phpunit/CRM/Export/BAO/ExportTest.php | 6 ++-- 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/CRM/Export/BAO/ExportProcessor.php b/CRM/Export/BAO/ExportProcessor.php index 8770d5dcc9..8453d02475 100644 --- a/CRM/Export/BAO/ExportProcessor.php +++ b/CRM/Export/BAO/ExportProcessor.php @@ -399,7 +399,7 @@ class CRM_Export_BAO_ExportProcessor { $this->setQueryOperator($queryOperator); $this->setRequestedFields($requestedFields); $this->setRelationshipTypes(); - $this->setIsMergeSameHousehold($isMergeSameHousehold); + $this->setIsMergeSameHousehold($isMergeSameHousehold || $isMergeSameAddress); $this->setIsPostalableOnly($isPostalableOnly); $this->setIsMergeSameAddress($isMergeSameAddress); $this->setReturnProperties($this->determineReturnProperties()); @@ -754,7 +754,8 @@ class CRM_Export_BAO_ExportProcessor { // This will require a generalised handling cleanup return ts('Campaign ID'); } - if ($this->isMergeSameHousehold() && $field === 'id') { + if ($this->isMergeSameHousehold() && !$this->isMergeSameAddress() && $field === 'id') { + // This is weird - even if we are merging households not every contact in the export is a household so this would not be accurate. return ts('Household ID'); } elseif (isset($this->getQueryFields()[$field]['title'])) { @@ -1927,9 +1928,8 @@ class CRM_Export_BAO_ExportProcessor { * Build array for merging same addresses. * * @param string $sql - * @param bool $sharedAddress */ - public function buildMasterCopyArray($sql, $sharedAddress = FALSE) { + public function buildMasterCopyArray($sql) { $parents = []; $dao = CRM_Core_DAO::executeQuery($sql); @@ -1961,7 +1961,7 @@ class CRM_Export_BAO_ExportProcessor { } $parents[$copyID] = $masterID; - if (!$sharedAddress && !array_key_exists($copyID, $this->contactsToMerge[$masterID]['copy'])) { + if (!array_key_exists($copyID, $this->contactsToMerge[$masterID]['copy'])) { $copyPostalGreeting = $this->getContactPortionOfGreeting((int) $dao->copy_contact_id, (int) $dao->copy_postal_greeting_id, 'postal_greeting', $dao->copy_postal_greeting); if ($copyPostalGreeting) { $this->contactsToMerge[$masterID]['postalGreeting'] = "{$this->contactsToMerge[$masterID]['postalGreeting']}, {$copyPostalGreeting}"; @@ -1987,26 +1987,6 @@ class CRM_Export_BAO_ExportProcessor { public function mergeSameAddress() { $tableName = $this->getTemporaryTable(); - // check if any records are present based on if they have used shared address feature, - // and not based on if city / state .. matches. - $sql = " -SELECT r1.id as copy_id, - r1.civicrm_primary_id as copy_contact_id, - r1.addressee as copy_addressee, - r1.addressee_id as copy_addressee_id, - r1.postal_greeting as copy_postal_greeting, - r1.postal_greeting_id as copy_postal_greeting_id, - r2.id as master_id, - r2.civicrm_primary_id as master_contact_id, - r2.postal_greeting as master_postal_greeting, - r2.postal_greeting_id as master_postal_greeting_id, - r2.addressee as master_addressee, - r2.addressee_id as master_addressee_id -FROM $tableName r1 -INNER JOIN civicrm_address adr ON r1.master_id = adr.id -INNER JOIN $tableName r2 ON adr.contact_id = r2.civicrm_primary_id -ORDER BY r1.id"; - $this->buildMasterCopyArray($sql, TRUE); // find all the records that have the same street address BUT not in a household // require match on city and state as well @@ -2025,11 +2005,9 @@ SELECT r1.id as master_id, r2.addressee_id as copy_addressee_id FROM $tableName r1 LEFT JOIN $tableName r2 ON ( r1.street_address = r2.street_address AND - r1.city = r2.city AND - r1.state_province_id = r2.state_province_id ) -WHERE ( r1.household_name IS NULL OR r1.household_name = '' ) -AND ( r2.household_name IS NULL OR r2.household_name = '' ) -AND ( r1.street_address != '' ) + r1.city = r2.city AND + r1.state_province_id = r2.state_province_id ) +WHERE ( r1.street_address != '' ) AND r2.id > r1.id ORDER BY r1.id "; diff --git a/tests/phpunit/CRM/Export/BAO/ExportTest.php b/tests/phpunit/CRM/Export/BAO/ExportTest.php index a6a59b88f4..f2495690df 100644 --- a/tests/phpunit/CRM/Export/BAO/ExportTest.php +++ b/tests/phpunit/CRM/Export/BAO/ExportTest.php @@ -1248,10 +1248,10 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase { 'ids' => $contactIDs, 'mergeSameAddress' => TRUE, ]); - // @todo - the below is commented out because it does not yet work. - //$this->assertCount(1, $this->csv); + + $this->assertCount(1, $this->csv); $row = $this->csv->fetchOne(); - //$this->assertEquals('Household', $this->csv->fetchOne()['Contact Type']); + $this->assertEquals('Household', $this->csv->fetchOne()['Contact Type']); } /** -- 2.25.1