Revert membership debug handling in IPN
authorEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 11 Jan 2022 18:37:58 +0000 (07:37 +1300)
committerEileen McNaughton <emcnaughton@wikimedia.org>
Tue, 11 Jan 2022 18:37:58 +0000 (07:37 +1300)
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....

CRM/Contribute/BAO/Contribution.php
CRM/Core/Payment/PayPalIPN.php
api/v3/Contribution.php
tests/phpunit/CRM/Core/Payment/PayPalIPNTest.php

index a44d125fb65a89697740934e2cd20faf1c372853..885fe7af9d4eded5784abb9b2059e972a4cde38a 100644 (file)
@@ -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;
index a626afd0ea4a04b1fdcc34ed59a83251620c0ae5..4a23cacdd62520d7a8ad6bd78a5f141639ac2505 100644 (file)
@@ -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;
index e3d49c086c09bb6ddb2bc7a3944c0fd8f1602f96..f9fd9811702d695c85e783252510898a4edb5082 100644 (file)
@@ -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));
index 03e9055456e2a0a9ea694ac2c988c6b094f067fd..f6075b1a988dfbdd30666c473034e9d41740af12 100644 (file)
@@ -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.
    */