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.

Sign up for GitHub

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#183 Alter Import process to use standardised CRM_Utils_SQL_T… #15797

Closed
wants to merge 1 commit into from
Closed

dev/core#183 Alter Import process to use standardised CRM_Utils_SQL_T… #15797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 5 additions & 3 deletions CRM/Contact/Import/Form/DataSource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,12 @@ public function preProcess() {

//Test database user privilege to create table(Temporary) CRM-4725
$errorScope = CRM_Core_TemporaryErrorScope::ignoreException();
$tempTable1 = CRM_Utils_SQL_TempTable::build()->getName();
$tempTable2 = CRM_Utils_SQL_TempTable::build()->getName();
$daoTestPrivilege = new CRM_Core_DAO();
$daoTestPrivilege->query("CREATE TEMPORARY TABLE import_job_permission_one(test int) ENGINE=InnoDB");
$daoTestPrivilege->query("CREATE TEMPORARY TABLE import_job_permission_two(test int) ENGINE=InnoDB");
$daoTestPrivilege->query("DROP TEMPORARY TABLE IF EXISTS import_job_permission_one, import_job_permission_two");
$daoTestPrivilege->query("CREATE TEMPORARY TABLE {$tempTable1} (test int) ENGINE=InnoDB");
$daoTestPrivilege->query("CREATE TEMPORARY TABLE {$tempTable2} (test int) ENGINE=InnoDB");
$daoTestPrivilege->query("DROP TEMPORARY TABLE IF EXISTS {$tempTable1}, {$tempTable2}");
unset($errorScope);

if ($daoTestPrivilege->_lastError) {
Expand Down
2 changes: 1 addition & 1 deletion CRM/Contact/Import/Form/Summary.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public function postProcess() {

$importTableName = $this->get('importTableName');
// do a basic sanity check here
if (strpos($importTableName, 'civicrm_import_job_') === 0) {
if (strpos($importTableName, 'civicrm_tmp') === 0 || strpos($importTableName, 'civicrm_import_job') === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do any of this? Why not just DROP TABLE IF EXISTS - also is this a temporary table? If so it should be DROP TEMPORARY TABLE IF EXISTS (respects transactions )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doh' good point these are actually not temporary tables doh seamus needs more coffee

$query = "DROP TABLE IF EXISTS $importTableName";
$db->query($query);
}
Expand Down
4 changes: 2 additions & 2 deletions CRM/Contact/Import/ImportJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function __construct($tableName = NULL, $createSql = NULL, $createTable =

// FIXME: we should regen this table's name if it exists rather than drop it
if (!$tableName) {
$tableName = 'civicrm_import_job_' . md5(uniqid(rand(), TRUE));
$tableName = CRM_Utils_SQL_TempTable::build()->setCategory('importjob')->getName();
}
$db->query("DROP TABLE IF EXISTS $tableName");
$db->query("CREATE TABLE $tableName ENGINE=InnoDB DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci $createSql");
Expand Down Expand Up @@ -411,7 +411,7 @@ public static function getIncompleteImportTables() {
$database = $dao->database();
Copy link
Contributor

Choose a reason for hiding this comment

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

If you trace back this function I think it can be removed entirely + the one function & one class that calls it

$query = "SELECT TABLE_NAME FROM INFORMATION_SCHEMA
WHERE TABLE_SCHEMA = ? AND
TABLE_NAME LIKE 'civicrm_import_job_%'
( TABLE_NAME LIKE 'civicrm_tmp_%_importjob%' OR TABLE_NAME LIKE 'civicrm_import_job_%')
ORDER BY TABLE_NAME";
$result = CRM_Core_DAO::executeQuery($query, array($database));
$incompleteImportTables = array();
Expand Down
2 changes: 1 addition & 1 deletion CRM/Import/DataSource/CSV.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private static function _CsvToTable(

// FIXME: we should regen this table's name if it exists rather than drop it
if (!$table) {
$table = 'civicrm_import_job_' . md5(uniqid(rand(), TRUE));
$table = CRM_Utils_SQL_TempTable::build()->setCategory('importjob')->getName();
}

$db->query("DROP TABLE IF EXISTS $table");
Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/CRM/Import/DataSource/CsvTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class CRM_Import_DataSource_CsvTest extends CiviUnitTestCase {
* @dataProvider getCsvFiles
*/
public function testToCsv($fileName) {
$dataSource = new CRM_Import_DataSource_Csv();
$dataSource = new CRM_Import_DataSource_CSV();
$params = [
'uploadFile' => [
'name' => __DIR__ . '/' . $fileName,
Expand Down