Merge pull request #25647 from mattwire/disableinvalidrelationship

Allow to disable an invalid relationship (eg. contact subtype was changed so no longer valid)
civicrm · Apr 4, 2023 · 2f74e53 · 2f74e53
2 parents ce0a77e + bafcad8
commit 2f74e53
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 26 deletions.
diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php
@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Relationship;
+
 /**
  * Class CRM_Contact_BAO_Relationship.
  */
@@ -49,14 +51,16 @@ class CRM_Contact_BAO_Relationship extends CRM_Contact_DAO_Relationship implemen
   public static function create(&$params) {
 
     $extendedParams = self::loadExistingRelationshipDetails($params);
-    // When id is specified we always wan't to update, so we don't need to
-    // check for duplicate relations.
+    // When id is specified we always want to update, so we don't need to check for duplicate relations.
     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;
-    if (!CRM_Contact_BAO_Relationship::checkRelationshipType($params['contact_id_a'], $params['contact_id_b'],
-      $params['relationship_type_id'])) {
+    // Check if this is a "simple" disable relationship. If it is don't check the relationshipType
+    if (!empty($params['id']) && array_key_exists('is_active', $params) && empty($params['is_active'])) {
+      $disableRelationship = TRUE;
+    }
+    if (empty($disableRelationship) && !CRM_Contact_BAO_Relationship::checkRelationshipType($params['contact_id_a'], $params['contact_id_b'], $params['relationship_type_id'])) {
       throw new CRM_Core_Exception('Invalid Relationship');
     }
     $relationship = self::add($params);
@@ -616,8 +620,25 @@ public static function disableEnableRelationship($id, $action, $params = [], $id
         ];
       }
       $contact_id_a = empty($params['contact_id_a']) ? $relationship->contact_id_a : $params['contact_id_a'];
-      // calling relatedMemberships to delete/add the memberships of
-      // related contacts.
+
+      // Check if relationship can be used for related memberships
+      $membershipTypes = \Civi\Api4\MembershipType::get(FALSE)
+        ->addSelect('relationship_type_id')
+        ->addGroupBy('relationship_type_id')
+        ->addWhere('relationship_type_id', 'IS NOT EMPTY')
+        ->execute();
+      foreach ($membershipTypes as $membershipType) {
+        // We have to loop through them because relationship_type_id is an array and we can't filter by a single
+        // relationship id using API.
+        if (in_array($relationship->relationship_type_id, $membershipType['relationship_type_id'])) {
+          $relationshipIsUsedForRelatedMemberships = TRUE;
+        }
+      }
+      if (empty($relationshipIsUsedForRelatedMemberships)) {
+        // This relationship is not configured for any related membership types
+        return;
+      }
+      // Call relatedMemberships to delete/add the memberships of related contacts.
       if ($action & CRM_Core_Action::DISABLE) {
         // @todo this could call a subset of the function that just relates to
         // cleaning up no-longer-inherited relationships
@@ -1377,10 +1398,9 @@ public static function getRelationType($targetContactType) {
    * @throws \CRM_Core_Exception
    */
   public static function relatedMemberships($contactId, $params, $ids, $action = CRM_Core_Action::ADD, $active = TRUE) {
-    // Check the end date and set the status of the relationship
-    // accordingly.
+    // Check the end date and set the status of the relationship accordingly.
     $status = self::CURRENT;
-    $targetContact = $targetContact = CRM_Utils_Array::value('contact_check', $params, []);
+    $targetContact = $params['contact_check'] ?? [];
     $today = date('Ymd');
 
     // If a relationship hasn't yet started, just return for now
@@ -1430,8 +1450,7 @@ public static function relatedMemberships($contactId, $params, $ids, $action = C
     // Build the 'values' array for
     // 1. ContactA
     // 2. ContactB
-    // This will allow us to check if either of the contacts in
-    // relationship have active memberships.
+    // This will allow us to check if either of the contacts in relationship have active memberships.
 
     $values = [];
 
@@ -1490,8 +1509,7 @@ public static function relatedMemberships($contactId, $params, $ids, $action = C
           ($status & self::PAST) &&
           ($membershipValues['owner_membership_id'])
         ) {
-          // If relationship is PAST and action is UPDATE
-          // then delete the RELATED membership
+          // If relationship is PAST and action is UPDATE then delete the RELATED membership
           CRM_Member_BAO_Membership::deleteRelatedMemberships($membershipValues['owner_membership_id'],
             $membershipValues['contact_id']
           );
@@ -1517,9 +1535,7 @@ public static function relatedMemberships($contactId, $params, $ids, $action = C
         }
         $relTypeDir = $details['relationshipTypeId'] . $details['relationshipTypeDirection'];
         if (in_array($relTypeDir, $relTypeDirs)) {
-          // Check if relationship being created/updated is
-          // similar to that of membership type's
-          // relationship.
+          // Check if relationship being created/updated is similar to that of membership type's relationship.
 
           $membershipValues['owner_membership_id'] = $membershipId;
           unset($membershipValues['id']);
@@ -1542,11 +1558,10 @@ public static function relatedMemberships($contactId, $params, $ids, $action = C
                 continue;
               }
 
-              //delete the membership record for related
-              //contact before creating new membership record.
+              // delete the membership record for related contact before creating new membership record.
               CRM_Member_BAO_Membership::deleteRelatedMemberships($membershipId, $relatedContactId);
             }
-            //skip status calculation for pay later memberships.
+            // skip status calculation for pay later memberships.
             if ('Pending' === CRM_Core_PseudoConstant::getName('CRM_Member_BAO_Membership', 'status_id', $membershipValues['status_id'])) {
               $membershipValues['skipStatusCal'] = TRUE;
             }
@@ -2152,13 +2167,14 @@ private static function isInheritedMembershipInvalidated($membershipValues, arra
    */
   private static function isContactHasValidRelationshipToInheritMembershipType(int $contactID, int $membershipTypeID, int $parentMembershipID): bool {
     $membershipType = CRM_Member_BAO_MembershipType::getMembershipType($membershipTypeID);
-    $existingRelationships = civicrm_api3('Relationship', 'get', [
-      'contact_id_a' => $contactID,
-      'contact_id_b' => $contactID,
-      'relationship_type_id' => ['IN' => $membershipType['relationship_type_id']],
-      'options' => ['or' => [['contact_id_a', 'contact_id_b']], 'limit' => 0],
-      'is_active' => 1,
-    ])['values'];
+
+    $existingRelationships = Relationship::get(FALSE)
+      ->addWhere('relationship_type_id', 'IN', $membershipType['relationship_type_id'])
+      ->addClause('OR', ['contact_id_a', '=', $contactID], ['contact_id_b', '=', $contactID])
+      ->addWhere('is_active', '=', TRUE)
+      ->execute()
+      ->indexBy('id')
+      ->getArrayCopy();
 
     if (empty($existingRelationships)) {
       return FALSE;

diff --git a/tests/phpunit/CRM/Contact/BAO/RelationshipTest.php b/tests/phpunit/CRM/Contact/BAO/RelationshipTest.php
@@ -9,6 +9,8 @@
  +--------------------------------------------------------------------+
  */
 
+use Civi\Api4\Relationship;
+
 /**
  * Test class for CRM_Contact_BAO_Relationship
  *
@@ -320,4 +322,56 @@ public function testBAOAdd() {
     $this->assertEquals($relationshipObj->end_date, $today);
   }
 
+  /**
+   * This tests that you can disable an invalid relationship.
+   * It is quite easy to end up with invalid relationships if you change contact subtypes.
+   *
+   * @return void
+   * @throws \CRM_Core_Exception
+   */
+  public function testDisableInvalidRelationship() {
+    $individualStaff = civicrm_api3('Contact', 'create', [
+      'display_name' => 'Individual A',
+      'contact_type' => 'Individual',
+      'contact_sub_type' => 'Staff',
+    ]);
+    $individualStudent = civicrm_api3('Contact', 'create', [
+      'display_name' => 'Individual B',
+      'contact_type' => 'Individual',
+      'contact_sub_type' => 'Student',
+    ]);
+
+    $personToOrgType = 'A_B_relationship';
+    $orgToPersonType = 'B_A_relationship';
+
+    $relationshipTypeID = civicrm_api3('RelationshipType', 'create', [
+      'name_a_b' => $personToOrgType,
+      'name_b_a' => $orgToPersonType,
+      'contact_type_a' => 'Individual',
+      'contact_type_b' => 'Individual',
+      'contact_sub_type_a' => 'Staff',
+      'contact_sub_type_b' => 'Student',
+    ])['id'];
+
+    // Create a relationship between the two individuals with sub types
+    $relationship = Relationship::create(FALSE)
+      ->addValue('contact_id_a', $individualStaff['id'])
+      ->addValue('contact_id_b', $individualStudent['id'])
+      ->addValue('relationship_type_id', $relationshipTypeID)
+      ->execute()
+      ->first();
+
+    // This makes the relationship invalid because one contact sub type is no longer matching the required one
+    civicrm_api3('Contact', 'create', [
+      'id' => $individualStaff['id'],
+      'contact_sub_type' => 'Parent',
+    ]);
+
+    // Check that we can disable the invalid relationship
+    Relationship::update(FALSE)
+      ->addValue('is_active', FALSE)
+      ->addWhere('id', '=', $relationship['id'])
+      ->execute();
+  }
+
 }