From 9a2485264925f5b208a9d456e30ec60400d832a4 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 8 Apr 2020 12:46:36 +1200 Subject: [PATCH] [REF] Clarify variable & tighten use. moveContactBelongings is only called from one place which passes in all the function variables, so we don't need defaults. The last parameter, is retrieved from self::getAffectedCustomTables which always returns an array - so all the handling for it being NULL can be removed.... --- CRM/Dedupe/Merger.php | 19 +++++++++---------- .../phpunit/api/v3/JobTestCustomDataTest.php | 19 ++++++++++++------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 7e2354a01a..5dfbb5ad7b 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -478,7 +478,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m * @param array $tableOperations * @param array $customTableToCopyFrom */ - public static function moveContactBelongings($mainId, $otherId, $tables = FALSE, $tableOperations = [], $customTableToCopyFrom = NULL) { + public static function moveContactBelongings($mainId, $otherId, $tables, $tableOperations, array $customTableToCopyFrom) { $cidRefs = self::cidRefs(); $eidRefs = self::eidRefs(); $cpTables = self::cpTables(); @@ -486,12 +486,10 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m // getting all custom tables $customTables = []; - if ($customTableToCopyFrom !== NULL) { - // @todo this duplicates cidRefs? - CRM_Core_DAO::appendCustomTablesExtendingContacts($customTables); - CRM_Core_DAO::appendCustomContactReferenceFields($customTables); - $customTables = array_keys($customTables); - } + // @todo this duplicates cidRefs? + CRM_Core_DAO::appendCustomTablesExtendingContacts($customTables); + CRM_Core_DAO::appendCustomContactReferenceFields($customTables); + $customTables = array_keys($customTables); $affected = array_merge(array_keys($cidRefs), array_keys($eidRefs)); @@ -514,7 +512,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m foreach ($affected as $table) { // skipping non selected single-value custom table's value migration if (!in_array($table, $multi_value_tables)) { - if ($customTableToCopyFrom !== NULL && in_array($table, $customTables) && !in_array($table, $customTableToCopyFrom)) { + if (in_array($table, $customTables) && !in_array($table, $customTableToCopyFrom)) { if (isset($cidRefs[$table]) && ($delCol = array_search('entity_id', $cidRefs[$table])) !== FALSE) { // remove entity_id from the field list unset($cidRefs[$table][$delCol]); @@ -549,7 +547,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m $preOperationSqls = self::operationSql($mainId, $otherId, $table, $tableOperations); $sqls = array_merge($sqls, $preOperationSqls); - if ($customTableToCopyFrom !== NULL && in_array($table, $customTableToCopyFrom) && !self::customRecordExists($mainId, $table, $field) && $field == 'entity_id') { + if (in_array($table, $customTableToCopyFrom) && !self::customRecordExists($mainId, $table, $field) && $field == 'entity_id') { // this is the entity_id column of a custom field group where: // - the custom table should be copied as indicated by $customTableToCopyFrom // e.g. because a field in the group was selected in a form @@ -1917,7 +1915,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m * @param $value * * @return array - * @throws \Exception + * @throws \CRM_Core_Exception */ protected static function processCustomFields($mainId, $key, $cFields, $submitted, $value) { if (substr($key, 0, 7) == 'custom_') { @@ -2021,6 +2019,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m * @param string $contactType * * @return array + * @throws \CRM_Core_Exception */ protected static function getCustomFieldMetadata($contactType) { $treeCache = []; diff --git a/tests/phpunit/api/v3/JobTestCustomDataTest.php b/tests/phpunit/api/v3/JobTestCustomDataTest.php index 07e783507e..33062c5848 100644 --- a/tests/phpunit/api/v3/JobTestCustomDataTest.php +++ b/tests/phpunit/api/v3/JobTestCustomDataTest.php @@ -399,24 +399,29 @@ class api_v3_JobTestCustomDataTest extends CiviUnitTestCase { /** * Check we get a conflict on the customs field when the data conflicts for booleans (reverse). + * + * @throws \CRM_Core_Exception */ - public function testBatchMergeCustomFieldConflictsOneBlank() { + public function testBatchMergeCustomFieldNoConflictsOneBlank() { $this->individualCreate(['custom_' . $this->customBoolFieldID => 1]); $this->individualCreate(); $result = $this->callAPISuccess('Job', 'process_batch_merge', []); - $this->assertEquals(1, count($result['values']['merged'])); - $this->assertEquals(0, count($result['values']['skipped'])); + $this->assertCount(1, $result['values']['merged']); + $this->assertCount(0, $result['values']['skipped']); } /** * Check we get a conflict on the customs field when the data conflicts for booleans (reverse). + * + * @throws \CRM_Core_Exception */ - public function testBatchMergeCustomFieldConflictsOneBlankReverse() { - $this->individualCreate(); + public function testBatchMergeCustomFieldNoConflictsOneBlankReverse() { + $contactID = $this->individualCreate(); $this->individualCreate(['custom_' . $this->customBoolFieldID => 1]); $result = $this->callAPISuccess('Job', 'process_batch_merge', []); - $this->assertEquals(1, count($result['values']['merged'])); - $this->assertEquals(0, count($result['values']['skipped'])); + $this->assertCount(1, $result['values']['merged']); + $this->assertCount(0, $result['values']['skipped']); + $this->assertEquals(1, $this->callAPISuccessGetValue('Contact', ['id' => $contactID, 'return' => 'custom_' . $this->customBoolFieldID])); } } -- 2.25.1