CRM-21737 loosen option value validation to support language option group
authoreileen <emcnaughton@wikimedia.org>
Mon, 5 Feb 2018 20:22:29 +0000 (09:22 +1300)
committereileen <emcnaughton@wikimedia.org>
Tue, 6 Feb 2018 00:18:07 +0000 (13:18 +1300)
CRM/Core/BAO/OptionValue.php
tests/phpunit/api/v3/OptionValueTest.php

index 9bb61d0543889e01eb58509ab799a4cd3e490efe..429d14ac782f09b3907b0fce01e2090fa1e084bb 100644 (file)
@@ -164,18 +164,19 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
    * @param array $params
    *   Reference array contains the values submitted by the form.
    * @param array $ids
-   *   Reference array contains the id.
+   *   deprecated Reference array contains the id.
    *
-   *
-   * @return CRM_Core_DAO_OptionValue
+   * @return \CRM_Core_DAO_OptionValue
+   * @throws \CRM_Core_Exception
    */
   public static function add(&$params, $ids = array()) {
+    $id = CRM_Utils_Array::value('id', $params, CRM_Utils_Array::value('optionValue', $ids));
     // CRM-10921: do not reset attributes to default if this is an update
     //@todo consider if defaults are being set in the right place. 'dumb' defaults like
     // these would be usefully set @ the api layer so they are visible to api users
     // complex defaults like the domain id below would make sense in the setDefauls function
     // but unclear what other ways this function is being used
-    if (empty($ids['optionValue'])) {
+    if (!$id) {
       $params['is_active'] = CRM_Utils_Array::value('is_active', $params, FALSE);
       $params['is_default'] = CRM_Utils_Array::value('is_default', $params, FALSE);
       $params['is_optgroup'] = CRM_Utils_Array::value('is_optgroup', $params, FALSE);
@@ -183,9 +184,19 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
     }
     // Update custom field data to reflect the new value
     elseif (isset($params['value'])) {
-      CRM_Core_BAO_CustomOption::updateValue($ids['optionValue'], $params['value']);
+      CRM_Core_BAO_CustomOption::updateValue($id, $params['value']);
     }
 
+    // We need to have option_group_id populated for validation so load if necessary.
+    if (empty($params['option_group_id'])) {
+      $params['option_group_id'] = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionValue',
+        $id, 'option_group_id', 'id'
+      );
+    }
+    $groupName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup',
+      $params['option_group_id'], 'name', 'id'
+    );
+
     // action is taken depending upon the mode
     $optionValue = new CRM_Core_DAO_OptionValue();
     $optionValue->copyValues($params);
@@ -206,33 +217,30 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
       CRM_Core_DAO::executeQuery($query, $p);
     }
 
-    // CRM-13814 : evalute option group id
-    if (!array_key_exists('option_group_id', $params) && !empty($ids['optionValue'])) {
-      $groupId = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionValue',
-        $ids['optionValue'], 'option_group_id', 'id'
-      );
-    }
-    else {
-      $groupId = $params['option_group_id'];
+    if (empty($params['domain_id']) && in_array($groupName, CRM_Core_OptionGroup::$_domainIDGroups)) {
+      $optionValue->domain_id = CRM_Core_Config::domainID();
     }
 
-    $groupName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup',
-      $groupId, 'name', 'id'
-    );
-    if (empty($ids['optionValue']) && empty($params['id']) && !empty($params['value'])) {
-      $domainSpecifc = in_array($groupName, CRM_Core_OptionGroup::$_domainIDGroups) ? TRUE : FALSE;
+    $groupsSupportingDuplicateValues = ['languages'];
+    if (!$id && !empty($params['value'])) {
       $dao = new CRM_Core_DAO_OptionValue();
-      $dao->value = $params['value'];
-      $dao->option_group_id = $groupId;
+      if (!in_array($groupName, $groupsSupportingDuplicateValues)) {
+        $dao->value = $params['value'];
+      }
+      else {
+        // CRM-21737 languages option group does not use unique values but unique names.
+        $dao->name = $params['name'];
+      }
+      if (in_array($groupName, CRM_Core_OptionGroup::$_domainIDGroups)) {
+        $dao->domain_id = $optionValue->domain_id;
+      }
+      $dao->option_group_id = $params['option_group_id'];
       if ($dao->find(TRUE)) {
         throw new CRM_Core_Exception('Value already exists in the database');
       }
     }
-    if (in_array($groupName, CRM_Core_OptionGroup::$_domainIDGroups)) {
-      $optionValue->domain_id = CRM_Utils_Array::value('domain_id', $params, CRM_Core_Config::domainID());
-    }
 
-    $optionValue->id = CRM_Utils_Array::value('optionValue', $ids);
+    $optionValue->id = $id;
     $optionValue->save();
     CRM_Core_PseudoConstant::flush();
     return $optionValue;
index 65593d919c5987a7cf8d508ab204b231af3fe54e..dc2195bfb16fca2c7de7a324804e8697e0d6a951 100644 (file)
@@ -378,7 +378,7 @@ class api_v3_OptionValueTest extends CiviUnitTestCase {
   }
 
   /**
-   * CRM-19346 Ensur that Option Values cannot share same value in the same option value group
+   * CRM-19346 Ensure that Option Values cannot share same value in the same option value group
    */
   public function testCreateOptionValueWithSameValue() {
     $og = $this->callAPISuccess('option_group', 'create', array(
@@ -390,11 +390,24 @@ class api_v3_OptionValueTest extends CiviUnitTestCase {
       array('option_group_id' => $og['id'], 'label' => 'test option value')
     );
     // update option value without 'option_group_id'
-    $ov2 = $this->callAPIFailure('option_value', 'create',
+    $this->callAPIFailure('option_value', 'create',
       array('option_group_id' => $og['id'], 'label' => 'Test 2nd option value', 'value' => $ov['values'][$ov['id']]['value'])
     );
   }
 
+  /**
+   * CRM-21737 Ensure that language Option Values CAN share same value.
+   */
+  public function testCreateOptionValueWithSameValueLanguagesException() {
+    $this->callAPISuccess('option_value', 'create',
+      ['option_group_id' => 'languages', 'label' => 'Quasi English', 'name' => 'en_Qu', 'value' => 'en']
+    );
+    $this->callAPISuccess('option_value', 'create',
+      ['option_group_id' => 'languages', 'label' => 'Semi English', 'name' => 'en_Se', 'value' => 'en']
+    );
+
+  }
+
   public function testCreateOptionValueWithSameValueDiffOptionGroup() {
     $og = $this->callAPISuccess('option_group', 'create', array(
       'name' => 'our test Option Group for with group id',