From 98fd6fc8bd5ac6d90082f722ad072c0ebce74ad9 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Sat, 3 Aug 2019 11:53:23 +1200
Subject: [PATCH] [REF] Extract chunk of code relating to whether to disabled
 an inherited relationship

---
 CRM/Contact/BAO/Relationship.php              | 45 ++++++++++++-------
 .../CRM/Contact/BAO/RelationshipTest.php      | 11 +++--
 2 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/CRM/Contact/BAO/Relationship.php b/CRM/Contact/BAO/Relationship.php
index 5d55ef141a82..cc46c9c1faf3 100644
--- a/CRM/Contact/BAO/Relationship.php
+++ b/CRM/Contact/BAO/Relationship.php
@@ -1670,21 +1670,9 @@ public static function relatedMemberships($contactId, &$params, $ids, $action =
       foreach ($details['memberships'] as $membershipId => $membershipValues) {
         $relTypeIds = [];
         if ($action & CRM_Core_Action::DELETE) {
-          // Delete memberships of the related contacts only if relationship type exists for membership type
-          $query = "
-SELECT relationship_type_id, relationship_direction
-  FROM civicrm_membership_type
- WHERE id = {$membershipValues['membership_type_id']}";
-          $dao = CRM_Core_DAO::executeQuery($query);
-          $relTypeDirs = [];
-          while ($dao->fetch()) {
-            $relTypeId = $dao->relationship_type_id;
-            $relDirection = $dao->relationship_direction;
-          }
-          $relTypeIds = explode(CRM_Core_DAO::VALUE_SEPARATOR, $relTypeId);
-          if (in_array($values[$cid]['relationshipTypeId'], $relTypeIds
-            //CRM-16300 check if owner membership exist for related membership
-            ) && !empty($membershipValues['owner_membership_id']) && !empty($values[$mainRelatedContactId]['memberships'][$membershipValues['owner_membership_id']])) {
+          // @todo don't return relTypeId here - but it seems to be used later in a cryptic way (hint cryptic is not a complement).
+          list($relTypeId, $isDeletable) = self::isInheritedMembershipInvalidated($membershipValues, $values, $cid, $mainRelatedContactId);
+          if ($isDeletable) {
             CRM_Member_BAO_Membership::deleteRelatedMemberships($membershipValues['owner_membership_id'], $membershipValues['membership_contact_id']);
           }
           continue;
@@ -2324,4 +2312,31 @@ private static function isRelationshipTypeCurrentEmployer(int $existingTypeID):
     return $isCurrentEmployerRelationshipType;
   }
 
+  /**
+   * Is the inherited relationship invalidated by this relationship change.
+   *
+   * @param $membershipValues
+   * @param array $values
+   * @param int $cid
+   * @param int $mainRelatedContactId
+   *
+   * @return array
+   */
+  private static function isInheritedMembershipInvalidated($membershipValues, array $values, $cid, $mainRelatedContactId): array {
+    // Delete memberships of the related contacts only if relationship type exists for membership type
+    $query = "
+SELECT relationship_type_id, relationship_direction
+  FROM civicrm_membership_type
+ WHERE id = {$membershipValues['membership_type_id']}";
+    $dao = CRM_Core_DAO::executeQuery($query);
+    while ($dao->fetch()) {
+      $relTypeId = $dao->relationship_type_id;
+    }
+    $relTypeIds = explode(CRM_Core_DAO::VALUE_SEPARATOR, $relTypeId);
+    $isDeletable = in_array($values[$cid]['relationshipTypeId'], $relTypeIds
+      //CRM-16300 check if owner membership exist for related membership
+      ) && !empty($membershipValues['owner_membership_id']) && !empty($values[$mainRelatedContactId]['memberships'][$membershipValues['owner_membership_id']]);
+    return [$relTypeId, $isDeletable];
+  }
+
 }
diff --git a/tests/phpunit/CRM/Contact/BAO/RelationshipTest.php b/tests/phpunit/CRM/Contact/BAO/RelationshipTest.php
index 7b3ed5bcecef..ddde7bc94d65 100644
--- a/tests/phpunit/CRM/Contact/BAO/RelationshipTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/RelationshipTest.php
@@ -249,15 +249,18 @@ public function testSingleMembershipForTwoRelationships() {
       'relationship_type_id' => $orgToPersonTypeId2,
     ]);
 
+    $this->callAPISuccessGetCount('Membership', ['contact_id' => $individualID], 1);
+
+    // Disable the relationship & check the membership is removed.
+    $relationshipOne['is_active'] = 0;
+    $this->callAPISuccess('Relationship', 'create', array_merge($relationshipOne, ['is_active' => 0]));
+    $this->callAPISuccessGetCount('Membership', ['contact_id' => $individualID], 0);
     /*
      * @todo this section not yet working due to bug in would-be-tested code.
-    $this->callAPISuccessGetCount('Membership', ['contact_id' => $individualID], 1);
     $relationshipTwo['is_active'] = 0;
     $this->callAPISuccess('Relationship', 'create', $relationshipTwo);
     $this->callAPISuccessGetCount('Membership', ['contact_id' => $individualID], 1);
-    $relationshipOne['is_active'] = 0;
-    $this->callAPISuccess('Relationship', 'create', $relationshipOne);
-    $this->callAPISuccessGetCount('Membership', ['contact_id' => $individualID], 0);
+
     $relationshipOne['is_active'] = 1;
     $this->callAPISuccess('Relationship', 'create', $relationshipOne);
     $this->callAPISuccessGetCount('Membership', ['contact_id' => $individualID], 1);