CRM-15203 fix situation where membership payment record is not created

diff --git a/CRM/Core/Payment/BaseIPN.php b/CRM/Core/Payment/BaseIPN.php
index db7b62c65089669952ad76902dc5f2b81a857ad4..26cf90cebd17c95313ff1c107213860044b16c11 100644 (file)
--- a/CRM/Core/Payment/BaseIPN.php
+++ b/CRM/Core/Payment/BaseIPN.php
@@ -205,7 +205,7 @@ class CRM_Core_Payment_BaseIPN {
 
     //add lineitems for recurring payments
     if (!empty($objects['contributionRecur']) && $objects['contributionRecur']->id && $addLineItems) {
-      $this->addrecurLineItems($objects['contributionRecur']->id, $contribution->id, CRM_Core_DAO::$_nullArray);
+      $this->addRecurLineItems($objects['contributionRecur']->id, $contribution);
     }
 
     //add new soft credit against current contribution id and
@@ -285,7 +285,7 @@ class CRM_Core_Payment_BaseIPN {
 
     //add lineitems for recurring payments
     if (!empty($objects['contributionRecur']) && $objects['contributionRecur']->id && $addLineItems) {
-      $this->addrecurLineItems($objects['contributionRecur']->id, $contribution->id, CRM_Core_DAO::$_nullArray);
+      $this->addRecurLineItems($objects['contributionRecur']->id, $contribution);
     }
 
     //add new soft credit against current $contribution and
@@ -570,8 +570,14 @@ LIMIT 1;";
     }
 
     //add lineitems for recurring payments
-    if (!empty($objects['contributionRecur']) && $objects['contributionRecur']->id && $addLineItems) {
-      $this->addrecurLineItems($objects['contributionRecur']->id, $contribution->id, $input);
+    if (!empty($objects['contributionRecur']) && $objects['contributionRecur']->id) {
+      if ($addLineItems) {
+        $input ['line_item'] = $this->addRecurLineItems($objects['contributionRecur']->id, $contribution);
+      }
+      else {
+        // this is just to prevent e-notices when we call recordFinancialAccounts - per comments on that line - intention is somewhat unclear
+        $input['line_item'] = array();
+      }
     }
 
     //copy initial contribution custom fields for recurring contributions
@@ -616,7 +622,7 @@ LIMIT 1;";
         $input['contribution_mode'] = 'membership';
       }
       //@todo writing a unit test I was unable to create a scenario where this line did not fatal on second
-      // and subsequent payments. In this case the line items are created at $this->addrecurLineItems
+      // and subsequent payments. In this case the line items are created at $this->addRecurLineItems
       // and since the contribution is saved prior to this line there is always a contribution-id,
       // however there is never a prevContribution (which appears to mean original contribution not previous
       // contribution - or preUpdateContributionObject most accurately)
@@ -624,6 +630,7 @@ LIMIT 1;";
       // to mean "are we updating an exisitng pending contribution"
       //I was able to make the unit test complete as fataling here doesn't prevent
       // the contribution being created - but activities would not be created or emails sent
+
       CRM_Contribute_BAO_Contribution::recordFinancialAccounts($input, NULL);
     }
 
@@ -930,31 +937,38 @@ LIMIT 1;";
 
   /**
    * @param $recurId
-   * @param $contributionId
-   * @param $input
+   * @param $contribution
+   *
+   * @internal param $contributionId
+   *
+   * @return array
    */
-  function addrecurLineItems($recurId, $contributionId, &$input) {
-    $lineSets = $lineItems = array();
-
-    //Get the first contribution id with recur id
-    if ($recurId) {
-      $contriID = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $recurId, 'id', 'contribution_recur_id');
-      $lineItems = CRM_Price_BAO_LineItem::getLineItems($contriID, 'contribution');
-      if (!empty($lineItems)) {
-        foreach ($lineItems as $key => $value) {
-          $pricesetID = new CRM_Price_DAO_PriceField();
-          $pricesetID->id = $value['price_field_id'];
-          $pricesetID->find(TRUE);
-          $lineSets[$pricesetID->price_set_id][] = $value;
+  function addRecurLineItems($recurId, $contribution) {
+    $lineSets = array();
+
+    $originalContributionID = CRM_Core_DAO::getFieldValue('CRM_Contribute_DAO_Contribution', $recurId, 'id', 'contribution_recur_id');
+    $lineItems = CRM_Price_BAO_LineItem::getLineItemsByContributionID($originalContributionID);
+    if (!empty($lineItems)) {
+      foreach ($lineItems as $key => $value) {
+        $priceField = new CRM_Price_DAO_PriceField();
+        $priceField->id = $value['price_field_id'];
+        $priceField->find(TRUE);
+        $lineSets[$priceField->price_set_id][] = $value;
+        if ($value['entity_table'] == 'civicrm_membership') {
+          try {
+            civicrm_api3('membership_payment', 'create', array('membership_id' => $value['entity_id'], 'contribution_id' => $contribution->id));
+          }
+          catch (CiviCRM_API3_Exception $e) {
+            // we are catching & ignoring errors as an extra precaution since lost IPNs may be more serious that lost membership_payment data
+            // this fn is unit-tested so risk of changes elsewhere breaking it are otherwise mitigated
+          }
         }
       }
-      if (!empty($input)) {
-        $input['line_item'] = $lineSets;
-      }
-      else {
-        CRM_Price_BAO_LineItem::processPriceSet($contributionId, $lineSets);
-      }
     }
+    else {
+      CRM_Price_BAO_LineItem::processPriceSet($contribution->id, $lineSets, $contribution);
+    }
+    return $lineSets;
   }
 
   // function to copy custom data of the

diff --git a/CRM/Price/BAO/LineItem.php b/CRM/Price/BAO/LineItem.php
index 363a8b0036eb3da191202b65d01258d737bb78eb..333685d6328ca7112ad4436bb2d2aa07a343b65b 100644 (file)
--- a/CRM/Price/BAO/LineItem.php
+++ b/CRM/Price/BAO/LineItem.php
@@ -123,6 +123,16 @@ AND li.entity_id = {$entityId}
     return $lineItemTotal;
   }
 
+  /**
+   * wrapper for line item retrieval when contribution ID is known
+   * @param $contributionID
+   *
+   * @return array
+   */
+  static function getLineItemsByContributionID($contributionID) {
+     return self::getLineItems($contributionID,'contribution', NULL, TRUE, TRUE, " WHERE contribution_id = " . (int) $contributionID);
+  }
+
   /**
    * Given a participant id/contribution id,
    * return contribution/fee line items
@@ -134,10 +144,14 @@ AND li.entity_id = {$entityId}
    * @param bool $isQtyZero
    * @param bool $relatedEntity
    *
+   * @param string $overrideWhereClause e.g "WHERE contribution id = 7 " per the getLineItemsByContributionID wrapper.
+   * this function precedes the convenience of the contribution id but since it does quite a bit more than just a db retrieval we need to be able to use it even
+   * when we don't want it's entity-id magix
+   *
    * @return array of line items
    */
-  static function getLineItems($entityId, $entity = 'participant', $isQuick = NULL , $isQtyZero = TRUE, $relatedEntity = FALSE) {
-    $selectClause = $whereClause = $fromClause = NULL;
+  static function getLineItems($entityId, $entity = 'participant', $isQuick = NULL , $isQtyZero = TRUE, $relatedEntity = FALSE, $overrideWhereClause = '') {
+    $whereClause = $fromClause = NULL;
     $selectClause = "
       SELECT    li.id,
       li.label,
@@ -145,6 +159,8 @@ AND li.entity_id = {$entityId}
       li.qty,
       li.unit_price,
       li.line_total,
+      li.entity_table,
+      li.entity_id,
       pf.label as field_title,
       pf.html_type,
       pfv.membership_type_id,
@@ -194,6 +210,10 @@ AND li.entity_id = {$entityId}
     $getTaxDetails = FALSE;
     $invoiceSettings = CRM_Core_BAO_Setting::getItem(CRM_Core_BAO_Setting::CONTRIBUTE_PREFERENCES_NAME,'contribution_invoice_settings');
     $invoicing = CRM_Utils_Array::value('invoicing', $invoiceSettings);
+    if ($overrideWhereClause) {
+      $whereClause = $overrideWhereClause;
+    }
+
     $dao = CRM_Core_DAO::executeQuery("$selectClause $fromClause $whereClause $orderByClause", $params);
     while ($dao->fetch()) {
       if (!$dao->id) {
@@ -210,7 +230,10 @@ AND li.entity_id = {$entityId}
         'field_title' => $dao->field_title,
         'html_type' => $dao->html_type,
         'description' => $dao->description,
-        'entity_id' => $entityId,
+        // the entity id seems prone to randomness but not sure if it has a reason - so if we are overriding the Where clause we assume
+        // we also JUST WANT TO KNOW the the entity_id in the DB
+        'entity_id' => empty($overrideWhereClause) ? $entityId : $dao->entity_id,
+        'entity_table' => $dao->entity_table,
         'contribution_id' => $dao->contribution_id,
         'financial_type_id' => $dao->financial_type_id,
         'membership_type_id' => $dao->membership_type_id,
@@ -356,7 +379,9 @@ AND li.entity_id = {$entityId}
 
       foreach ($values as $line) {
         $line['entity_table'] = $entityTable;
-        $line['entity_id'] = $entityId;
+        if (empty($line['entity_id'])) {
+          $line['entity_id'] = $entityId;
+        }
         if(!empty($line['membership_type_id'])) {
           $line['entity_table'] = 'civicrm_membership';
         }