Details
-
Type:
Bug
-
Status: Done/Fixed
-
Priority:
Minor
-
Resolution: Fixed/Completed
-
Affects Version/s: 4.3.0
-
Fix Version/s: 4.3.0
-
Component/s: CiviCRM API
-
Labels:
Description
When the Group API get function is called with an id value, it should return the Group with the specified id, but instead it returns all Groups. Other fields, e.g. title, can still be used to return a specific Group. This bug was introduced in the 4.3 code and is present in 4.3 beta 3.
Attachments
Activity
The _civicrm_api3_get_options_from_params function takes any param called "id" and sets it to "group_id". However, CRM_Contact_BAO_Group::getGroups doesn't know how to deal with group_id. Instead of changing the BAO, I just made the API copy the value of "group_id" to "id".
Created a pull request for this--branch called group_id-to-id.
Yeah, this falls between the cracks. The api layer DAO based get calls know how to handle '{$entity}_id', the BAO query based ones return erratic results unless they get {$entity}_id - but this BAO fn does something else again.
I'm wondering about how one would unit-test to prevent this in the future. Perhaps an update to api_v3_SyntaxConformanceAllEntitiesTest – maybe enhance testSimple_get(), enhance testAcceptsOnlyID_get(), or add a new case?
(The crux of the test would be: call civicrm_api($entity, 'get', array('id' => $x) and verify that it returns exactly one entity (and that the entity has the right ID.)
SyntaxConformance is probably to the right place. In the past SyntaxConformance tests that fail have pretty much just been removed which has defeated the purpose of it - but using Jenkins has the potential to change that for all the obvious reasons.
I don't understand why this patch is necessary, doesn't the civicrm_api wrapper always alias foo_id to id and vice-versa?
Yes, the api wrapper converts to DB field names for 'create' requests & to unique field names for 'get' requests - which reflects the most prevalent BAO requirements.
However, this BAO function doesn't want a unique name - it wants a DB field name.
Yeah, I didn't want to mess around with the BAO function so I re-rewrite it back to "id".
I think this was your patch that changed it wasn't it? Do you want to take a look?