From 119664d66964c126f7c2d6ca133091992cd71b09 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 24 May 2019 00:11:38 +1200 Subject: [PATCH] Add unit test for merge handling on custom data. This also moves the start & stop of the transaction to being further out so a hard fail later doesn't leave a half-merged state --- CRM/Dedupe/Merger.php | 8 +- .../CRMTraits/Custom/CustomDataTrait.php | 56 +++++++++++-- tests/phpunit/api/v3/ContactTest.php | 61 ++++++++++++++ tests/phpunit/api/v3/RelationshipTest.php | 80 +++++++++---------- 4 files changed, 151 insertions(+), 54 deletions(-) diff --git a/CRM/Dedupe/Merger.php b/CRM/Dedupe/Merger.php index 684393da6c..0c8bec8f59 100644 --- a/CRM/Dedupe/Merger.php +++ b/CRM/Dedupe/Merger.php @@ -579,13 +579,10 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m // Allow hook_civicrm_merge() to add SQL statements for the merge operation. CRM_Utils_Hook::merge('sqls', $sqls, $mainId, $otherId, $tables); - // call the SQL queries in one transaction - $transaction = new CRM_Core_Transaction(); foreach ($sqls as $sql) { CRM_Core_DAO::executeQuery($sql, [], TRUE, NULL, TRUE); } CRM_Dedupe_Merger::addMembershipToRealtedContacts($mainId); - $transaction->commit(); } /** @@ -1526,6 +1523,8 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m if (empty($migrationInfo)) { return FALSE; } + // Encapsulate in a transaction to avoid half-merges. + $transaction = new CRM_Core_Transaction(); $contactType = $migrationInfo['main_details']['contact_type']; $relTables = CRM_Dedupe_Merger::relTables(); @@ -1670,7 +1669,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m CRM_Contact_BAO_Contact::createProfileContact($submitted, CRM_Core_DAO::$_nullArray, $mainId); } - + $transaction->commit(); CRM_Utils_Hook::post('merge', 'Contact', $mainId); self::createMergeActivities($mainId, $otherId); @@ -2416,6 +2415,7 @@ INNER JOIN civicrm_membership membership2 ON membership1.membership_type_id = m $sql = "SELECT entity_id, {$columnName} AS file_id FROM {$tableName} WHERE entity_id IN ({$mainId}, {$otherId})"; $dao = CRM_Core_DAO::executeQuery($sql); while ($dao->fetch()) { + // @todo - this is actually broken - fix & or remove - see testMergeCustomFields $fileIds[$dao->entity_id] = $dao->file_id; if ($dao->entity_id == $mainId) { CRM_Core_BAO_File::deleteFileReferences($fileIds[$mainId], $mainId, $customId); diff --git a/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php b/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php index 8ff45ac853..3b5d9a197f 100644 --- a/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php +++ b/tests/phpunit/CRMTraits/Custom/CustomDataTrait.php @@ -32,6 +32,16 @@ */ trait CRMTraits_Custom_CustomDataTrait { + /** + * Create a custom group with fields of multiple types. + * + * @param array $groupParams + */ + public function createCustomGroupWithFieldsOfAllTypes($groupParams = []) { + $this->createCustomGroup($groupParams); + $this->ids['CustomField'] = $this->createCustomFieldsOfAllTypes(); + } + /** * Create a custom group. * @@ -68,7 +78,7 @@ trait CRMTraits_Custom_CustomDataTrait { ]; $customField = $this->callAPISuccess('CustomField', 'create', $params); - $ids[] = $customField['id']; + $ids['text'] = $customField['id']; $optionValue[] = [ 'label' => 'Red', @@ -102,7 +112,7 @@ trait CRMTraits_Custom_CustomDataTrait { ]; $customField = $this->callAPISuccess('custom_field', 'create', $params); - $ids[] = $customField['id']; + $ids['select_string'] = $customField['id']; $params = [ 'custom_group_id' => $customGroupID, @@ -112,14 +122,12 @@ trait CRMTraits_Custom_CustomDataTrait { 'data_type' => 'Date', 'default_value' => '20090711', 'weight' => 3, - 'is_required' => 1, - 'is_searchable' => 0, - 'is_active' => 1, + 'time_format' => 1, ]; $customField = $this->callAPISuccess('custom_field', 'create', $params); - $ids[] = $customField['id']; + $ids['select_date'] = $customField['id']; $params = [ 'custom_group_id' => $customGroupID, 'name' => 'test_link', @@ -134,8 +142,42 @@ trait CRMTraits_Custom_CustomDataTrait { ]; $customField = $this->callAPISuccess('custom_field', 'create', $params); - $ids[] = $customField['id']; + + $ids['link'] = $customField['id']; + $fileField = $this->customFieldCreate([ + 'custom_group_id' => $customGroupID, + 'data_type' => 'File', + 'html_type' => 'File', + 'default_value' => '', + ]); + + $ids['file'] = $fileField['id']; + $ids['country'] = $this->customFieldCreate([ + 'custom_group_id' => $customGroupID, + 'data_type' => 'Int', + 'html_type' => 'Select Country', + 'default_value' => '', + 'label' => 'Country', + 'option_type' => 0, + ])['id']; + return $ids; } + /** + * Get the custom field name for the relevant key. + * + * e.g returns 'custom_5' where 5 is the id of the field using the key. + * + * Generally keys map to data types. + * + * @param string $key + * + * @return string + */ + protected function getCustomFieldName($key) { + $linkField = 'custom_' . $this->ids['CustomField'][$key]; + return $linkField; + } + } diff --git a/tests/phpunit/api/v3/ContactTest.php b/tests/phpunit/api/v3/ContactTest.php index d0d5ed7466..4cab52bddc 100644 --- a/tests/phpunit/api/v3/ContactTest.php +++ b/tests/phpunit/api/v3/ContactTest.php @@ -37,6 +37,8 @@ * @group headless */ class api_v3_ContactTest extends CiviUnitTestCase { + use CRMTraits_Custom_CustomDataTrait; + public $DBResetRequired = FALSE; protected $_apiversion; protected $_entity; @@ -45,6 +47,14 @@ class api_v3_ContactTest extends CiviUnitTestCase { protected $_contactID; protected $_financialTypeId = 1; + + /** + * Entity to be extended. + * + * @var string + */ + protected $entity = 'Contact'; + /** * Test setup for every test. * @@ -3369,6 +3379,57 @@ class api_v3_ContactTest extends CiviUnitTestCase { } + /** + * Test merging 2 contacts with custom fields. + * + * @throws \Exception + */ + public function testMergeCustomFields() { + $contact1 = $this->individualCreate(); + /* Not sure this is quite right but it does get it into the file table + $file = $this->callAPISuccess('Attachment', 'create', [ + 'name' => 'header.txt', + 'mime_type' => 'text/plain', + 'description' => 'My test description', + 'content' => 'My test content', + 'entity_table' => 'civicrm_contact', + 'entity_id' => $contact1, + ]); + */ + + $this->createCustomGroupWithFieldsOfAllTypes(); + $fileField = $this->getCustomFieldName('file'); + $linkField = $this->getCustomFieldName('link'); + $dateField = $this->getCustomFieldName('select_date'); + $selectField = $this->getCustomFieldName('select_string'); + $countryField = $this->getCustomFieldName('country'); + + $countriesByName = array_flip(CRM_Core_PseudoConstant::country(FALSE, FALSE)); + $customFieldValues = [ + // @todo fix the fatal bug on this & uncomment - see dev/core#723 + // $fileField => $file['id'], + $linkField => 'http://example.org', + $dateField => '2018-01-01 17:10:56', + $selectField => 'G', + // Currently broken. + //$countryField => $countriesByName['New Zealand'], + ]; + $this->callAPISuccess('Contact', 'create', array_merge([ + 'id' => $contact1, + ], $customFieldValues)); + + $contact2 = $this->individualCreate(); + $this->callAPISuccess('contact', 'merge', [ + 'to_keep_id' => $contact2, + 'to_remove_id' => $contact1, + 'auto_flip' => FALSE, + ]); + $contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact2, 'return' => array_keys($customFieldValues)]); + foreach ($customFieldValues as $key => $value) { + $this->assertEquals($value, $contact[$key]); + } + } + /** * Test retrieving merged contacts. * diff --git a/tests/phpunit/api/v3/RelationshipTest.php b/tests/phpunit/api/v3/RelationshipTest.php index 1fdc04b337..6d42e0755b 100644 --- a/tests/phpunit/api/v3/RelationshipTest.php +++ b/tests/phpunit/api/v3/RelationshipTest.php @@ -398,35 +398,32 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { * Check relationship creation with custom data. */ public function testRelationshipCreateEditWithCustomData() { - $this->createCustomGroup(); - $this->_ids = $this->createCustomFieldsOfAllTypes(); + $this->createCustomGroupWithFieldsOfAllTypes(); //few custom Values for comparing - $custom_params = array( - "custom_{$this->_ids[0]}" => 'Hello! this is custom data for relationship', - "custom_{$this->_ids[1]}" => 'Y', - "custom_{$this->_ids[2]}" => '2009-07-11 00:00:00', - "custom_{$this->_ids[3]}" => 'http://example.com', - ); - - $params = array( + $custom_params = [ + $this->getCustomFieldName('text') => 'Hello! this is custom data for relationship', + $this->getCustomFieldName('select_string') => 'Y', + $this->getCustomFieldName('select_date') => '2009-07-11 00:00:00', + $this->getCustomFieldName('link') => 'http://example.com', + ]; + + $params = [ 'contact_id_a' => $this->_cId_a, 'contact_id_b' => $this->_cId_b, 'relationship_type_id' => $this->_relTypeID, 'start_date' => '2008-12-20', 'is_active' => 1, - ); + ]; $params = array_merge($params, $custom_params); $result = $this->callAPISuccess('relationship', 'create', $params); - $relationParams = array( - 'id' => $result['id'], - ); + $relationParams = ['id' => $result['id']]; $this->assertDBState('CRM_Contact_DAO_Relationship', $result['id'], $relationParams); //Test Edit of custom field from the form. $getParams = array('id' => $result['id']); $updateParams = array_merge($getParams, array( - "custom_{$this->_ids[0]}" => 'Edited Text Value', + $this->getCustomFieldName('text') => 'Edited Text Value', 'relationship_type_id' => $this->_relTypeID . '_b_a', 'related_contact_id' => $this->_cId_a, )); @@ -436,7 +433,7 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { $reln->submit($updateParams); $check = $this->callAPISuccess('relationship', 'get', $getParams); - $this->assertEquals("Edited Text Value", $check['values'][$check['id']]["custom_{$this->_ids[0]}"]); + $this->assertEquals("Edited Text Value", $check['values'][$check['id']][$this->getCustomFieldName('text')]); $params['id'] = $result['id']; $this->callAPISuccess('relationship', 'delete', $params); @@ -571,21 +568,20 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { * should be OK if the custom field values differ. */ public function testRelationshipCreateDuplicateWithCustomFields() { - $this->createCustomGroup(); - $this->_ids = $this->createCustomFieldsOfAllTypes(); + $this->createCustomGroupWithFieldsOfAllTypes(); $custom_params_1 = array( - "custom_{$this->_ids[0]}" => 'Hello! this is custom data for relationship', - "custom_{$this->_ids[1]}" => 'Y', - "custom_{$this->_ids[2]}" => '2009-07-11 00:00:00', - "custom_{$this->_ids[3]}" => 'http://example.com', + $this->getCustomFieldName('text') => 'Hello! this is custom data for relationship', + $this->getCustomFieldName('select_string') => 'Y', + $this->getCustomFieldName('select_date') => '2009-07-11 00:00:00', + $this->getCustomFieldName('link') => 'http://example.com', ); $custom_params_2 = array( - "custom_{$this->_ids[0]}" => 'Hello! this is other custom data', - "custom_{$this->_ids[1]}" => 'Y', - "custom_{$this->_ids[2]}" => '2009-07-11 00:00:00', - "custom_{$this->_ids[3]}" => 'http://example.org', + $this->getCustomFieldName('text') => 'Hello! this is other custom data', + $this->getCustomFieldName('select_string') => 'Y', + $this->getCustomFieldName('select_date') => '2009-07-11 00:00:00', + $this->getCustomFieldName('link') => 'http://example.org', ); $params = array( @@ -615,23 +611,22 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { * does. */ public function testRelationshipCreateDuplicateWithCustomFields2() { - $this->createCustomGroup(); - $this->_ids = $this->createCustomFieldsOfAllTypes(); + $this->createCustomGroupWithFieldsOfAllTypes(); $custom_params_2 = array( - "custom_{$this->_ids[0]}" => 'Hello! this is other custom data', - "custom_{$this->_ids[1]}" => 'Y', - "custom_{$this->_ids[2]}" => '2009-07-11 00:00:00', - "custom_{$this->_ids[3]}" => 'http://example.org', + $this->getCustomFieldName('text') => 'Hello! this is other custom data', + $this->getCustomFieldName('select_string') => 'Y', + $this->getCustomFieldName('select_date') => '2009-07-11 00:00:00', + $this->getCustomFieldName('link') => 'http://example.org', ); - $params_1 = array( + $params_1 = [ 'contact_id_a' => $this->_cId_a, 'contact_id_b' => $this->_cId_b, 'relationship_type_id' => $this->_relTypeID, 'start_date' => '2008-12-20', 'is_active' => 1, - ); + ]; $params_2 = array_merge($params_1, $custom_params_2); @@ -651,15 +646,14 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { * does not. */ public function testRelationshipCreateDuplicateWithCustomFields3() { - $this->createCustomGroup(); - $this->_ids = $this->createCustomFieldsOfAllTypes(); - - $custom_params_1 = array( - "custom_{$this->_ids[0]}" => 'Hello! this is other custom data', - "custom_{$this->_ids[1]}" => 'Y', - "custom_{$this->_ids[2]}" => '2009-07-11 00:00:00', - "custom_{$this->_ids[3]}" => 'http://example.org', - ); + $this->createCustomGroupWithFieldsOfAllTypes(); + + $custom_params_1 = [ + $this->getCustomFieldName('text') => 'Hello! this is other custom data', + $this->getCustomFieldName('select_string') => 'Y', + $this->getCustomFieldName('select_date') => '2009-07-11 00:00:00', + $this->getCustomFieldName('link') => 'http://example.org', + ]; $params_2 = array( 'contact_id_a' => $this->_cId_a, -- 2.25.1