From ca275c674ddcb7939dd416ca11c59e29ab06f1ee Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 30 Apr 2021 12:11:56 +1200 Subject: [PATCH] [REF] Cleanup on import rows error Trying (and not yet succeeding) to replicat https://lab.civicrm.org/dev/core/-/issues/2566 I was able to step through & eliminate one more place where we call dao->query() instead of CRM_Core_DAO::executeQuery --- CRM/Contact/Import/Parser.php | 48 ++++++++++++++----------- CRM/Contact/Import/Parser/Contact.php | 51 ++++++--------------------- 2 files changed, 38 insertions(+), 61 deletions(-) diff --git a/CRM/Contact/Import/Parser.php b/CRM/Contact/Import/Parser.php index e6bfd26f2a..99932d242a 100644 --- a/CRM/Contact/Import/Parser.php +++ b/CRM/Contact/Import/Parser.php @@ -691,30 +691,38 @@ abstract class CRM_Contact_Import_Parser extends CRM_Import_Parser { /** * Update the record with PK $id in the import database table. * + * @deprecated - call setImportStatus directly as the parameters are simpler, + * * @param int $id * @param array $params */ - public function updateImportRecord($id, &$params) { - $statusFieldName = $this->_statusFieldName; - $primaryKeyName = $this->_primaryKeyName; - - if ($statusFieldName && $primaryKeyName) { - $dao = new CRM_Core_DAO(); - $db = $dao->getDatabaseConnection(); - - $query = "UPDATE $this->_tableName - SET $statusFieldName = ?, - ${statusFieldName}Msg = ? - WHERE $primaryKeyName = ?"; - $args = [ - $params[$statusFieldName], - CRM_Utils_Array::value("${statusFieldName}Msg", $params), - $id, - ]; - - //print "Running query: $query
With arguments: ".$params[$statusFieldName].", ".$params["${statusFieldName}Msg"].", $id
"; + public function updateImportRecord($id, $params): void { + $this->setImportStatus((int) $id, $params[$this->_statusFieldName] ?? '', $params["{$this->_statusFieldName}Msg"] ?? ''); + } - $db->query($query, $args); + /** + * Set the import status for the given record. + * + * If this is a sql import then the sql table will be used and the update + * will not happen as the relevant fields don't exist in the table - hence + * the checks that statusField & primary key are set. + * + * @param int $id + * @param string $status + * @param string $message + */ + public function setImportStatus(int $id, string $status, string $message): void { + if ($this->_statusFieldName && $this->_primaryKeyName) { + CRM_Core_DAO::executeQuery(" + UPDATE $this->_tableName + SET $this->_statusFieldName = %1, + {$this->_statusFieldName}Msg = %2 + WHERE $this->_primaryKeyName = %3 + ", [ + 1 => [$status, 'String'], + 2 => [$message, 'String'], + 3 => [$id, 'Integer'], + ]); } } diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 3cc2600b25..c27da774e5 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -242,10 +242,10 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { * @return bool * the result of this processing */ - public function summary(&$values) { + public function summary(&$values): int { $erroneousField = NULL; - $response = $this->setActiveFieldValues($values, $erroneousField); - + $this->setActiveFieldValues($values, $erroneousField); + $rowNumber = (int) ($values[count($values) - 1]); $errorMessage = NULL; $errorRequired = FALSE; switch ($this->_contactType) { @@ -280,12 +280,10 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { break; } - $statusFieldName = $this->_statusFieldName; - if ($this->_emailIndex >= 0) { /* If we don't have the required fields, bail */ - if ($this->_contactType == 'Individual' && !$this->_updateWithId) { + if ($this->_contactType === 'Individual' && !$this->_updateWithId) { if ($errorRequired && empty($values[$this->_emailIndex])) { if ($errorMessage) { $errorMessage .= ' ' . ts('OR') . ' ' . ts('Email Address'); @@ -294,11 +292,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { $errorMessage = ts('Missing required field:') . ' ' . ts('Email Address'); } array_unshift($values, $errorMessage); - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + $this->setImportStatus($rowNumber, 'ERROR', $errorMessage); return CRM_Import_Parser::ERROR; } @@ -311,11 +305,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { if (!CRM_Utils_Rule::email($email)) { $errorMessage = ts('Invalid Email address'); array_unshift($values, $errorMessage); - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + $this->setImportStatus($rowNumber, 'ERROR', $errorMessage); return CRM_Import_Parser::ERROR; } @@ -332,11 +322,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { $errorMessage = ts('Missing required field:') . ' ' . ts('Email Address'); } array_unshift($values, $errorMessage); - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + $this->setImportStatus($rowNumber, 'ERROR', $errorMessage); return CRM_Import_Parser::ERROR; } @@ -349,11 +335,7 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { if ($externalDupe = CRM_Utils_Array::value($externalID, $this->_allExternalIdentifiers)) { $errorMessage = ts('External ID conflicts with record %1', [1 => $externalDupe]); array_unshift($values, $errorMessage); - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $errorMessage, - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + $this->setImportStatus($rowNumber, 'ERROR', $errorMessage); return CRM_Import_Parser::ERROR; } //otherwise, count it and move on @@ -381,24 +363,12 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { $this->isErrorInCoreData($params, $errorMessage); if ($errorMessage) { $tempMsg = "Invalid value for field(s) : $errorMessage"; - // put the error message in the import record in the DB - $importRecordParams = [ - $statusFieldName => 'ERROR', - "${statusFieldName}Msg" => $tempMsg, - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + $this->setImportStatus($rowNumber, 'ERROR', $tempMsg); array_unshift($values, $tempMsg); $errorMessage = NULL; return CRM_Import_Parser::ERROR; } - - //if user correcting errors by walking back - //need to reset status ERROR msg to null - //now currently we are having valid data. - $importRecordParams = [ - $statusFieldName => 'NEW', - ]; - $this->updateImportRecord($values[count($values) - 1], $importRecordParams); + $this->setImportStatus($rowNumber, 'NEW', ''); return CRM_Import_Parser::VALID; } @@ -431,7 +401,6 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Contact_Import_Parser { * @throws \API_Exception */ public function import($onDuplicate, &$values, $doGeocodeAddress = FALSE) { - $config = CRM_Core_Config::singleton(); $this->_unparsedStreetAddressContacts = []; if (!$doGeocodeAddress) { // CRM-5854, reset the geocode method to null to prevent geocoding -- 2.25.1