From: eileen <emcnaughton@wikimedia.org>
Date: Wed, 8 Apr 2020 00:46:36 +0000 (+1200)
Subject: [REF] Clarify variable & tighten use.
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=9a2485264925f5b208a9d456e30ec60400d832a4;p=civicrm-core.git

[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....
---

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]));
   }
 
 }