From 5a9eb1973e4f615ca54fbc2733f5507165b82cb5 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Fri, 25 Nov 2022 14:39:56 +1300 Subject: [PATCH] Fix Relationship to permit disabled relationships to work & fix test --- CRM/Contact/BAO/Contact/Utils.php | 2 +- CRM/Contact/BAO/Relationship.php | 12 +++--- CRM/Contact/Import/Parser/Contact.php | 4 +- CRM/Core/BAO/Address.php | 2 +- .../api/v4/Entity/RelationshipTest.php | 42 +++++++++++++------ 5 files changed, 39 insertions(+), 23 deletions(-) diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php index 7916e81b07..40fdbd7151 100644 --- a/CRM/Contact/BAO/Contact/Utils.php +++ b/CRM/Contact/BAO/Contact/Utils.php @@ -418,7 +418,7 @@ WHERE id={$contactId}; "; $relMembershipParams['contact_check'][$employerId] = 1; //get relationship id. - if (CRM_Contact_BAO_Relationship::checkDuplicateRelationship($relMembershipParams, $contactId, $employerId)) { + if (CRM_Contact_BAO_Relationship::checkDuplicateRelationship($relMembershipParams, (int) $contactId, (int) $employerId)) { $relationship = new CRM_Contact_DAO_Relationship(); $relationship->contact_id_a = $contactId; $relationship->contact_id_b = $employerId; diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php index 4b6b86605f..fd99c8841b 100644 --- a/CRM/Contact/BAO/Relationship.php +++ b/CRM/Contact/BAO/Relationship.php @@ -51,7 +51,7 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship implemen $extendedParams = self::loadExistingRelationshipDetails($params); // When id is specified we always wan't to update, so we don't need to // check for duplicate relations. - if (!isset($params['id']) && self::checkDuplicateRelationship($extendedParams, $extendedParams['contact_id_a'], $extendedParams['contact_id_b'], CRM_Utils_Array::value('id', $extendedParams, 0))) { + if (!isset($params['id']) && self::checkDuplicateRelationship($extendedParams, (int) $extendedParams['contact_id_a'], (int) $extendedParams['contact_id_b'], CRM_Utils_Array::value('id', $extendedParams, 0))) { throw new CRM_Core_Exception('Duplicate Relationship'); } $params = $extendedParams; @@ -201,9 +201,9 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship implemen if ( self::checkDuplicateRelationship( $contactFields, - $contactID, + (int) $contactID, // step 2 - $relatedContactID + (int) $relatedContactID ) ) { return [0, 1]; @@ -842,14 +842,14 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship implemen * @return bool * true if record exists else false */ - public static function checkDuplicateRelationship($params, $id, $contactId = 0, $relationshipId = 0) { + public static function checkDuplicateRelationship(array $params, int $id, int $contactId = 0, int $relationshipId = 0): bool { $relationshipTypeId = $params['relationship_type_id'] ?? NULL; [$type] = explode('_', $relationshipTypeId); - $queryString = " + $queryString = ' SELECT id FROM civicrm_relationship -WHERE relationship_type_id = " . CRM_Utils_Type::escape($type, 'Integer'); +WHERE is_active = 1 AND relationship_type_id = ' . CRM_Utils_Type::escape($type, 'Integer'); /* * CRM-11792 - date fields from API are in ISO format, but this function diff --git a/CRM/Contact/Import/Parser/Contact.php b/CRM/Contact/Import/Parser/Contact.php index 5c96f7c0e4..49c14051ce 100644 --- a/CRM/Contact/Import/Parser/Contact.php +++ b/CRM/Contact/Import/Parser/Contact.php @@ -266,9 +266,9 @@ class CRM_Contact_Import_Parser_Contact extends CRM_Import_Parser { if ( CRM_Contact_BAO_Relationship::checkDuplicateRelationship( $contactFields, - $contactID, + (int) $contactID, // step 2 - $relatedContactID + (int) $relatedContactID ) ) { return [0, 1]; diff --git a/CRM/Core/BAO/Address.php b/CRM/Core/BAO/Address.php index 7d81b4a1a4..5b114112fc 100644 --- a/CRM/Core/BAO/Address.php +++ b/CRM/Core/BAO/Address.php @@ -1125,7 +1125,7 @@ SELECT is_primary, // If already there is a relationship record of $relParam criteria, avoid creating relationship again or else // it will casue CRM-16588 as the Duplicate Relationship Exception will revert other contact field values on update - if (CRM_Contact_BAO_Relationship::checkDuplicateRelationship($relParam, $currentContactId, $sharedContactId)) { + if (CRM_Contact_BAO_Relationship::checkDuplicateRelationship($relParam, (int) $currentContactId, (int) $sharedContactId)) { return; } diff --git a/tests/phpunit/api/v4/Entity/RelationshipTest.php b/tests/phpunit/api/v4/Entity/RelationshipTest.php index f3d9f4f22c..a73f637eb1 100644 --- a/tests/phpunit/api/v4/Entity/RelationshipTest.php +++ b/tests/phpunit/api/v4/Entity/RelationshipTest.php @@ -23,6 +23,8 @@ use api\v4\Api4TestBase; use Civi\Api4\Relationship; use Civi\Api4\RelationshipCache; use Civi\Test\TransactionalInterface; +use DateInterval; +use DateTime; /** * Assert that interchanging data between APIv3 and APIv4 yields consistent @@ -32,7 +34,12 @@ use Civi\Test\TransactionalInterface; */ class RelationshipTest extends Api4TestBase implements TransactionalInterface { - public function testRelCacheCount() { + /** + * Test relationship cache tracks created relationships. + * + * @throws \CRM_Core_Exception + */ + public function testRelationshipCacheCount(): void { $c1 = Contact::create(FALSE)->addValue('first_name', '1')->execute()->first()['id']; $c2 = Contact::create(FALSE)->addValue('first_name', '2')->execute()->first()['id']; Relationship::create(FALSE) @@ -47,6 +54,9 @@ class RelationshipTest extends Api4TestBase implements TransactionalInterface { $this->assertCount(2, $cacheRecords); } + /** + * @throws \CRM_Core_Exception + */ public function testRelationshipCacheCalcFields(): void { $c1 = Contact::create(FALSE)->addValue('first_name', '1')->execute()->first()['id']; $c2 = Contact::create(FALSE)->addValue('first_name', '2')->execute()->first()['id']; @@ -77,10 +87,15 @@ class RelationshipTest extends Api4TestBase implements TransactionalInterface { $this->assertEquals($relationship['modified_date'], $cacheRecords[$c2]['relationship_modified_date']); } + /** + * Test that a relationship can be created with the same values as a disabled relationship. + * + * @throws \CRM_Core_Exception + */ public function testRelationshipDisableCreate(): void { - $today = new \DateTime('today'); - $future = new \DateTime('today'); - $future->add(new \DateInterval('P1Y')); + $today = new DateTime('today'); + $future = new DateTime('today'); + $future->add(new DateInterval('P1Y')); $c1 = Contact::create(FALSE)->addValue('first_name', '1')->execute()->first()['id']; $c2 = Contact::create(FALSE)->addValue('first_name', '2')->execute()->first()['id']; @@ -98,11 +113,11 @@ class RelationshipTest extends Api4TestBase implements TransactionalInterface { $relationship = Relationship::get(FALSE) ->addWhere('id', '=', $relationship['id']) ->execute()->first(); - $relationshipDisable = Relationship::update(FALSE) + Relationship::update(FALSE) ->addWhere('id', '=', $relationship['id']) ->addValue('is_active', FALSE) ->execute()->first(); - $relationship2 = Relationship::create(FALSE) + Relationship::create(FALSE) ->setValues([ 'contact_id_a' => $c1, 'contact_id_b' => $c2, @@ -117,22 +132,23 @@ class RelationshipTest extends Api4TestBase implements TransactionalInterface { $cacheRecords = RelationshipCache::get(FALSE) ->addWhere('near_contact_id', 'IN', [$c1]) ->addWhere('is_active', '=', FALSE) - ->addSelect('near_contact_id', 'orientation', 'description', 'start_date', 'end_date') + ->addSelect('near_contact_id', 'orientation', 'description', 'start_date', 'end_date', 'is_active') ->execute()->indexBy('near_contact_id'); $this->assertCount(1, $cacheRecords); $this->assertEquals(FALSE, $cacheRecords[$c1]['is_active']); - $this->assertEquals($today, $cachedRecords[$c1]['start_date']); - $this->assertEquals($future, $cachedRecords[$c1]['end_date']); + $this->assertEquals($today->format('Y-m-d'), $cacheRecords[$c1]['start_date']); + $this->assertEquals($future->format('Y-m-d'), $cacheRecords[$c1]['end_date']); $cacheRecords = RelationshipCache::get(FALSE) ->addWhere('near_contact_id', 'IN', [$c1]) ->addWhere('is_active', '=', TRUE) - ->addSelect('near_contact_id', 'orientation', 'description', 'start_date', 'end_date') + ->addSelect('near_contact_id', 'orientation', 'description', 'start_date', 'end_date', 'is_active') ->execute()->indexBy('near_contact_id'); $this->assertCount(1, $cacheRecords); - $this->assertEquals(TRUE, $cacheRecords[$c1]['is_active']); - $this->assertEquals($today, $cachedRecords[$c1]['start_date']); - $this->assertEquals($future, $cachedRecords[$c1]['end_date']); + $cacheRecord = $cacheRecords->first(); + $this->assertEquals(TRUE, $cacheRecord['is_active']); + $this->assertEquals($today->format('Y-m-d'), $cacheRecord['start_date']); + $this->assertEquals($future->format('Y-m-d'), $cacheRecord['end_date']); } } -- 2.25.1