dev/core#3063 APIv3 - Fix numeric option matching

Before: Option matching was skipped for all FK fields if a numeric value was given
After: Only skipped for `campaign_id` field, if positive integer given

The optimization was overly broad and had unintended side-effects

diff --git a/api/v3/utils.php b/api/v3/utils.php
index 1bbc77ef459fb4bc9970cd99007db506c12fd292..a7ce83160e70e303744f34ef99e70cf3e937e9a0 100644 (file)
--- a/api/v3/utils.php
+++ b/api/v3/utils.php
@@ -2077,10 +2077,10 @@ function _civicrm_api3_validate_integer(&$params, $fieldName, &$fieldInfo, $enti
       }
     }
     if (
-    (!empty($fieldInfo['pseudoconstant']) || !empty($fieldInfo['options']) || $fieldName === 'campaign_id')
-     // if it is already numeric AND it is an FK field we don't need to validate because
-     // sql will do that for us on insert (this also saves a big lookup)
-     && (!is_numeric($fieldValue) || empty($fieldInfo['FKClassName']))
+      !empty($fieldInfo['pseudoconstant']) ||
+      !empty($fieldInfo['options']) ||
+      // Special case for campaign_id which is no longer a pseudoconstant
+      ($fieldName === 'campaign_id' && !CRM_Utils_Rule::positiveInteger($fieldValue))
     ) {
       $additional_lookup_params = [];
       if (strtolower($entity) === 'address' && $fieldName == 'state_province_id') {

diff --git a/tests/phpunit/api/v3/FinancialTypeTest.php b/tests/phpunit/api/v3/FinancialTypeTest.php
index 4b9a65312389e6798757401b8b4ba0abee9f95f2..44805af8d0c577ad5de53d1c0093b688a52b04cb 100644 (file)
--- a/tests/phpunit/api/v3/FinancialTypeTest.php
+++ b/tests/phpunit/api/v3/FinancialTypeTest.php
@@ -114,4 +114,48 @@ class api_v3_FinancialTypeTest extends CiviUnitTestCase {
     $this->assertEquals('INC', $result['account_type_code'], 'Financial account created is not an income account.');
   }
 
+  public function testMatchFinancialTypeOptions() {
+    // Just a string name, should be simple to match on
+    $nonNumericOption = $this->callAPISuccess('FinancialType', 'create', [
+      'name' => 'StringName',
+    ])['id'];
+    // A numeric name, but a number that won't match any existing id
+    $numericOptionUnique = $this->callAPISuccess('FinancialType', 'create', [
+      'name' => '999',
+    ])['id'];
+    // Here's the kicker, a numeric name that matches an existing id!
+    $numericOptionMatchingExistingId = $this->callAPISuccess('FinancialType', 'create', [
+      'name' => $nonNumericOption,
+    ])['id'];
+    $cid = $this->individualCreate();
+
+    // Create a contribution matching non-numeric name
+    $contributionWithNonNumericType = $this->callAPISuccess('Contribution', 'create', [
+      'financial_type_id' => 'StringName',
+      'total_amount' => 100,
+      'contact_id' => $cid,
+      'sequential' => TRUE,
+    ]);
+    $this->assertEquals($nonNumericOption, $contributionWithNonNumericType['values'][0]['financial_type_id']);
+
+    // Create a contribution matching unique numeric name
+    $contributionWithUniqueNumericType = $this->callAPISuccess('Contribution', 'create', [
+      'financial_type_id' => '999',
+      'total_amount' => 100,
+      'contact_id' => $cid,
+      'sequential' => TRUE,
+    ]);
+    $this->assertEquals($numericOptionUnique, $contributionWithUniqueNumericType['values'][0]['financial_type_id']);
+
+    // Create a contribution matching the id of the non-numeric option, which is ambiguously the name of another option
+    $contributionWithAmbiguousNumericType = $this->callAPISuccess('Contribution', 'create', [
+      'financial_type_id' => "$nonNumericOption",
+      'total_amount' => 100,
+      'contact_id' => $cid,
+      'sequential' => TRUE,
+    ]);
+    // The id should have taken priority over matching by name
+    $this->assertEquals($nonNumericOption, $contributionWithAmbiguousNumericType['values'][0]['financial_type_id']);
+  }
+
 }