Merge pull request #3071 from eileenmcnaughton/CRM-14449

CRM-14449 add test to check custom data (& some fixes to pass)

api/v3/Activity.php

@@ -64,8 +64,6 @@ function civicrm_api3_activity_create($params) {
     );
   }
 
-  $errors = array();
-
   // check for various error and required conditions
   // note that almost all the processing in there should be managed by the wrapper layer
   // & should be removed - needs testing
@@ -79,7 +77,7 @@ function civicrm_api3_activity_create($params) {
 
 
   // processing for custom data
-  $values = array();
+  $values = $activityArray = array();
   _civicrm_api3_custom_format_params($params, $values, 'Activity');
 
   if (!empty($values['custom'])) {
@@ -104,7 +102,7 @@ function civicrm_api3_activity_create($params) {
         CRM_Activity_BAO_Activity::retrieve($oldActivityParams, $oldActivityValues);
       }
       if (empty($oldActivityValues)) {
-        return civicrm_api3_create_error(ts("Unable to locate existing activity."));
+        throw new API_Exception(ts("Unable to locate existing activity."));
       }
       else {
         $activityDAO = new CRM_Activity_DAO_Activity();
@@ -114,7 +112,7 @@ function civicrm_api3_activity_create($params) {
           if (is_object($activityDAO)) {
             $activityDAO->free();
           }
-          return civicrm_api3_create_error(ts("Unable to revision existing case activity."));
+          throw new API_Exception(ts("Unable to revision existing case activity."));
         }
         $createRevision = TRUE;
       }
@@ -157,7 +155,7 @@ function civicrm_api3_activity_create($params) {
       $params['original_id'] = empty($oldActivityValues['original_id']) ? $oldActivityValues['id'] : $oldActivityValues['original_id'];
     }
     else {
-      return civicrm_api3_create_error(ts("Unable to create new revision of case activity."));
+      throw new API_Exception(ts("Unable to create new revision of case activity."));
     }
   }
 
@@ -294,9 +292,9 @@ function _civicrm_api3_activity_get_formatResult($params, $activities) {
   }
   if (!empty($activities) && (!empty($returnProperties) || !empty($params['contact_id']))) {
     foreach ($activities as $activityId => $values) {
-      _civicrm_api3_custom_data_get($activities[$activityId], 'Activity', $activityId, NULL, $values['activity_type_id']);
+      //@todo - should possibly load activity type id if not loaded (update with id)
+      _civicrm_api3_custom_data_get($activities[$activityId], 'Activity', $activityId, NULL, CRM_Utils_Array::value('activity_type_id', $values));
     }
-    return $activities;
   }
   return $activities;
 }
@@ -308,6 +306,7 @@ function _civicrm_api3_activity_get_formatResult($params, $activities) {
  * @param array $params array holding 'id' of activity to be deleted
  * {@getfields activity_delete}
  *
+ * @throws API_Exception
  * @return void|CRM_Core_Error  An error if 'activityName or ID' is invalid,
  *                         permissions are insufficient, etc. or CiviCRM success array
  *
@@ -315,7 +314,6 @@ function _civicrm_api3_activity_get_formatResult($params, $activities) {
  *
  * @example ActivityDelete.php Standard Delete Example
  *
- *
  */
 function civicrm_api3_activity_delete($params) {
 
@@ -323,15 +321,18 @@ function civicrm_api3_activity_delete($params) {
     return civicrm_api3_create_success(1, $params, 'activity', 'delete');
   }
   else {
-    return civicrm_api3_create_error('Could not delete activity');
+    throw new API_Exception('Could not delete activity');
   }
 }
 
 /**
  * Function to check for required params
  *
- * @param array   $params  associated array of fields
- * @param boolean $addMode true for add mode
+ * @param array $params associated array of fields
+ *
+ * @throws API_Exception
+ * @throws Exception
+ * @internal param bool $addMode true for add mode
  *
  * @return array $error array with errors
  */
@@ -370,7 +371,7 @@ SELECT  count(*)
   FROM  civicrm_contact
  WHERE  id IN (' . implode(', ', $contactIds) . ' )';
     if (count($contactIds) != CRM_Core_DAO::singleValueQuery($sql)) {
-      return civicrm_api3_create_error('Invalid ' .  ' Contact Id');
+      throw new API_Exception('Invalid ' .  ' Contact Id');
     }
   }
 
@@ -384,7 +385,7 @@ SELECT  count(*)
     if ($value &&
       !CRM_Core_DAO::getFieldValue('CRM_Activity_DAO_Activity', $value, 'id')
     ) {
-      return civicrm_api3_create_error('Invalid ' . ucfirst($id) . ' Id');
+      throw new API_Exception('Invalid ' . ucfirst($id) . ' Id');
     }
   }
   // this should be handled by wrapper layer & probably the api would already manage it
@@ -408,21 +409,21 @@ SELECT  count(*)
       throw new Exception($errorString);
     }
     elseif ($activityTypeId && ($activityTypeId != $activityTypeIdInList)) {
-      return civicrm_api3_create_error('Mismatch in Activity');
+      throw new API_Exception('Mismatch in Activity');
     }
     $params['activity_type_id'] = $activityTypeIdInList;
   }
   elseif ($activityTypeId &&
     !array_key_exists($activityTypeId, $activityTypes)
   ) {
-    return civicrm_api3_create_error('Invalid Activity Type ID');
+    throw new API_Exception('Invalid Activity Type ID');
   }
 
   // check for activity duration minutes
   // this should be validated @ the wrapper layer not here
   // needs testing
   if (isset($params['duration_minutes']) && !is_numeric($params['duration_minutes'])) {
-    return civicrm_api3_create_error('Invalid Activity Duration (in minutes)');
+    throw new API_Exception('Invalid Activity Duration (in minutes)');
   }
 
 

api/v3/Case.php

@@ -40,8 +40,8 @@
 /**
  * Open a new case, add client and manager roles, and add standard timeline
  *
- * @param  array(
-    //REQUIRED:
+ * @param  array (
+ * //REQUIRED:
  * 'case_type_id' => int OR
  * 'case_type' => str (provide one or the other)
  * 'contact_id' => int // case client
@@ -56,7 +56,8 @@
  * 'duration' => int // in minutes
  * 'details' => str // html format
  *
- * @return sucessfully opened case
+ * @throws API_Exception
+ * @return array api result array
  *
  * @access public
  * {@getfields case_create}
@@ -72,10 +73,10 @@ function civicrm_api3_case_create($params) {
 
   // If format_params didn't find what it was looking for, return error
   if (empty($params['case_type_id'])) {
-    return civicrm_api3_create_error('Invalid case_type. No such case type exists.');
+    throw new API_Exception('Invalid case_type. No such case type exists.');
   }
   if (empty($params['case_type'])) {
-    return civicrm_api3_create_error('Invalid case_type_id. No such case type exists.');
+    throw new API_Exception('Invalid case_type_id. No such case type exists.');
   }
 
   // Fixme: can we safely pass raw params to the BAO?
@@ -91,7 +92,7 @@ function civicrm_api3_case_create($params) {
   $caseBAO = CRM_Case_BAO_Case::create($newParams);
 
   if (!$caseBAO) {
-    return civicrm_api3_create_error('Case not created. Please check input params.');
+    throw new API_Exception('Case not created. Please check input params.');
   }
 
   foreach ((array) $params['contact_id'] as $cid) {
@@ -134,7 +135,6 @@ function civicrm_api3_case_create($params) {
 function _civicrm_api3_case_get_spec(&$params) {
   $params['contact_id']['api.aliases'] = array('client_id');
   $params['contact_id']['title'] = 'Case Client';
-  $params['creator_id']['api.default'] = 'user_contact_id';
 }
 
 /**
@@ -147,6 +147,9 @@ function _civicrm_api3_case_create_spec(&$params) {
   $params['contact_id']['title'] = 'Case Client';
   $params['contact_id']['api.required'] = 1;
   $params['status_id']['api.default'] = 1;
+  $params['creator_id']['api.default'] = 'user_contact_id';
+  $params['creator_id']['type'] = CRM_Utils_Type::T_INT;
+  $params['start_date']['api.default'] = 'now';
   $params['medium_id'] = array(
     'name' => 'medium_id',
     'title' => 'Activity Medium',
@@ -176,8 +179,8 @@ function _civicrm_api3_case_delete_spec(&$params) {
  *
  * Please provide one (and only one) of the four get/search parameters:
  *
- * @param array(
-    'id' => if set, will get all available info about a case, including contacts and activities
+ * @param array (
+ * 'id' => if set, will get all available info about a case, including contacts and activities
  *
  * // if no case_id provided, this function will use one of the following search parameters:
  * 'client_id' => finds all cases with a specific client
@@ -186,8 +189,7 @@ function _civicrm_api3_case_delete_spec(&$params) {
  *
  * {@getfields case_get}
  *
- * @return (get mode, case_id provided): Array with case details, case roles, case activity ids, (search mode, case_id not provided): Array of cases found
- * @access public
+ * @return array (get mode, case_id provided): Array with case details, case roles, case activity ids, (search mode, case_id not provided): Array of cases found@access public
  * @todo Erik Hommel 16 dec 2010 check if all DB fields are returned
  */
 function civicrm_api3_case_get($params) {
@@ -213,7 +215,7 @@ function civicrm_api3_case_get($params) {
   //search by activity
   if (!empty($params['activity_id'])) {
     if (!is_numeric($params['activity_id'])) {
-      return civicrm_api3_create_error('Invalid parameter: activity_id. Must provide a numeric value.');
+      throw new API_Exception('Invalid parameter: activity_id. Must provide a numeric value.');
     }
     $caseId = CRM_Case_BAO_Case::getCaseIdByActivityId($params['activity_id']);
     if (!$caseId) {
@@ -226,7 +228,7 @@ function civicrm_api3_case_get($params) {
   //search by contacts
   if ($contact = CRM_Utils_Array::value('contact_id', $params)) {
     if (!is_numeric($contact)) {
-      return civicrm_api3_create_error('Invalid parameter: contact_id.  Must provide a numeric value.');
+      throw new API_Exception('Invalid parameter: contact_id.  Must provide a numeric value.');
     }
 
     $sql = "
@@ -271,8 +273,8 @@ function civicrm_api3_case_activity_create($params) {
 /**
  * Update a specified case.
  *
- * @param  array(
-    //REQUIRED:
+ * @param  array (
+ * //REQUIRED:
  * 'case_id' => int
  *
  * //OPTIONAL
@@ -280,22 +282,21 @@ function civicrm_api3_case_activity_create($params) {
  * 'start_date' => str datestamp
  * 'contact_id' => int // case client
  *
- * @return Updated case
+ * @throws API_Exception
+ * @return array api result array
  *
  * @access public
- *
  */
 function civicrm_api3_case_update($params) {
   //check parameters
   civicrm_api3_verify_mandatory($params, NULL, array('id'));
 
-  // return error if modifing creator id
+  // return error if modifying creator id
   if (array_key_exists('creator_id', $params)) {
-    return civicrm_api3_create_error(ts('You cannot update creator id'));
+    throw new API_Exception(ts('You cannot update creator id'));
   }
 
-  $mCaseId = array();
-  $origContactIds = array();
+  $mCaseId = $origContactIds = array();
 
   // get original contact id and creator id of case
   if (!empty($params['contact_id'])) {
@@ -306,10 +307,10 @@ function civicrm_api3_case_update($params) {
   if (count($origContactIds) > 1) {
     // check valid orig contact id
     if (!empty($params['orig_contact_id']) && !in_array($params['orig_contact_id'], $origContactIds)) {
-      return civicrm_api3_create_error('Invalid case contact id (orig_contact_id)');
+      throw new API_Exception('Invalid case contact id (orig_contact_id)');
     }
     elseif (empty($params['orig_contact_id'])) {
-      return civicrm_api3_create_error('Case is linked with more than one contact id. Provide the required params orig_contact_id to be replaced');
+      throw new API_Exception('Case is linked with more than one contact id. Provide the required params orig_contact_id to be replaced');
     }
     $origContactId = $params['orig_contact_id'];
   }
@@ -358,7 +359,7 @@ function civicrm_api3_case_delete($params) {
     return civicrm_api3_create_success($params, $params, 'case', 'delete');
   }
   else {
-    return civicrm_api3_create_error('Could not delete case.');
+    throw new API_Exception('Could not delete case.');
   }
 }
 
@@ -373,7 +374,9 @@ function civicrm_api3_case_delete($params) {
  *
  * @param int $caseId
  *
- * @param $params
+ * @param $options
+ *
+ * @internal param $params
  *
  * @internal param $options
  *

api/v3/Group.php

@@ -53,17 +53,7 @@
  * @access public
  */
 function civicrm_api3_group_create($params) {
-
-  $group = CRM_Contact_BAO_Group::create($params);
-
-  if (is_null($group)) {
-    return civicrm_api3_create_error('Group not created');
-  }
-  else {
-    $values = array();
-    _civicrm_api3_object_to_array_unique_fields($group, $values[$group->id]);
-    return civicrm_api3_create_success($values, $params, 'group', 'create', $group);
-  }
+  return _civicrm_api3_basic_create(_civicrm_api3_get_BAO(__FUNCTION__), $params, 'Group');
 }
 
 /**
@@ -90,34 +80,16 @@ function _civicrm_api3_group_create_spec(&$params) {
  * @access public
  */
 function civicrm_api3_group_get($params) {
-
-  $options          = _civicrm_api3_get_options_from_params($params, TRUE, 'group', 'get');
-  $sort             = CRM_Utils_Array::value('sort', $options, NULL);
-  $offset           = CRM_Utils_Array::value('offset', $options);
-  $rowCount         = CRM_Utils_Array::value('limit', $options);
-  $returnProperties = CRM_Utils_Array::value('return', $options, NULL);
-  $inputParams      = CRM_Utils_Array::value('input_params', $options, array());
-  if(is_array($returnProperties) && !empty($returnProperties)){
-    // group function takes $returnProperties in non standard format & doesn't add id
-    unset($returnProperties['group_id']);
-    $returnProperties['id'] = 1;
-    $returnProperties = array_keys($returnProperties);
-  }
-  if (!empty($inputParams['group_id'])) {
-    $inputParams['id'] = $inputParams['group_id'];
-  }
-  $groupObjects = CRM_Contact_BAO_Group::getGroups($inputParams, $returnProperties, $sort, $offset, $rowCount);
-  if (empty($groupObjects)) {
-    return civicrm_api3_create_success(FALSE);
-  }
-  $groups = array();
-  foreach ($groupObjects as $group) {
-    _civicrm_api3_object_to_array($group, $groups[$group->id]);
-    _civicrm_api3_custom_data_get($groups[$group->id], 'Group', $group->id);
+  $options = _civicrm_api3_get_options_from_params($params, TRUE, 'group', 'get');
+  if(empty($options['return']) || !in_array('member_count', $options['return'])) {
+    return _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, TRUE, 'Group');
   }
 
-
-  return civicrm_api3_create_success($groups, $params, 'group', 'create');
+  $groups = _civicrm_api3_basic_get(_civicrm_api3_get_BAO(__FUNCTION__), $params, FALSE, 'Group');
+  foreach ($groups as $id => $group) {
+    $groups[$id]['member_count'] = CRM_Contact_BAO_Group::memberCount($id);
+  }
+  return civicrm_api3_create_success($groups, $params, 'group', 'get');
 }
 
 /**

api/v3/utils.php

@@ -804,11 +804,13 @@ function _civicrm_api3_get_unique_name_array(&$bao) {
 /**
  * Converts an DAO object to an array
  *
- * @param  object $dao           (reference )object to convert
+ * @param  object $dao (reference )object to convert
  * @param null $params
  * @param bool $uniqueFields
  * @param string $entity
  *
+ * @param bool $autoFind
+ *
  * @return array
  *
  * @params array of arrays (key = id) of array of fields
@@ -1562,7 +1564,6 @@ function _civicrm_api3_swap_out_aliases(&$apiRequest, $fields) {
  * @internal param array $fieldinfo array of fields from getfields function
  */
 function _civicrm_api3_validate_integer(&$params, &$fieldName, &$fieldInfo, $entity) {
-  //if fieldname exists in params
   if (!empty($params[$fieldName])) {
     // if value = 'user_contact_id' (or similar), replace value with contact id
     if (!is_numeric($params[$fieldName]) && is_scalar($params[$fieldName])) {

tests/phpunit/api/v3/SyntaxConformanceTest.php

@@ -118,6 +118,22 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase {
     return static::entities(static::toBeSkipped_delete(TRUE));
   }
 
+  public static function custom_data_entities_get() {
+    return static::custom_data_entities();
+  }
+
+  public static function custom_data_entities() {
+   $entities = CRM_Core_BAO_CustomQuery::$extendsMap;
+   $customDataEntities = array();
+    $invalidEntities = array('Individual', 'Organization', 'Household');
+   foreach ($entities as $entityName => $entity ) {
+     if(!in_array($entityName, $invalidEntities)) {
+       $customDataEntities[] = array($entityName );
+     }
+   }
+    return $customDataEntities;
+  }
+
   public static function toBeSkipped_get($sequential = FALSE) {
     $entitiesWithoutGet = array('MailingEventSubscribe', 'MailingEventConfirm', 'MailingEventResubscribe', 'MailingEventUnsubscribe', 'MailingGroup', 'Location');
     if ($sequential === TRUE) {
@@ -451,6 +467,26 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase {
     }
   }
 
+  /**
+   * @dataProvider custom_data_entities_get
+   */
+  public function testCustomDataGet($entityName) {
+    $this->createLoggedInUser();// so subsidiary activities are created
+    $ids = $this->entityCustomGroupWithSingleFieldCreate(__FUNCTION__, $entityName . 'Test.php');
+    $customFieldName = 'custom_' . $ids['custom_field_id'];
+    $objects = $this->getMockableBAOObjects($entityName, 1);
+    $params = array('id' => $objects[0]->id, 'custom_' . $ids['custom_field_id'] => "custom string");
+    $result = $this->callAPISuccess($entityName, 'create', $params);
+
+    $getParams = array('id' => $result['id'], 'return' => array($customFieldName));
+    $check = $this->callAPISuccess($entityName, 'get', $getParams);
+    $this->assertEquals("custom string", $check['values'][$check['id']][$customFieldName]);
+
+    $this->customFieldDelete($ids['custom_field_id']);
+    $this->customGroupDelete($ids['custom_group_id']);
+    $this->callAPISuccess($entityName, 'delete', array('id' => $result['id']));
+  }
+
   /**
    * @dataProvider entities_get
    */