Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev/core#1093 add a bulkCreate action for many customFields in one go #14694

Merged
merged 2 commits into from
Jul 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 86 additions & 6 deletions CRM/Core/BAO/CustomField.php
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@ public static function dataToHtml() {
public static function create($params) {
$customField = self::createCustomFieldRecord($params);
$op = empty($params['id']) ? 'add' : 'modify';
$indexExist = empty($params['id']) ? FALSE : CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $params['id'], 'is_searchable');
self::createField($customField, $op, $indexExist, CRM_Utils_Array::value('triggerRebuild', $params, TRUE));
self::createField($customField, $op, CRM_Utils_Array::value('triggerRebuild', $params, TRUE));

CRM_Utils_Hook::post(($op === 'add' ? 'create' : 'edit'), 'CustomField', $customField->id, $customField);

Expand All @@ -164,6 +163,53 @@ public static function create($params) {
return $customField;
}

/**
* Create/ update several fields at once in a mysql efficient way.
*
* https://lab.civicrm.org/dev/core/issues/1093
*
* The intention is that apiv4 would expose any BAO with bulkSave as a new action.
*
* @param array $bulkParams
* Array of arrays as would be passed into create
* @param array $defaults
* Default parameters to be be merged into each of the params.
*/
public static function bulkSave($bulkParams, $defaults) {
$sql = [];
$tables = [];
$customFields = [];
foreach ($bulkParams as $fieldParams) {
$params = array_merge($defaults, $fieldParams);
$operation = empty($params['id']) ? 'add' : 'modify';
$customField = self::createCustomFieldRecord($params);
$fieldSQL = self::getAlterFieldSQL($customField, $operation);
if (!isset($params['custom_group_id'])) {
$params['custom_group_id'] = civicrm_api3('CustomField', 'getvalue', ['id' => $customField->id, 'return' => 'custom_group_id']);
}
if (!isset($params['table_name'])) {
if (!isset($tables[$params['custom_group_id']])) {
$tables[$params['custom_group_id']] = civicrm_api3('CustomGroup', 'getvalue', [
'id' => $params['custom_group_id'],
'return' => 'table_name',
]);
}
$params['table_name'] = $tables[$params['custom_group_id']];
}
$sql[$params['table_name']][] = $fieldSQL;
$customFields[] = $customField;
}
foreach ($sql as $tableName => $statements) {
// CRM-7007: do not i18n-rewrite this query
CRM_Core_DAO::executeQuery("ALTER TABLE $tableName " . implode(', ', $statements), [], TRUE, NULL, FALSE, FALSE);
Civi::service('sql_triggers')->rebuild($params['table_name'], TRUE);
}
CRM_Utils_System::flushCache();
foreach ($customFields as $customField) {
CRM_Utils_Hook::post($operation === 'add' ? 'create' : 'edit', 'CustomField', $customField->id, $customField);
}
}

/**
* Fetch object based on array of properties.
*
Expand Down Expand Up @@ -1646,13 +1692,47 @@ public static function defaultCustomTableSchema($params) {
*
* @param CRM_Core_DAO_CustomField $field
* @param string $operation
* @param bool $indexExist
* @param bool $triggerRebuild
*/
public static function createField($field, $operation, $indexExist = FALSE, $triggerRebuild = TRUE) {
$params = self::prepareCreateParams($field, $operation);
public static function createField($field, $operation, $triggerRebuild = TRUE) {
$sql = str_repeat(' ', 8);
Copy link
Member

@colemanw colemanw Jul 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

????????

<?php
  echo str_repeat('?', 8);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah weird but retained from 'create'

$tableName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomGroup', $field->custom_group_id, 'table_name');
$sql .= "ALTER TABLE " . $tableName;
$sql .= self::getAlterFieldSQL($field, $operation);

// CRM-7007: do not i18n-rewrite this query
CRM_Core_DAO::executeQuery($sql, [], TRUE, NULL, FALSE, FALSE);

$config = CRM_Core_Config::singleton();
if ($config->logging) {
// CRM-16717 not sure why this was originally limited to add.
// For example custom tables can have field length changes - which need to flow through to logging.
// Are there any modifies we DON'T was to call this function for (& shouldn't it be clever enough to cope?)
if ($operation === 'add' || $operation === 'modify') {
$logging = new CRM_Logging_Schema();
$logging->fixSchemaDifferencesFor($tableName, [trim(strtoupper($operation)) => [$field->column_name]]);
}
}

if ($triggerRebuild) {
Civi::service('sql_triggers')->rebuild($tableName, TRUE);
}

}

CRM_Core_BAO_SchemaHandler::alterFieldSQL($params, $indexExist, $triggerRebuild);
/**
* @param CRM_Core_DAO_CustomField $field
* @param string $operation
*
* @return bool
*/
public static function getAlterFieldSQL($field, $operation) {
$indexExist = $operation === 'add' ? FALSE : CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField', $field->id, 'is_searchable');
$params = self::prepareCreateParams($field, $operation);
// Let's suppress the required flag, since that can cause an sql issue... for unknown reasons since we are calling
// a function only used by Custom Field creation...
$params['required'] = FALSE;
return CRM_Core_BAO_SchemaHandler::getFieldAlterSQL($params, $indexExist);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion CRM/Core/BAO/SchemaHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ public static function buildForeignKeySQL($params, $separator, $prefix, $tableNa
* @return bool
*/
public static function alterFieldSQL($params, $indexExist = FALSE, $triggerRebuild = TRUE) {

CRM_Core_Error::deprecatedFunctionWarning('function no longer in use / supported');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should add a @deprecated annotation too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I'll add that to the last PR (rather than invalidate tests on this)

// lets suppress the required flag, since that can cause sql issue
$params['required'] = FALSE;

Expand Down
1 change: 1 addition & 0 deletions CRM/Utils/Migrate/Import.php
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ public function customFields(&$xml, &$idMap) {

// Only rebuild the table's trigger on the last field added to avoid un-necessary
// and slow rebuilds when adding many fields at the same time.
// @todo - call bulkSave instead.
$triggerRebuild = FALSE;
if ($count == $total) {
$triggerRebuild = TRUE;
Expand Down
33 changes: 33 additions & 0 deletions tests/phpunit/CRM/Core/BAO/CustomFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -636,4 +636,37 @@ public function testGetFieldsForImport() {
$this->assertEquals($expected, CRM_Core_BAO_CustomField::getFieldsForImport());
}

/**
* Test the bulk create function works.
*/
public function testBulkCreate() {
$customGroup = $this->customGroupCreate([
'extends' => 'Individual',
'title' => 'my bulk group',
]);
CRM_Core_BAO_CustomField::bulkSave([
[
'label' => 'Test',
'data_type' => 'String',
'html_type' => 'Text',
'column_name' => 'my_text',
],
[
'label' => 'test_link',
'data_type' => 'Link',
'html_type' => 'Link',
'is_search_range' => '0',
],
],
[
'custom_group_id' => $customGroup['id'],
'is_active' => 1,
'is_searchable' => 1,
]);
$dao = CRM_Core_DAO::executeQuery(('SHOW CREATE TABLE ' . $customGroup['values'][$customGroup['id']]['table_name']));
$dao->fetch();
$this->assertContains('`test_link_2` varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL', $dao->Create_Table);
$this->assertContains('KEY `INDEX_my_text` (`my_text`)', $dao->Create_Table);
}

}
4 changes: 2 additions & 2 deletions tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ public function testDropColumn() {
];

// drop col1
CRM_Core_BAO_SchemaHandler::alterFieldSQL($alterParams, FALSE, TRUE);
CRM_Core_DAO::executeQuery(CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($alterParams, FALSE));

$create_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE civicrm_test_drop_column");
while ($create_table->fetch()) {
Expand All @@ -328,7 +328,7 @@ public function testDropColumn() {

// drop col2
$alterParams['name'] = 'col2';
CRM_Core_BAO_SchemaHandler::alterFieldSQL($alterParams, FALSE, TRUE);
CRM_Core_DAO::executeQuery(CRM_Core_BAO_SchemaHandler::buildFieldChangeSql($alterParams, FALSE));

$create_table = CRM_Core_DAO::executeQuery("SHOW CREATE TABLE civicrm_test_drop_column");
while ($create_table->fetch()) {
Expand Down
5 changes: 4 additions & 1 deletion tests/phpunit/api/v3/CustomFieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,9 @@ public function testMakeSearchableContactReferenceFieldUnsearchable() {
$result = $this->callAPISuccess('CustomField', 'create', $params);
}

/**
* Test disabling a searchable contact reference field.
*/
public function testDisableSearchableContactReferenceField() {
$customGroup = $this->customGroupCreate([
'name' => 'testCustomGroup',
Expand All @@ -647,7 +650,7 @@ public function testDisableSearchableContactReferenceField() {
'id' => $result['id'],
'is_active' => 0,
];
$result = $this->callAPISuccess('CustomField', 'create', $params);
$this->callAPISuccess('CustomField', 'create', $params);
}

}