From 02374bae3aedc62946175baa4f655f8b312da62d Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Sun, 29 May 2022 23:18:28 +1200 Subject: [PATCH] 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 --- CRM/Contact/BAO/Contact.php | 9 +- CRM/Contact/Import/Parser/Contact.php | 33 +--- CRM/Import/Parser.php | 23 +-- tests/phpunit/CRM/Contact/BAO/ContactTest.php | 141 ++++-------------- .../CRM/Contact/Import/Parser/ContactTest.php | 15 +- tests/phpunit/CRM/Dedupe/MergerTest.php | 2 +- 6 files changed, 56 insertions(+), 167 deletions(-) diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php index cf3261c9ba..4dd1a86343 100644 --- 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 e81a34394e..638371f443 100644 --- 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 3c0a56d810..0549390315 100644 --- 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 6259bf4df7..0f2398a4e2 100644 --- 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('135', $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 619b06f97b..78ec496f9f 100644 --- 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 fae48c4596..157a014905 100644 --- 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', -- 2.25.1