From 1bec3b51fa2a2022bb0a8c9aa17cc4b48f9d6ac3 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 22 Dec 2021 10:48:30 +1300 Subject: [PATCH] [REF] Extract & stdise AmountBlockIsActive It took me a bit to figure out what it even meant - so extracted into a function which can provide explanation. Part of trying to stop passing known values around - see pass to isSeparateMembershipTransaction - that function should require no input params & be callable with consistent results anywhere --- CRM/Contribute/Form/Contribution/Confirm.php | 14 +++++----- CRM/Contribute/Form/Contribution/Main.php | 2 +- CRM/Contribute/Form/ContributionBase.php | 28 +++++++++++++++----- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index bc9ec89272..9b1e7b9769 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -487,8 +487,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr $this->buildCustom($this->_values['honoree_profile_id'], 'honoreeProfileFields', TRUE, 'honor', $fieldTypes); } $this->assign('receiptFromEmail', $this->_values['receipt_from_email'] ?? NULL); - $amount_block_is_active = $this->get('amount_block_is_active'); - $this->assign('amount_block_is_active', $amount_block_is_active); + $this->assign('amount_block_is_active', $this->isFormSupportsNonMembershipContributions()); // Make a copy of line items array to use for display only $tplLineItems = $this->_lineItem; @@ -1438,9 +1437,9 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr //enabled and contribution amount is not selected. fix for CRM-3010 $isPaidMembership = TRUE; } - $isProcessSeparateMembershipTransaction = $this->isSeparateMembershipTransaction($this->_id, $this->_values['amount_block_is_active']); + $isProcessSeparateMembershipTransaction = $this->isSeparateMembershipTransaction($this->_id); - if ($this->_values['amount_block_is_active']) { + if ($this->isFormSupportsNonMembershipContributions()) { $financialTypeID = $this->_values['financial_type_id']; } else { @@ -1876,13 +1875,12 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr * transaction AND a membership transaction AND the payment processor supports double financial transactions (ie. NOT doTransferCheckout style) * * @param int $formID - * @param bool $amountBlockActiveOnForm * * @return bool */ - public function isSeparateMembershipTransaction($formID, $amountBlockActiveOnForm) { + protected function isSeparateMembershipTransaction($formID): bool { $memBlockDetails = CRM_Member_BAO_Membership::getMembershipBlock($formID); - if (!empty($memBlockDetails['is_separate_payment']) && $amountBlockActiveOnForm) { + if (!empty($memBlockDetails['is_separate_payment']) && $this->isFormSupportsNonMembershipContributions()) { return TRUE; } return FALSE; @@ -2495,7 +2493,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr if (!empty($membershipParams['selectMembership'])) { // CRM-12233 $membershipLineItems = $formLineItems; - if ($this->_separateMembershipPayment && $this->_values['amount_block_is_active']) { + if ($this->_separateMembershipPayment && $this->isFormSupportsNonMembershipContributions()) { $membershipLineItems = []; foreach ($this->_values['fee'] as $key => $feeValues) { if ($feeValues['name'] == 'membership_amount') { diff --git a/CRM/Contribute/Form/Contribution/Main.php b/CRM/Contribute/Form/Contribution/Main.php index 17db610f09..9d6a15c9b9 100644 --- a/CRM/Contribute/Form/Contribution/Main.php +++ b/CRM/Contribute/Form/Contribution/Main.php @@ -946,7 +946,7 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu // CRM-12233 if ($membershipIsActive && empty($self->_membershipBlock['is_required']) - && $self->_values['amount_block_is_active'] + && $self->isFormSupportsNonMembershipContributions() ) { $membershipFieldId = $contributionFieldId = $errorKey = $otherFieldId = NULL; foreach ($self->_values['fee'] as $fieldKey => $fieldValue) { diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php index f9bd0a3677..963044d2aa 100644 --- a/CRM/Contribute/Form/ContributionBase.php +++ b/CRM/Contribute/Form/ContributionBase.php @@ -365,7 +365,6 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { // this avoids getting E_NOTICE errors in php $setNullFields = [ - 'amount_block_is_active', 'is_allow_other_amount', 'footer_text', ]; @@ -450,16 +449,15 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { // check if one of the (amount , membership) blocks is active or not. $this->_membershipBlock = $this->get('membershipBlock'); - if (!$this->_values['amount_block_is_active'] && + if (!$this->isFormSupportsNonMembershipContributions() && !$this->_membershipBlock['is_active'] && !$this->_priceSetId ) { CRM_Core_Error::statusBounce(ts('The requested online contribution page is missing a required Contribution Amount section or Membership section or Price Set. Please check with the site administrator for assistance.')); } - - if ($this->_values['amount_block_is_active']) { - $this->set('amount_block_is_active', $this->_values['amount_block_is_active']); - } + // This can probably go as nothing it 'getting it' anymore since the values data is loaded + // on every form, rather than being passed from form to form. + $this->set('amount_block_is_active', $this->isFormSupportsNonMembershipContributions()); $this->_contributeMode = $this->get('contributeMode'); $this->assign('contributeMode', $this->_contributeMode); @@ -1242,4 +1240,22 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form { } } + /** + * Is payment for (non membership) contributions enabled on this form. + * + * This would be true in a case of contributions only or where both + * memberships and non-membership contributions are enabled (whether they + * are using quick config price sets or explicit price sets). + * + * The value is a database value in the config for the contribution page. It + * is loaded into values in ContributionBase::preProcess (called by this). + * + * @internal function is public to support validate but is for core use only. + * + * @return bool + */ + public function isFormSupportsNonMembershipContributions(): bool { + return (bool) ($this->_values['amount_block_is_active'] ?? FALSE); + } + } -- 2.25.1