Cleanup gender handling in contact import

civicrm · May 18, 2022 · 46a3cd3 · 46a3cd3
1 parent 78dcfbb
commit 46a3cd3
Show file tree

Hide file tree

Showing 5 changed files with 188 additions and 78 deletions.
diff --git a/CRM/Contact/BAO/Contact.php b/CRM/Contact/BAO/Contact.php
@@ -781,7 +781,6 @@ public static function resolveDefaults(&$defaults, $reverse = FALSE) {
 
     CRM_Utils_Array::lookupValue($defaults, 'prefix', CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'prefix_id'), $reverse);
     CRM_Utils_Array::lookupValue($defaults, 'suffix', CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'suffix_id'), $reverse);
-    CRM_Utils_Array::lookupValue($defaults, 'gender', CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'gender_id'), $reverse);
     CRM_Utils_Array::lookupValue($defaults, 'communication_style', CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'communication_style_id'), $reverse);
 
     //lookup value of email/postal greeting, addressee, CRM-4575

diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php
@@ -1305,41 +1305,6 @@ public static function isErrorInCustomData($params, &$errorMessage, $csType = NU
     }
   }
 
-  /**
-   * Check if value present in all genders or.
-   * as a substring of any gender value, if yes than return corresponding gender.
-   * eg value might be  m/M, ma/MA, mal/MAL, male return 'Male'
-   * but if value is 'maleabc' than return false
-   *
-   * @param string $gender
-   *   Check this value across gender values.
-   *
-   * retunr gender value / false
-   *
-   * @return bool
-   */
-  public function checkGender($gender) {
-    $gender = trim($gender, '.');
-    if (!$gender) {
-      return FALSE;
-    }
-
-    $allGenders = CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'gender_id');
-    foreach ($allGenders as $key => $value) {
-      if (strlen($gender) > strlen($value)) {
-        continue;
-      }
-      if ($gender == $value) {
-        return $value;
-      }
-      if (substr_compare($value, $gender, 0, strlen($gender), TRUE) === 0) {
-        return $value;
-      }
-    }
-
-    return FALSE;
-  }
-
   /**
    * Check if an error in Core( non-custom fields ) field
    *
@@ -1350,6 +1315,9 @@ public function checkGender($gender) {
   public function isErrorInCoreData($params, &$errorMessage) {
     $errors = [];
     foreach ($params as $key => $value) {
+      if ($value === 'invalid_import_value') {
+        $errors[] = $this->getFieldMetadata($key)['title'];
+      }
       if ($value) {
         $session = CRM_Core_Session::singleton();
         $dateType = $session->get("dateTypes");
@@ -1383,12 +1351,6 @@ public function isErrorInCoreData($params, &$errorMessage) {
             }
             break;
 
-          case 'gender_id':
-            if (!self::checkGender($value)) {
-              $errors[] = ts('Gender');
-            }
-            break;
-
           case 'preferred_communication_method':
             $preffComm = [];
             $preffComm = explode(',', $value);
@@ -2338,23 +2300,14 @@ private function getParams(array $values): array {
       $locationFields = ['location_type_id', 'phone_type_id', 'provider_id', 'website_type_id'];
       $locationValues = array_filter(array_intersect_key($mappedField, array_fill_keys($locationFields, 1)));
 
-      $contactArray = &$params;
       if ($relatedContactKey) {
         if (!isset($params[$relatedContactKey])) {
           $params[$relatedContactKey] = ['contact_type' => $this->getRelatedContactType($mappedField['relationship_type_id'], $mappedField['relationship_direction'])];
         }
-        $contactArray = &$params[$relatedContactKey];
-      }
-
-      if (!empty($locationValues)) {
-        $locationValues[$fieldName] = $importedValue;
-        $contactArray[$fieldName] = (array) ($contactArray[$fieldName] ?? []);
-        $contactArray[$fieldName][] = $locationValues;
+        $this->addFieldToParams($params[$relatedContactKey], $locationValues, $fieldName, $importedValue);
       }
       else {
-        // @todo - this is really the best point to convert labels
-        // to values.
-        $contactArray[$fieldName] = $importedValue;
+        $this->addFieldToParams($params, $locationValues, $fieldName, $importedValue);
       }
     }
 
@@ -2556,17 +2509,6 @@ protected function formatContactParameters(&$values, &$params) {
       return TRUE;
     }
 
-    if (isset($values['gender'])) {
-      if (!empty($params['gender_id'])) {
-        $genders = CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'gender_id');
-        $params['gender'] = $genders[$params['gender_id']];
-      }
-      else {
-        $params['gender'] = $values['gender'];
-      }
-      return TRUE;
-    }
-
     if (!empty($values['preferred_communication_method'])) {
       $comm = [];
       $pcm = array_change_key_case(array_flip(CRM_Core_PseudoConstant::get('CRM_Contact_DAO_Contact', 'preferred_communication_method')), CASE_LOWER);
@@ -3070,4 +3012,26 @@ protected function getRelationshipType(int $relationshipTypeID): array {
     return Civi::$statics[__CLASS__][$cacheKey];
   }
 
+  /**
+   * Add the given field to the contact array.
+   *
+   * @param array $contactArray
+   * @param array $locationValues
+   * @param string $fieldName
+   * @param mixed $importedValue
+   *
+   * @return void
+   * @throws \API_Exception
+   */
+  private function addFieldToParams(array &$contactArray, array $locationValues, string $fieldName, $importedValue): void {
+    if (!empty($locationValues)) {
+      $locationValues[$fieldName] = $importedValue;
+      $contactArray[$fieldName] = (array) ($contactArray[$fieldName] ?? []);
+      $contactArray[$fieldName][] = $locationValues;
+    }
+    else {
+      $contactArray[$fieldName] = $this->getTransformedFieldValue($fieldName, $importedValue);
+    }
+  }
+
 }
diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php
@@ -1175,4 +1175,73 @@ protected function validateRequiredFields(array $requiredFields, array $params,
     throw new CRM_Core_Exception(($prefixString ? ($prefixString . ' ') : '') . ts('Missing required fields:') . ' ' . implode(' ' . ts('OR') . ' ', $missingFields));
   }
 
+  /**
+   * Get the field value, transformed by metadata.
+   *
+   * @param string $fieldName
+   * @param string|int $importedValue
+   *   Value as it came in from the datasource.
+   *
+   * @return string|array|bool|int
+   * @throws \API_Exception
+   */
+  protected function getTransformedFieldValue(string $fieldName, $importedValue) {
+    // For now only do gender_id as we need to work through removing duplicate handling
+    if ($fieldName !== 'gender_id' || empty($importedValue)) {
+      return $importedValue;
+    }
+    return $this->getFieldOptions($fieldName)[$importedValue] ?? 'invalid_import_value';
+  }
+
+  /**
+   * @param string $fieldName
+   *
+   * @return false|array
+   *
+   * @throws \API_Exception
+   */
+  protected function getFieldOptions(string $fieldName) {
+    return $this->getFieldMetadata($fieldName, TRUE)['options'];
+  }
+
+  /**
+   * Get the metadata for the field.
+   *
+   * @param string $fieldName
+   * @param bool $loadOptions
+   *
+   * @return array
+   * @throws \API_Exception
+   */
+  protected function getFieldMetadata(string $fieldName, bool $loadOptions = FALSE): array {
+    $fieldMetadata = $this->getImportableFieldsMetadata()[$fieldName];
+    if ($loadOptions && !isset($fieldMetadata['options'])) {
+      if (empty($fieldMetadata['pseudoconstant'])) {
+        $this->importableFieldsMetadata[$fieldName]['options'] = FALSE;
+      }
+      else {
+        $options = civicrm_api4($fieldMetadata['entity'], 'getFields', [
+          'loadOptions' => ['id', 'name', 'label'],
+          'where' => [['name', '=', $fieldMetadata['name']]],
+          'select' => ['options'],
+        ])->first()['options'];
+        // We create an array of the possible variants - notably including
+        // name AND label as either might be used, and capitalisation variants.
+        $values = [];
+        foreach ($options as $option) {
+          $values[$option['id']] = $option['id'];
+          $values[$option['label']] = $option['id'];
+          $values[$option['name']] = $option['id'];
+          $values[strtoupper($option['name'])] = $option['id'];
+          $values[strtolower($option['name'])] = $option['id'];
+          $values[strtoupper($option['label'])] = $option['id'];
+          $values[strtolower($option['label'])] = $option['id'];
+        }
+        $this->importableFieldsMetadata[$fieldName]['options'] = $values;
+      }
+      return $this->importableFieldsMetadata[$fieldName];
+    }
+    return $fieldMetadata;
+  }
+
 }
diff --git a/tests/phpunit/CRM/Contact/Import/Form/data/individual_genders.csv b/tests/phpunit/CRM/Contact/Import/Form/data/individual_genders.csv
@@ -0,0 +1,17 @@
+first_name,Last Name,Gender,mum_first_name,mum_last_name,mum_gender,Expected
+Madame,1,Female,Mum,1,,Valid
+Madame,2,FEMALE,Mum,2,,Valid
+Madame,3,female,Mum,3,,Valid
+Madame,4,Female.,Mum,4,,Invalid
+Madame,5,F,Mum,5,,Invalid
+Madame,6,f,Mum,6,,Invalid
+Madame,7,Cat,Mum,7,,Invalid
+Madame,8,1,Mum,8,,Valid
+Daughter,9,,Madame,9,Female,Valid
+Daughter,10,,Madame,10,FEMALE,Valid
+Daughter,11,,Madame,11,female,Valid
+Daughter,12,,Madame,12,Female.,Invalid
+Daughter,13,,Madame,13,F,Invalid
+Daughter,14,,Madame,14,f,Invalid
+Daughter,15,,Madame,15,Cat,Invalid
+Daughter,16,,Madame,16,1,Valid
diff --git a/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php b/tests/phpunit/CRM/Contact/Import/Parser/ContactTest.php
@@ -14,6 +14,7 @@
  * File for the CRM_Contact_Imports_Parser_ContactTest class.
  */
 
+use Civi\Api4\Contact;
 use Civi\Api4\UserJob;
 
 /**
@@ -882,6 +883,52 @@ public function importDataProvider(): array {
     ];
   }
 
+  /**
+   * Test the handling of validation when importing genders.
+   *
+   * If it's not gonna import it should fail at the validation stage...
+   *
+   * @throws \API_Exception
+   * @throws \CRM_Core_Exception
+   */
+  public function testImportGenders(): void {
+    $mapper = [
+      ['first_name'],
+      ['last_name'],
+      ['gender_id'],
+      ['1_a_b', 'first_name'],
+      ['1_a_b', 'last_name'],
+      ['1_a_b', 'gender_id'],
+      ['do_not_import'],
+    ];
+    $csv = 'individual_genders.csv';
+    /* @var CRM_Import_DataSource_CSV $dataSource */
+    /* @var \CRM_Contact_Import_Parser_Contact $parser */
+    [$dataSource, $parser] = $this->getDataSourceAndParser($csv, $mapper, []);
+    while ($values = $dataSource->getRow()) {
+      try {
+        $parser->validateValues(array_values($values));
+        if ($values['expected'] !== 'Valid') {
+          $this->fail($values['gender'] . ' should not have been valid');
+        }
+      }
+      catch (CRM_Core_Exception $e) {
+        if ($values['expected'] !== 'Invalid') {
+          $this->fail($values['gender'] . ' should have been valid');
+        }
+      }
+    }
+
+    $this->importCSV($csv, $mapper);
+    $contacts = Contact::get()
+      ->addWhere('first_name', '=', 'Madame')
+      ->addSelect('gender_id:name')->execute();
+    foreach ($contacts as $contact) {
+      $this->assertEquals('Female', $contact['gender_id:name']);
+    }
+    $this->assertCount(8, $contacts);
+  }
+
   /**
    * Test that setting duplicate action to fill doesn't blow away data
    * that exists, but does fill in where it's empty.
@@ -1108,6 +1155,33 @@ protected function runImport(array $originalValues, $onDuplicateAction, $expecte
     $this->assertEquals($expectedResult, $parser->import($onDuplicateAction, $values), 'Return code from parser import was not as expected');
   }
 
+  /**
+   * @param string $csv
+   * @param array $mapper
+   * @param array $submittedValues
+   *
+   * @return array
+   * @throws \API_Exception
+   * @throws \Civi\API\Exception\UnauthorizedException
+   */
+  protected function getDataSourceAndParser(string $csv, array $mapper, array $submittedValues): array {
+    $userJobID = $this->getUserJobID(array_merge([
+      'uploadFile' => ['name' => __DIR__ . '/../Form/data/' . $csv],
+      'skipColumnHeader' => TRUE,
+      'fieldSeparator' => ',',
+      'onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP,
+      'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
+      'mapper' => $mapper,
+      'dataSource' => 'CRM_Import_DataSource_CSV',
+    ], $submittedValues));
+
+    $dataSource = new CRM_Import_DataSource_CSV($userJobID);
+    $parser = new CRM_Contact_Import_Parser_Contact();
+    $parser->setUserJobID($userJobID);
+    $parser->init();
+    return [$dataSource, $parser];
+  }
+
   /**
    * @param array $fields Array of fields to be imported
    * @param array $allfields Array of all fields which can be part of import
@@ -1272,20 +1346,7 @@ protected function getUserJobID($submittedValues = []) {
    * @throws \CRM_Core_Exception
    */
   protected function validateCSV(string $csv, array $mapper, $submittedValues): void {
-    $userJobID = $this->getUserJobID(array_merge([
-      'uploadFile' => ['name' => __DIR__ . '/../Form/data/' . $csv],
-      'skipColumnHeader' => TRUE,
-      'fieldSeparator' => ',',
-      'onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP,
-      'contactType' => CRM_Import_Parser::CONTACT_INDIVIDUAL,
-      'mapper' => $mapper,
-      'dataSource' => 'CRM_Import_DataSource_CSV',
-    ], $submittedValues));
-
-    $dataSource = new CRM_Import_DataSource_CSV($userJobID);
-    $parser = new CRM_Contact_Import_Parser_Contact();
-    $parser->setUserJobID($userJobID);
-    $parser->init();
+    [$dataSource, $parser] = $this->getDataSourceAndParser($csv, $mapper, $submittedValues);
     $parser->validateValues(array_values($dataSource->getRow()));
   }