From: eileenmcnaugton <eileen@fuzion.co.nz>
Date: Tue, 13 Oct 2015 14:18:48 +0000 (+1300)
Subject: CRM-17256 further fix to pay-later with billing_required
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=f48e6cf7851f9cf1ce8d8ac2ba98009c3cfaf2d6;p=civicrm-core.git

CRM-17256 further fix to pay-later with billing_required
---

diff --git a/CRM/Contribute/Form/ContributionBase.php b/CRM/Contribute/Form/ContributionBase.php
index 1bc1a51747..0984975146 100644
--- a/CRM/Contribute/Form/ContributionBase.php
+++ b/CRM/Contribute/Form/ContributionBase.php
@@ -451,17 +451,6 @@ class CRM_Contribute_Form_ContributionBase extends CRM_Core_Form {
       $this->_isBillingAddressRequiredForPayLater = CRM_Utils_Array::value('is_billing_required', $this->_values);
       $this->assign('isBillingAddressRequiredForPayLater', $this->_isBillingAddressRequiredForPayLater);
     }
-
-    // We save the fact that the profile 'billing' is required on the payment form.
-    // Currently pay-later is the only 'processor' that takes notice of this - but ideally
-    // 1) it would be possible to select the minimum_billing_profile_id for the contribution form
-    // 2) that profile_id would be set on the payment processor
-    // 3) the payment processor would return a billing form that combines these user-configured
-    // minimums with the payment processor minimums. This would lead to fields like 'postal_code'
-    // only being on the form if either the admin has configured it as wanted or the processor
-    // requires it.
-    $this->assign('billing_profile_id', ($this->_isBillingAddressRequiredForPayLater ? 'billing' : ''));
-
   }
 
   /**
diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php
index f5ba798e2b..eb971c01f1 100644
--- a/CRM/Core/Form.php
+++ b/CRM/Core/Form.php
@@ -681,6 +681,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
     else {
       throw new CRM_Core_Exception(ts('A payment processor configured for this page might be disabled (contact the site administrator for assistance).'));
     }
+
   }
 
   /**
@@ -762,6 +763,15 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
       CRM_Financial_Form_Payment::addCreditCardJs();
     }
     $this->assign('paymentProcessorID', $this->_paymentProcessorID);
+    // We save the fact that the profile 'billing' is required on the payment form.
+    // Currently pay-later is the only 'processor' that takes notice of this - but ideally
+    // 1) it would be possible to select the minimum_billing_profile_id for the contribution form
+    // 2) that profile_id would be set on the payment processor
+    // 3) the payment processor would return a billing form that combines these user-configured
+    // minimums with the payment processor minimums. This would lead to fields like 'postal_code'
+    // only being on the form if either the admin has configured it as wanted or the processor
+    // requires it.
+    $this->assign('billing_profile_id', (CRM_Utils_Array::value('is_billing_required', $this->_values) ? 'billing' : ''));
   }
 
   /**
diff --git a/CRM/Core/Payment/Manual.php b/CRM/Core/Payment/Manual.php
index b438b8faaf..df49d590f3 100644
--- a/CRM/Core/Payment/Manual.php
+++ b/CRM/Core/Payment/Manual.php
@@ -36,12 +36,8 @@ class CRM_Core_Payment_Manual extends CRM_Core_Payment {
 
   /**
    * This function checks to see if we have the right config values.
-   *
-   * @return string
-   *   the error message if any
    */
-  public function checkConfig() {
-  }
+  public function checkConfig() {}
 
   /**
    * Get billing fields required for this processor.
@@ -158,4 +154,5 @@ class CRM_Core_Payment_Manual extends CRM_Core_Payment {
   public function getPaymentTypeLabel() {
     return '';
   }
+
 }
diff --git a/CRM/Event/Cart/Form/Checkout/Payment.php b/CRM/Event/Cart/Form/Checkout/Payment.php
index 292adda6f8..bbbac759d9 100644
--- a/CRM/Event/Cart/Form/Checkout/Payment.php
+++ b/CRM/Event/Cart/Form/Checkout/Payment.php
@@ -394,13 +394,14 @@ class CRM_Event_Cart_Form_Checkout_Payment extends CRM_Event_Cart_Form_Cart {
       CRM_Core_Form::validateMandatoryFields($self->_fields, $fields, $errors);
 
       // validate payment instrument values (e.g. credit card number)
-      CRM_Core_Payment_Form::validatePaymentInstrument($self->_paymentProcessor['id'], $fields, $errors, $self);
+      CRM_Core_Payment_Form::validatePaymentInstrument($self->_paymentProcessor['id'], $fields, $errors, NULL);
     }
 
     return empty($errors) ? TRUE : $errors;
   }
 
   /**
+   * @todo this should surely go! Test & remove.
    * @return bool
    */
   public function validate() {
diff --git a/CRM/Event/Form/Participant.php b/CRM/Event/Form/Participant.php
index 0be659cf56..05e0822f87 100644
--- a/CRM/Event/Form/Participant.php
+++ b/CRM/Event/Form/Participant.php
@@ -806,7 +806,7 @@ class CRM_Event_Form_Participant extends CRM_Contribute_Form_AbstractEditPayment
 
     if (!empty($values['payment_processor_id'])) {
       // make sure that payment instrument values (e.g. credit card number and cvv) are valid
-      CRM_Core_Payment_Form::validatePaymentInstrument($values['payment_processor_id'], $values, $errorMsg, $self);
+      CRM_Core_Payment_Form::validatePaymentInstrument($values['payment_processor_id'], $values, $errorMsg, NULL);
     }
 
     if (!empty($values['record_contribution'])) {
diff --git a/CRM/Event/Form/Registration.php b/CRM/Event/Form/Registration.php
index ce05f73807..f37433b2c9 100644
--- a/CRM/Event/Form/Registration.php
+++ b/CRM/Event/Form/Registration.php
@@ -176,8 +176,6 @@ class CRM_Event_Form_Registration extends CRM_Core_Form {
 
   /**
    * Set variables up before form is built.
-   *
-   * @return void
    */
   public function preProcess() {
     $this->_eventId = CRM_Utils_Request::retrieve('id', 'Positive', $this, TRUE);
@@ -414,7 +412,12 @@ class CRM_Event_Form_Registration extends CRM_Core_Form {
       $this->_values['event']['campaign_id'] = $campID;
     }
 
+    // Set the same value for is_billing_required as contribution page so code can be shared.
+    $this->_values['is_billing_required'] = CRM_Utils_Array::value('is_billing_required', $this->_values['event']);
     // check if billing block is required for pay later
+    // note that I have started removing the use of isBillingAddressRequiredForPayLater in favour of letting
+    // the CRM_Core_Payment_Manual class handle it - but there are ~300 references to it in the code base so only
+    // removing in very limited cases.
     if (CRM_Utils_Array::value('is_pay_later', $this->_values['event'])) {
       $this->_isBillingAddressRequiredForPayLater = CRM_Utils_Array::value('is_billing_required', $this->_values['event']);
       $this->assign('isBillingAddressRequiredForPayLater', $this->_isBillingAddressRequiredForPayLater);
diff --git a/CRM/Event/Form/Registration/AdditionalParticipant.php b/CRM/Event/Form/Registration/AdditionalParticipant.php
index 62f6d89113..9aadd5978d 100644
--- a/CRM/Event/Form/Registration/AdditionalParticipant.php
+++ b/CRM/Event/Form/Registration/AdditionalParticipant.php
@@ -561,7 +561,7 @@ class CRM_Event_Form_Registration_AdditionalParticipant extends CRM_Event_Form_R
 
     // validate supplied payment instrument values (e.g. credit card number and cvv)
     $payment_processor_id = $self->_params[0]['payment_processor'];
-    CRM_Core_Payment_Form::validatePaymentInstrument($payment_processor_id, $self->_params[0], $errors, $self);
+    CRM_Core_Payment_Form::validatePaymentInstrument($payment_processor_id, $self->_params[0], $errors, (!$self->_isBillingAddressRequiredForPayLater ? NULL : 'billing'));
 
     if (!empty($errors)) {
       return FALSE;
diff --git a/CRM/Event/Form/Registration/Confirm.php b/CRM/Event/Form/Registration/Confirm.php
index 6f5471da5c..4cca3d8e3e 100644
--- a/CRM/Event/Form/Registration/Confirm.php
+++ b/CRM/Event/Form/Registration/Confirm.php
@@ -329,6 +329,10 @@ class CRM_Event_Form_Registration_Confirm extends CRM_Event_Form_Registration {
     //consider total amount.
     $this->assign('isAmountzero', ($this->_totalAmount <= 0) ? TRUE : FALSE);
 
+    // @todo this needs to GO! We are getting rid of references to processor types in the code base in favour of
+    // over-ride-able functions on them.
+    // The processor effectively has a 'buildForm' hook it can use if it needs to.
+    // The tricky thing is that we have no way of testing this code out - perhaps it hasn't worked for years!
     if ($this->_paymentProcessor['payment_processor_type'] == 'Google_Checkout' && empty($this->_params[0]['is_pay_later']) && !($this->_params[0]['amount'] == 0) &&
       !$this->_allowWaitlist && !$this->_requireApproval
     ) {
diff --git a/CRM/Event/Form/Registration/Register.php b/CRM/Event/Form/Registration/Register.php
index 24b22cf286..6d39361aa9 100644
--- a/CRM/Event/Form/Registration/Register.php
+++ b/CRM/Event/Form/Registration/Register.php
@@ -34,7 +34,6 @@
 
 /**
  * This class generates form components for processing Event
- *
  */
 class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration {
 
@@ -73,8 +72,6 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration {
 
   /**
    * Set variables up before form is built.
-   *
-   * @return void
    */
   public function preProcess() {
     parent::preProcess();
@@ -844,21 +841,12 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration {
       }
     }
 
+    // @todo - can we remove the 'is_monetary' concept?
     if ($self->_values['event']['is_monetary']) {
       if (empty($self->_requireApproval) && !empty($fields['amount']) && $fields['amount'] > 0 && !isset
         ($fields['payment_processor_id'])) {
         $errors['payment_processor_id'] = ts('Please select a Payment Method');
       }
-      // return if this is express mode
-      if ($self->_paymentProcessor &&
-        $self->_paymentProcessor['billing_mode'] & CRM_Core_Payment::BILLING_MODE_BUTTON
-      ) {
-        if (!empty($fields[$self->_expressButtonName . '_x']) || !empty($fields[$self->_expressButtonName . '_y']) ||
-          CRM_Utils_Array::value($self->_expressButtonName, $fields)
-        ) {
-          return empty($errors) ? TRUE : $errors;
-        }
-      }
 
       $isZeroAmount = $skipPaymentValidation = FALSE;
       if (!empty($fields['priceSetId'])) {
@@ -885,14 +873,19 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration {
         $skipPaymentValidation = TRUE;
       }
 
-      // also return if paylater mode or zero fees for valid members
-      if (!empty($fields['is_pay_later']) || !empty($fields['bypass_payment']) ||
+      // also return if zero fees for valid members
+      if (!empty($fields['bypass_payment']) ||
         $skipPaymentValidation ||
         (!$self->_allowConfirmation && ($self->_requireApproval || $self->_allowWaitlist))
       ) {
         return empty($errors) ? TRUE : $errors;
       }
-      CRM_Core_Payment_Form::validatePaymentInstrument($self->_paymentProcessorID, $fields, $errors, $self);
+      CRM_Core_Payment_Form::validatePaymentInstrument(
+        $fields['payment_processor_id'],
+        $fields,
+        $errors,
+        (!$self->_isBillingAddressRequiredForPayLater ? NULL : 'billing')
+      );
     }
 
     foreach (CRM_Contact_BAO_Contact::$_greetingTypes as $greeting) {
diff --git a/CRM/Financial/BAO/PaymentProcessor.php b/CRM/Financial/BAO/PaymentProcessor.php
index 4e9f92c059..382bf8a096 100644
--- a/CRM/Financial/BAO/PaymentProcessor.php
+++ b/CRM/Financial/BAO/PaymentProcessor.php
@@ -305,9 +305,11 @@ class CRM_Financial_BAO_PaymentProcessor extends CRM_Financial_DAO_PaymentProces
 
     // Add the pay-later pseudo-processor.
     $processors['values'][0] = array(
-      'object' =>  new CRM_Core_Payment_Manual(),
+      'object' => new CRM_Core_Payment_Manual(),
       'id' => 0,
       'payment_processor_type_id' => 0,
+      // This shouldn't be required but there are still some processors hacked into core with nasty 'if's.
+      'payment_processor_type' => 'Manual',
       'class_name' => 'Payment_Manual',
       'name' => 'pay_later',
       'billing_mode' => '',
diff --git a/CRM/Member/Form/Membership.php b/CRM/Member/Form/Membership.php
index fa394ac5c5..fedfa1e9eb 100644
--- a/CRM/Member/Form/Membership.php
+++ b/CRM/Member/Form/Membership.php
@@ -806,7 +806,7 @@ class CRM_Member_Form_Membership extends CRM_Member_Form {
 
     if (!empty($params['payment_processor_id'])) {
       // validate payment instrument (e.g. credit card number)
-      CRM_Core_Payment_Form::validatePaymentInstrument($params['payment_processor_id'], $params, $errors, $self);
+      CRM_Core_Payment_Form::validatePaymentInstrument($params['payment_processor_id'], $params, $errors, NULL);
     }
 
     $joinDate = NULL;