From 72d57998d17a80c776ed85460cd0699b03a050a3 Mon Sep 17 00:00:00 2001
From: eileen <emcnaughton@wikimedia.org>
Date: Mon, 11 Mar 2019 12:44:36 +1300
Subject: [PATCH] Further cleanup on getRelatedMemberships - just get them with
 the api

Per work by Matt it makes sense to just retrieve as an api rather than
do weird object wrangling. This is intended to not change the logic about what is fetched
---
 CRM/Contribute/BAO/Contribution.php | 45 ++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
index 989129001f..f7d7abd632 100644
--- a/CRM/Contribute/BAO/Contribution.php
+++ b/CRM/Contribute/BAO/Contribution.php
@@ -949,20 +949,24 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution {
    * @return array
    */
   protected static function getRelatedMemberships($contributionID) {
-    $contribution = new CRM_Contribute_BAO_Contribution();
-    $contribution->id = $contributionID;
-    $contribution->fetch(TRUE);
-    $contribution->loadRelatedMembershipObjects();
-    $result = CRM_Utils_Array::value('membership', $contribution->_relatedObjects, []);
-    $memberships = [];
-    foreach ($result as $membership) {
-      if (empty($membership)) {
-        continue;
-      }
-      // @todo - remove this again & just call api in the first place.
-      _civicrm_api3_object_to_array($membership, $memberships[$membership->id]);
-    }
-    return $memberships;
+    $membershipPayments = civicrm_api3('MembershipPayment', 'get', [
+      'return' => 'membership_id',
+      'contribution_id' => (int) $contributionID,
+    ])['values'];
+    $membershipIDs = [];
+    foreach ($membershipPayments as $membershipPayment) {
+      $membershipIDs[] = $membershipPayment['membership_id'];
+    }
+    if (empty($membershipIDs)) {
+      return [];
+    }
+    // We could combine this with the MembershipPayment.get - we'd
+    // need to re-wrangle the params (here or in the calling function)
+    // as they would then me membership.contact_id, membership.is_test etc
+    return civicrm_api3('Membership', 'get', [
+      'id' => ['IN' => $membershipIDs],
+      'return' => ['id', 'contact_id', 'membership_type_id', 'is_test']
+    ])['values'];
   }
 
   /**
@@ -2414,6 +2418,14 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
    * @throws Exception
    */
   public function loadRelatedObjects(&$input, &$ids, $loadAll = FALSE) {
+    // @todo deprecate this function - the steps should be
+    // 1) add additional functions like 'getRelatedMemberships'
+    // 2) switch all calls that refer to ->_relatedObjects to
+    // using the helper functions
+    // 3) make ->_relatedObjects noisy in some way (deprecation won't work for properties - hmm
+    // 4) make ->_relatedObjects protected
+    // 5) hone up the individual functions to not use rely on this having been called
+    // 6) deprecate like mad
     if ($loadAll) {
       $ids = array_merge($this->getComponentDetails($this->id), $ids);
       if (empty($ids['contact']) && isset($this->contact_id)) {
@@ -2476,6 +2488,8 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
       }
     }
 
+    // These are probably no longer accessed from anywhere
+    // @todo remove this line, after ensuring not used.
     $ids = $this->loadRelatedMembershipObjects($ids);
 
     if ($this->_component != 'contribute') {
@@ -4518,6 +4532,7 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
       $input['participant_id'] = $contribution->_relatedObjects['participant']->id;
     }
     elseif (!empty($contribution->_relatedObjects['membership'])) {
+      // @todo - use getRelatedMemberships instead
       $input['contribution_mode'] = 'membership';
       $contribution->contribution_status_id = $contributionParams['contribution_status_id'];
       $contribution->trxn_id = CRM_Utils_Array::value('trxn_id', $input);
@@ -4652,6 +4667,8 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac
   /**
    * Load related memberships.
    *
+   * @deprecated
+   *
    * Note that in theory it should be possible to retrieve these from the line_item table
    * with the membership_payment table being deprecated. Attempting to do this here causes tests to fail
    * as it seems the api is not correctly linking the line items when the contribution is created in the flow
-- 
2.25.1