From 49f8272dffabdf0801411bc2a870201bb57c5fb1 Mon Sep 17 00:00:00 2001 From: Eileen Date: Tue, 22 Oct 2013 18:25:18 +0000 Subject: [PATCH] CRM-13632 relationship update functions, call pre, post hooks on enable, disable, update correctly from api when id is set ---------------------------------------- * CRM-13632: Post Hook not called when a relationship is disabled via cron http://issues.civicrm.org/jira/browse/CRM-13632 --- CRM/Contact/BAO/Relationship.php | 46 +++++++++++-------- api/v3/examples/JobCreate.php | 2 +- api/v3/examples/JobDelete.php | 2 +- .../Relationship/BetweenRelationshipType.php | 24 +++++----- .../Relationship/INRelationshipType.php | 18 ++++---- .../NotBetweenRelationshipType.php | 12 ++--- .../Relationship/NotInRelationshipType.php | 18 ++++---- .../examples/Relationship/filterIsCurrent.php | 8 ++-- api/v3/examples/RelationshipCreate.php | 10 ++-- api/v3/examples/RelationshipDelete.php | 2 +- api/v3/examples/RelationshipGet.php | 8 ++-- tests/phpunit/api/v3/JobTest.php | 35 +++++++++++++- tests/phpunit/api/v3/RelationshipTest.php | 17 ++++++- 13 files changed, 128 insertions(+), 74 deletions(-) diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php index 23472e6b26..86ad794c31 100644 --- a/CRM/Contact/BAO/Relationship.php +++ b/CRM/Contact/BAO/Relationship.php @@ -113,7 +113,9 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { } else { // check for duplicate relationship - + //@todo this code doesn't cope well with updates - causes e-Notices. API has a lot of code to work around + // this but should review this code & remove the extra handling from the api + // it seems doubtful any of this is relevant if the contact fields & relationship type fields are not set if ( self::checkDuplicateRelationship( $params, @@ -196,13 +198,14 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { * @access public * @static */ - static function add(&$params, &$ids, $contactId) { - if (CRM_Utils_Array::value('relationship', $ids)) { - CRM_Utils_Hook::pre('edit', 'Relationship', $ids['relationship'], $params); - } - else { - CRM_Utils_Hook::pre('create', 'Relationship', NULL, $params); + static function add(&$params, $ids = array(), $contactId = NULL) { + $relationshipId = CRM_Utils_Array::value('relationship', $ids, CRM_Utils_Array::value('id', $params)); + $hook = 'create'; + if($relationshipId) { + $hook = 'edit'; } + //@todo hook are called from create and add - remove one + CRM_Utils_Hook::pre($hook , 'Relationship', $relationshipId, $params); $relationshipTypes = CRM_Utils_Array::value('relationship_type_id', $params); @@ -219,10 +222,12 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { } $relationship = new CRM_Contact_BAO_Relationship(); + //@todo this code needs to be updated for the possibility that not all fields are set + // (update) $relationship->contact_id_b = $contact_b; $relationship->contact_id_a = $contact_a; $relationship->relationship_type_id = $type; - $relationship->id = CRM_Utils_Array::value('relationship', $ids); + $relationship->id = $relationshipId; $dateFields = array('end_date', 'start_date'); @@ -238,7 +243,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { $relationship->$defaultField = $params[$defaultField]; } } - elseif(empty($relationship->id)){ + elseif(!$relationshipId){ $relationship->$defaultField = $defaultValue; } } @@ -252,12 +257,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { $relationship->free(); - if (CRM_Utils_Array::value('relationship', $ids)) { - CRM_Utils_Hook::post('edit', 'Relationship', $relationship->id, $relationship); - } - else { - CRM_Utils_Hook::post('create', 'Relationship', $relationship->id, $relationship); - } + CRM_Utils_Hook::post($hook, 'Relationship', $relationshipId, $relationship); return $relationship; } @@ -729,9 +729,15 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship { * @static */ static function setIsActive($id, $is_active) { - CRM_Core_DAO::setFieldValue('CRM_Contact_DAO_Relationship', $id, 'is_active', $is_active); - - // call hook + // as both the create & add functions have a bunch of logic in them that + // doesn't seem to cope with a normal update we will call the api which + // has tested handling for this + // however, a longer term solution would be to simplify the add, create & api functions + // to be more standard. It is debatable @ that point whether it's better to call the BAO + // direct as the api is more tested. + civicrm_api3('relationship', 'create', array('id' => $id, 'is_active' => $is_active)); + + // call (undocumented possibly deprecated) hook CRM_Utils_Hook::enableDisable('CRM_Contact_BAO_Relationship', $id, $is_active); return TRUE; @@ -1445,7 +1451,7 @@ WHERE id IN ( {$contacts} ) $query = " SELECT cc.id as id, cc.sort_name as name FROM civicrm_relationship cr, civicrm_contact cc -WHERE +WHERE cr.contact_id_a = %1 AND cr.relationship_type_id = %2 AND cr.is_permission_a_b = 1 AND @@ -1456,7 +1462,7 @@ cc.id = cr.contact_id_b"; if (!empty($name)) { $name = CRM_Utils_Type::escape($name, 'String'); $query .= " -AND cc.sort_name LIKE '%$name%'"; +AND cc.sort_name LIKE '%$name%'"; } $args = array(1 => array($contactID, 'Integer'), 2 => array($relTypeId, 'Integer')); diff --git a/api/v3/examples/JobCreate.php b/api/v3/examples/JobCreate.php index 3ab7c9bbc1..5146b95671 100644 --- a/api/v3/examples/JobCreate.php +++ b/api/v3/examples/JobCreate.php @@ -79,4 +79,4 @@ function job_create_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/JobDelete.php b/api/v3/examples/JobDelete.php index 9e548d9805..ec65017206 100644 --- a/api/v3/examples/JobDelete.php +++ b/api/v3/examples/JobDelete.php @@ -58,4 +58,4 @@ function job_delete_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/Relationship/BetweenRelationshipType.php b/api/v3/examples/Relationship/BetweenRelationshipType.php index 152a9ca8b8..ce263b9046 100644 --- a/api/v3/examples/Relationship/BetweenRelationshipType.php +++ b/api/v3/examples/Relationship/BetweenRelationshipType.php @@ -7,8 +7,8 @@ function relationship_get_example(){ $params = array( 'relationship_type_id' => array( 'BETWEEN' => array( - '0' => 32, - '1' => 34, + '0' => 33, + '1' => 35, ), ), ); @@ -39,9 +39,9 @@ function relationship_get_expectedresult(){ 'values' => array( '2' => array( 'id' => '2', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '32', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '33', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -50,9 +50,9 @@ function relationship_get_expectedresult(){ ), '3' => array( 'id' => '3', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '33', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '34', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -61,9 +61,9 @@ function relationship_get_expectedresult(){ ), '4' => array( 'id' => '4', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '34', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '35', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -97,4 +97,4 @@ function relationship_get_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/Relationship/INRelationshipType.php b/api/v3/examples/Relationship/INRelationshipType.php index b1b8d7c588..ea7a9ac965 100644 --- a/api/v3/examples/Relationship/INRelationshipType.php +++ b/api/v3/examples/Relationship/INRelationshipType.php @@ -7,8 +7,8 @@ function relationship_get_example(){ $params = array( 'relationship_type_id' => array( 'IN' => array( - '0' => 32, - '1' => 33, + '0' => 33, + '1' => 34, ), ), ); @@ -39,9 +39,9 @@ function relationship_get_expectedresult(){ 'values' => array( '2' => array( 'id' => '2', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '32', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '33', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -50,9 +50,9 @@ function relationship_get_expectedresult(){ ), '3' => array( 'id' => '3', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '33', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '34', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -86,4 +86,4 @@ function relationship_get_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/Relationship/NotBetweenRelationshipType.php b/api/v3/examples/Relationship/NotBetweenRelationshipType.php index 40037d60d6..463dbb29fb 100644 --- a/api/v3/examples/Relationship/NotBetweenRelationshipType.php +++ b/api/v3/examples/Relationship/NotBetweenRelationshipType.php @@ -7,8 +7,8 @@ function relationship_get_example(){ $params = array( 'relationship_type_id' => array( 'NOT BETWEEN' => array( - '0' => 32, - '1' => 34, + '0' => 33, + '1' => 35, ), ), ); @@ -40,9 +40,9 @@ function relationship_get_expectedresult(){ 'values' => array( '1' => array( 'id' => '1', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '31', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '32', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -76,4 +76,4 @@ function relationship_get_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/Relationship/NotInRelationshipType.php b/api/v3/examples/Relationship/NotInRelationshipType.php index 299a8b7e79..26d9877d5d 100644 --- a/api/v3/examples/Relationship/NotInRelationshipType.php +++ b/api/v3/examples/Relationship/NotInRelationshipType.php @@ -7,8 +7,8 @@ function relationship_get_example(){ $params = array( 'relationship_type_id' => array( 'NOT IN' => array( - '0' => 32, - '1' => 33, + '0' => 33, + '1' => 34, ), ), ); @@ -39,9 +39,9 @@ function relationship_get_expectedresult(){ 'values' => array( '1' => array( 'id' => '1', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '31', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '32', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -50,9 +50,9 @@ function relationship_get_expectedresult(){ ), '4' => array( 'id' => '4', - 'contact_id_a' => '63', - 'contact_id_b' => '64', - 'relationship_type_id' => '34', + 'contact_id_a' => '87', + 'contact_id_b' => '89', + 'relationship_type_id' => '35', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -86,4 +86,4 @@ function relationship_get_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/Relationship/filterIsCurrent.php b/api/v3/examples/Relationship/filterIsCurrent.php index 0116fd8697..9199531dca 100644 --- a/api/v3/examples/Relationship/filterIsCurrent.php +++ b/api/v3/examples/Relationship/filterIsCurrent.php @@ -37,9 +37,9 @@ function relationship_get_expectedresult(){ 'values' => array( '2' => array( 'id' => '2', - 'contact_id_a' => '60', - 'contact_id_b' => '61', - 'relationship_type_id' => '30', + 'contact_id_a' => '83', + 'contact_id_b' => '85', + 'relationship_type_id' => '31', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -73,4 +73,4 @@ function relationship_get_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/RelationshipCreate.php b/api/v3/examples/RelationshipCreate.php index 8708deaa32..ff07700dc7 100644 --- a/api/v3/examples/RelationshipCreate.php +++ b/api/v3/examples/RelationshipCreate.php @@ -5,8 +5,8 @@ */ function relationship_create_example(){ $params = array( - 'contact_id_a' => 24, - 'contact_id_b' => 25, + 'contact_id_a' => 31, + 'contact_id_b' => 33, 'relationship_type_id' => 18, 'start_date' => '2010-10-30', 'end_date' => '2010-12-30', @@ -41,8 +41,8 @@ function relationship_create_expectedresult(){ 'values' => array( '1' => array( 'id' => '1', - 'contact_id_a' => '24', - 'contact_id_b' => '25', + 'contact_id_a' => '31', + 'contact_id_b' => '33', 'relationship_type_id' => '18', 'start_date' => '2013-07-29 00:00:00', 'end_date' => '2013-08-04 00:00:00', @@ -79,4 +79,4 @@ function relationship_create_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/RelationshipDelete.php b/api/v3/examples/RelationshipDelete.php index daa948f180..5460670fd0 100644 --- a/api/v3/examples/RelationshipDelete.php +++ b/api/v3/examples/RelationshipDelete.php @@ -58,4 +58,4 @@ function relationship_delete_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/api/v3/examples/RelationshipGet.php b/api/v3/examples/RelationshipGet.php index 7e2fb5a40f..85603ac531 100644 --- a/api/v3/examples/RelationshipGet.php +++ b/api/v3/examples/RelationshipGet.php @@ -35,9 +35,9 @@ function relationship_get_expectedresult(){ 'values' => array( '1' => array( 'id' => '1', - 'contact_id_a' => '33', - 'contact_id_b' => '34', - 'relationship_type_id' => '21', + 'contact_id_a' => '47', + 'contact_id_b' => '49', + 'relationship_type_id' => '22', 'start_date' => '2013-07-29 00:00:00', 'is_active' => '1', 'description' => '', @@ -73,4 +73,4 @@ function relationship_get_expectedresult(){ * * API Standards documentation: * http://wiki.civicrm.org/confluence/display/CRM/API+Architecture+Standards -*/ \ No newline at end of file +*/ diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php index 0d3a221ecd..143cf3f788 100644 --- a/tests/phpunit/api/v3/JobTest.php +++ b/tests/phpunit/api/v3/JobTest.php @@ -38,7 +38,6 @@ require_once 'CiviTest/CiviUnitTestCase.php'; class api_v3_JobTest extends CiviUnitTestCase { protected $_apiversion = 3; - public $_eNoticeCompliant = TRUE; public $DBResetRequired = FALSE; public $_entity = 'Job'; @@ -60,6 +59,7 @@ class api_v3_JobTest extends CiviUnitTestCase { function tearDown() { $this->quickCleanup(array('civicrm_job')); + CRM_Utils_Hook::singleton()->reset(); parent::tearDown(); } @@ -175,5 +175,38 @@ class api_v3_JobTest extends CiviUnitTestCase { $ct = 'Individual,Household'; $result = $this->callAPISuccess($this->_entity, 'update_greeting', array('gt' => $gt, 'ct' => $ct)); } + + public function testCallDisableExpiredRelationships() { + $individualID = $this->individualCreate(); + $orgID = $this->organizationCreate(); + CRM_Utils_Hook_UnitTests::singleton()->setHook('civicrm_pre', array($this, 'hookPreRelationship')); + $relationshipTypeID = $this->callAPISuccess('relationship_type', 'getvalue', array('return' => 'id', 'name_a_b' => 'Employee of')); + $result = $this->callAPISuccess('relationship', 'create', array( + 'relationship_type_id' => $relationshipTypeID, + 'contact_id_a' => $individualID, + 'contact_id_b' => $orgID, + 'is_active' => 1, + 'end_date' => 'yesterday', + )); + $relationshipID = $result['id']; + $this->assertEquals('Hooked', $result['values'][$relationshipID]['description']); + $this->callAPISuccess($this->_entity, 'disable_expired_relationships', array()); + $result = $this->callAPISuccess('relationship', 'get', array()); + $this->assertEquals('Go Go you good thing', $result['values'][$relationshipID]['description']); + $this->contactDelete($individualID); + $this->contactDelete($orgID); + } + + function hookPreRelationship($op, $objectName, $id, &$params ) { + if($op == 'delete') { + return; + } + if($params['is_active']) { + $params['description'] = 'Hooked'; + } + else { + $params['description'] = 'Go Go you good thing'; + } + } } diff --git a/tests/phpunit/api/v3/RelationshipTest.php b/tests/phpunit/api/v3/RelationshipTest.php index 59e508aa45..0b8a8d2115 100644 --- a/tests/phpunit/api/v3/RelationshipTest.php +++ b/tests/phpunit/api/v3/RelationshipTest.php @@ -288,7 +288,22 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { $params['id'] = $result['id']; $this->callAPISuccess('relationship', 'delete', $params); } - + /** + * ensure disabling works + */ + function testRelationshipUpdate() { + $result = $this->callAPISuccess('relationship', 'create', $this->_params); + $relID = $result['id']; + $result = $this->callAPISuccess('relationship', 'create', array('id' => $relID, 'description' => 'blah')); + $this->assertEquals($relID, $result['id']); + $this->assertEquals('blah', $result['values'][$result['id']]['description']); + $result = $this->callAPISuccess('relationship', 'create', array('id' => $relID, 'is_permission_b_a' => 1)); + $this->assertEquals(1, $result['values'][$result['id']]['is_permission_b_a']); + $result = $this->callAPISuccess('relationship', 'create', array('id' => $result['id'], 'is_active' => 0)); + $this->assertEquals(0, $result['values'][$result['id']]['is_active']); + $this->assertEquals('blah', $result['values'][$result['id']]['description']); + $this->assertEquals(1, $result['values'][$result['id']]['is_permission_b_a']); + } /** * check relationship creation */ -- 2.25.1