From 9fe6051a27cf9136f652ba1da9f7e20bcb2c61d8 Mon Sep 17 00:00:00 2001
From: eileen <eileen@fuzion.co.nz>
Date: Fri, 22 Mar 2013 11:46:51 +1300
Subject: [PATCH] fixes to value and weight handling CRM-12133

---
 CRM/Core/BAO/OptionValue.php | 50 ++++++++++++++++++++++++------------
 api/v3/CustomSearch.php      |  1 -
 api/v3/ReportTemplate.php    |  1 -
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/CRM/Core/BAO/OptionValue.php b/CRM/Core/BAO/OptionValue.php
index 7c949e6962..8098ed1d63 100644
--- a/CRM/Core/BAO/OptionValue.php
+++ b/CRM/Core/BAO/OptionValue.php
@@ -40,11 +40,9 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
   function __construct() {
     parent::__construct();
   }
-  /*
+  /**
    * Create option value - note that the create function calls 'add' but
   * has more business logic
-  * Note that this is the right place to add pre & post hooks if we want them
-  * ? any reason not to?
   *
   * @param array $params input parameters
   */
@@ -64,11 +62,13 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
    * This functions sets default parameters if not set:
    * - name & label are set to each other as required (it might make more sense for one
    * to be required but this would mean a change to the api level)
-   * - ditto weight & value - but they both default to next weight
-   * NB am not sure that weight should be set to value as higher priority to
-   * setting it to the next weight - although this is existing logic
+   * - weight & value will be set to their respective option groups next values
+   * if nothing is passed in.
+   *
+   * Note this function does not check for presence of $params['id'] so should only be called
+   * if 'id' is not present
    *
-   * @param unknown_type $params
+   * @param array $params
    */
   static function setDefaults(&$params){
     if(empty($params['label'])){
@@ -78,19 +78,32 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
       $params['name'] = $params['label'];
     }
     if(empty($params['weight'])){
-      //@todo consider this logic - see block comments
-      $params['weight'] = CRM_Utils_Array::value('value', $params,
-        (int) CRM_Utils_Weight::getDefaultWeight('CRM_Core_DAO_OptionValue',
-          array('option_group_id' => $params['option_group_id']))
-      );
+      $params['weight'] = (int) CRM_Utils_Weight::getDefaultWeight('CRM_Core_DAO_OptionValue',
+          array('option_group_id' => $params['option_group_id']));
     }
     if(empty($params['value'])){
-      $params['value'] = CRM_Utils_Array::value('weight', $params,
-        (int) CRM_Utils_Weight::getDefaultWeight('CRM_Core_DAO_OptionValue',
-          array('option_group_id' => $params['option_group_id']))
-      );
+      $params['value'] = self::getNextValue($params);
     }
   }
+  /**
+   * Get next available value
+   * We will take the highest numeric value (or 0 if no numeric values exist)
+   * and add one. The calling function is responsible for any
+   * more complex decision making
+   * @param array $params
+   */
+  static function getNextValue($params){
+     $bao = new CRM_Core_BAO_OptionValue();
+     $bao->option_group_id = $params['option_group_id'];
+     if(isset($params['domain_id'])){
+       $bao->domain_id = $params['domain_id'];
+     }
+     $bao->selectAdd();
+     $bao->whereAdd("value REGEXP '^[0-9]+$'");
+     $bao->selectAdd('(ROUND(COALESCE(MAX(value),0)) +1) as nextvalue');
+     $bao->find(TRUE);
+     return $bao->nextvalue;
+  }
   /**
    * Takes a bunch of params that are needed to match certain criteria and
    * retrieves the relevant objects. Typically the valid params are only
@@ -141,6 +154,10 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
    */
   static function add(&$params, &$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 (!CRM_Utils_Array::value('optionValue', $ids)) {
       $params['is_active'] = CRM_Utils_Array::value('is_active', $params, FALSE);
       $params['is_default'] = CRM_Utils_Array::value('is_default', $params, FALSE);
@@ -167,7 +184,6 @@ class CRM_Core_BAO_OptionValue extends CRM_Core_DAO_OptionValue {
       $p = array(1 => array($params['option_group_id'], 'Integer'));
       CRM_Core_DAO::executeQuery($query, $p);
     }
-
     $groupName = CRM_Core_DAO::getFieldValue('CRM_Core_DAO_OptionGroup',
       $params['option_group_id'], 'name', 'id'
     );
diff --git a/api/v3/CustomSearch.php b/api/v3/CustomSearch.php
index fe36d0126c..1860ae48e5 100644
--- a/api/v3/CustomSearch.php
+++ b/api/v3/CustomSearch.php
@@ -54,7 +54,6 @@ function civicrm_api3_custom_search_create($params) {
 function _civicrm_api3_custom_search_create_spec(&$params) {
   require_once 'api/v3/OptionValue.php';
   _civicrm_api3_option_value_create_spec($params);
-  $params['weight']['api.default'] = 'next';
   $params['name']['api.aliases'] = array('class_name');
 }
 
diff --git a/api/v3/ReportTemplate.php b/api/v3/ReportTemplate.php
index 86d748ccb3..1ad88c2a47 100644
--- a/api/v3/ReportTemplate.php
+++ b/api/v3/ReportTemplate.php
@@ -50,7 +50,6 @@ function civicrm_api3_report_template_create($params) {
 function _civicrm_api3_report_template_create_spec(&$params) {
   require_once 'api/v3/OptionValue.php';
   _civicrm_api3_option_value_create_spec($params);
-  $params['weight']['api.default'] = 'next';
   $params['value']['api.aliases'] = array('report_url');
   $params['name']['api.aliases'] = array('class_name');
   // $params['component']['api.required'] = TRUE;
-- 
2.25.1