From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Mon, 5 Sep 2022 02:14:17 +0000 (+1200)
Subject: Fix contribution import mapping validation, add test
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=b0de4744a705dab4ae9fadd1a4fc4082049b1e8a;p=civicrm-core.git

Fix contribution import mapping validation, add test
---

diff --git a/CRM/Contribute/Import/Form/MapField.php b/CRM/Contribute/Import/Form/MapField.php
index 4ba33999ea..8188e3cd5a 100644
--- a/CRM/Contribute/Import/Form/MapField.php
+++ b/CRM/Contribute/Import/Form/MapField.php
@@ -20,38 +20,6 @@
  */
 class CRM_Contribute_Import_Form_MapField extends CRM_Import_Form_MapField {
 
-  /**
-   * Check if required fields are present.
-   *
-   * @param CRM_Contribute_Import_Form_MapField $self
-   * @param string $contactORContributionId
-   * @param array $importKeys
-   * @param array $errors
-   * @param int $weightSum
-   * @param int $threshold
-   * @param string $fieldMessage
-   *
-   * @return array
-   */
-  protected static function checkRequiredFields($self, string $contactORContributionId, array $importKeys, array $errors, int $weightSum, $threshold, string $fieldMessage): array {
-    // FIXME: should use the schema titles, not redeclare them
-    $requiredFields = [
-      'contribution_contact_id' => ts('Contact ID'),
-    ];
-
-    foreach ($requiredFields as $field => $title) {
-      if (!in_array($field, $importKeys)) {
-        if ($field == 'contribution_contact_id') {
-          if (!($weightSum >= $threshold || in_array('external_identifier', $importKeys))
-          ) {
-            $errors['_qf_default'] .= ts('Missing required contact matching fields.') . " $fieldMessage " . ts('(Sum of all weights should be greater than or equal to threshold: %1).', [1 => $threshold]) . '<br />';
-          }
-        }
-      }
-    }
-    return $errors;
-  }
-
   /**
    * Set variables up before form is built.
    */
@@ -162,41 +130,25 @@ class CRM_Contribute_Import_Form_MapField extends CRM_Import_Form_MapField {
    *   list of errors to be posted back to the form
    */
   public static function formRule($fields, $files, $self) {
-    $errors = [];
-    $fieldMessage = NULL;
-    $contactORContributionId = $self->isUpdateExisting() ? 'contribution_id' : 'contribution_contact_id';
-    if (!array_key_exists('savedMapping', $fields)) {
-      $importKeys = [];
-      foreach ($fields['mapper'] as $mapperPart) {
-        $importKeys[] = $mapperPart[0];
-      }
-
-      $params = [
-        'used' => 'Unsupervised',
-        'contact_type' => $self->getContactType(),
-      ];
-      [$ruleFields, $threshold] = CRM_Dedupe_BAO_DedupeRuleGroup::dedupeRuleFieldsWeight($params);
-      $weightSum = 0;
-      foreach ($importKeys as $key => $val) {
-        if (array_key_exists($val, $ruleFields)) {
-          $weightSum += $ruleFields[$val];
-        }
-      }
-      foreach ($ruleFields as $field => $weight) {
-        $fieldMessage .= ' ' . $field . '(weight ' . $weight . ')';
-      }
-      try {
-        $parser = $self->getParser();
-        $parser->validateMapping($fields['mapper']);
-      }
-      catch (CRM_Core_Exception $e) {
-        $errors['_qf_default'] = $e->getMessage();
-      }
+    $mapperError = [];
+    try {
+      $parser = $self->getParser();
+      $rule = $parser->getDedupeRule($self->getContactType());
       if (!$self->isUpdateExisting()) {
-        $errors = self::checkRequiredFields($self, $contactORContributionId, $importKeys, $errors, $weightSum, $threshold, $fieldMessage);
+        $missingDedupeFields = $self->validateDedupeFieldsSufficientInMapping($rule, $fields['mapper']);
+        if ($missingDedupeFields) {
+          $mapperError[] = $missingDedupeFields;
+        }
       }
+      $parser->validateMapping($fields['mapper']);
     }
-    return !empty($errors) ? $errors : TRUE;
+    catch (CRM_Core_Exception $e) {
+      $mapperError[] = $e->getMessage();
+    }
+    if (!empty($mapperError)) {
+      return ['_qf_default' => implode('<br/>', $mapperError)];
+    }
+    return TRUE;
   }
 
   /**
@@ -253,4 +205,33 @@ class CRM_Contribute_Import_Form_MapField extends CRM_Import_Form_MapField {
     return $defaults;
   }
 
+  /**
+   * Validate the the mapped fields contain enough to meet the dedupe rule lookup requirements.
+   *
+   * @param array $rule
+   * @param array $mapper
+   *
+   * @return string|false
+   *   Error string if insufficient.
+   */
+  protected function validateDedupeFieldsSufficientInMapping(array $rule, array $mapper): ?string {
+    $threshold = $rule['threshold'];
+    $ruleFields = $rule['fields'];
+    $weightSum = 0;
+    foreach ($mapper as $mapping) {
+      if ($mapping[0] === 'external_identifier') {
+        // It is enough to have external identifier mapped.
+        $weightSum = $threshold;
+        break;
+      }
+      if (array_key_exists($mapping[0], $ruleFields)) {
+        $weightSum += $ruleFields[$mapping[0]];
+      }
+    }
+    if ($weightSum < $threshold) {
+      return $rule['rule_message'];
+    }
+    return NULL;
+  }
+
 }
diff --git a/CRM/Import/Parser.php b/CRM/Import/Parser.php
index 4512a9ccf7..d780929d60 100644
--- a/CRM/Import/Parser.php
+++ b/CRM/Import/Parser.php
@@ -12,6 +12,7 @@
 use Civi\Api4\Campaign;
 use Civi\Api4\Contact;
 use Civi\Api4\CustomField;
+use Civi\Api4\DedupeRule;
 use Civi\Api4\DedupeRuleGroup;
 use Civi\Api4\Event;
 use Civi\Api4\UserJob;
@@ -279,6 +280,8 @@ abstract class CRM_Import_Parser implements UserJobInterface {
    */
   private $_fields;
 
+  private $dedupeRules = [];
+
   /**
    * Metadata for all available fields, keyed by unique name.
    *
@@ -311,42 +314,24 @@ abstract class CRM_Import_Parser implements UserJobInterface {
    * @param string $contactType
    *
    * @return array[]
-   * @throws \CRM_Core_Exception
    */
   protected function getContactFields(string $contactType): array {
     $contactFields = CRM_Contact_BAO_Contact::importableFields($contactType, NULL);
+    $dedupeFields = $this->getDedupeFields($contactType);
 
-    // Using new Dedupe rule.
-    $ruleParams = [
-      'contact_type' => $contactType,
-      'used' => 'Unsupervised',
-    ];
-    $fieldsArray = CRM_Dedupe_BAO_DedupeRule::dedupeRuleFields($ruleParams);
-    $tmpContactField = [];
-    if (is_array($fieldsArray)) {
-      foreach ($fieldsArray as $value) {
-        //skip if there is no dupe rule
-        if ($value === 'none') {
-          continue;
-        }
-        $customFieldId = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_CustomField',
-          $value,
-          'id',
-          'column_name'
-        );
-        $value = trim($customFieldId ? 'custom_' . $customFieldId : $value);
-        $tmpContactField[$value] = $contactFields[$value];
-        $title = $tmpContactField[$value]['title'] . ' ' . ts('(match to contact)');
-        $tmpContactField[$value]['title'] = $title;
-        // When we switch to apiv4 getfields this will already be set for
-        // all fields (including custom which it isn't yet)
-        $tmpContactField[$value]['entity'] = 'Contact';
+    $contactFieldsForContactLookup = [];
+    foreach ($dedupeFields as $fieldName => $dedupeField) {
+      if (!isset($contactFields[$fieldName])) {
+        continue;
       }
+      $contactFieldsForContactLookup[$fieldName] = $contactFields[$fieldName];
+      $contactFieldsForContactLookup[$fieldName]['title'] . ' ' . ts('(match to contact)');
+      $contactFieldsForContactLookup[$fieldName]['entity'] = 'Contact';
     }
 
-    $tmpContactField['external_identifier'] = $contactFields['external_identifier'];
-    $tmpContactField['external_identifier']['title'] = $contactFields['external_identifier']['title'] . ' ' . ts('(match to contact)');
-    return $tmpContactField;
+    $contactFieldsForContactLookup['external_identifier'] = $contactFields['external_identifier'];
+    $contactFieldsForContactLookup['external_identifier']['title'] = $contactFields['external_identifier']['title'] . ' ' . ts('(match to contact)');
+    return $contactFieldsForContactLookup;
   }
 
   /**
@@ -960,9 +945,13 @@ abstract class CRM_Import_Parser implements UserJobInterface {
    * @param string $contactType
    *
    * @return string
+   * @throws \CRM_Core_Exception
    */
   protected function getDefaultRuleForContactType(string $contactType): string {
-    return $contactType . '.Unsupervised';
+    return DedupeRuleGroup::get(FALSE)
+      ->addWhere('contact_type', '=', $contactType)
+      ->addWhere('used', '=', 'Unsupervised')
+      ->addSelect('id', 'name')->execute()->first()['name'];
   }
 
   /**
@@ -981,6 +970,72 @@ abstract class CRM_Import_Parser implements UserJobInterface {
       ->execute()->first()['name'];
   }
 
+  /**
+   * Get the dedupe rule, including an array of fields with weights.
+   *
+   * The fields are keyed according to the metadata.
+   *
+   * @param string $contactType
+   * @param string|null $name
+   *
+   * @return array
+   * @noinspection PhpUnhandledExceptionInspection
+   * @noinspection PhpDocMissingThrowsInspection
+   */
+  public function getDedupeRule(string $contactType, ?string $name = NULL): array {
+    if (!$name) {
+      $name = $this->getDefaultRuleForContactType($contactType);
+    }
+    if (empty($this->dedupeRules[$name])) {
+      $this->dedupeRules[$name] = (array) DedupeRuleGroup::get(FALSE)
+        ->addWhere('name', '=', $name)
+        ->addSelect('threshold', 'name', 'id', 'title', 'contact_type')
+        ->execute()->first();
+      $fields = [];
+      $this->dedupeRules[$name]['rule_message'] = $fieldMessage = '';
+      // Now we add the fields in a format like ['first_name' => 6, 'custom_8' => 9]
+      // The number is the weight and we add both api three & four style fields so the
+      // array can be used for converted & unconverted.
+      $ruleFields = DedupeRule::get(FALSE)
+        ->addWhere('dedupe_rule_group_id', '=', $this->dedupeRules[$name]['id'])
+        ->addSelect('id', 'rule_table', 'rule_field', 'rule_weight')->execute();
+      foreach ($ruleFields as $ruleField) {
+        $fieldMessage .= ' ' . $ruleField['rule_field'] . '(weight ' . $ruleField['rule_weight'] . ')';
+        if ($ruleField['rule_table'] === 'civicrm_contact') {
+          $fields[$ruleField['rule_field']] = $ruleField['rule_weight'];
+        }
+        // If not a contact field we add both api variants of fields.
+        elseif ($ruleField['rule_table'] === 'civicrm_phone') {
+          // Actually the dedupe rule for phone should always be phone_numeric. so checking 'phone' is probably unncessary
+          if (in_array($ruleField['rule_field'], ['phone', 'phone_numeric'], TRUE)) {
+            $fields['phone'] = $ruleField['rule_weight'];
+            $fields['phone_primary.phone'] = $ruleField['rule_weight'];
+          }
+        }
+        elseif ($ruleField['rule_field'] === 'email') {
+          $fields['email'] = $ruleField['rule_weight'];
+          $fields['email_primary.email'] = $ruleField['rule_weight'];
+        }
+        elseif ($ruleField['rule_table'] === 'civicrm_address') {
+          $fields[$ruleField['rule_field']] = $ruleField['rule_weight'];
+          $fields['address_primary' . $ruleField['rule_field']] = $ruleField['rule_weight'];
+        }
+        else {
+          // At this point it must be a custom field.
+          $customField = CustomField::get(FALSE)->addWhere('custom_group_id.table_name', '=', $ruleField['rule_table'])
+            ->addWhere('column_name', '=', $ruleField['rule_field'])
+            ->addSelect('id', 'name', 'custom_group_id.name')->execute()->first();
+          $fields['custom_' . $customField['id']] = $ruleField['rule_weight'];
+          $fields[$customField['custom_group_id.name'] . '.' . $customField['name']] = $ruleField['rule_weight'];
+        }
+      }
+      $this->dedupeRules[$name]['rule_message'] = ts('Missing required contact matching fields.') . " $fieldMessage " . ts('(Sum of all weights should be greater than or equal to threshold: %1).', [1 => $this->dedupeRules[$name]['threshold']]) . '<br />';
+
+      $this->dedupeRules[$name]['fields'] = $fields;
+    }
+    return $this->dedupeRules[$name];
+  }
+
   /**
    * This function adds the contact variable in $values to the
    * parameter list $params.  For most cases, $values should have length 1.  If
@@ -994,6 +1049,7 @@ abstract class CRM_Import_Parser implements UserJobInterface {
    *
    * @return bool|CRM_Utils_Error
    *
+   * @throws \CRM_Core_Exception
    * @deprecated
    */
   private function _civicrm_api3_deprecated_add_formatted_param(&$values, &$params) {
@@ -1695,6 +1751,9 @@ abstract class CRM_Import_Parser implements UserJobInterface {
 
   /**
    * Get the field metadata for fields to be be offered to match the contact.
+   * @todo this is very similar to getContactFields - this is called by participant and that
+   * by contribution import. They should be reconciled - but note that one is being fixed
+   * to support api4 style fields on contribution import - with this import to follow.
    *
    * @return array
    * @noinspection PhpDocMissingThrowsInspection
@@ -2275,4 +2334,16 @@ abstract class CRM_Import_Parser implements UserJobInterface {
     return $contactID;
   }
 
+  /**
+   * Get the fields for the dedupe rule.
+   *
+   * @param string $contactType
+   *
+   * @return array
+   * @throws \CRM_Core_Exception
+   */
+  protected function getDedupeFields(string $contactType): array {
+    return $this->getDedupeRule($contactType)['fields'];
+  }
+
 }
diff --git a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php
index 5e28e86c1c..babfd41f96 100644
--- a/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php
+++ b/tests/phpunit/CRM/Contribute/Import/Parser/ContributionTest.php
@@ -186,8 +186,6 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
     $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE);
     $contribution = $this->callAPISuccess('Contribution', 'get', ['contact_id' => $contactID, $this->getCustomFieldName('radio') => 'Red Testing']);
     $this->assertEquals(5, $contribution['values'][$contribution['id']]['custom_' . $this->ids['CustomField']['radio']]);
-    $this->callAPISuccess('CustomField', 'delete', ['id' => $this->ids['CustomField']['radio']]);
-    $this->callAPISuccess('CustomGroup', 'delete', ['id' => $this->ids['CustomGroup']['Custom Group']]);
   }
 
   /**
@@ -243,7 +241,7 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
     ]);
     $parser = new CRM_Contribute_Import_Parser_Contribution();
     $parser->setUserJobID($this->getUserJobID());
-    $fields = $parser->getAvailableFields();
+    $fields = $parser->getFieldsMetadata();
     $this->assertArrayHasKey('phone', $fields);
     $this->callApiSuccess('RuleGroup', 'create', [
       'id' => $unsupervisedRuleGroup['id'],
@@ -274,8 +272,8 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
     $this->runImport($values, CRM_Import_Parser::DUPLICATE_UPDATE, NULL);
 
     $updatedContribution = $this->callAPISuccessGetSingle('Contribution', ['id' => $initialContribution['id']]);
-    $this->assertNotContains('L', $updatedContribution[$customField], "Contribution Duplicate Update Import does not contain L");
-    $this->assertContains('V', $updatedContribution[$customField], "Contribution Duplicate Update Import contains V");
+    $this->assertNotContains('L', $updatedContribution[$customField], 'Contribution Duplicate Update Import does not contain L');
+    $this->assertContains('V', $updatedContribution[$customField], 'Contribution Duplicate Update Import contains V');
 
   }
 
@@ -350,7 +348,7 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
       ['name' => 'financial_type_id'],
       ['name' => 'total_amount'],
     ];
-    foreach ($data['fields']  as $field) {
+    foreach ($data['fields'] as $field) {
       $mappings[] = ['name' => $field === 'custom' ? $this->getCustomFieldName() : $field];
     }
     $this->submitDataSourceForm('contributions.csv', ['onDuplicate' => CRM_Import_Parser::DUPLICATE_SKIP]);
@@ -372,18 +370,18 @@ class CRM_Contribute_Import_Parser_ContributionTest extends CiviUnitTestCase {
    * @return array
    */
   public function validateData(): array {
-   return [
-     'email_first_name_last_name' => [['fields' => ['email', 'first_name', 'last_name'], 'valid' => TRUE]],
-     'email_last_name' => [['fields' => ['email', 'last_name'], 'valid' => TRUE]],
-     'email_first_name' => [['fields' => ['email', 'first_name',], 'valid' => TRUE]],
-     'first_name_last_name' => [['fields' => ['first_name', 'last_name'], 'valid' => TRUE]],
-     'email' => [['fields' => ['email'], 'valid' => TRUE]],
-     'first_name' => [['fields' => ['first_name'], 'valid' => FALSE]],
-     'last_name' => [['fields' => ['last_name'], 'valid' => FALSE]],
-     'last_name_custom' => [['fields' => ['last_name', 'custom'], 'valid' => TRUE]],
-     'first_name_custom' => [['fields' => ['first_name', 'custom'], 'valid' => TRUE]],
-     'custom' => [['fields' => ['first_name', 'custom'], 'valid' => FALSE]],
-   ];
+    return [
+      'email_first_name_last_name' => [['fields' => ['email', 'first_name', 'last_name'], 'valid' => TRUE]],
+      'email_last_name' => [['fields' => ['email', 'last_name'], 'valid' => TRUE]],
+      'email_first_name' => [['fields' => ['email', 'first_name'], 'valid' => TRUE]],
+      'first_name_last_name' => [['fields' => ['first_name', 'last_name'], 'valid' => TRUE]],
+      'email' => [['fields' => ['email'], 'valid' => TRUE]],
+      'first_name' => [['fields' => ['first_name'], 'valid' => FALSE]],
+      'last_name' => [['fields' => ['last_name'], 'valid' => FALSE]],
+      'last_name_custom' => [['fields' => ['last_name', 'custom'], 'valid' => TRUE]],
+      'first_name_custom' => [['fields' => ['first_name', 'custom'], 'valid' => TRUE]],
+      'custom' => [['fields' => ['custom'], 'valid' => FALSE]],
+    ];
   }
 
   /**