From d4fd16100252a88fb637fc0bac08478d4a9e8df9 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 18 Jan 2022 19:04:01 +1300 Subject: [PATCH] Fix createEmployerRelationship to work with the data it has when doing a lookup, test The duplicate check has a bug in it but also it's really complicated give we specifically know the ids & type and not any other values --- CRM/Contact/BAO/Contact/Utils.php | 62 +++++++++++++---------- tests/phpunit/CRM/Dedupe/MergerTest.php | 49 ++++++++++++++++++ tests/phpunit/api/v3/RelationshipTest.php | 5 +- 3 files changed, 85 insertions(+), 31 deletions(-) diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php index fb8d5bacac..da904c31b8 100644 --- a/CRM/Contact/BAO/Contact/Utils.php +++ b/CRM/Contact/BAO/Contact/Utils.php @@ -10,6 +10,7 @@ */ use Civi\Api4\Contact; +use Civi\Api4\Relationship; /** * @@ -272,17 +273,38 @@ WHERE id IN ( $idString ) CRM_Core_Error::deprecatedWarning('attempting to create an employer with invalid contact types is deprecated'); return; } + $relationshipIds = []; - $duplicate = CRM_Contact_BAO_Relationship::checkDuplicateRelationship( - [ - 'contact_id_a' => $contactID, - 'contact_id_b' => $employerID, - 'relationship_type_id' => $relationshipTypeID, - ], - $contactID, - $employerID - ); - if (!$duplicate) { + $ids = []; + $action = CRM_Core_Action::ADD; + $existingRelationship = Relationship::get(FALSE) + ->setWhere([ + ['contact_id_a', '=', $contactID], + ['contact_id_b', '=', $employerID], + ['OR', [['start_date', '<=', 'now'], ['start_date', 'IS EMPTY']]], + ['OR', [['end_date', '>=', 'now'], ['end_date', 'IS EMPTY']]], + ['relationship_type_id', '=', $relationshipTypeID], + ['is_active', 'IN', [0, 1]], + ]) + ->setSelect(['id', 'is_active', 'start_date', 'end_date', 'contact_id_a.employer_id']) + ->addOrderBy('is_active', 'DESC') + ->setLimit(1) + ->execute()->first(); + + if (!empty($existingRelationship)) { + if ($existingRelationship['is_active']) { + // My work here is done. + return; + } + + $action = CRM_Core_Action::UPDATE; + // No idea why we set these ids but it's either legacy cruft or used by `relatedMemberships` + $ids['contact'] = $contactID; + $ids['contactTarget'] = $employerID; + $ids['relationship'] = $existingRelationship['id']; + CRM_Contact_BAO_Relationship::setIsActive($existingRelationship['id'], TRUE); + } + else { $params = [ 'is_active' => TRUE, 'contact_check' => [$employerID => TRUE], @@ -308,25 +330,9 @@ WHERE id IN ( $idString ) // set current employer self::setCurrentEmployer([$contactID => $employerID]); - $ids = []; - $action = CRM_Core_Action::ADD; - - //we do not know that triggered relationship record is active. - if ($duplicate) { - $relationship = new CRM_Contact_DAO_Relationship(); - $relationship->contact_id_a = $contactID; - $relationship->contact_id_b = $employerID; - $relationship->relationship_type_id = $relationshipTypeID; - if ($relationship->find(TRUE)) { - $action = CRM_Core_Action::UPDATE; - $ids['contact'] = $contactID; - $ids['contactTarget'] = $employerID; - $ids['relationship'] = $relationship->id; - CRM_Contact_BAO_Relationship::setIsActive($relationship->id, TRUE); - } - } - //need to handle related memberships. CRM-3792 + // @todo - this probably duplicates the work done in the setIsActive + // for duplicates... if ($previousEmployerID != $employerID) { CRM_Contact_BAO_Relationship::relatedMemberships($contactID, [ 'relationship_ids' => $relationshipIds, diff --git a/tests/phpunit/CRM/Dedupe/MergerTest.php b/tests/phpunit/CRM/Dedupe/MergerTest.php index fae48c4596..3ed1125b83 100644 --- a/tests/phpunit/CRM/Dedupe/MergerTest.php +++ b/tests/phpunit/CRM/Dedupe/MergerTest.php @@ -7,6 +7,8 @@ */ class CRM_Dedupe_MergerTest extends CiviUnitTestCase { + use CRMTraits_Custom_CustomDataTrait; + protected $_groupId; protected $_contactIds = []; @@ -1470,6 +1472,33 @@ WHERE $this->callAPISuccess('Contact', 'merge', ['to_keep_id' => $contact1, 'to_remove_id' => $contact2]); } + /** + * Test that a custom field attached to the relationship does not block merge. + * + * @throws \CRM_Core_Exception + */ + public function testMergeWithRelationshipWithCustomFields(): void { + $contact1 = $this->individualCreate(); + $this->createCustomGroupWithFieldsOfAllTypes(['extends' => 'Relationship']); + $contact2 = $this->createContactWithEmployerRelationship([ + $this->getCustomFieldName('text') => 'blah', + $this->getCustomFieldName('boolean') => TRUE, + ]); + $this->callAPISuccess('Contact', 'merge', ['to_keep_id' => $contact1, 'to_remove_id' => $contact2]); + $this->callAPISuccessGetSingle('Relationship', [ + 'contact_id_a' => $contact1, + ]); + + $contact2 = $this->createContactWithEmployerRelationship([ + $this->getCustomFieldName('boolean') => TRUE, + $this->getCustomFieldName('text') => '', + ]); + $this->callAPISuccess('Contact', 'merge', ['to_keep_id' => $contact2, 'to_remove_id' => $contact1]); + $this->callAPISuccessGetSingle('Relationship', [ + 'contact_id_a' => $contact2, + ]); + } + /** * Implements hook_civicrm_entityTypes(). * @@ -1496,4 +1525,24 @@ WHERE $links[] = new CRM_Core_Reference_Basic('civicrm_im', 'name', 'civicrm_contact', 'first_name'); } + /** + * Create an individual with a relationship of type employee. + * + * @param array $params + * + * @return int + * @throws \CRM_Core_Exception + */ + protected function createContactWithEmployerRelationship(array $params): int { + $contact2 = $this->individualCreate(); + // Test the merge can also happen if the other contact has an empty text field. + $this->callAPISuccess('Relationship', 'create', array_merge([ + 'contact_id_a' => $contact2, + 'contact_id_b' => CRM_Core_BAO_Domain::getDomain()->contact_id, + 'relationship_type_id' => 'Employee of', + 'is_current_employer' => TRUE, + ], $params)); + return $contact2; + } + } diff --git a/tests/phpunit/api/v3/RelationshipTest.php b/tests/phpunit/api/v3/RelationshipTest.php index 3d09f5a277..6fe9020efa 100644 --- a/tests/phpunit/api/v3/RelationshipTest.php +++ b/tests/phpunit/api/v3/RelationshipTest.php @@ -652,11 +652,10 @@ class api_v3_RelationshipTest extends CiviUnitTestCase { $params_2 = array_merge($params_1, $custom_params_2); - $this->callAPISuccess('relationship', 'create', $params_1); - $result_2 = $this->callAPISuccess('relationship', 'create', $params_2); + $this->callAPISuccess('Relationship', 'create', $params_1); + $result_2 = $this->callAPISuccess('Relationship', 'create', $params_2); $this->assertNotNull($result_2['id']); - $this->assertEquals(0, $result_2['is_error']); } /** -- 2.25.1