eprecate crazy BAO handling of preferred_communication_method

In the context of import this is tested in testImportFieldsWithVariousOptions - for the api it is tested via testPseudoFields - although in api context it is just a deprecation
civicrm · May 29, 2022 · baa1b04 · baa1b04
1 parent d2256e5
commit baa1b04
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 85 deletions.
diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
@@ -122,9 +122,12 @@ public static function add(&$params) {
     }
 
     if (isset($params['preferred_communication_method']) && is_array($params['preferred_communication_method'])) {
-      CRM_Utils_Array::formatArrayKeys($params['preferred_communication_method']);
-      $contact->preferred_communication_method = CRM_Utils_Array::implodePadded($params['preferred_communication_method']);
-      unset($params['preferred_communication_method']);
+      if (!empty($params['preferred_communication_method']) && empty($params['preferred_communication_method'][0])) {
+        CRM_Core_Error::deprecatedWarning(' Form layer formatting should never get to the BAO');
+        CRM_Utils_Array::formatArrayKeys($params['preferred_communication_method']);
+        $contact->preferred_communication_method = CRM_Utils_Array::implodePadded($params['preferred_communication_method']);
+        unset($params['preferred_communication_method']);
+      }
     }
 
     $defaults = ['source' => $params['contact_source'] ?? NULL];

diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php
@@ -101,6 +101,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
     'suffix_id',
     'communication_style',
     'preferred_language',
+    'preferred_communication_method',
     'phone',
     'im',
     'openid',
@@ -941,15 +942,6 @@ public function isErrorInCoreData($params, &$errorMessage) {
       if ($value) {
 
         switch ($key) {
-          case 'preferred_communication_method':
-            $preffComm = [];
-            $preffComm = explode(',', $value);
-            foreach ($preffComm as $v) {
-              if (!self::in_value(trim($v), CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'preferred_communication_method'))) {
-                $errors[] = ts('Preferred Communication Method');
-              }
-            }
-            break;
 
           case 'state_province':
             if (!empty($value)) {
@@ -2183,17 +2175,6 @@ protected function formatContactParameters(&$values, &$params) {
     }
 
     if (!empty($values['preferred_communication_method'])) {
-      $comm = [];
-      $pcm = array_change_key_case(array_flip(CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'preferred_communication_method')), CASE_LOWER);
-
-      $preffComm = explode(',', $values['preferred_communication_method']);
-      foreach ($preffComm as $v) {
-        $v = strtolower(trim($v));
-        if (array_key_exists($v, $pcm)) {
-          $comm[$pcm[$v]] = 1;
-        }
-      }
-
       $params['preferred_communication_method'] = $comm;
       return TRUE;
     }

diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php
@@ -1223,6 +1223,13 @@ protected function getTransformedFieldValue(string $fieldName, $importedValue) {
       return $importedValue;
     }
     $fieldMetadata = $this->getFieldMetadata($fieldName);
+    if (!empty($fieldMetadata['serialize']) && count(explode(',', $importedValue)) > 1) {
+      $values = [];
+      foreach (explode(',', $importedValue) as $value) {
+        $values[] = $this->getTransformedFieldValue($fieldName, $value);
+      }
+      return $values;
+    }
     if ($fieldName === 'url') {
       return CRM_Utils_Rule::url($importedValue) ? $importedValue : 'invalid_import_value';
     }

diff --git a/tests/phpunit/CRM/Contact/BAO/ContactTest.php b/tests/phpunit/CRM/Contact/BAO/ContactTest.php
@@ -11,7 +11,7 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
    *
    * test with empty params.
    */
-  public function testAddWithEmptyParams() {
+  public function testAddWithEmptyParams(): void {
     $params = [];
     $contact = CRM_Contact_BAO_Contact::add($params);
 
@@ -24,7 +24,7 @@ public function testAddWithEmptyParams() {
    *
    * Test with names (create and update modes)
    */
-  public function testAddWithNames() {
+  public function testAddWithNames(): void {
     $firstName = 'Shane';
     $lastName = 'Whatson';
     $params = [
@@ -66,12 +66,12 @@ public function testAddWithNames() {
    * Test with all contact params
    * (create and update modes)
    */
-  public function testAddWithAll() {
+  public function testAddWithAll(): void {
     // Take the common contact params.
     $params = $this->contactParams();
 
     unset($params['location']);
-    $prefComm = $params['preferred_communication_method'];
+
     $contact = CRM_Contact_BAO_Contact::add($params);
     $contactId = $contact->id;
 
@@ -88,9 +88,7 @@ public function testAddWithAll() {
     $this->assertEquals('1', $contact->is_opt_out, 'Check for is_opt_out creation.');
     $this->assertEquals($params['external_identifier'], $contact->external_identifier, 'Check for external_identifier creation.');
     $this->assertEquals($params['last_name'] . ', ' . $params['first_name'], $contact->sort_name, 'Check for sort_name creation.');
-    $this->assertEquals($params['preferred_mail_format'], $contact->preferred_mail_format,
-      'Check for preferred_mail_format creation.'
-    );
+
     $this->assertEquals($params['contact_source'], $contact->source, 'Check for contact_source creation.');
     $this->assertEquals($params['prefix_id'], $contact->prefix_id, 'Check for prefix_id creation.');
     $this->assertEquals($params['suffix_id'], $contact->suffix_id, 'Check for suffix_id creation.');
@@ -103,16 +101,7 @@ public function testAddWithAll() {
     $this->assertEquals(CRM_Utils_Date::processDate($params['deceased_date']),
       $contact->deceased_date, 'Check for deceased_date creation.'
     );
-    $dbPrefComm = explode(CRM_Core_DAO::VALUE_SEPARATOR,
-      $contact->preferred_communication_method
-    );
-    $checkPrefComm = [];
-    foreach ($dbPrefComm as $key => $value) {
-      if ($value) {
-        $checkPrefComm[$value] = 1;
-      }
-    }
-    $this->assertAttributesEquals($checkPrefComm, $prefComm);
+    $this->assertEquals('135', $contact->preferred_communication_method);
 
     $updateParams = [
       'contact_type' => 'Individual',
@@ -778,7 +767,7 @@ public function testCreateProfileContact() {
     //check the values in DB.
     foreach ($params as $key => $val) {
       if (!is_array($params[$key])) {
-        if ($key == 'contact_source') {
+        if ($key === 'contact_source') {
           $this->assertDBCompareValue('CRM_Contact_DAO_Contact', $contactId, 'source',
             'id', $params[$key], "Check for {$key} creation."
           );
@@ -813,16 +802,10 @@ public function testCreateProfileContact() {
       'id', $params['deceased_date'], 'Check for deceased_date creation.'
     );
 
-    $dbPrefComm = explode(CRM_Core_DAO::VALUE_SEPARATOR,
+    $dbPrefComm = array_values(array_filter(explode(CRM_Core_DAO::VALUE_SEPARATOR,
       CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $contactId, 'preferred_communication_method', 'id', TRUE)
-    );
-    $checkPrefComm = [];
-    foreach ($dbPrefComm as $key => $value) {
-      if ($value) {
-        $checkPrefComm[$value] = 1;
-      }
-    }
-    $this->assertAttributesEquals($checkPrefComm, $params['preferred_communication_method']);
+    )));
+    $this->assertEquals($dbPrefComm, $params['preferred_communication_method']);
 
     //Now check DB for Address
     $searchParams = [
@@ -927,13 +910,7 @@ public function testCreateProfileContact() {
         'do_not_phone' => 1,
         'do_not_email' => 1,
       ],
-      'preferred_communication_method' => [
-        '1' => 0,
-        '2' => 1,
-        '3' => 0,
-        '4' => 1,
-        '5' => 0,
-      ],
+      'preferred_communication_method' => [2, 4],
     ];
 
     $updatePfParams = [
@@ -1000,7 +977,7 @@ public function testCreateProfileContact() {
     //check the values in DB.
     foreach ($updateCParams as $key => $val) {
       if (!is_array($updateCParams[$key])) {
-        if ($key == 'contact_source') {
+        if ($key === 'contact_source') {
           $this->assertDBCompareValue('CRM_Contact_DAO_Contact', $contactId, 'source',
             'id', $updateCParams[$key], "Check for {$key} creation."
           );
@@ -1034,17 +1011,8 @@ public function testCreateProfileContact() {
     $this->assertDBCompareValue('CRM_Contact_DAO_Contact', $contactId, 'deceased_date', 'id',
       $updateCParams['deceased_date'], 'Check for deceased_date creation.'
     );
-
-    $dbPrefComm = explode(CRM_Core_DAO::VALUE_SEPARATOR,
-      CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $contactId, 'preferred_communication_method', 'id', TRUE)
-    );
-    $checkPrefComm = [];
-    foreach ($dbPrefComm as $key => $value) {
-      if ($value) {
-        $checkPrefComm[$value] = 1;
-      }
-    }
-    $this->assertAttributesEquals($checkPrefComm, $updateCParams['preferred_communication_method']);
+    $created = $this->callAPISuccessGetSingle('Contact', ['id' => $contactId]);
+    $this->assertEquals($created['preferred_communication_method'], $updateCParams['preferred_communication_method']);
 
     //Now check DB for Address
     $searchParams = [
@@ -1315,7 +1283,6 @@ private function contactParams() {
       ],
       'contact_source' => 'test contact',
       'external_identifier' => 123456789,
-      'preferred_mail_format' => 'Both',
       'is_opt_out' => 1,
       'legal_identifier' => '123456789',
       'image_URL' => 'http://image.com',
@@ -1327,13 +1294,7 @@ private function contactParams() {
         'do_not_mail' => 1,
         'do_not_trade' => 1,
       ],
-      'preferred_communication_method' => [
-        '1' => 1,
-        '2' => 0,
-        '3' => 1,
-        '4' => 0,
-        '5' => 1,
-      ],
+      'preferred_communication_method' => [1, 3, 5],
     ];
 
     $params['address'] = [];

diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -1494,18 +1494,19 @@ public function testImportFieldsWithVariousOptions(): void {
     $importer->import(CRM_Import_Parser::DUPLICATE_NOCHECK, $fields);
     $contact = $this->callAPISuccessGetSingle('Contact', ['last_name' => 'Texter']);
 
-    $this->assertEquals([4, 1], $contact['preferred_communication_method'], "Import multiple preferred communication methods using labels.");
-    $this->assertEquals(1, $contact['gender_id'], "Import gender with label.");
-    $this->assertEquals('da_DK', $contact['preferred_language'], "Import preferred language with label.");
+    $this->assertEquals([4, 1], $contact['preferred_communication_method'], 'Import multiple preferred communication methods using labels.');
+    $this->assertEquals(1, $contact['gender_id'], 'Import gender with label.');
+    $this->assertEquals('da_DK', $contact['preferred_language'], 'Import preferred language with label.');
+    $this->callAPISuccess('Contact', 'delete', ['id' => $contact['id']]);
 
     $importer = $processor->getImporterObject();
-    $fields = ['Ima', 'Texter', "4,1", "1", "da_DK"];
+    $fields = ['Ima', 'Texter', '4,1', '1', 'da_DK'];
     $importer->import(CRM_Import_Parser::DUPLICATE_NOCHECK, $fields);
     $contact = $this->callAPISuccessGetSingle('Contact', ['last_name' => 'Texter']);
 
-    $this->assertEquals([4, 1], $contact['preferred_communication_method'], "Import multiple preferred communication methods using values.");
-    $this->assertEquals(1, $contact['gender_id'], "Import gender with id.");
-    $this->assertEquals('da_DK', $contact['preferred_language'], "Import preferred language with value.");
+    $this->assertEquals([4, 1], $contact['preferred_communication_method'], 'Import multiple preferred communication methods using values.');
+    $this->assertEquals(1, $contact['gender_id'], 'Import gender with id.');
+    $this->assertEquals('da_DK', $contact['preferred_language'], 'Import preferred language with value.');
   }
 
   /**

diff --git a/tests/phpunit/CRM/Dedupe/MergerTest.php b/tests/phpunit/CRM/Dedupe/MergerTest.php
@@ -705,7 +705,7 @@ public function testGetMatchesInGroup() {
    * @throws \CRM_Core_Exception
    * @throws \CiviCRM_API3_Exception
    */
-  public function testGetRowsElementsAndInfoSpecialInfo() {
+  public function testGetRowsElementsAndInfoSpecialInfo(): void {
     $contact1 = $this->individualCreate([
       'preferred_communication_method' => [],
       'communication_style_id' => 'Familiar',