Fix createEmployerRelationship to work with the data it has when doing a lookup,...

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

diff --git a/CRM/Contact/BAO/Contact/Utils.php b/CRM/Contact/BAO/Contact/Utils.php
index fb8d5bacac001d3490cd8514e65ef4b8b3e0f518..da904c31b8c559c5c17fa1e85fedf2a65a3ce58b 100644 (file)
--- 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 fae48c45960c5047f88e606e6e2732761c8d09c5..3ed1125b83e20335d7276e2b4af98f44d2b21b02 100644 (file)
--- 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 3d09f5a2771ba0df973401946162f1c12c6c8bc6..6fe9020efa2b0c0942cf57b5e05ea86e54b5ee0a 100644 (file)
--- 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']);
   }
 
   /**