Merge pull request #24603 from eileenmcnaughton/import

Fix civiimport crash on unmapped fields, remove overzealous cleanup, add api to help debug & test

CRM/Contribute/Import/Parser/Contribution.php

@@ -208,7 +208,7 @@ public function getRequiredFieldsForMatch(): array {
   public function getMappedRow(array $values): array {
     $params = [];
     foreach ($this->getFieldMappings() as $i => $mappedField) {
-      if ($mappedField['name'] === 'do_not_import' || !$mappedField['name']) {
+      if (empty($mappedField['name']) || $mappedField['name'] === 'do_not_import') {
         continue;
       }
       $fieldSpec = $this->getFieldMetadata($mappedField['name']);
@@ -774,7 +774,7 @@ public function getMappedFieldLabel(array $mappedField): string {
     if (empty($this->importableFieldsMetadata)) {
       $this->setFieldMetadata();
     }
-    if ($mappedField['name'] === '') {
+    if (empty($mappedField['name'])) {
       return '';
     }
     $title = [];

CRM/Import/DataSource.php

@@ -384,18 +384,6 @@ protected function getTableName(): ?string {
       return NULL;
     }
     if (!$this->tableName) {
-      // If we are just loading this table we will do some validation.
-      // In the case of viewing historical jobs the table could have
-      // been deleted so we check that when we first load it.
-      if (strpos($tableName, 'civicrm_tmp_') !== 0
-        || !CRM_Utils_Rule::alphanumeric($tableName)) {
-        // The table name is generated and stored by code, not users so it
-        // should be safe - but a check seems prudent all the same.
-        throw new CRM_Core_Exception('Table cannot be deleted');
-      }
-      if (!CRM_Core_DAO::singleValueQuery('SHOW TABLES LIKE %1', [1 => [$tableName, 'String']])) {
-        throw new CRM_Import_Exception_ImportTableUnavailable('table deleted');
-      }
       $this->tableName = $tableName;
     }
     return $this->tableName;

CRM/Import/Parser.php

@@ -632,7 +632,12 @@ protected function getRequiredFieldsContactCreate(): array {
     ];
   }
 
-  protected function doPostImportActions() {
+  /**
+   * Core function - do not call from outside core.
+   *
+   * @internal
+   */
+  public function doPostImportActions() {
     $userJob = $this->getUserJob();
     $summaryInfo = $userJob['metadata']['summary_info'] ?? [];
     $actions = $userJob['metadata']['post_actions'] ?? [];
@@ -1820,15 +1825,7 @@ protected function getFieldEntity(string $fieldName) {
   public function validate(): void {
     $dataSource = $this->getDataSourceObject();
     while ($row = $dataSource->getRow()) {
-      try {
-        $rowNumber = $row['_id'];
-        $values = array_values($row);
-        $this->validateValues($values);
-        $this->setImportStatus($rowNumber, 'VALID', '');
-      }
-      catch (CRM_Core_Exception $e) {
-        $this->setImportStatus($rowNumber, 'ERROR', $e->getMessage());
-      }
+      $this->validateRow($row);
     }
   }
 
@@ -2551,4 +2548,19 @@ protected function getDedupeRules(array $dedupeRuleIDs, $contact_type) {
     return $dedupeRules;
   }
 
+  /**
+   * @param array|null $row
+   */
+  public function validateRow(?array $row): void {
+    try {
+      $rowNumber = $row['_id'];
+      $values = array_values($row);
+      $this->validateValues($values);
+      $this->setImportStatus($rowNumber, 'VALID', '');
+    }
+    catch (CRM_Core_Exception $e) {
+      $this->setImportStatus($rowNumber, 'ERROR', $e->getMessage());
+    }
+  }
+
 }

ext/civiimport/Civi/Api4/Import.php

@@ -17,6 +17,8 @@
 use Civi\Api4\Import\Create;
 use Civi\Api4\Import\Save;
 use Civi\Api4\Import\Update;
+use Civi\Api4\Import\Import as ImportAction;
+use Civi\Api4\Import\Validate;
 
 /**
  * Import entity.
@@ -27,6 +29,14 @@
  */
 class Import {
 
+  /**
+   * Constructor.
+   *
+   * This is here cos otherwise phpcs complains about the `import` function
+   * having the same name as the class.
+   */
+  public function __construct() {}
+
   /**
    * @param int $userJobID
    * @param bool $checkPermissions
@@ -63,7 +73,8 @@ public static function save(int $userJobID, bool $checkPermissions = TRUE): Save
   /**
    * @param int $userJobID
    * @param bool $checkPermissions
-   * @return \Civi\Api4\Generic\DAOCreateAction
+   *
+   * @return \Civi\Api4\Import\Create
    * @throws \API_Exception
    */
   public static function create(int $userJobID, bool $checkPermissions = TRUE): Create {
@@ -101,6 +112,30 @@ public static function checkAccess(int $userJobID): CheckAccessAction {
     return new CheckAccessAction('Import_' . $userJobID, __FUNCTION__);
   }
 
+  /**
+   * @param int $userJobID
+   * @param bool $checkPermissions
+   *
+   * @return \Civi\Api4\Import\Import
+   *
+   * @throws \API_Exception
+   */
+  public static function import(int $userJobID, bool $checkPermissions = TRUE): ImportAction {
+    return (new ImportAction('Import_' . $userJobID, __FUNCTION__))
+      ->setCheckPermissions($checkPermissions);
+  }
+
+  /**
+   * @param int $userJobID
+   * @param bool $checkPermissions
+   *
+   * @return \Civi\Api4\Import\Validate
+   * @throws \API_Exception
+   */
+  public static function validate(int $userJobID, bool $checkPermissions = TRUE): Validate {
+    return (new Validate('Import_' . $userJobID, __FUNCTION__))->setCheckPermissions($checkPermissions);
+  }
+
   /**
    * We need to implement these elsewhere as we permit based on 'created_id'.
    *

ext/civiimport/Civi/Api4/Import/Import.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Api4\Import;
+
+use Civi\Api4\Generic\DAOGetAction;
+use Civi\Api4\Generic\Result;
+
+class Import extends DAOGetAction {
+
+  use ImportProcessTrait;
+
+  /**
+   * @throws \CRM_Core_Exception
+   */
+  public function _run(Result $result): void {
+    $userJobID = (int) str_replace('Import_', '', $this->_entityName);
+    $where = $this->where;
+    $this->addWhere('_status', 'IN', ['new', 'valid']);
+    $this->getImportRows($result);
+    $parser = $this->getParser($userJobID);
+    foreach ($result as $row) {
+      $parser->import(array_values($row));
+    }
+    $parser->doPostImportActions();
+
+    // Re-fetch the validated result with updated messages.
+    $this->where = $where;
+    $this->addSelect('*');
+    parent::_run($result);
+  }
+
+}

ext/civiimport/Civi/Api4/Import/ImportProcessTrait.php

@@ -0,0 +1,84 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Api4\Import;
+
+use Civi\Api4\Generic\Result;
+use Civi\Api4\Import;
+use Civi\Api4\UserJob;
+
+/**
+ * Code shared by Import Save/Update actions.
+ *
+ * @method getCheckPermissions()
+ */
+trait ImportProcessTrait {
+
+  /**
+   * Get the parser for the import
+   *
+   * @return \CRM_Import_Parser|\CRM_Contribute_Import_Parser_Contribution
+   *
+   * @throws \CRM_Core_Exception
+   */
+  protected function getParser(int $userJobID) {
+    $userJob = UserJob::get($this->getCheckPermissions())
+      ->addWhere('id', '=', $userJobID)
+      ->addSelect('job_type')
+      ->execute()
+      ->first();
+    $parserClass = NULL;
+    foreach (\CRM_Core_BAO_UserJob::getTypes() as $userJobType) {
+      if ($userJob['job_type'] === $userJobType['id']) {
+        $parserClass = $userJobType['class'];
+      }
+    }
+    /** @var \CRM_Import_Parser|\CRM_Contribute_Import_Parser_Contribution $parser */
+    $parser = new $parserClass();
+    $parser->setUserJobID($userJobID);
+    $parser->init();
+    return $parser;
+  }
+
+  /**
+   * Get the selected import rows.
+   *
+   * @param \Civi\Api4\Generic\Result $result
+   *
+   * @throws \CRM_Core_Exception
+   */
+  protected function getImportRows(Result $result): void {
+    $userJobID = (int) str_replace('Import_', '', $this->_entityName);
+    $this->addSelect('*');
+    $importFields = array_keys((array) Import::getFields($userJobID, $this->getCheckPermissions())
+      ->addSelect('name')
+      ->addWhere('name', 'NOT LIKE', '_%')
+      ->execute()
+      ->indexBy('name'));
+    $importFields[] = '_id';
+    $importFields[] = '_entity_id';
+    $this->setSelect($importFields);
+    parent::_run($result);
+    foreach ($result as &$row) {
+      if ($row['_entity_id']) {
+        // todo - how should we handle this? Skip, exception. At this case
+        // it is non ui accessible so this is good for now.
+        throw new \CRM_Core_Exception('Row already imported');
+      }
+      // Push ID to the end as the get has moved it to the front & order matters here.
+      $rowID = $row['_id'];
+      unset($row['_id'], $row['_entity_id']);
+      $row['_id'] = $rowID;
+    }
+  }
+
+}

ext/civiimport/Civi/Api4/Import/Validate.php

@@ -0,0 +1,39 @@
+<?php
+
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+namespace Civi\Api4\Import;
+
+use Civi\Api4\Generic\DAOGetAction;
+use Civi\Api4\Generic\Result;
+
+class Validate extends DAOGetAction {
+
+  use ImportProcessTrait;
+
+  /**
+   * @throws \CRM_Core_Exception
+   */
+  public function _run(Result $result): void {
+    $userJobID = (int) str_replace('Import_', '', $this->_entityName);
+
+    $this->getImportRows($result);
+    $parser = $this->getParser($userJobID);
+    foreach ($result as $row) {
+      $parser->validateRow($row);
+    }
+
+    // Re-fetch the validated result with updated messages.
+    $this->addSelect('*');
+    parent::_run($result);
+  }
+
+}

ext/civiimport/Civi/Api4/Service/Spec/Provider/ImportSpecProvider.php

@@ -41,6 +41,7 @@ public function modifySpec(RequestSpec $spec): void {
       $field = new FieldSpec($column['name'], $spec->getEntity(), 'String');
       $field->setTitle(ts('Import field') . ':' . $column['label']);
       $field->setLabel($column['label']);
+      $field->setType('Field');
       $field->setReadonly($isInternalField);
       $field->setDescription(ts('Data being imported into the field.'));
       $field->setColumnName($column['name']);

ext/civiimport/tests/phpunit/CiviApiImportTest.php

@@ -63,6 +63,13 @@ public function testApiActions():void {
           'dedupe_rule_id' => NULL,
           'dateFormats' => CRM_Core_Form_Date::DATE_yyyy_mm_dd,
         ],
+        'import_mappings' => [
+          ['name' => 'external_identifier'],
+          ['name' => 'total_amount'],
+          ['name' => 'receive_date'],
+          ['name' => 'financial_type_id'],
+          [],
+        ],
       ],
       'status_id:name' => 'draft',
       'job_type' => 'contribution_import',
@@ -82,11 +89,12 @@ public function testApiActions():void {
     ])->execute();
 
     $import = Import::get($userJobID)->setSelect(['external_identifier', 'amount_given', '_status'])->execute()->first();
+    $rowID = $import['_id'];
     $this->assertEquals('80', $import['amount_given']);
 
     Import::update($userJobID)->setValues([
       'amount_given' => NULL,
-      '_id' => $import['_id'],
+      '_id' => $rowID,
       '_status' => 'IMPORTED',
     ])->execute();
 
@@ -98,23 +106,35 @@ public function testApiActions():void {
         'external_identifier' => 999,
         'amount_given' => 9,
         '_status' => 'ERROR',
+        '_id' => $rowID,
       ],
     ])->execute();
 
-    $import = Import::get($userJobID)->setSelect(['external_identifier', 'amount_given', '_status'])->addWhere('_id', '>', $import['_id'])->execute()->first();
+    $import = Import::get($userJobID)->setSelect(['external_identifier', 'amount_given', '_status'])->addWhere('_id', '=', $rowID)->execute()->first();
     $this->assertEquals(9, $import['amount_given']);
 
     Import::save($userJobID)->setRecords([
       [
         'external_identifier' => 777,
-        '_id' => $import['_id'],
+        '_id' => $rowID,
         '_status' => 'ERROR',
       ],
     ])->execute();
 
-    $import = Import::get($userJobID)->setSelect(['external_identifier', 'amount_given', '_status'])->addWhere('_id', '=', $import['_id'])->execute()->first();
+    $import = Import::get($userJobID)->setSelect(['external_identifier', 'amount_given', '_status'])->addWhere('_id', '=', $rowID)->execute()->first();
     $this->assertEquals(777, $import['external_identifier']);
 
+    $validate = Import::validate($userJobID)->addWhere('_id', '=', $rowID)->setLimit(1)->execute()->first();
+    $this->assertEquals('Missing required fields: Contribution ID OR Invoice Reference OR Transaction ID OR Financial Type ID', $validate['_status_message']);
+    $this->assertEquals('ERROR', $validate['_status']);
+
+    Import::update($userJobID)->setValues(['financial_type' => 'Donation'])->addWhere('_id', '=', $rowID)->execute();
+    $validate = Import::validate($userJobID)->addWhere('_id', '=', $rowID)->setLimit(1)->execute()->first();
+    $this->assertEquals('', $validate['_status_message']);
+    $this->assertEquals('VALID', $validate['_status']);
+    $imported = Import::import($userJobID)->addWhere('_id', '=', $rowID)->setLimit(1)->execute()->first();
+    $this->assertEquals('ERROR', $imported['_status']);
+    $this->assertEquals('No matching Contact found', $imported['_status_message']);
   }
 
   /**