Merge pull request #23526 from eileenmcnaughton/import_sub

[Import] Check subtype validity in validate rather than wait for 'imort'
civicrm · May 20, 2022 · 69d6d95 · 69d6d95
2 parents ceac046 + e0b8f9a
commit 69d6d95
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 55 deletions.
diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php
@@ -93,6 +93,8 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser {
    */
   protected $metadataHandledFields = [
     'gender_id',
+    'contact_type',
+    'contact_sub_type',
   ];
 
   /**
@@ -434,34 +436,21 @@ public function import($onDuplicate, &$values) {
     }
 
     $params = $this->getMappedRow($values);
-    $formatted = [
-      'contact_type' => $this->getContactType(),
-    ];
+    $formatted = array_filter(array_intersect_key($params, array_fill_keys($this->metadataHandledFields, 1)));
 
     $contactFields = CRM_Contact_DAO_Contact::import();
 
     $params['contact_sub_type'] = $this->getContactSubType() ?: ($params['contact_sub_type'] ?? NULL);
 
-    if ($params['contact_sub_type']) {
-      if (CRM_Contact_BAO_ContactType::isExtendsContactType($params['contact_sub_type'], $this->getContactType(), FALSE, 'label')) {
-        // I think this bit is switching a passed in label to
-        // a name.
-        $subTypes = CRM_Contact_BAO_ContactType::subTypePairs($this->getContactType(), FALSE, NULL);
-        $params['contact_sub_type'] = array_search($params['contact_sub_type'], $subTypes);
-      }
-    }
-
     try {
-      if ($params['contact_sub_type'] && !CRM_Contact_BAO_ContactType::isExtendsContactType($params['contact_sub_type'], $this->getContactType())) {
-        throw new CRM_Core_Exception('Mismatched or Invalid Contact Subtype.', CRM_Import_Parser::NO_MATCH);
-      }
       $params['id'] = $formatted['id'] = $this->lookupContactID($params, ($this->isSkipDuplicates() || $this->isIgnoreDuplicates()));
     }
     catch (CRM_Core_Exception $e) {
       $statuses = [CRM_Import_Parser::DUPLICATE => 'DUPLICATE', CRM_Import_Parser::ERROR => 'ERROR', CRM_Import_Parser::NO_MATCH => 'invalid_no_match'];
       $this->setImportStatus((int) $values[count($values) - 1], $statuses[$e->getErrorCode()], $e->getMessage());
       return FALSE;
     }
+
     // Get contact id to format common data in update/fill mode,
     // prioritising a dedupe rule check over an external_identifier check, but falling back on ext id.
 
@@ -919,21 +908,7 @@ private static function legacyCreateMultiple($params, $ids = []) {
    *   Contact DAO fields.
    */
   private function formatCommonData($params, &$formatted, $contactFields) {
-    $csType = [
-      CRM_Utils_Array::value('contact_type', $formatted),
-    ];
-
-    //CRM-5125
-    //add custom fields for contact sub type
-    if (!empty($this->_contactSubType)) {
-      $csType = $this->_contactSubType;
-    }
-
-    if ($relCsType = CRM_Utils_Array::value('contact_sub_type', $formatted)) {
-      $csType = $relCsType;
-    }
-
-    $customFields = CRM_Core_BAO_CustomField::getFields($formatted['contact_type'], FALSE, FALSE, $csType);
+    $customFields = CRM_Core_BAO_CustomField::getFields($formatted['contact_type'], FALSE, FALSE, $formatted['contact_sub_type'] ?? NULL);
 
     $addressCustomFields = CRM_Core_BAO_CustomField::getFields('Address');
     $customFields = $customFields + $addressCustomFields;
@@ -1243,6 +1218,10 @@ public static function isErrorInCustomData($params, &$errorMessage, $csType = NU
    */
   public function isErrorInCoreData($params, &$errorMessage) {
     $errors = [];
+    if (!empty($params['contact_sub_type']) && !CRM_Contact_BAO_ContactType::isExtendsContactType($params['contact_sub_type'], $params['contact_type'])) {
+      $errors[] = ts('Mismatched or Invalid Contact Subtype.');
+    }
+
     foreach ($params as $key => $value) {
       if ($value === 'invalid_import_value') {
         $errors[] = $this->getFieldMetadata($key)['title'];
@@ -2014,16 +1993,6 @@ public function deprecated_contact_check_params(
     $dupeCheck = TRUE,
     $dedupeRuleGroupID = NULL) {
 
-    if (isset($params['id']) && is_numeric($params['id'])) {
-      // @todo - ensure this is tested & remove - expectation is api call further
-      // down validates it.
-      if ($csType = CRM_Utils_Array::value('contact_sub_type', $params)) {
-        if (!(CRM_Contact_BAO_ContactType::isExtendsContactType($csType, $params['contact_type']))) {
-          throw new CRM_Core_Exception("Invalid or Mismatched Contact Subtype: " . implode(', ', (array) $csType));
-        }
-      }
-    }
-
     if ($dupeCheck) {
       // @todo switch to using api version
       // $dupes = civicrm_api3('Contact', 'duplicatecheck', (array('match' => $params, 'dedupe_rule_id' => $dedupeRuleGroupID)));
@@ -2713,7 +2682,7 @@ public function getMappedFieldLabel(array $mappedField): string {
     if ($mappedField['relationship_type_id']) {
       $title[] = $this->getRelationshipLabel($mappedField['relationship_type_id'], $mappedField['relationship_direction']);
     }
-    $title[] = $this->getImportableFieldsMetadata()[$mappedField['name']]['title'];
+    $title[] = $this->getFieldMetadata($mappedField['name'])['title'];
     if ($mappedField['location_type_id']) {
       $title[] = CRM_Core_PseudoConstant::getLabel('CRM_Core_BAO_Address', 'location_type_id', $mappedField['location_type_id']);
     }
@@ -2772,6 +2741,9 @@ protected function getRelationshipLabel(int $id, string $direction): string {
   public function getMappedRow(array $values): array {
     $params = $this->getParams($values);
     $params['contact_type'] = $this->getContactType();
+    if ($this->getContactSubType()) {
+      $params['contact_sub_type'] = $this->getContactSubType();
+    }
     return $params;
   }
 
@@ -2811,16 +2783,8 @@ public function validateValues(array $values): void {
     //date-format part ends
 
     $errorMessage = NULL;
-
-    //CRM-5125
-    //add custom fields for contact sub type
-    $csType = NULL;
-    if (!empty($this->_contactSubType)) {
-      $csType = $this->_contactSubType;
-    }
-
     //checking error in custom data
-    $this->isErrorInCustomData($params, $errorMessage, $csType);
+    $this->isErrorInCustomData($params, $errorMessage, $params['contact_sub_type'] ?? NULL);
 
     //checking error in core data
     $this->isErrorInCoreData($params, $errorMessage);

diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php
@@ -1220,12 +1220,17 @@ protected function getFieldOptions(string $fieldName) {
    *
    * @param string $fieldName
    * @param bool $loadOptions
+   * @param bool $limitToContactType
+   *   Only show fields for the type to import (not appropriate when looking up
+   *   related contact fields).
+   *
    *
    * @return array
    * @throws \API_Exception
+   * @throws \Civi\API\Exception\NotImplementedException
    */
-  protected function getFieldMetadata(string $fieldName, bool $loadOptions = FALSE): array {
-    $fieldMetadata = $this->getImportableFieldsMetadata()[$fieldName];
+  protected function getFieldMetadata(string $fieldName, bool $loadOptions = FALSE, $limitToContactType = FALSE): array {
+    $fieldMetadata = $this->getImportableFieldsMetadata()[$fieldName] ?? ($limitToContactType ? NULL : CRM_Contact_BAO_Contact::importableFields('All')[$fieldName]);
     if ($loadOptions && !isset($fieldMetadata['options'])) {
       if (empty($fieldMetadata['pseudoconstant'])) {
         $this->importableFieldsMetadata[$fieldName]['options'] = FALSE;

diff --git a/tests/phpunit/CRM/Contact/Import/Form/data/individual_contact_sub_types.csv b/tests/phpunit/CRM/Contact/Import/Form/data/individual_contact_sub_types.csv
@@ -0,0 +1,8 @@
+first_name,Last Name,Organization,Contact Subtype,Organization SubType,expected
+Joe,Green,Greenfingers,baby,,Valid
+Joe,Green,Greenfingers,Infant,,Valid
+Joe,Green,Greenfingers,infant,,Valid
+Joe,Green,Greenfingers,rando,,Invalid
+Joe,Green,Greenfingers,,baby,Invalid
+Joe,Green,Greenfingers,,Infant,Invalid
+Joe,Green,Greenfingers,,infant,Invalid
diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -16,6 +16,7 @@
 
 use Civi\Api4\Address;
 use Civi\Api4\Contact;
+use Civi\Api4\ContactType;
 use Civi\Api4\RelationshipType;
 use Civi\Api4\UserJob;
 
@@ -41,6 +42,7 @@ class CRM_Contact_Import_Parser_ContactTest extends CiviUnitTestCase {
   public function tearDown(): void {
     $this->quickCleanup(['civicrm_address', 'civicrm_phone', 'civicrm_email', 'civicrm_user_job', 'civicrm_relationship'], TRUE);
     RelationshipType::delete()->addWhere('name_a_b', '=', 'Dad to')->execute();
+    ContactType::delete()->addWhere('name', '=', 'baby')->execute();
     parent::tearDown();
   }
 
@@ -990,7 +992,7 @@ public function importDataProvider(): array {
         'csv' => 'individual_invalid_contact_sub_type.csv',
         'mapper' => [['first_name'], ['last_name'], ['contact_sub_type']],
         'expected_error' => '',
-        'expected_outcomes' => [CRM_Import_Parser::NO_MATCH => 1],
+        'expected_outcomes' => [CRM_Import_Parser::ERROR => 1],
       ],
     ];
   }
@@ -1051,15 +1053,53 @@ public function testValidateDateData(): void {
     // Date types should be picked up from submitted values but still some clean up to do.
     CRM_Core_Session::singleton()->set('dateTypes', $dateType);
     $this->validateMultiRowCsv($csv, $mapper, 'custom_' . $addressCustomFieldID, ['dateFormats' => $dateType]);
-    $fields = ['contact_id.birth_date', 'contact_id.deceased_date', 'contact_id.custom_' . $contactCustomFieldID, $addressCustomFieldID];
-    $contacts = Address::get()->addWhere('contact_id.first_name', '=', 'Joe')->setSelect($fields)->execute();
+    $fields = [
+      'contact_id.birth_date',
+      'contact_id.deceased_date',
+      'contact_id.custom_' . $contactCustomFieldID,
+      $addressCustomFieldID,
+    ];
+    $contacts = Address::get()
+      ->addWhere('contact_id.first_name', '=', 'Joe')
+      ->setSelect($fields)
+      ->execute();
     foreach ($contacts as $contact) {
       foreach ($fields as $field) {
         $this->assertEquals('2008-09-01', $contact[$field]);
       }
     }
   }
 
+  /**
+   * @throws \API_Exception
+   */
+  public function testImportContactSubTypes(): void {
+    ContactType::create()->setValues([
+      'name' => 'baby',
+      'label' => 'Infant',
+      'parent_id:name' => 'Individual',
+    ])->execute();
+    $mapper = [
+      ['first_name'],
+      ['last_name'],
+      ['5_a_b', 'organization_name'],
+      ['contact_sub_type'],
+      ['5_a_b', 'contact_sub_type'],
+    ];
+    $csv = 'individual_contact_sub_types.csv';
+    $field = 'contact_sub_type';
+
+    $this->validateMultiRowCsv($csv, $mapper, $field);
+    $this->importCSV($csv, $mapper);
+    $contacts = Contact::get()
+      ->addWhere('last_name', '=', 'Green')
+      ->addSelect('contact_sub_type:name')->execute();
+    foreach ($contacts as $contact) {
+      $this->assertEquals(['baby'], $contact['contact_sub_type:name']);
+    }
+    $this->assertCount(3, $contacts);
+  }
+
   /**
    * Test that setting duplicate action to fill doesn't blow away data
    * that exists, but does fill in where it's empty.

diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/data/.~lock.contributions.csv# b/tests/phpunit/CRM/Contribute/Import/Parser/data/.~lock.contributions.csv#
@@ -0,0 +1 @@
+,eileen,eileen-laptop,20.05.2022 17:00,file:///home/eileen/.config/libreoffice/4;
diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions.CSV b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions.CSV
@@ -0,0 +1,2 @@
+External Identifier,Total Amount,Receive Date,Financial Type,Soft Credit to
+bob,65,2008-09-20,Donation,[email protected]
diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions.csv b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions.csv
@@ -0,0 +1,2 @@
+External Identifier,Total Amount,Receive Date,Financial Type,Soft Credit to
+bob,65,2008-09-20,Donation,[email protected]
diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions.txt b/tests/phpunit/CRM/Contribute/Import/Parser/data/contributions.txt
@@ -0,0 +1,2 @@
+External Identifier,Total Amount,Receive Date,Financial Type,Soft Credit to
+bob,65,2008-09-20,Donation,[email protected]
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		,eileen,eileen-laptop,20.05.2022 17:00,file:///home/eileen/.config/libreoffice/4;
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		External Identifier,Total Amount,Receive Date,Financial Type,Soft Credit to
		bob,65,2008-09-20,Donation,[email protected]