Skip to content

Commit

Permalink
dev/core#723 Fix fatal error when trying to merge contacts with a cus…
Browse files Browse the repository at this point in the history 
…tom file field

Technical comments here https://lab.civicrm.org/dev/core/issues/723#note_16231
  • Loading branch information
@eileenmcnaughton
eileenmcnaughton committed May 26, 2019
1 parent d4d1ab5 commit e947f02
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 77 deletions.
63 changes: 63 additions & 0 deletions CRM/Core/BAO/CustomField.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2010,6 +2010,68 @@ public static function moveField($fieldID, $newGroupID) {
CRM_Utils_System::flushCache();
}

/**
* Move custom data from one contact to another.
*
* This is currently the start of a refactoring. The theory is each
* entity could have a 'move' function with a DAO default one to fall back on.
*
* At the moment this only does a small part of the process - ie deleting a file field that
* is about to be overwritten. However, the goal is the whole process around the move for
* custom data should be in here.
*
* This is currently called by the merge class but it makes sense that api could
* expose move actions as moving (e.g) contributions feels like a common
* ask that should be handled by the form layer.
*
* @param int $oldContactID
* @param int $newContactID
* @param int[] $fieldIDs
* Optional list field ids to move.
*
* @throws \CiviCRM_API3_Exception
* @throws \Exception
*/
public function move($oldContactID, $newContactID, $fieldIDs) {
if (empty($fieldIDs)) {
return;
}
$fields = civicrm_api3('CustomField', 'get', ['id' => ['IN' => $fieldIDs], 'return' => ['custom_group_id.is_multiple', 'custom_group_id.table_name', 'column_name', 'data_type'], 'options' => ['limit' => 0]])['values'];
$return = [];
foreach ($fieldIDs as $fieldID) {
$return[] = 'custom_' . $fieldID;
}
$oldContact = civicrm_api3('Contact', 'getsingle', ['id' => $oldContactID, 'return' => $return]);
$newContact = civicrm_api3('Contact', 'getsingle', ['id' => $newContactID, 'return' => $return]);

// The moveaAllBelongings function has functionality to move custom fields. It doesn't work very well...
// @todo handle all fields here but more immediately Country since that is broken at the moment.
$fieldTypesNotHandledInMergeAttempt = ['File'];
foreach ($fields as $field) {
$isMultiple = !empty($field['custom_group_id.is_multiple']);
if ($field['data_type'] === 'File' && !$isMultiple) {
if (!empty($oldContact['custom_' . $field['id']]) && !empty($newContact['custom_' . $field['id']])) {
CRM_Core_BAO_File::deleteFileReferences($oldContact['custom_' . $field['id']], $oldContactID, $field['id']);
}
if (!empty($oldContact['custom_' . $field['id']])) {
CRM_Core_DAO::executeQuery("
UPDATE civicrm_entity_file
SET entity_id = $newContactID
WHERE file_id = {$oldContact['custom_' . $field['id']]}"
);
}
}
if (in_array($field['data_type'], $fieldTypesNotHandledInMergeAttempt) && !$isMultiple) {
CRM_Core_DAO::executeQuery(
"INSERT INTO {$field['custom_group_id.table_name']} (entity_id, {$field['column_name']})
VALUES ($newContactID, {$oldContact['custom_' . $field['id']]})
ON DUPLICATE KEY UPDATE
{$field['column_name']} = {$oldContact['custom_' . $field['id']]}
");
}
}
}

/**
* Get the database table name and column name for a custom field.
*
Expand All @@ -2020,6 +2082,7 @@ public static function moveField($fieldID, $newGroupID) {
*
* @return array
* fatal is fieldID does not exists, else array of tableName, columnName
* @throws \Exception
*/
public static function getTableColumnGroup($fieldID, $force = FALSE) {
$cacheKey = "CRM_Core_DAO_CustomField_CustomGroup_TableColumn_{$fieldID}";
Expand Down
87 changes: 19 additions & 68 deletions CRM/Dedupe/Merger.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1512,12 +1512,13 @@ public static function getRowsElementsAndInfo($mainId, $otherId, $checkPermissio
* @param int $otherId
* Duplicate contact which would be deleted after merge operation.
*
* @param $migrationInfo
* @param array $migrationInfo
*
* @param bool $checkPermissions
* Respect logged in user permissions.
*
* @return bool
* @throws \CiviCRM_API3_Exception
*/
public static function moveAllBelongings($mainId, $otherId, $migrationInfo, $checkPermissions = TRUE) {
if (empty($migrationInfo)) {
Expand Down Expand Up @@ -1558,6 +1559,10 @@ public static function moveAllBelongings($mainId, $otherId, $migrationInfo, $che

// **** Do contact related migrations
$customTablesToCopyValues = self::getAffectedCustomTables($submittedCustomFields);
// @todo - move all custom field processing to the move class & eventually have an
// overridable DAO class for it.
$customFieldBAO = new CRM_Core_BAO_CustomField();
$customFieldBAO->move($otherId, $mainId, $submittedCustomFields);
CRM_Dedupe_Merger::moveContactBelongings($mainId, $otherId, $moveTables, $tableOperations, $customTablesToCopyValues);
unset($moveTables, $tableOperations);

Expand Down Expand Up @@ -1601,13 +1606,10 @@ public static function moveAllBelongings($mainId, $otherId, $migrationInfo, $che
if (!isset($submitted)) {
$submitted = [];
}
$customFiles = [];
foreach ($submitted as $key => $value) {
list($cFields, $customFiles, $submitted) = self::processCustomFields($mainId, $key, $cFields, $customFiles, $submitted, $value);
list($cFields, $submitted) = self::processCustomFields($mainId, $key, $cFields, $submitted, $value);
}

self::processCustomFieldFiles($mainId, $otherId, $customFiles);

// move view only custom fields CRM-5362
$viewOnlyCustomFields = [];
foreach ($submitted as $key => $value) {
Expand Down Expand Up @@ -2146,29 +2148,36 @@ protected static function swapOutFieldsAffectedByQFZeroBug(&$migrationInfo) {
/**
* Honestly - what DOES this do - hopefully some refactoring will reveal it's purpose.
*
* Update this is formatting fields to be processed through 'ProfileContactCreate action
* - for some fields it fails - e.g Country - per testMergeCustomFields.
*
* Goal is to move all custom field handling into 'move' functions on the various BAO
* with an underlying DAO function. For custom fields it has been started on the BAO.
*
* @param $mainId
* @param $key
* @param $cFields
* @param $customFiles
* @param $submitted
* @param $value
*
* @return array
* @throws \Exception
*/
protected static function processCustomFields($mainId, $key, $cFields, $customFiles, $submitted, $value) {
protected static function processCustomFields($mainId, $key, $cFields, $submitted, $value) {
if (substr($key, 0, 7) == 'custom_') {
$fid = (int) substr($key, 7);
if (empty($cFields[$fid])) {
return [$cFields, $customFiles, $submitted];
return [$cFields, $submitted];
}
$htmlType = $cFields[$fid]['attributes']['html_type'];
switch ($htmlType) {
case 'File':
$customFiles[] = $fid;
// Handled in CustomField->move(). Tested in testMergeCustomFields.
unset($submitted["custom_$fid"]);
break;

case 'Select Country':
// @todo Test in testMergeCustomFields disabled as this does not work, Handle in CustomField->move().
case 'Select State/Province':
$submitted[$key] = CRM_Core_BAO_CustomField::displayValue($value, $fid);
break;
Expand Down Expand Up @@ -2247,7 +2256,7 @@ protected static function processCustomFields($mainId, $key, $cFields, $customFi
break;
}
}
return [$cFields, $customFiles, $submitted];
return [$cFields, $submitted];
}

/**
Expand Down Expand Up @@ -2390,64 +2399,6 @@ public static function getConflicts(&$migrationInfo, $mainId, $otherId, $mode) {
return $conflicts;
}

/**
* Do file custom fields related migrations.
* FIXME: move this someplace else (one of the BAOs) after discussing
* where to, and whether CRM_Core_BAO_File::deleteFileReferences() shouldn't actually,
* like, delete a file...
*
* Note outstanding bug https://lab.civicrm.org/dev/core/issues/723
* relates to this code....
*
* @param $mainId
* @param $otherId
* @param $customFiles
*/
protected static function processCustomFieldFiles($mainId, $otherId, $customFiles) {
foreach ($customFiles as $customId) {
list($tableName, $columnName, $groupID) = CRM_Core_BAO_CustomField::getTableColumnGroup($customId);

// get the contact_id -> file_id mapping
$fileIds = [];
$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);
}
}

// move the other contact's file to main contact
//NYSS need to INSERT or UPDATE depending on whether main contact has an existing record
if (CRM_Core_DAO::singleValueQuery("SELECT id FROM {$tableName} WHERE entity_id = {$mainId}")) {
$sql = "UPDATE {$tableName} SET {$columnName} = {$fileIds[$otherId]} WHERE entity_id = {$mainId}";
}
else {
$sql = "INSERT INTO {$tableName} ( entity_id, {$columnName} ) VALUES ( {$mainId}, {$fileIds[$otherId]} )";
}
CRM_Core_DAO::executeQuery($sql);

if (CRM_Core_DAO::singleValueQuery("
SELECT id
FROM civicrm_entity_file
WHERE entity_table = '{$tableName}' AND file_id = {$fileIds[$otherId]}")
) {
$sql = "
UPDATE civicrm_entity_file
SET entity_id = {$mainId}
WHERE entity_table = '{$tableName}' AND file_id = {$fileIds[$otherId]}";
}
else {
$sql = "
INSERT INTO civicrm_entity_file ( entity_table, entity_id, file_id )
VALUES ( '{$tableName}', {$mainId}, {$fileIds[$otherId]} )";
}
CRM_Core_DAO::executeQuery($sql);
}
}

/**
* @param $rule_group_id
* @param $group_id
Expand Down
18 changes: 9 additions & 9 deletions tests/phpunit/api/v3/ContactTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3386,16 +3386,15 @@ public function testMerge() {
*/
public function testMergeCustomFields() {
$contact1 = $this->individualCreate();
/* Not sure this is quite right but it does get it into the file table
// 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,
'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');
Expand All @@ -3407,7 +3406,7 @@ public function testMergeCustomFields() {
$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'],
$fileField => $file['id'],
$linkField => 'http://example.org',
$dateField => '2018-01-01 17:10:56',
$selectField => 'G',
Expand All @@ -3425,6 +3424,7 @@ public function testMergeCustomFields() {
'auto_flip' => FALSE,
]);
$contact = $this->callAPISuccessGetSingle('Contact', ['id' => $contact2, 'return' => array_keys($customFieldValues)]);
$this->assertEquals($contact2, CRM_Core_DAO::singleValueQuery('SELECT entity_id FROM civicrm_entity_file WHERE file_id = ' . $file['id']));
foreach ($customFieldValues as $key => $value) {
$this->assertEquals($value, $contact[$key]);
}
Expand Down

0 comments on commit e947f02

Please sign in to comment.