We have an intermittent regression on extending memberships off paypal payments. This adds debug messages
and handling / tests for the scenario I suspect may be in play.
The 'decision' the code makes about how to create recurring transactions is all dependent on the 'template transaction'
- if that is not correct the follow on payments will not be.
This PR allows the membershipID input from paypal to be passed through and used. This will override any
intended changes - but that just restores previous behaviour and we do not have good handling for these
https://lab.civicrm.org/dev/membership/issues/13
return [TRUE, $isARefund];
}
+ /**
+ * 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']]);
+ }
+ }
+
/**
* @inheritDoc
*/
/**
* Repeat a transaction as part of a recurring series.
*
- * Only call this via the api as it is being refactored. The intention is that the repeatTransaction function
- * (possibly living on the ContributionRecur BAO) would be called first to create a pending contribution with a
- * subsequent call to the contribution.completetransaction api.
- *
- * The completeTransaction functionality has historically been overloaded to both complete and repeat payments.
+ * The ideal flow is
+ * 1) Processor calls contribution.repeattransaction with contribution_status_id = Pending
+ * 2) The repeattransaction loads the 'template contribution' and calls a hook to allow altering of it .
+ * 3) Repeat transaction calls order.create to create the pending contribution with correct line items
+ * and associated entities.
+ * 4) The calling code calls Payment.create which in turn calls CompleteOrder (if completing)
+ * which updates the various entities and sends appropriate emails.
+ *
+ * Gaps in the above (@todo)
+ * 1) many processors still call repeattransaction with contribution_status_id = Completed
+ * 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.
+ * 5) the template transaction is tricky - historically we used the first contribution
+ * linked to a recurring contribution. More recently that was changed to be the most recent.
+ * Ideally it would be an actual template - not a contribution used as a template which
+ * would give more appropriate flexibility. Note line_items have an entity so that table
+ * could be used for the line item template - the difficulty is the custom fields...
+ * 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.
*
* @param CRM_Contribute_BAO_Contribution $contribution
* @param array $input
$createContribution = civicrm_api3('Contribution', 'create', $contributionParams);
$contribution->id = $createContribution['id'];
CRM_Contribute_BAO_ContributionRecur::copyCustomValues($contributionParams['contribution_recur_id'], $contribution->id);
+ self::handleMembershipIDOverride($contribution->id, $input);
return TRUE;
}
}
$input['component'] = $component;
$ids['contact'] = $this->retrieve('contactID', 'Integer', TRUE);
- $ids['contribution'] = $this->retrieve('contributionID', 'Integer', TRUE);
+ $contributionID = $ids['contribution'] = $this->retrieve('contributionID', 'Integer', TRUE);
+ $membershipID = $this->retrieve('membershipID', 'Integer', FALSE);
+ $contributionRecurID = $this->retrieve('contributionRecurID', 'Integer', FALSE);
$this->getInput($input, $ids);
}
else {
// get the optional ids
- $ids['membership'] = $this->retrieve('membershipID', 'Integer', FALSE);
- $ids['contributionRecur'] = $this->retrieve('contributionRecurID', 'Integer', FALSE);
+ $ids['membership'] = $membershipID;
+ $ids['contributionRecur'] = $contributionRecurID;
$ids['contributionPage'] = $this->retrieve('contributionPageID', 'Integer', FALSE);
$ids['related_contact'] = $this->retrieve('relatedContactID', 'Integer', FALSE);
$ids['onbehalf_dupe_alert'] = $this->retrieve('onBehalfDupeAlert', 'Integer', FALSE);
}
- $paymentProcessorID = self::getPayPalPaymentProcessorID($input, $ids);
+ $paymentProcessorID = $this->getPayPalPaymentProcessorID($input, $ids);
Civi::log()->debug('PayPalIPN: Received (ContactID: ' . $ids['contact'] . '; trxn_id: ' . $input['trxn_id'] . ').');
+ if ($this->retrieve('membershipID', 'Integer', FALSE)) {
+ $templateContribution = CRM_Contribute_BAO_ContributionRecur::getTemplateContribution($contributionRecurID);
+ $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);
+ }
+ if ($contributionRecurID) {
+ $recurLinks = civicrm_api3('ContributionRecur', 'get', [
+ 'membership_id' => $membershipID,
+ 'contribution_recur_id' => $contributionRecurID,
+ ]);
+ Civi::log()->debug('PayPalIPN: Membership should be linked to contribution recur record ' . $contributionRecurID
+ . ' ' . $recurLinks['count'] . 'links found'
+ );
+ }
+ }
if (!$this->validateData($input, $ids, $objects, TRUE, $paymentProcessorID)) {
return;
}
'fee_amount',
'financial_type_id',
'contribution_status_id',
+ 'membership_id',
];
$input = array_intersect_key($params, array_fill_keys($passThroughParams, NULL));
/**
* Test IPN response updates contribution_recur & contribution for first & second contribution.
+ *
+ * @throws \CRM_Core_Exception
+ * @throws \CiviCRM_API3_Exception
*/
public function testIPNPaymentMembershipRecurSuccess() {
$durationUnit = 'year';
$this->assertEquals(1, $contribution['contribution_status_id']);
$this->assertEquals('8XA571746W2698126', $contribution['trxn_id']);
// source gets set by processor
- $this->assertTrue(substr($contribution['contribution_source'], 0, 20) == "Online Contribution:");
+ $this->assertTrue(substr($contribution['contribution_source'], 0, 20) === "Online Contribution:");
$contributionRecur = $this->callAPISuccess('contribution_recur', 'getsingle', ['id' => $this->_contributionRecurID]);
$this->assertEquals(5, $contributionRecur['contribution_status_id']);
$paypalIPN = new CRM_Core_Payment_PaypalIPN($this->getPaypalRecurSubsequentTransaction());
'entity_table' => 'civicrm_membership',
]);
$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() {
+ $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']]);
}
/**
projects / civicrm-core.git / commitdiff