From 6ff69e9ca9d4432dc35fd1b800374c84f1c6b69f Mon Sep 17 00:00:00 2001 From: eileen Date: Sat, 24 Oct 2020 11:44:26 +1300 Subject: [PATCH] dev/core#927 Move cancelling of related memberships to extension from BaseIPN This moves the code to cancel memberships on a related contribution cancel to the contributioncancelactions core extension. Before The cancellations are done using convoluted & elsewhere duplicated logic in BaseIPN using convoluted input params After When a contribution is updated & the status_id of 'Cancellled' is set then the hook will kick in, look for any related pending memberships and cancel them. The test demonstrates that the api call will ensure related pending memberships are cancelled. Note that I am using line_items table to get the membership - we have been storing this membership_payment link there for log enough it should be reliable but if there are remaining data issues then this is a lower risk flow to flush them out. Also note that once this is cleaned up I'll move to the 2 other flows I'm aware of Order.create api & Contribuion form edits --- CRM/Core/Payment/BaseIPN.php | 42 ++++++++++++------- .../contributioncancelactions.php | 28 ++++++++++++- .../tests/phpunit/IPNCancelTest.php | 23 ++++++---- 3 files changed, 67 insertions(+), 26 deletions(-) diff --git a/CRM/Core/Payment/BaseIPN.php b/CRM/Core/Payment/BaseIPN.php index c20c3b4010..daa0988b56 100644 --- a/CRM/Core/Payment/BaseIPN.php +++ b/CRM/Core/Payment/BaseIPN.php @@ -224,6 +224,13 @@ class CRM_Core_Payment_BaseIPN { /** * Process cancelled payment outcome. * + * @deprecated The intended replacement code is + * + * Contribution::update(FALSE)->setValues([ + * 'cancel_date' => 'now', + * 'contribution_status_id:name' => 'Cancelled', + * ])->addWhere('id', '=', $contribution->id)->execute(); + * * @param array $objects * * @return bool @@ -231,15 +238,11 @@ class CRM_Core_Payment_BaseIPN { */ public function cancelled($objects) { $contribution = &$objects['contribution']; - $memberships = []; - if (!empty($objects['membership'])) { - $memberships = &$objects['membership']; - if (is_numeric($memberships)) { - $memberships = [$objects['membership']]; - } - } if (empty($contribution->id)) { + // This code is believed to be unreachable. + // this entire function is due to be deprecated in the near future so + // this code will live in a deprecated function until it gets removed. $addLineItems = TRUE; // CRM-15546 $contributionStatuses = CRM_Core_PseudoConstant::get('CRM_Contribute_DAO_Contribution', 'contribution_status_id', [ @@ -253,6 +256,20 @@ class CRM_Core_Payment_BaseIPN { if (!empty($objects['contributionRecur']) && $objects['contributionRecur']->id && $addLineItems) { CRM_Contribute_BAO_ContributionRecur::addRecurLineItems($objects['contributionRecur']->id, $contribution); } + $memberships = []; + if (!empty($objects['membership'])) { + $memberships = &$objects['membership']; + if (is_numeric($memberships)) { + $memberships = [$objects['membership']]; + } + } + if (!empty($memberships)) { + foreach ($memberships as $membership) { + if ($membership) { + $this->cancelMembership($membership, $membership->status_id); + } + } + } } else { Contribution::update(FALSE)->setValues([ @@ -262,14 +279,6 @@ class CRM_Core_Payment_BaseIPN { } $participant = &$objects['participant']; - if (!empty($memberships)) { - foreach ($memberships as $membership) { - if ($membership) { - $this->cancelMembership($membership, $membership->status_id); - } - } - } - if ($participant) { $this->cancelParticipant($participant->id); } @@ -319,9 +328,10 @@ class CRM_Core_Payment_BaseIPN { * @param boolean $onlyCancelPendingMembership * Do we only cancel pending memberships? OR memberships in any status? (see CRM-18688) * @fixme Historically failed() cancelled membership in any status, cancelled() cancelled only pending memberships so we retain that behaviour for now. - * + * @deprecated */ private function cancelMembership($membership, $membershipStatusID, $onlyCancelPendingMembership = TRUE) { + CRM_Core_Error::deprecatedFunctionWarning('use the api'); // @fixme https://lab.civicrm.org/dev/core/issues/927 Cancelling membership etc is not desirable for all use-cases and we should be able to disable it // Cancel only Pending memberships $pendingMembershipStatusId = CRM_Core_PseudoConstant::getKey('CRM_Member_BAO_Membership', 'status_id', 'Pending'); diff --git a/ext/contributioncancelactions/contributioncancelactions.php b/ext/contributioncancelactions/contributioncancelactions.php index 6cac3f39d9..2ec3aae284 100644 --- a/ext/contributioncancelactions/contributioncancelactions.php +++ b/ext/contributioncancelactions/contributioncancelactions.php @@ -4,16 +4,40 @@ require_once 'contributioncancelactions.civix.php'; // phpcs:disable use CRM_Contributioncancelactions_ExtensionUtil as E; // phpcs:enable +use Civi\Api4\LineItem; /** * Implements hook_civicrm_preProcess(). * + * This enacts the following + * - find and cancel any related pending memberships + * - (not yet implemented) find and cancel any related pending participant records + * - (not yet implemented) find any related pledge payment records. Remove the contribution id. + * * @link https://docs.civicrm.org/dev/en/latest/hooks/hook_civicrm_post */ function contributioncancelactions_civicrm_post($op, $objectName, $objectId, $objectRef) { if ($op === 'edit' && $objectName === 'Contribution') { - if ($objectRef->contribution_status_id === CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Cancelled')) { - // Do the stuff. + if ('Cancelled' === CRM_Core_PseudoConstant::getName('CRM_Contribute_BAO_Contribution', 'contribution_status_id', $objectRef->contribution_status_id)) { + // Find and cancel any pending memberships. + $connectedMemberships = (array) LineItem::get(FALSE)->setWhere([ + ['contribution_id', '=', $objectId], + ['entity_table', '=', 'civicrm_membership'], + ])->execute()->indexBy('entity_id'); + if (empty($connectedMemberships)) { + return; + } + // @todo we don't have v4 membership api yet so v3 for now. + $connectedMemberships = array_keys(civicrm_api3('Membership', 'get', [ + 'status_id' => 'Pending', + 'id' => ['IN' => array_keys($connectedMemberships)], + ])['values']); + if (empty($connectedMemberships)) { + return; + } + foreach ($connectedMemberships as $membershipID) { + civicrm_api3('Membership', 'create', ['status_id' => 'Cancelled', 'id' => $membershipID, 'is_override' => 1]); + } } } } diff --git a/ext/contributioncancelactions/tests/phpunit/IPNCancelTest.php b/ext/contributioncancelactions/tests/phpunit/IPNCancelTest.php index 9f808765e8..95816b6df5 100644 --- a/ext/contributioncancelactions/tests/phpunit/IPNCancelTest.php +++ b/ext/contributioncancelactions/tests/phpunit/IPNCancelTest.php @@ -1,11 +1,12 @@ ids['contact'][0] = Civi\Api4\Contact::create()->setValues(['first_name' => 'Brer', 'last_name' => 'Rabbit'])->execute()->first()['id']; + $this->createMembershipType(); + Relationship::create()->setValues([ + 'contact_id_a' => $this->ids['contact'][0], + 'contact_id_b' => Contact::create()->setValues(['first_name' => 'Bugs', 'last_name' => 'Bunny'])->execute()->first()['id'], + 'relationship_type_id' => RelationshipType::get()->addWhere('name_a_b', '=', 'AB')->execute()->first()['id'], + ])->execute(); + $this->createMembershipOrder(); - $membership = $this->callAPISuccessGetSingle('Membership', []); - $membership['owner_membership_id'] = $membership['id']; - $membership['contact_id'] = Contact::create()->setValues(['first_name' => 'Bugs', 'last_name' => 'Bunny'])->execute()->first()['id']; - unset($membership['id']); - $this->callAPISuccess('Membership', 'create', $membership); + + $memberships = $this->callAPISuccess('Membership', 'get')['values']; + $this->assertCount(2, $memberships); $ipn = new CRM_Core_Payment_PayPalProIPN([ 'rp_invoice_id' => http_build_query([ @@ -92,8 +99,6 @@ class IPNCancelTest extends \PHPUnit\Framework\TestCase implements HeadlessInter * @throws \Civi\API\Exception\UnauthorizedException */ protected function createMembershipOrder() { - $this->ids['contact'][0] = Civi\Api4\Contact::create()->setValues(['first_name' => 'Brer', 'last_name' => 'Rabbit'])->execute()->first()['id']; - $this->createMembershipType(); $priceFieldID = $this->callAPISuccessGetValue('price_field', [ 'return' => 'id', 'label' => 'Membership Amount', @@ -154,6 +159,8 @@ class IPNCancelTest extends \PHPUnit\Framework\TestCase implements HeadlessInter 'member_of_contact_id' => 1, 'domain_id' => 1, 'financial_type_id' => 2, + 'relationship_type_id' => RelationshipType::create(FALSE)->setValues(['name_a_b' => 'AB', 'name_b_a' => 'BA'])->execute()->first()['id'], + 'relationship_direction' => 'a_b', 'is_active' => 1, 'sequential' => 1, 'visibility' => 'Public', -- 2.25.1