CiviCRM Community Forums (archive)

Just wanted to get your opinions....I am digging into bug CRM-6895 which was reported on the Relationship API, and created a fix for that. But there was also unexpected behaviour on the Activity API: the target_contact_id was gone after an update. And now it turns into a little bowl of spaghetti:

• the activity_get function did not retrieve any targets or assignees. So I changed that, the get now includes an array targets, containing the contact_ids of the targets, and the same for the assignees
• next issue is that currently the API only expects one target_contact_id, whilst we have many in CiviCRM

Now this is the point where I can make a fix for the update that merges the retrieved targets/assignees with the parameter targets/assignees whilst also taking care of the situation where only one is entered. But I feel this is then getting kind of messy.
The solution I would prefer is:

• the activity_create/update is only used for the entity Activity, and only updates or creates fields in that entity (so I remove target and assignee processing from the
  current one)
• we have additional functions activity_target_create/update and activitiy_assignee_create/update that take care of the entities ActivityTarget and ActivityAssignee
• we change the documentation to reflect this

To me this is a more structured approach? Let me know what you think and if I am missing anything obvious (and let me tell you I am good at that :-))
Erik

Hi Eileen,
so what you are saying is that from a API user point of view, having one that takes care of all the possibilities would be your preference as you would otherwise be creating wrappers around it?

I see what you mean. On the other hand, if we are trying to keep the API's as a standardized set of fairly easy maintainable things then having them a little simpler would be a great benefit. For example, if we would split location API into one for location, one for adress, one for phone and one for email I would create a wrapper that would call them all in one go but have a predictable behaviour on the API's. I guess I would prefer that to having one that does a lot but I actually have to get into the code to find out how, why and what?
Erik

I agree with you on bringing back the add. My point would be that it would not be based on a table but on an entity. In that way the behaviour would be so much more predictable I would have to know less about the CiviCRM internal stuff. For me as a relative newbie to API's my greatest suffering what the inconsistent behaviour, not the understanding the entities. Having predictable behaviour would also mean the documentation is easier to keep up to date. I think we are both trying to achieve the same thing: consistent and predictable API's that mean we do not have to know the details of CiviCRM. Let's see what Xavier has to say about this :-) Apparenty the three of us are on the API team anyway :-)
Erik

It is a tough one.....I will throw some questions on this level on the location API tomorrow when I dig into it. We can then try to get a best approach for that based on the actual issues. I am off to bed now, early morning and English breakfast tomorrow 

Hi,

1) Having to call apis in a specific order to get an "atomic" result is not a good rest api
2) Having zillions of test cases and still not covering all the options of how to populate the params is not a good api

Pick your poison, you can have one of the two it seems

From a user pov of this api, I would find logical to have to call a second api if I want to assign it to a group of contacts. However, if there is only one contact to assign, I would find illogical to have the owner as a regular param, but having to call a second api just to add the assigned.

What about "flatening"  the api, and consider that you can set the assigned contact directly only if there is one:

It takes 0 or 1 assign, if there is one, it replaces the existing one. If you want more, call the specific api

In general, what about that convention? if it's one, (eg one phone, one email..), you can set it from the contact api, if not, use the specific api.

You have covered 80% of the cases in one call, and by leaving out the 20%, we can keep the api code managable?

X+

Probably not because I would still wind up writing a wrapper to pass into that chain since the chain is still pretty complex. I would still write a wrapper converting a single multi-dimensional array into that chain of functions.

That's not really disimilar to the location api at all is it?

I had always imagined the address API should work like

phone =array(array('phone_type' =>1, 'phone' => 123,'add'=1),array('phone_type'=>2,'phone'=>234)));

ie add the first phone number in the array but for the second you should only add it if one doesn't already exist.