Skip to content

Commit

Permalink
Deprecate crazy BAO handling of preferred_communication_method
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed May 30, 2022
1 parent e09e156 commit 02374ba
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 167 deletions.
9 changes: 6 additions & 3 deletions CRM/Contact/BAO/Contact.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down
33 changes: 1 addition & 32 deletions CRM/Contact/Import/Parser/Contact.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -723,13 +724,6 @@ private function formatCommonData($params, &$formatted, $contactFields) {
$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;
}
Expand Down Expand Up @@ -941,15 +935,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)) {
Expand Down Expand Up @@ -2182,22 +2167,6 @@ protected function formatContactParameters(&$values, &$params) {
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'])) {
Expand Down
23 changes: 7 additions & 16 deletions CRM/Import/Parser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -866,22 +866,6 @@ private function _civicrm_api3_deprecated_add_formatted_param(&$values, &$params
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;
Expand Down Expand Up @@ -1223,6 +1207,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';
}
Expand Down
141 changes: 33 additions & 108 deletions tests/phpunit/CRM/Contact/BAO/ContactTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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 = [
Expand Down Expand Up @@ -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;

Expand All @@ -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.');
Expand All @@ -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',
Expand All @@ -133,7 +122,6 @@ public function testAddWithAll() {
],
'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',
Expand All @@ -143,70 +131,35 @@ public function testAddWithAll() {
'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 = [
Expand Down Expand Up @@ -778,7 +731,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."
);
Expand Down Expand Up @@ -813,16 +766,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 = [
Expand Down Expand Up @@ -927,13 +874,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 = [
Expand Down Expand Up @@ -1000,7 +941,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."
);
Expand Down Expand Up @@ -1034,17 +975,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 = [
Expand Down Expand Up @@ -1315,7 +1247,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',
Expand All @@ -1327,13 +1258,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'] = [];
Expand Down
15 changes: 8 additions & 7 deletions tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1501,18 +1501,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.');
}

/**
Expand Down
Loading

0 comments on commit 02374ba

Please sign in to comment.