CiviCRM Community Forums (archive)

I just got done writing the CustomValue api and then read your post. Aside from being lazy and not wanting to change it all around, I can think of a couple reasons not to output in nested arrays:
1 it's less consistent and potentially confusing
2 the way I've implemented it, people have a choice of a simple custom_7 or they can use custom_group_name:field_name. And output will be given in same format as input (if they asked for the field by name, they'll get it by name, if they asked for it by ID, they'll get it by ID). Since the input is flat, I think the output should be flat, and since users use Id sometimes instead of groupName:fieldName, we can't expect them to know the names of things and know how to parse nested output.
3 No matter what we do, variables in smarty will be different than in PHP. They just are. Whether it's {$GroupName.FieldName} or {$GroupName_FieldName}, who cares as long as we're consistent. I vote we just convert ':' to '_' before sending to smarty.

Here is the new code:
<?php

/**
 * Files required for this package
 */
require_once 'api/v3/utils.php';
require_once 'CRM/Core/BAO/CustomField.php';
require_once 'CRM/Core/BAO/CustomValueTable.php';


/**
 * Sets custom values for an entity.
 *
 *
 * @param $params  expected keys are in format custom_fieldID:recordID or custom_groupName:fieldName:recordID
 * for example:
 * 'id' => 123, // entity ID. You do not need to specify entity type, we figure it out based on the fields you're using
 * 'custom_6' => 'foo', // (omitting :id) inserts or updates a field in a single-valued group
 * 'custom_24' => array('bar', 'baz'), // custom_24 is checkbox or multiselect, so pass items as an array
 * 'custom_33:5' => value, // in this case custom_33 is part of a multi-valued group, and we're updating record id 5
 * 'custom_33:-1' => value, // inserts new record in multi-valued group
 * 'custom_33:-2' => value, // inserts another new record in multi-valued group
 * 'custom_some_group:my_field => 'myinfo', // you can use group_name:field_name instead of ID
 * 'custom_some_big_group:my_other_field:8 => 'myinfo', // updates record ID 8 in my_other_field in multi-valued some_big_group
 *
 *
 * @return array('values' => TRUE) or array('is_error' => 1, 'error_message' => 'what went wrong')
 *
 * @access public
 * 
 */
function civicrm_api3_custom_value_create( $params ) {
  civicrm_api3_verify_mandatory($params, null, array('id'));

  $create = array('entityID' => $params['id']);

  // Translate names and
  //Convert arrays to multi-value strings
  $sp = CRM_Core_DAO::VALUE_SEPARATOR;
  foreach ($params as $id => $param) {
    if (is_array($param)) {
      $param = $sp . implode($sp, $param) . $sp;
    }
    list($c, $id) = explode('_', $id, 2);
    if ($c != 'custom') {
      continue;
    }
    list($i, $n, $x) = explode(':', $id);
    if (is_numeric($i)) {
      $key = $i;
      $x = $n;
    } else {
      // Lookup names if ID was not supplied
      $key = CRM_Core_BAO_CustomField::getCustomFieldID($i, $n);
      if (!$key) {
        continue;
      }
    }
    if ($x && is_numeric($x)) {
      $key .= '_' . $x;
    }
    $create['custom_' . $key] = $param;
  }

  $result = CRM_Core_BAO_CustomValueTable::setValues($create);
  if ($result['is_error']) {
    return civicrm_api3_create_error($result['error_message']);
  }
  return civicrm_api3_create_success(true, $params);
}


/**
 * Use this API to get existing custom values for an entity.
 *
 * @param $params  array specifying the id of the entity
 * Optionally include entity_type param, i.e. 'entity_type' => 'Activity'
 * If no entity_type is supplied, it will be determined based on the fields you request.
 * If no entity_type is supplied and no fields are specified, 'Contact' will be assumed and it will return all custom fields.
 * Optionally include the desired custom data to be fetched (or else all custom data for this entity will be returned)
 * Example: 'id' => 123, 'return.custom_6' => 1, 'return.custom_33' => 1
 * If you do not know the ID, you may use group name : field name, for example 'return.custom_foo_stuff:my_field' => 1
 * (note that custom is not part of the group name in this example; all keys should start with return.custom_)
 * If names are given instead of IDs, then data will be output in the same format.
 *
 * @return array. For fields in single-value custom fieldsets, format will be 'custom_6' => 'foo'
 * For multi-value fieldsets, format will be 'custom_6:11' => 'foo', 'custom_6:12' => 'bar', 'custom_6:13' => 'baz'
 *
 * @access public
 * 
 **/
function civicrm_api3_custom_value_get($params) {
  civicrm_api3_verify_mandatory($params, null, array('id'));

  $getParams = array('entityID' => $params['id']);
  $getParams['entityType'] = $params['entity_type'];
  foreach ($params as $id => $param) {
    if ($param && substr($id, 0, 6) == 'return') {
      list($c, $i) = explode('_', substr($id, 7), 2);
      if ($c != 'custom') {
        continue;
      }
      if (is_numeric($i)) {
        $names['custom_' . $i] = 'custom_' . $i;
        $id = $i;
      } else {
        // Lookup names if ID was not supplied
        list($group, $field) = explode(':', $i, 2);
        $id = CRM_Core_BAO_CustomField::getCustomFieldID($field, $group);
        if (!$id) {
          continue;
        }
        $names['custom_' .$id] = 'custom_' . $i;
      }
      $getParams['custom_' . $id] = 1;
    }
  }
  $result = CRM_Core_BAO_CustomValueTable::getValues($getParams);
  if ($result['is_error']) {
    return civicrm_api3_create_error($result['error_message']);
  } else {
    $entity_id = $result['entityID'];
    unset($result['is_error'], $result['entityID']);

    // Translate IDs to names if they were given that way and
    // Convert multi-value strings to arrays
    $sp = CRM_Core_DAO::VALUE_SEPARATOR;
    foreach ($result as $id => $value) {
      if (strpos($value, $sp) !== FALSE) {
        $value = explode($sp, trim($value, $sp));
      }
      list($c, $i, $n) = explode('_', $id);
      if ($name = $names[$c .'_'. $i]) {
        $id = $name;
      }
      elseif ($c == 'custom') {
        $id = $c .'_'. $i;
      }
      if ($n && $c == 'custom') {
        $id .= ':' . $n;
      }
      $values[$id] = $value;
    }
    return civicrm_api3_create_success(array($entity_id => $values), $params);
  }
}


NB - not 100% sure what you mean about the nested arrays. All api return arrays are nested to the extent that they follow the format in the api/v3/examples directory but assume that's not what you're referring to.

I don't mean indexing would be

['values']['grouipd_fieldID']['entity_id']['custom_123']

the entity_id would be dropped out of that chaining

My vote: as a general, keep the tree as a tree for as long as possible. One should only consider flattening/mapping/binding data when you get to the boundary and *must* pass data to another environment.

To be more concrete, take the example expression:

$params['custom_some_big_group:my_other_field:8'] = 'myinfo';
PHP: $params['custom_some_big_group:my_other_field:8']
Smarty: {$params.custom_some_big_group___my_other_field___8}
JS: params['custom_some_big_group:my_other_field:8'] = 'myinfo'
XML: <property key="custom_some_big_group:my_other_field:8">myinfo</property>
HTML Form: custom_some_big_group___my_other_field___8

In this case, there's an implicit tree of data, and that tree has been flattened. But now anyone who wants to seriously work with that data in any of these environments will have to do lots of string manipulation to parse and construct keys. This feels like reinventing the wheel when every major environment -- PHP, Smarty, JSON, XML, HTML form-encoders, etc -- already has a canonical way to represent tree data, and the canonical ways get the best tooling support (e.g. "foreach" in PHP, "for" in JS, "selectors" in jQuery, XSLT in XML).

It would be better to let each environment use a representation like:

PHP: $params['custom']['some_big_group'][8]['my_other_field']
Smarty: {$params.custom.some_big_group.8.my_other_field}
JS: params.some_big_group[8].my_other_field
XML: <custom><some_big_group id=8><my_other_field>myinfo</my_other_field></some_big_group></custom>
HTML Form: custom[some_big_group][8][my_other_field]

OK, had to add an new fuction to the BAO for looking up names. Here's the result of a get request now. Note that all arrays are nested, and single-value custom groups always have index 0.
Array
(
  [is_error] => 0
  [version] => 3
  [count] => 3
  [values] => Array
    (
      [Additional_Info] => Array
        (
          [0] => Array
            (
              [HS_Graduation_Date] => 1998-06-01 00:00:00
            )
        )
      [Single_Stuff] => Array
        (
          [0] => Array
            (
              [Some_text] => text!!!
              [Some_Date] => 2011-05-25 12:34:00
              [Some_More_Info] => Ipsum lorem...
            )
        )
      [My_Multi] => Array
        (
          [4] => Array
            (
              [Here_s_some_text] => yea!
              [Are_you_cool_] => 0
            )
          [5] => Array
            (
              [Here_s_some_text] => more
              [Are_you_cool_] => 0
            )
          [6] => Array
            (
              [Here_s_some_text] => getting there
              [Are_you_cool_] => 0
            )
          [7] => Array
            (
              [Here_s_some_text] => ok, ok, you're cool
              [Are_you_cool_] => 1
            )
        )
    )
)


Hi Coleman,

I've been thinking about your post but the only conclusion I have come to so far is that I probably don't understand it quite well enough yet & that I should do some testing of the output so I am sure I'm not making dumb assumptions (I'll do it once I get home).

Not sure if I've pointed this out - but the reason we are picking at the details of your code is that it's a great contribution & worthy of becoming a key part of the api.

Anyway. I've been thinking about the point of the id indexing (apart from it just being part of the standard / consistency).

A couple of notes -

1) if there is only one entity (ie. count = 1) dealt with the civicrm_api will populate the return array with that as 'id'

2) the civicrm_create_success will re-index any functions to use a numerical index if you pass in 'sequential' => 1 as a param

3) I set the above to be the default on nested api calls (so you can change it by passing 'sequential' => 0 but otherwise it will be numerically indexed anyway). I did that because when I wrote a couple of tests it quickly seemed like it would be easier to work with

4) The id in the id array is supposed to be the 'id' of the relevant table. In the case of customValue the relevant table is the value table. However, I think that might not always be unique as there is more than one table involved. Also, until I've taken another look at how your code outputs I won't be clear in my own head if that would fit anyway.

Note that the level at which the id indexing takes place is the level straight under 'values' - ie. in your code 'Additional_Info'.

Why would we want to have 'entity_id' be a param when every other API uses 'id'?

- because it's the field name in the relevant table (id is the customvalue id). ie. if by 'id' you mean 'contact_id' then that is what I would rename to 'entity_id' - if you mean 'custom value id' then that's fine as 'id'
- because it would fit with the proposed implementation of nesting (where the id from the parent function gets passed in as $entity_id AND as entity_id and the parent function entity gets passed in as 'entity'

Here's some more that I'd like to add:
FOR THE GET FUNCITON:
-Returning arrays keyed by name is very nice and probably should be the default. However sometimes you want it keyed by group ID and field ID rather than the names. So I propose adding a 'return_format' => 'id' optional param.

http://forum.civicrm.org/index.php/topic,19460.msg81325.html#msg81325

we are trying to introduce namespace to separate what's "field" param and filter or option param
option.custom_id OR option.custom_name ?

(return_format => id doesn't tell me it's about custom fields)

-Specifying what you want returned could use some love. I don't really like the format 'return.my_group:some_field'. Since you're specifying the group, why not just leave it at that? Why would you ever not want every field in a group returned?

Because when you do an ajax call, you don't want to have loads of stuff you don't want, and for what I've seen, lots have one single "misc" group with dozen of fields

btw, you can have return=>"my_group:some_field, my_group:some_other_field" as a syntax (from ajax at least) and pretty sure return can be an array (I personally think that return.xx =1 should be obsoleted)

-Lobo suggests that for multi-valued fields (checkboxes, etc) we should return value => label rather than just an array of values. Makes sense, hopefully there's a BAO to fetch that info.

I think in a lot of cases you wouldn't need it (ie. you will code around it, that's the value that matters, not the label). It would be nice to have option.custom_label =1 to do as lobo suggest but still keep what is done now as default

As for fetching the label, pretty sure with the api OptionGroup OptionValues you can (try from civicrm/ajax/doc)

Another option would be to chain the api calls and do a last one api.OptionValues.Get ('group_id' => 42) and get the list of all the values=> label for that field.

FOR THE CREATE FUNCTION:
Haven't touched this since we changed around the get function to return a nested array. So currently it takes input like this:
 * 'custom_6' => 'foo', // (omitting :id) inserts or updates a field in a single-valued group
 * 'custom_24' => array('bar', 'baz'), // custom_24 is checkbox or multiselect, so pass items as an array
 * 'custom_33:5' => value, // in this case custom_33 is part of a multi-valued group, and we're updating record id 5
 * 'custom_33:-1' => value, // inserts new record in multi-valued group
 * 'custom_33:-2' => value, // inserts another new record in multi-valued group
 * 'custom_some_group:my_field => 'myinfo', // you can use group_name:field_name instead of ID
 * 'custom_some_big_group:my_other_field:8 => 'myinfo', // updates record ID 8 in my_other_field in multi-valued some_big_group

Personally I'm fine with that, but now it's inconsistent with our output. Suggestions?

I think we are doing the same error here than for location: force trying to put a multi-dimensional structure in a flat one. We tried and failed.

I would suggest that we only cover a single syntax (custom_id or custom_group:field) for the most common use
- if mono-valued create/get it
- if multi-valued add one/get the latest (or whatever the behaviour today)

if you need the most advanced modes, let's use api chaining  so

to return all the custom fields (multi or mono value)

api.customField.Get ('group_name' => 'xxx', entity_id = '$values.contact_id')

to update a custom field (multi value)

api.customField.Create ('id' => id of the field, ...)

to create a new value in a multi value

api.customField.Create ('group_name' => 'xxx', entity_id = '$values.contact_id' ...)


The more I see the power of chaining the api, the more I like it. Eileen will swim on a sea of warm beer at civicon.


as create it (or create a new one) for the create and the


X+


P.S. More radical option (I like it)

As we always have chaining the api to get/create exactly what we want, I would suggest to skip the name of the group in custom_group:fieldname and go for custom_fieldname

if you want to be able to use custom_fieldname, create your fieldname so they are unique (not only unique within the same group).

if you want to have the same field name in different groups, then use custom_id or chain the api.

Hi you said your code is in svn now but I'm not seeing it