CiviCRM Community Forums (archive)

Hi,

I realised on doing the membership_type_create that if 'id' is passed in it makes sense to fetch any existing values BEFORE checking if all the params are present - that way just an id is sufficient for an update request & our creates are 'update-friendly'

/**
 * API to Create or update a Membership Type
 * 
 * @param   array  $params  an associative array of name/value property values of civicrm_membership_type
 * @return array $result newly created or updated membership type property values.
 * @access public
 */
function civicrm_membership_type_create(&$params)
{
  _civicrm_initialize();
  try{
    $values = $params;
    if(!empty($params['id'])){
     $getparams = array('id' => $params['id'],'version' => 3);
     $result = civicrm_membership_type_get($getparams);
     $values = array_merge($result['values'][$params['id']],$params); 
    }
    civicrm_verify_mandatory($values,'CRM_Member_DAO_MembershipType' ,array('name',  'duration_unit','duration_interval'));
     
    $ids['membershipType']   = CRM_Utils_Array::value( 'id', $params );
    $ids['memberOfContact']  = CRM_Utils_Array::value( 'member_of_contact_id', $params );
    $ids['contributionType'] = CRM_Utils_Array::value( 'contribution_type_id', $params );

    require_once 'CRM/Member/BAO/MembershipType.php';
    $membershipTypeBAO = CRM_Member_BAO_MembershipType::add($values, $ids);
    $membershipType = array();   
    _civicrm_object_to_array( $membershipTypeBAO, $membershipType[$membershipTypeBAO->id] );
    $membershipTypeBAO->free;
    return civicrm_create_success($membershipType,$params);

  } catch (PEAR_Exception $e) {
    return civicrm_create_error( $e->getMessage() );
  } catch (Exception $e) {
    return civicrm_create_error( $e->getMessage() );
  }
} 

Like it nice move,

Have you tested what happen if the id is invalid ? I *think* I had an option so the api would throw an error instead of returning an is_error, I'll dig it if doesn't work straight out of the box.

X+

NB - I suppose we could move into a helper function to turn it into a one-liner

Code: [Select]

    $values = $params;
    if(!empty($params['id'])){
     $getparams = array('id' => $params['id'],'version' => 3);
     $result = civicrm_membership_type_get($getparams);
     $values = array_merge($result['values'][$params['id']],$params); 
    }

Isn't the param entity_id not id ?

As for the function to get the entity from the function of file name (what I'm doing on Entity.php)

This starts lookging weird: every call of the api is supposed to have action and entity separated (eg. civicrm_api (entity,action), ajax/rest?action=&entity= ...)

Then we mungle and put them as civicrm_entity_action, then we need to split them again

Wondering if the call of that shouldn't be done before the civicrm_entity_action, and that we add the result of the get as params['previous']

     $values = array_merge($result['values'][$params['id']],$params);
Still would prefer the sequential by default and
     $values = array_merge($result['values'][0],$params);

Hi,

Just looking at how this function would look in the utils - just wondering what the latest is on what it should call. Also, does our existing stuff work with the UFGroup pattern - ie. two caps that don't represent U_F_Group

Code: [Select]

/*
 * Function to get existing values when an 'id' is passed into a Create api
 *
 * @params array $params input params
 * @return array $valuse params with existing values from contact
 */

function  civicrm_update_get_existing($params,$function, $filename){
    $values = $params;
// figure out here how to determine the function call we need from function or filename - nb make sure it copes with UFField etc anomolies
    if(!empty($params['id'])){
     $getparams = array('id' => $params['id'],'version' => 3);
     $result = civicrm_membership_type_get($getparams);
     $values = array_merge($result['values'][$params['id']],$params);
    }
    return $values;
}

So, you're suggesting we do the getting before it even gets to the _create function.

Makes sense but it takes a bit of getting my head around the idea of the functions not being self-contained. I'd been thinking of the api.php as a handy extra rather than a necessary .... good.

So, something like

/*
 * Function to get existing values when an 'id' is passed into a Create api
 *
 * @params array $params input params
 * @return array $valuse params with existing values from contact
 */

function  civicrm_update_get_existing($params, $filename){
    $entity = substr ( basename($filename) ,0, strlen(basename($filename))-8 );
    if (strstr($entity,'UF')){// a cleverer person than me would do it in a single regex
       $fnPrefix = strtolower(preg_replace('/(?<! )(?<!^)(?<=UF)[A-Z]/','_$0', $entity));         
    }else{
        $fnPrefix = strtolower(preg_replace('/(?<! )(?<!^)[A-Z]/','_$0', $entity));
     }   
    $function = "civicrm_" . $fnPrefix . "_get";
    $values = $params;
    if(!empty($params['id'])){
     $getparams = array('id' => $params['id'],'version' => 3);
     $result = $$function($getparams);
     $values = array_merge($result['values'][$params['id']],$params);
    }
    return $values;
}   

yeah - good point - that's even simpler - I copied it from somewhere where I couldn't do a simple replace

This one is a serious bummer.

What do we do when reality and theory don't match ?

I'd personally be tempted to modify and say that it isn't &$params that you pass as a param to the civicrm_entity_create but $params.

while we are a it could do it as well for the others, cause it's not going to be long arrays anyway.

But really not sure, beside it's a problem.

X+

Plan B:

We don't touch the _create (ie. if you don't put the existing fields, they are erased).

We create a new action in civicrm_api/crmAPI/ajax: modify

if (modify)
  current = civicrm_api (entity, 'get', param)
  tmp = array merge (param,current);
  civicrm_api (entity, 'create')


What do you think ?

And the modify api is only available if you use the civicrm_api & friends. If you want to call directly the civicrm_entity_create, fine, but you have to deal yourself with copying the current fields.

X+