[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....
civicrm · Apr 8, 2020 · 9a24852 · 9a24852
1 parent 0ff7d95
commit 9a24852
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 17 deletions.
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
@@ -478,20 +478,18 @@ public static function removeContactBelongings($otherID, $tables) {
    * @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();
     $paymentTables = self::paymentTables();
 
     // 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 @@ public static function moveContactBelongings($mainId, $otherId, $tables = FALSE,
     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 @@ public static function moveContactBelongings($mainId, $otherId, $tables = FALSE,
           $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 @@ protected static function swapOutFieldsAffectedByQFZeroBug(&$migrationInfo) {
    * @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 @@ protected static function processCustomFields($mainId, $key, $cFields, $submitte
    * @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
@@ -399,24 +399,29 @@ public function testBatchMergeCustomFieldConflictsReverse() {
 
   /**
    * 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]));
   }
 
 }