Merge pull request #14260 from eileenmcnaughton/spec

[REF+test] clean up code for getting labels for merge screen, stdise
civicrm · May 19, 2019 · 0896a16 · 0896a16
2 parents 0c68f8e + 045b216
commit 0896a16
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 48 deletions.
diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php
@@ -1080,8 +1080,6 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
 
     $main = self::getMergeContactDetails($mainId);
     $other = self::getMergeContactDetails($otherId);
-    $specialValues['main'] = self::getSpecialValues($main);
-    $specialValues['other'] = self::getSpecialValues($other);
 
     $compareFields = self::retrieveFields($main, $other);
 
@@ -1093,12 +1091,21 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
         continue;
       }
       foreach (['main' => $main, 'other' => $other] as $moniker => $contact) {
-        $value = CRM_Utils_Array::value($field, $contact);
-        if (isset($specialValues[$moniker][$field]) && is_string($specialValues[$moniker][$field])) {
-          $value = CRM_Core_DAO::VALUE_SEPARATOR . trim($specialValues[$moniker][$field], CRM_Core_DAO::VALUE_SEPARATOR) . CRM_Core_DAO::VALUE_SEPARATOR;
+        $value = $label = CRM_Utils_Array::value($field, $contact);
+        $fieldSpec = $fields[$field];
+        if (!empty($fieldSpec['serialize']) && is_array($value)) {
+          // In practice this only applies to preferred_communication_method as the sub types are skipped above
+          // and no others are serialized.
+          $labels = [];
+          foreach ($value as $individualValue) {
+            $labels[] = CRM_Core_PseudoConstant::getLabel('CRM_Contact_BAO_Contact', $field, $individualValue);
+          }
+          $label = implode(', ', $labels);
+          // We serialize this due to historic handling but it's likely that if we just left it as an
+          // array all would be well & we would have less code.
+          $value = CRM_Core_DAO::serializeField($value, $fieldSpec['serialize']);
         }
-        $label = isset($specialValues[$moniker]["{$field}_display"]) ? $specialValues[$moniker]["{$field}_display"] : $value;
-        if (!empty($fields[$field]['type']) && $fields[$field]['type'] == CRM_Utils_Type::T_DATE) {
+        elseif (!empty($fieldSpec['type']) && $fieldSpec['type'] == CRM_Utils_Type::T_DATE) {
           if ($value) {
             $value = str_replace('-', '', $value);
             $label = CRM_Utils_Date::customFormat($label);
@@ -1115,15 +1122,8 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
             $label = ts('[x]');
           }
         }
-        elseif ($field == 'prefix_id') {
-          $label = CRM_Utils_Array::value('individual_prefix', $contact);
-        }
-        elseif ($field == 'suffix_id') {
-          $label = CRM_Utils_Array::value('individual_suffix', $contact);
-        }
-        elseif ($field == 'gender_id' && !empty($value)) {
-          $genderOptions = civicrm_api3('contact', 'getoptions', ['field' => 'gender_id']);
-          $label = $genderOptions['values'][$value];
+        elseif (!empty($fieldSpec['pseudoconstant'])) {
+          $label = CRM_Core_PseudoConstant::getLabel('CRM_Contact_BAO_Contact', $field, $value);
         }
         elseif ($field == 'current_employer_id' && !empty($value)) {
           $label = "$value (" . CRM_Contact_BAO_Contact::displayName($value) . ")";
@@ -1881,38 +1881,6 @@ public static function getMergeCacheKeyString($rule_group_id, $group_id, $criter
     return $cacheKeyString;
   }
 
-  /**
-   * @param array $contact
-   * @return array
-   *   $specialValues
-   */
-  public static function getSpecialValues($contact) {
-    $preferred_communication_method = CRM_Utils_Array::value('preferred_communication_method', $contact);
-    $value = empty($preferred_communication_method) ? [] : $preferred_communication_method;
-    $specialValues = [
-      'preferred_communication_method' => $value,
-      'communication_style_id' => $value,
-    ];
-
-    if (!empty($contact['preferred_communication_method'])) {
-      // api 3 returns pref_comm_method as an array, which breaks the lookup; so we reconstruct
-      $prefCommList = is_array($specialValues['preferred_communication_method']) ? implode(CRM_Core_DAO::VALUE_SEPARATOR, $specialValues['preferred_communication_method']) : $specialValues['preferred_communication_method'];
-      $specialValues['preferred_communication_method'] = CRM_Core_DAO::VALUE_SEPARATOR . $prefCommList . CRM_Core_DAO::VALUE_SEPARATOR;
-    }
-    $names = [
-      'preferred_communication_method' => [
-        'newName' => 'preferred_communication_method_display',
-        'groupName' => 'preferred_communication_method',
-      ],
-    ];
-    CRM_Core_OptionGroup::lookupValues($specialValues, $names);
-
-    if (!empty($contact['communication_style'])) {
-      $specialValues['communication_style_id_display'] = $contact['communication_style'];
-    }
-    return $specialValues;
-  }
-
   /**
    * Get the metadata for the merge fields.
    *

diff --git a/tests/phpunit/CRM/Dedupe/MergerTest.php b/tests/phpunit/CRM/Dedupe/MergerTest.php
@@ -485,6 +485,26 @@ public function testGetMatchesInGroup() {
     ), $pairs);
   }
 
+  /**
+   * Test the special info handling is unchanged after cleanup.
+   *
+   * Note the handling is silly - we are testing to lock in over short term changes not to imply any contract on the
+   * function.
+   */
+  public function testgetRowsElementsAndInfoSpecialInfo() {
+    $contact1 = $this->individualCreate(['preferred_communication_method' => [], 'communication_style_id' => 'Familiar', 'prefix_id' => 'Mrs.', 'suffix_id' => 'III']);
+    $contact2 = $this->individualCreate(['preferred_communication_method' => ['SMS', 'Fax'], 'communication_style_id' => 'Formal', 'gender_id' => 'Female']);
+    $rowsElementsAndInfo = CRM_Dedupe_Merger::getRowsElementsAndInfo($contact1, $contact2);
+    $rows = $rowsElementsAndInfo['rows'];
+    $this->assertEquals(['main' => 'Mrs.', 'other' => 'Mr.', 'title' => 'Individual Prefix'], $rows['move_prefix_id']);
+    $this->assertEquals(['main' => 'III', 'other' => 'II', 'title' => 'Individual Suffix'], $rows['move_suffix_id']);
+    $this->assertEquals(['main' => '', 'other' => 'Female', 'title' => 'Gender'], $rows['move_gender_id']);
+    $this->assertEquals(['main' => 'Familiar', 'other' => 'Formal', 'title' => 'Communication Style'], $rows['move_communication_style_id']);
+    $this->assertEquals(1, $rowsElementsAndInfo['migration_info']['move_communication_style_id']);
+    $this->assertEquals(['main' => '', 'other' => 'SMS, Fax', 'title' => 'Preferred Communication Method'], $rows['move_preferred_communication_method']);
+    $this->assertEquals('45', $rowsElementsAndInfo['migration_info']['move_preferred_communication_method']);
+  }
+
   /**
    * Test migration of Membership.
    */