From d0488daf4eb90280a436db97d3047b274f6a1235 Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 6 Feb 2018 09:22:29 +1300 Subject: [PATCH] CRM-21737 loosen option value validation to support language option group --- CRM/Core/BAO/OptionValue.php | 56 ++++++++++++++---------- tests/phpunit/api/v3/OptionValueTest.php | 17 ++++++- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/CRM/Core/BAO/OptionValue.php b/CRM/Core/BAO/OptionValue.php index 9bb61d0543..429d14ac78 100644 --- a/CRM/Core/BAO/OptionValue.php +++ b/CRM/Core/BAO/OptionValue.php @@ -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; diff --git a/tests/phpunit/api/v3/OptionValueTest.php b/tests/phpunit/api/v3/OptionValueTest.php index 65593d919c..dc2195bfb1 100644 --- a/tests/phpunit/api/v3/OptionValueTest.php +++ b/tests/phpunit/api/v3/OptionValueTest.php @@ -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', -- 2.25.1