From f956cd2487422e5fd179d40d976a36e17f37b283 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 21 Oct 2019 17:43:40 +1300 Subject: [PATCH] Fix bug whereby cidZero does not prepopulate billing details for selected contact for pay later. To replicate 1) create a contribution page & select pay later (and no other processor) with 'billing address required' 2) access the contribution page with cid=0 in the url 3) select a contact - witness the contacts detaisl have NOT loaded into the billing block 4) follow the same steps on event I was trying to reduce complexity on this form for another change when I realised that the variable to determine whether to show the block was wrong when billing details were required for pay later and on testing there seemed now downside (other than a very minor performance hit in obscure circumstances) in just always loading the billing profile details. I saw no js error when the billing block was NOT present and we are already checking the person has access to see the contact so that is not an issue here (Profile.get does a permission check --- CRM/Contribute/Form/Contribution/Main.php | 7 +------ CRM/Core/Form.php | 8 ++------ CRM/Event/Form/Registration/Register.php | 6 +----- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/CRM/Contribute/Form/Contribution/Main.php b/CRM/Contribute/Form/Contribution/Main.php index 1a23147d47..69fc0192cb 100644 --- a/CRM/Contribute/Form/Contribution/Main.php +++ b/CRM/Contribute/Form/Contribution/Main.php @@ -336,13 +336,8 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $this->add('text', 'total_amount', ts('Total Amount'), ['readonly' => TRUE], FALSE); } $pps = []; - //@todo - this should be replaced by a check as to whether billing fields are set - $onlinePaymentProcessorEnabled = FALSE; if (!empty($this->_paymentProcessors)) { foreach ($this->_paymentProcessors as $key => $name) { - if ($name['billing_mode'] == 1) { - $onlinePaymentProcessorEnabled = TRUE; - } $pps[$key] = $name['name']; } } @@ -367,7 +362,7 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $contactID = $this->getContactID(); if ($this->getContactID() === 0) { - $this->addCidZeroOptions($onlinePaymentProcessorEnabled); + $this->addCidZeroOptions(); } //build pledge block. diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php index 5f61bbd296..9b10eb4199 100644 --- a/CRM/Core/Form.php +++ b/CRM/Core/Form.php @@ -2248,10 +2248,8 @@ class CRM_Core_Form extends HTML_QuickForm_Page { * that small pieces of duplication are not being refactored into separate functions because their only shared parent * is this form. Inserting a class FrontEndForm.php between the contribution & event & this class would allow functions like this * and a dozen other small ones to be refactored into a shared parent with the reduction of much code duplication - * - * @param $onlinePaymentProcessorEnabled */ - public function addCIDZeroOptions($onlinePaymentProcessorEnabled) { + public function addCIDZeroOptions() { $this->assign('nocid', TRUE); $profiles = []; if ($this->_values['custom_pre_id']) { @@ -2260,9 +2258,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page { if ($this->_values['custom_post_id']) { $profiles = array_merge($profiles, (array) $this->_values['custom_post_id']); } - if ($onlinePaymentProcessorEnabled) { - $profiles[] = 'billing'; - } + $profiles[] = 'billing'; if (!empty($this->_values)) { $this->addAutoSelector($profiles); } diff --git a/CRM/Event/Form/Registration/Register.php b/CRM/Event/Form/Registration/Register.php index bb69047bc3..ac1388a35b 100644 --- a/CRM/Event/Form/Registration/Register.php +++ b/CRM/Event/Form/Registration/Register.php @@ -413,18 +413,14 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration { $pps = []; //@todo this processor adding fn is another one duplicated on contribute - a shared // common class would make this sort of thing extractable - $onlinePaymentProcessorEnabled = FALSE; if (!empty($this->_paymentProcessors)) { foreach ($this->_paymentProcessors as $key => $name) { - if ($name['billing_mode'] == 1) { - $onlinePaymentProcessorEnabled = TRUE; - } $pps[$key] = $name['name']; } } if ($this->getContactID() === 0 && !$this->_values['event']['is_multiple_registrations']) { //@todo we are blocking for multiple registrations because we haven't tested - $this->addCIDZeroOptions($onlinePaymentProcessorEnabled); + $this->addCIDZeroOptions(); } if (!empty($this->_values['event']['is_pay_later']) && ($this->_allowConfirmation || (!$this->_requireApproval && !$this->_allowWaitlist)) -- 2.25.1