Deprecate 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

diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
index cf3261c9bab6afa15a619d22dff368386c6358af..4dd1a8634379381b9db24644103a24568a6e6c9a 100644 (file)
--- a/CRM/Contact/BAO/Contact.php
+++ b/CRM/Contact/BAO/Contact.php
@@ -122,9 +122,12 @@ class CRM_Contact_BAO_Contact extends CRM_Contact_DAO_Contact implements Civi\Co
     }
 
     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
index e81a34394e78ab548ff8e8dd03ba254a7449ac58..638371f443873f56bbc7f2c992b76f17ddfc6f01 100644 (file)
--- 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',
@@ -723,13 +724,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
         $key => $field,
       ];
 
-      if (($key !== 'preferred_communication_method') && (array_key_exists($key, $contactFields))) {
-        // due to merging of individual table and
-        // contact table, we need to avoid
-        // preferred_communication_method forcefully
-        $formatValues['contact_type'] = $formatted['contact_type'];
-      }
-
       if ($key == 'id' && isset($field)) {
         $formatted[$key] = $field;
       }
@@ -941,15 +935,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
       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)) {
@@ -2182,22 +2167,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
       return TRUE;
     }
 
-    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;
-    }
-
     if (isset($values['note'])) {
       // add a note field
       if (!isset($params['note'])) {

diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php
index 3c0a56d8101958060cad1ad7bba1df8d53dcd15c..0549390315aa4bdb6b5c4bd1510d798d987b4d0d 100644 (file)
--- a/CRM/Import/Parser.php
+++ b/CRM/Import/Parser.php
@@ -866,22 +866,6 @@ abstract class CRM_Import_Parser {
       return TRUE;
     }
 
-    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;
-    }
-
     // format the website params.
     if (!empty($values['url'])) {
       static $websiteFields;
@@ -1223,6 +1207,13 @@ abstract class CRM_Import_Parser {
       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
index 6259bf4df7394d117a8f6a3012cfc85bf15ef7b2..0f2398a4e208f203e62af59842cd8b2cf6f92b3c 100644 (file)
--- 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 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
    *
    * Test with names (create and update modes)
    */
-  public function testAddWithNames() {
+  public function testAddWithNames(): void {
     $firstName = 'Shane';
     $lastName = 'Whatson';
     $params = [
@@ -66,12 +66,12 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
    * 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 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
     $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 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
     $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('\ 11\ 13\ 15\ 1', $contact->preferred_communication_method);
 
     $updateParams = [
       'contact_type' => 'Individual',
@@ -133,7 +122,6 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
       ],
       'contact_source' => 'test update contact',
       'external_identifier' => 111111111,
-      'preferred_mail_format' => 'Both',
       'is_opt_out' => 0,
       'deceased_date' => '1981-03-03',
       'birth_date' => '1951-04-04',
@@ -143,70 +131,35 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
         'do_not_mail' => 0,
         'do_not_trade' => 0,
       ],
-      'preferred_communication_method' => [
-        '1' => 0,
-        '2' => 1,
-        '3' => 0,
-        '4' => 1,
-        '5' => 0,
-      ],
+      'preferred_communication_method' => [2, 4],
     ];
 
-    $prefComm = $updateParams['preferred_communication_method'];
     $updateParams['contact_id'] = $contactId;
-    $contact = CRM_Contact_BAO_Contact::create($updateParams);
-    $contactId = $contact->id;
-
-    $this->assertInstanceOf('CRM_Contact_DAO_Contact', $contact, 'Check for created object');
-
-    $this->assertEquals($updateParams['first_name'], $contact->first_name, 'Check for first name creation.');
-    $this->assertEquals($updateParams['last_name'], $contact->last_name, 'Check for last name creation.');
-    $this->assertEquals($updateParams['middle_name'], $contact->middle_name, 'Check for middle name creation.');
-    $this->assertEquals($updateParams['contact_type'], $contact->contact_type, 'Check for contact type creation.');
-    $this->assertEquals('0', $contact->do_not_email, 'Check for do_not_email creation.');
-    $this->assertEquals('0', $contact->do_not_phone, 'Check for do_not_phone creation.');
-    $this->assertEquals('0', $contact->do_not_mail, 'Check for do_not_mail creation.');
-    $this->assertEquals('0', $contact->do_not_trade, 'Check for do_not_trade creation.');
-    $this->assertEquals('0', $contact->is_opt_out, 'Check for is_opt_out creation.');
-    $this->assertEquals($updateParams['external_identifier'], $contact->external_identifier,
-      'Check for external_identifier creation.'
-    );
-    $this->assertEquals($updateParams['last_name'] . ', ' . $updateParams['first_name'],
-      $contact->sort_name, 'Check for sort_name creation.'
-    );
-    $this->assertEquals($updateParams['preferred_mail_format'], $contact->preferred_mail_format,
-      'Check for preferred_mail_format creation.'
-    );
-    $this->assertEquals($updateParams['contact_source'], $contact->source, 'Check for contact_source creation.');
-    $this->assertEquals($updateParams['prefix_id'], $contact->prefix_id, 'Check for prefix_id creation.');
-    $this->assertEquals($updateParams['suffix_id'], $contact->suffix_id, 'Check for suffix_id creation.');
-    $this->assertEquals($updateParams['job_title'], $contact->job_title, 'Check for job_title creation.');
-    $this->assertEquals($updateParams['gender_id'], $contact->gender_id, 'Check for gender_id creation.');
-    $this->assertEquals('1', $contact->is_deceased, 'Check for is_deceased creation.');
-    $this->assertEquals(CRM_Utils_Date::processDate($updateParams['birth_date']),
-      date('YmdHis', strtotime($contact->birth_date)), 'Check for birth_date creation.'
-    );
-    $this->assertEquals(CRM_Utils_Date::processDate($updateParams['deceased_date']),
-      date('YmdHis', strtotime($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;
+    // Annoyingly `create` alters params
+    $preUpdateParams = $updateParams;
+    CRM_Contact_BAO_Contact::create($updateParams);
+    $return = array_merge(array_keys($updateParams), ['do_not_phone', 'do_not_email', 'do_not_trade', 'do_not_mail']);
+    $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contactId, 'return' => $return]);
+    foreach ($preUpdateParams as $key => $value) {
+      if ($key === 'website') {
+        continue;
+      }
+      if ($key === 'privacy') {
+        foreach ($value as $privacyKey => $privacyValue) {
+          $this->assertEquals($privacyValue, $contact[$privacyKey], $key);
+        }
+      }
+      else {
+        $this->assertEquals($value, $contact[$key], $key);
       }
     }
-    $this->assertAttributesEquals($checkPrefComm, $prefComm);
-
     $this->contactDelete($contactId);
   }
 
   /**
    * Test case for add( ) with All contact types.
    */
-  public function testAddWithAllContactTypes() {
+  public function testAddWithAllContactTypes(): void {
     $firstName = 'Bill';
     $lastName = 'Adams';
     $params = [
@@ -778,7 +731,7 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
     //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 +766,10 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
       '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 +874,7 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
         '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 +941,7 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
     //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 +975,8 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
     $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 +1247,6 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
       ],
       '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 +1258,7 @@ class CRM_Contact_BAO_ContactTest extends CiviUnitTestCase {
         '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
index 619b06f97b4ac03b01cc88f764d81ce64159d01d..78ec496f9f09b969ea96e8b87d42ac9ef5c23409 100644 (file)
--- a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
+++ b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -1501,18 +1501,19 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
     $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
index fae48c45960c5047f88e606e6e2732761c8d09c5..157a014905875a23301dad355b4fe518321ea78b 100644 (file)
--- a/tests/phpunit/CRM/Dedupe/MergerTest.php
+++ b/tests/phpunit/CRM/Dedupe/MergerTest.php
@@ -705,7 +705,7 @@ class CRM_Dedupe_MergerTest extends CiviUnitTestCase {
    * @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',