From 34a100a7b59eb54f42fc8926d10797b7627fa1d6 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 4 Nov 2014 23:37:58 +1300 Subject: [PATCH] CRM-15203 fix situation where membership payment record is not created --- CRM/Core/Payment/BaseIPN.php | 66 ++++++++++++++++++++++-------------- CRM/Price/BAO/LineItem.php | 33 +++++++++++++++--- 2 files changed, 69 insertions(+), 30 deletions(-) diff --git a/CRM/Core/Payment/BaseIPN.php b/CRM/Core/Payment/BaseIPN.php index db7b62c650..26cf90cebd 100644 --- 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 363a8b0036..333685d632 100644 --- 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'; } -- 2.25.1