Author Topic: Multiple address, phone or email records and location_type, is_primary and API (Read 1990 times)

davej · November 21, 2011, 08:05:06 am

Hi,

Thought it would be good to clarify what the "correct" behaviour is in the following scenarios. I found some discussion of the is_primary aspect at http://issues.civicrm.org/jira/browse/CRM-8689.

(1) Trying to create two addresses of same location_type for a contact.
How it works in 4.0.6:
UI: disallowed by form validation.
API v3 Address create specifying contact_id but not id: updates existing address record - supplied fields overwrite existing data, other fields left unchanged.

(2) Trying to create two phone records of same location_type and phone_type for a contact.
How it works in 4.0.6:
UI: allowed; is_primary determined by radio buttons.
API v3 Phone create specifying contact_id but not id: allowed; is_primary not specified -> set to 0; is_primary=1 -> may result in multiple records with is_primary=1.

(3) Trying to create two email records of same location_type for a contact.
How it works in 4.0.6:
UI: allowed; is_primary determined by radio buttons.
API v3 Email create specifying contact_id but not id: allowed; is_primary not specified -> set to 0; is_primary=1 -> may result in multiple records with is_primary=1.

So I guess the bits I'm trying to clarify are:

- Is it intended that phones & emails are treated differently from addresses, in respect of allowing multiple records with same location type (+ phone type)? (A client thinks they want e.g. multiple Home addresses; they can probably be dissuaded but it would be good to know whether the prohibition on this is deliberate & likely to remain!)

- Should enforcement of one and only one address / phone / email being primary be the responsibility of the API or of the application code that uses the API?

Dave J

Erik Hommel · November 21, 2011, 11:02:20 pm

Hi Dave,

see if I can throw some answers up in the air:

Quote

Is it intended that phones & emails are treated differently from addresses, in respect of allowing multiple records with same location type (+ phone type)? (A client thinks they want e.g. multiple Home addresses; they can probably be dissuaded but it would be good to know whether the prohibition on this is deliberate & likely to remain!)

As far as I know, phones and emails are treated differently in Core because of the shared address functionality. Question in return: what do you think it should do? Seems to me there opinions behind the questions :-)

Quote

- Should enforcement of one and only one address / phone / email being primary be the responsibility of the API or of the application code that uses the API?

Nice one.....personally I would like to have this type of validation in the API, but in general we do assume in quite some cases that the person calling the API knows what he/she is doing. Again, what is your opinion?

xavier · November 22, 2011, 03:14:29 am

Right now, is_primary is defaulted to 1 if not set.

IMO it would be better when you call the api without defining the is_primary that the first entity is primary and when creating subsequent ones it isn't. patch welcome

X+

fen · January 09, 2012, 10:01:18 pm

When 'create'ing an address record for a contact with location_type_id=LID and is_primary unspecified,
is_primary is set to 1 when an existing address record is updated (for contact_id=CID and location_type_id=LID) or
when a new address record is created (when the contact does not have an existing address record with location_type_id=LID).

But CiviCRM wants there to be only one address record across *all* location types that has is_primary set despite the fact that the API will cheerfully create new is_primary address records of other location_types.

Also, @davej's note that existing address records are not replaced but updated points out an issue that I had previously missed when importing new contact records: in my use case, if an existing contact is found, the new address should completely replace the old one.

Though not true at the time of this function's writing, I assume that each contact has at most one address record with each location_type_id and (if the contact has at least one address record) exactly one address record with is_primary=1 (see http://forum.civicrm.org/index.php/topic,11659.msg59809.html#msg59809 for SQL to make sure this is so - thanks, @eileen!)

With all this in mind, here's some (as yet untested and lacking of error checking) code to import contact addresses the way I want them to be imported (using v3.4.8 )

2012-01-10 10:50am ET: Updated code to examine multiple records of same location_type. This should not happen, but it does.

Code: [Select]

/**
 * We are importing contacts via the API, so replace rather than update existing addresses.
 * We assume that each contact with one or more address records has exactly one address record with is_primary=1
 *
 * @param array $params - address params including contact_id and location_type_id
 * @return  int $id     - the address record id that was created or updated
 * @see http://forum.civicrm.org/index.php/topic,11659.msg59809.html#msg59809
 */
function create_or_update_contact_address($params) {
  // Get all addresses for this contact
  $result = civicrm_api('Address', 'get', array('version' => 3, 'contact_id' => $params['contact_id']));

  // If there is at least one address...
  if ($result['count']) {
    $ids = array();
    foreach ($result['values'] as $id => $address) {
      // If there's a location_type match, check for an address match, too
      if ($address['location_type_id'] == $params['location_type_id']) {
        // The implementation of address_match() is left as an exercise for the reader.
        // (I've always wanted to say that!) ;)
        if (address_match($params, $address)) {
          // We found a matching address, so we're good
          // FIXME: Would it be better to update the old address with fields from the new one?
          return $id;
        }
        // We found an address with a matching location_type_id; save its id
        $ids[] = $id;
      }
    }
    // If there is one (or more) address(es) with a location_type_id match...
    if ($ids) {
      // Get the oldest address record record by id
      // FIXME: What would be a better algorithm to use here?
      $id = [min($ids)];
      // We're going to replace this address so save its is_primary value
      $params['is_primary'] = $result['values'][$id]['is_primary'];
      // Delete the found address record to make way for a completely new one.
      // Otherwise, address->create will simply update supplied address fields
      // leaving other fields untouched.
      $result = civicrm_api('Address', 'delete', array('version' => 3, 'id' => $id));
    }
    else {
      // No address of this location_type found.
      // Assume some other existing address (with a different location_type) has is_primary=1
      // FIXME: If (say) a shipping address is_primary and a Home address is added,
      //        should the Home address become the is_primary address?
      $params['is_primary'] = 0;
    }
  }
  else {
    // No addresses for this contact so create a primary one
    $params['is_primary'] = 1;
  }
  // Create the address record
  $result = civicrm_api('Address', 'create', $params);
  return $result['id'];
}

Eileen · January 10, 2012, 12:39:58 am

Hi,

I think we push this level of processing into the BAO. One conversation we need to have is what function EVERY BAO should have that is consistently called by the API - then we'll know what we are working towards - at the moment we have a mismash of create, add & random function name called by the API. We should settle on the same function every time & also agree that that function should always call pre & post process hooks

Once we know that I think the 3 location BAO should each hold the functionality to ensure primary is set if there is only one address & if more than one then the one that is currently primary has primary unset.

fen · January 10, 2012, 08:05:08 am

Quote from: Eileen on January 10, 2012, 12:39:58 am

Once we know that I think the 3 location BAO should each hold the functionality to ensure primary is set if there is only one address & if more than one then the one that is currently primary has primary unset.

I'm not quite sure what you mean here. Trying to clarify:

The UI enforces only one address for each location_type and at most one is_primary.

Legacy databases may have more than one address record for each location_type, and more than one address record set with is_primary.

The API (currently) will create new is_primary records, one for each location_type.

The goal for the API might be, when adding a new address record, something like this pseudocode:

Code: [Select]

if ($params['is_primary']) {
  remove_is_primary_from_existing_address_records($params['contact_id']);
}
else {
  if (address_records_exist($params['contact_id'])) {
    // Assume one has is_primary set
    // Should there be an assurance step to confirm this?
    $params['is_primary'] = 0;
  }
  else {
    $params['is_primary'] = 1;
  }
}

Or am I off the mark here?

xavier · January 10, 2012, 08:28:07 am

@fen,

You are right about the pseudo code, but Eileen point was that it shouldn't be a duplication of the existing feature (in the ui form code) in the API, but that this code should be re-factored so the test is moved from the UI to the BAO, so that's the same code for both the UI & the API.

With the API it should be possible to create/update an address while forcing it to be primary, forcing it not to be primary (in that case how to select the primary?) or let the BAO decides (the first entity is primary, the subsequent ones aren't)

Would be great if someone could try to implement it (hint hint

X+

fen · January 10, 2012, 08:44:13 am

@xavier,

I get your point (and Eileen's) and the hint - though I barely know how to spell BAO.

Once I get the client on their feet again, I'll take a look and see if I can manage not to get too lost.

=Fen

Eileen · January 10, 2012, 11:22:59 am

B-A-O

I guess one of the recurrent difficulties in CiviCRM is that a lot of CiviCRM logic is stored in forms rather than in the BAO which makes it tricky for the API to tap into the functionity. I think it's kind of a sign of the times (ie. that was put in place a while ago) but we should work to make it more consistent over time. (although the nature of open source is that there are new developments pulling against that consistency)

CiviCRM Community Forums (archive)

News:

Author Topic: Multiple address, phone or email records and location_type, is_primary and API (Read 1990 times)

davej

Multiple address, phone or email records and location_type, is_primary and API

Erik Hommel

Re: Multiple address, phone or email records and location_type, is_primary and API

xavier

Re: Multiple address, phone or email records and location_type, is_primary and API

fen

Re: Multiple address, phone or email records and location_type, is_primary and API

Eileen

Re: Multiple address, phone or email records and location_type, is_primary and API

fen

Re: Multiple address, phone or email records and location_type, is_primary and API

xavier

Re: Multiple address, phone or email records and location_type, is_primary and API

fen

Re: Multiple address, phone or email records and location_type, is_primary and API

Eileen

Re: Multiple address, phone or email records and location_type, is_primary and API