[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

diff --git a/CRM/Contact/Import/Parser.php b/CRM/Contact/Import/Parser.php
index e6bfd26f2a460de2982afe4aaa18a7eb66ed6f24..99932d242a4aebfbf90fc828a191d86c88438b00 100644 (file)
--- 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<br/>With arguments: ".$params[$statusFieldName].", ".$params["${statusFieldName}Msg"].", $id<br/>";
+  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 3cc2600b25580dfa7c09e43d5cad631cbf105ff9..c27da774e572303f44433ab6c306c4813d54c768 100644 (file)
--- 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