From: Eileen McNaughton Date: Tue, 11 Jan 2022 18:37:58 +0000 (+1300) Subject: Revert membership debug handling in IPN X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=19d9044caf20d929febca79072d7f06dc94d94a4;p=civicrm-core.git Revert membership debug handling in IPN A looong time ago we added handling for the possibility of messed up memberships going through the paypal IPN. https://github.com/civicrm/civicrm-core/pull/15053 At the time we added a lot of debug to see if it was ever hit as the handling was an attempt to figure out a bug that we couldn't reproduce. We never got any evidence the debug was hit or the fix helped - but it does create a lot of code complexity and makes clean up more difficult so this removes it again.... --- diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index a44d125fb6..885fe7af9d 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -964,27 +964,6 @@ class CRM_Contribute_BAO_Contribution extends CRM_Contribute_DAO_Contribution { ])['values']; } - /** - * It is possible to override the membership id that is updated from the payment processor. - * - * Historically Paypal does this & it still does if it determines data is messed up - see - * https://lab.civicrm.org/dev/membership/issues/13 - * - * Read the comment block on repeattransaction for more information - * about how things should work. - * - * @param int $contributionID - * @param array $input - * - * @throws \CiviCRM_API3_Exception - */ - protected static function handleMembershipIDOverride($contributionID, $input) { - if (!empty($input['membership_id'])) { - Civi::log()->debug('The related membership id has been overridden - this may impact data - see https://github.com/civicrm/civicrm-core/pull/15053'); - civicrm_api3('MembershipPayment', 'create', ['contribution_id' => $contributionID, 'membership_id' => $input['membership_id']]); - } - } - /** * Get transaction information about the contribution. * @@ -2223,17 +2202,6 @@ LEFT JOIN civicrm_contribution contribution ON ( componentPayment.contribution_ * 2) repeattransaction code is current munged into completeTransaction code for historical bad coding reasons * 3) Repeat transaction duplicates rather than calls Order.create * 4) Use of payment.create still limited - completetransaction is more common. - * 6) the determination of the membership to be linked is tricksy. The prioritised method is - * to load the membership(s) referred to via line items in the template transactions. Any other - * method is likely to lead to incorrect line items & related entities being created (as the line_item - * link is a required part of 'correct data'). However there are 3 other methods to determine it - * - membership_payment record - * - civicrm_membership.contribution_recur_id - * - input override. - * Passing in an input override WILL ensure the membership is extended to prevent regressions - * of historical processors since this has been handled 'forever' - specifically for paypal. - * albeit by an even nastier mechanism than the current input override. - * The count is out on how correct related entities wind up in this case. */ protected static function repeatTransaction(array $input, array $contributionParams) { $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution( @@ -2259,7 +2227,6 @@ LEFT JOIN civicrm_contribution contribution ON ( componentPayment.contribution_ $createContribution = civicrm_api3('Contribution', 'create', $contributionParams); $temporaryObject = new CRM_Contribute_BAO_Contribution(); $temporaryObject->copyCustomFields($templateContribution['id'], $createContribution['id']); - self::handleMembershipIDOverride($createContribution['id'], $input); // Add new soft credit against current $contribution. CRM_Contribute_BAO_ContributionRecur::addrecurSoftCredit($contributionParams['contribution_recur_id'], $createContribution['id']); return $createContribution; diff --git a/CRM/Core/Payment/PayPalIPN.php b/CRM/Core/Payment/PayPalIPN.php index a626afd0ea..4a23cacdd6 100644 --- a/CRM/Core/Payment/PayPalIPN.php +++ b/CRM/Core/Payment/PayPalIPN.php @@ -229,37 +229,11 @@ class CRM_Core_Payment_PayPalIPN extends CRM_Core_Payment_BaseIPN { $input['component'] = $component; $contributionID = $this->getContributionID(); - $membershipID = $this->retrieve('membershipID', 'Integer', FALSE); - $this->getInput($input); $paymentProcessorID = $this->getPayPalPaymentProcessorID($input, $this->getContributionRecurID()); Civi::log()->debug('PayPalIPN: Received (ContactID: ' . $this->getContactID() . '; trxn_id: ' . $input['trxn_id'] . ').'); - - // Debugging related to possible missing membership linkage - if ($this->getContributionRecurID() && $this->retrieve('membershipID', 'Integer', FALSE)) { - $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution($this->getContributionRecurID()); - $membershipPayment = civicrm_api3('MembershipPayment', 'get', [ - 'contribution_id' => $templateContribution['id'], - 'membership_id' => $membershipID, - ]); - $lineItems = civicrm_api3('LineItem', 'get', [ - 'contribution_id' => $templateContribution['id'], - 'entity_id' => $membershipID, - 'entity_table' => 'civicrm_membership', - ]); - Civi::log()->debug('PayPalIPN: Received payment for membership ' . (int) $membershipID - . '. Original contribution was ' . (int) $contributionID . '. The template for this contribution is ' - . $templateContribution['id'] . ' it is linked to ' . $membershipPayment['count'] - . 'payments for this membership. It has ' . $lineItems['count'] . ' line items linked to this membership.' - . ' it is expected the original contribution will be linked by both entities to the membership.' - ); - if (empty($membershipPayment['count']) && empty($lineItems['count'])) { - Civi::log()->debug('PayPalIPN: Will attempt to compensate'); - $input['membership_id'] = $this->retrieve('membershipID', 'Integer', FALSE); - } - } $contribution = $this->getContribution(); $input['payment_processor_id'] = $paymentProcessorID; diff --git a/api/v3/Contribution.php b/api/v3/Contribution.php index e3d49c086c..f9fd981170 100644 --- a/api/v3/Contribution.php +++ b/api/v3/Contribution.php @@ -620,7 +620,6 @@ function civicrm_api3_contribution_repeattransaction($params) { 'fee_amount', 'financial_type_id', 'contribution_status_id', - 'membership_id', 'payment_processor_id', ]; $input = array_intersect_key($params, array_fill_keys($passThroughParams, NULL)); diff --git a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php index 03e9055456..f6075b1a98 100644 --- a/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php +++ b/tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php @@ -198,48 +198,6 @@ class CRM_Core_Payment_PayPalIPNTest extends CiviUnitTestCase { $this->callAPISuccessGetSingle('membership_payment', ['contribution_id' => $contribution['values'][1]['id']]); } - /** - * Test IPN that we can force membership when the membership payment has been deleted. - * - * https://lab.civicrm.org/dev/membership/issues/13 - * - * In this scenario the membership payment record was deleted (or not created) for the first contribution but we - * 'recover' by using the input membership id. - * - * @throws \CRM_Core_Exception - * @throws \CiviCRM_API3_Exception - */ - public function testIPNPaymentInputMembershipRecurSuccess(): void { - $durationUnit = 'year'; - $this->setupMembershipRecurringPaymentProcessorTransaction(['duration_unit' => $durationUnit, 'frequency_unit' => $durationUnit]); - $membershipPayment = $this->callAPISuccessGetSingle('membership_payment', []); - $paypalIPN = new CRM_Core_Payment_PayPalIPN(array_merge($this->getPaypalRecurTransaction(), ['membershipID' => $membershipPayment['membership_id']])); - $paypalIPN->main(); - $membershipEndDate = $this->callAPISuccessGetValue('membership', ['return' => 'end_date']); - CRM_Core_DAO::executeQuery('DELETE FROM civicrm_membership_payment WHERE id = ' . $membershipPayment['id']); - CRM_Core_DAO::executeQuery("UPDATE civicrm_line_item SET entity_table = 'civicrm_contribution' WHERE entity_table = 'civicrm_membership'"); - - $paypalIPN = new CRM_Core_Payment_PaypalIPN(array_merge($this->getPaypalRecurSubsequentTransaction(), ['membershipID' => $membershipPayment['membership_id']])); - $paypalIPN->main(); - $renewedMembershipEndDate = $this->membershipRenewalDate($durationUnit, $membershipEndDate); - $this->assertEquals($renewedMembershipEndDate, $this->callAPISuccessGetValue('membership', ['return' => 'end_date'])); - $contribution = $this->callAPISuccess('contribution', 'get', [ - 'contribution_recur_id' => $this->_contributionRecurID, - 'sequential' => 1, - ]); - $this->assertEquals(2, $contribution['count']); - $this->assertEquals('secondone', $contribution['values'][1]['trxn_id']); - $this->callAPISuccessGetCount('line_item', [ - 'entity_id' => $this->ids['membership'], - 'entity_table' => 'civicrm_membership', - ], 1); - $this->callAPISuccessGetSingle('line_item', [ - 'contribution_id' => $contribution['values'][1]['id'], - 'entity_table' => 'civicrm_membership', - ]); - $this->callAPISuccessGetSingle('membership_payment', ['contribution_id' => $contribution['values'][1]['id']]); - } - /** * Get IPN style details for an incoming recurring transaction. */