From d286de6a14a0fb6de4f9ccc3c4c3d2e36288ae85 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Thu, 14 Dec 2023 23:53:31 +1300 Subject: [PATCH] Standardise fetching of entity variables on recur management forms --- CRM/Contribute/BAO/ContributionRecur.php | 3 + CRM/Contribute/Form/ContributionRecur.php | 84 ++++++++++++++++------- CRM/Contribute/Form/UpdateBilling.php | 2 +- 3 files changed, 64 insertions(+), 25 deletions(-) diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php index e38b7c937f..7e7644c151 100644 --- a/CRM/Contribute/BAO/ContributionRecur.php +++ b/CRM/Contribute/BAO/ContributionRecur.php @@ -384,6 +384,7 @@ SELECT rec.id as recur_id, mp.membership_id"; if ($entity == 'recur') { + // This should be always true now. $sql .= " FROM civicrm_contribution_recur rec LEFT JOIN civicrm_contribution con ON ( con.contribution_recur_id = rec.id ) @@ -391,6 +392,7 @@ LEFT JOIN civicrm_membership_payment mp ON ( mp.contribution_id = con.id ) WHERE rec.id = %1"; } elseif ($entity == 'contribution') { + CRM_Core_Error::deprecatedWarning('no longer used'); $sql .= " FROM civicrm_contribution con INNER JOIN civicrm_contribution_recur rec ON ( con.contribution_recur_id = rec.id ) @@ -398,6 +400,7 @@ LEFT JOIN civicrm_membership_payment mp ON ( mp.contribution_id = con.id ) WHERE con.id = %1"; } elseif ($entity == 'membership') { + CRM_Core_Error::deprecatedWarning('no longer used'); $sql .= " FROM civicrm_membership_payment mp INNER JOIN civicrm_membership mem ON ( mp.membership_id = mem.id ) diff --git a/CRM/Contribute/Form/ContributionRecur.php b/CRM/Contribute/Form/ContributionRecur.php index 09fb3fd3c0..d02af5e59c 100644 --- a/CRM/Contribute/Form/ContributionRecur.php +++ b/CRM/Contribute/Form/ContributionRecur.php @@ -21,6 +21,9 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { use CRM_Core_Form_EntityFormTrait; + use CRM_Contribute_Form_ContributeFormTrait; + use CRM_Member_Form_MembershipFormTrait; + use CRM_Financial_Form_PaymentProcessorFormTrait; /** * Contribution ID. @@ -33,6 +36,8 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { * Contribution Recur ID. * * @var int + * + * @internal */ protected $_crid = NULL; @@ -43,6 +48,8 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { * we should probably deprecate $_crid. * * @var int + * + * @internal */ protected $contributionRecurID = NULL; @@ -50,8 +57,10 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { * Membership ID. * * @var int + * + * @internal */ - protected $_mid = NULL; + protected $_mid; /** * Payment processor object. @@ -121,10 +130,11 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { */ public function preProcess() { $this->setAction(CRM_Core_Action::UPDATE); - $this->_mid = CRM_Utils_Request::retrieve('mid', 'Integer', $this, FALSE); - $this->_crid = CRM_Utils_Request::retrieve('crid', 'Integer', $this, FALSE); - $this->contributionRecurID = $this->_crid; - $this->_coid = CRM_Utils_Request::retrieve('coid', 'Integer', $this, FALSE); + // These 2 gets shouldn't be needed long term but for now they ensure the + // properties are set - but we should deprecate the properties in favour of the + // standardised get methods. + $this->getMembershipID(); + $this->getContributionID(); $this->setSubscriptionDetails(); $this->setPaymentProcessor(); if ($this->getSubscriptionContactID()) { @@ -139,34 +149,20 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { * but this is good choice of function to call. */ protected function setPaymentProcessor() { - if ($this->_crid) { - $this->_paymentProcessor = CRM_Contribute_BAO_ContributionRecur::getPaymentProcessor($this->contributionRecurID); + if ($this->getContributionRecurID()) { + $this->_paymentProcessor = CRM_Contribute_BAO_ContributionRecur::getPaymentProcessor($this->getContributionRecurID()); if (!$this->_paymentProcessor) { CRM_Core_Error::statusBounce(ts('There is no valid processor for this subscription so it cannot be updated')); } $this->_paymentProcessorObj = $this->_paymentProcessor['object']; } - elseif ($this->_mid) { - $this->_paymentProcessorObj = CRM_Financial_BAO_PaymentProcessor::getProcessorForEntity($this->_mid, 'membership', 'obj'); - $this->_paymentProcessor = $this->_paymentProcessorObj->getPaymentProcessor(); - } } /** * Set the subscription details on the form. */ protected function setSubscriptionDetails() { - if ($this->contributionRecurID) { - $this->subscriptionDetails = $this->_subscriptionDetails = CRM_Contribute_BAO_ContributionRecur::getSubscriptionDetails($this->_crid); - } - elseif ($this->_coid) { - $this->subscriptionDetails = $this->_subscriptionDetails = CRM_Contribute_BAO_ContributionRecur::getSubscriptionDetails($this->_coid, 'contribution'); - } - elseif ($this->_mid) { - $this->subscriptionDetails = CRM_Contribute_BAO_ContributionRecur::getSubscriptionDetails($this->_mid, 'membership'); - } - // This is being set temporarily - we should eventually just use the getter fn. - $this->_subscriptionDetails = $this->subscriptionDetails; + $this->subscriptionDetails = $this->_subscriptionDetails = CRM_Contribute_BAO_ContributionRecur::getSubscriptionDetails($this->getContributionRecurID()); } /** @@ -191,10 +187,50 @@ class CRM_Contribute_Form_ContributionRecur extends CRM_Core_Form { /** * Get the recurring contribution ID. * + * @api This function will not change in a minor release and is supported for + * use outside of core. This annotation / external support for properties + * is only given where there is specific test cover. + * * @return int */ - protected function getContributionRecurID(): int { - return $this->getSubscriptionDetails()->recur_id; + public function getContributionRecurID(): int { + $id = CRM_Utils_Request::retrieve('crid', 'Integer', $this, FALSE); + if (!$id && $this->getContributionID()) { + $id = $this->getContributionValue('contribution_recur_id'); + } + if (!$id) { + $id = $this->getMembershipValue('contribution_recur_id'); + } + $this->contributionRecurID = $this->_crid = $id; + return (int) $id; + } + + /** + * Get the selected Contribution ID. + * + * @api This function will not change in a minor release and is supported for + * use outside of core. This annotation / external support for properties + * is only given where there is specific test cover. + * + * @noinspection PhpUnhandledExceptionInspection + */ + public function getContributionID(): ?int { + $this->_coid = CRM_Utils_Request::retrieve('coid', 'Integer', $this); + return $this->_coid ? (int) $this->_coid : NULL; + } + + /** + * Get the membership ID. + * + * @api This function will not change in a minor release and is supported for + * use outside of core. This annotation / external support for properties + * is only given where there is specific test cover. + * + * @return int + */ + protected function getMembershipID(): ?int { + $this->_mid = CRM_Utils_Request::retrieve('mid', 'Integer', $this, FALSE); + return $this->_mid ? (int) $this->_mid : NULL; } /** diff --git a/CRM/Contribute/Form/UpdateBilling.php b/CRM/Contribute/Form/UpdateBilling.php index 97401469e0..ba14937e27 100644 --- a/CRM/Contribute/Form/UpdateBilling.php +++ b/CRM/Contribute/Form/UpdateBilling.php @@ -30,7 +30,7 @@ class CRM_Contribute_Form_UpdateBilling extends CRM_Contribute_Form_Contribution */ public function preProcess() { parent::preProcess(); - if ($this->_crid) { + if ($this->getContributionRecurID()) { // Are we cancelling a recurring contribution that is linked to an auto-renew membership? if ($this->getSubscriptionDetails()->membership_id) { $this->_mid = $this->getSubscriptionDetails()->membership_id; -- 2.25.1