From: eileenmcnaugton <eileen@fuzion.co.nz>
Date: Tue, 13 Oct 2015 09:06:22 +0000 (+1300)
Subject: CRM-17256 move validation of billing fields to the payment processor
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=c319039f0e77ae39844f95b44970d3cba32384de;p=civicrm-core.git

CRM-17256 move validation of billing fields to the payment processor

Note that this wasn't done previously due to trying to 'work around' the billing-for-pay later
but that doesn't turn out to be working properly at the moment so better to do the 'proper' fix for this
and for that than to keep trying to work around the way that code was working
---

diff --git a/CRM/Contribute/Form/Contribution/Main.php b/CRM/Contribute/Form/Contribution/Main.php
index 2613ef2005..59d431f716 100644
--- a/CRM/Contribute/Form/Contribution/Main.php
+++ b/CRM/Contribute/Form/Contribution/Main.php
@@ -905,9 +905,6 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu
       return $errors;
     }
 
-    if (!empty($self->_paymentFields)) {
-      CRM_Core_Form::validateMandatoryFields($self->_paymentFields, $fields, $errors);
-    }
     CRM_Core_Payment_Form::validatePaymentInstrument($fields['payment_processor_id'], $fields, $errors, $self);
 
     foreach (CRM_Contact_BAO_Contact::$_greetingTypes as $greeting) {
diff --git a/CRM/Core/Payment.php b/CRM/Core/Payment.php
index 48bad78010..e80568779e 100644
--- a/CRM/Core/Payment.php
+++ b/CRM/Core/Payment.php
@@ -297,6 +297,7 @@ abstract class CRM_Core_Payment {
    * @param array $errors
    */
   public function validatePaymentInstrument($values, &$errors) {
+    CRM_Core_Form::validateMandatoryFields($this->getMandatoryFields(), $values, $errors);
     if ($this->_paymentProcessor['payment_type'] == 1) {
       CRM_Core_Payment_Form::validateCreditCard($values, $errors);
     }
@@ -388,6 +389,31 @@ abstract class CRM_Core_Payment {
     return $this->_paymentProcessor['payment_type'] == 1 ? $this->getCreditCardFormFields() : $this->getDirectDebitFormFields();
   }
 
+  /**
+   * Get the metadata for all required fields.
+   *
+   * @return array;
+   */
+  protected function getMandatoryFields() {
+    $mandatoryFields = array();
+    foreach ($this->getAllFields() as $field_name => $field_spec) {
+      if (!empty($field_spec['is_required'])) {
+        $mandatoryFields[$field_name] = $field_spec;
+      }
+    }
+    return $mandatoryFields;
+  }
+
+  /**
+   * Get the metadata of all the fields configured for this processor.
+   *
+   * @return array
+   */
+  protected function getAllFields() {
+    $paymentFields = array_intersect_key($this->getPaymentFormFieldsMetadata(), array_flip($this->getPaymentFormFields()));
+    $billingFields = array_intersect_key($this->getBillingAddressFieldsMetadata(), array_flip($this->getBillingAddressFields()));
+    return array_merge($paymentFields, $billingFields);
+  }
   /**
    * Get array of fields that should be displayed on the payment form for credit cards.
    *
diff --git a/CRM/Core/Payment/Form.php b/CRM/Core/Payment/Form.php
index cb882254ee..6fe963ab1e 100644
--- a/CRM/Core/Payment/Form.php
+++ b/CRM/Core/Payment/Form.php
@@ -223,32 +223,28 @@ class CRM_Core_Payment_Form {
    *
    * @param CRM_Core_Form $form
    *   Form that the payment fields are to be added to.
-   * @param bool $useRequired
-   *   Effectively this means are the fields required for pay-later - see above.
    * @param array $paymentFields
    *   Fields that are to be shown on the payment form.
    */
-  protected static function addCommonFields(&$form, $useRequired, $paymentFields) {
+  protected static function addCommonFields(&$form, $paymentFields) {
     $requiredPaymentFields = array();
     foreach ($paymentFields as $name => $field) {
       if (!empty($field['cc_field'])) {
         if ($field['htmlType'] == 'chainSelect') {
-          $form->addChainSelect($field['name'], array('required' => $useRequired && $field['is_required']));
+          $form->addChainSelect($field['name'], array('required' => FALSE));
         }
         else {
           $form->add($field['htmlType'],
             $field['name'],
             $field['title'],
             $field['attributes'],
-            $useRequired ? $field['is_required'] : FALSE
+            FALSE
           );
         }
       }
-      // CRM-17071 We seem to be tying ourselves in knots around the addition
-      // of requiring billing fields for pay-later. Here we 'tell' the form the
-      // field is required if it is not otherwise marked as required and is
-      // required for the billing block.
-      $requiredPaymentFields[$field['name']] = !$useRequired ? $field['is_required'] : FALSE;
+      // This will cause the fields to be marked as required - but it is up to the payment processor to
+      // validate it.
+      $requiredPaymentFields[$field['name']] = $field['is_required'];
     }
     $form->assign('requiredPaymentFields', $requiredPaymentFields);
   }
@@ -360,7 +356,7 @@ class CRM_Core_Payment_Form {
     }
 
     self::setPaymentFieldsByProcessor($form, $processor, empty($isBillingDataOptional), $isBackOffice);
-    self::addCommonFields($form, !$isBillingDataOptional, $form->_paymentFields);
+    self::addCommonFields($form, $form->_paymentFields);
     self::addRules($form, $form->_paymentFields);
     return (!empty($form->_paymentFields));
   }
diff --git a/CRM/Core/Payment/PayPalImpl.php b/CRM/Core/Payment/PayPalImpl.php
index 5bb6842051..70803c409d 100644
--- a/CRM/Core/Payment/PayPalImpl.php
+++ b/CRM/Core/Payment/PayPalImpl.php
@@ -175,8 +175,9 @@ class CRM_Core_Payment_PayPalImpl extends CRM_Core_Payment {
    * @param array $errors
    */
   public function validatePaymentInstrument($values, &$errors) {
-    if ($this->_paymentProcessor['payment_processor_type'] == 'PayPal_Pro') {
+    if ($this->_paymentProcessor['payment_processor_type'] == 'PayPal') {
       CRM_Core_Payment_Form::validateCreditCard($values, $errors);
+      CRM_Core_Form::validateMandatoryFields($this->getMandatoryFields(), $values, $errors);
     }
   }
 
@@ -447,7 +448,7 @@ class CRM_Core_Payment_PayPalImpl extends CRM_Core_Payment {
    */
   public function doPayment(&$params, $component = 'contribute') {
     if ($this->_paymentProcessor['payment_processor_type'] == 'PayPal_Express'
-      || ($this->_paymentProcessor['payment_processor_type'] == 'PayPal_Pro' && !empty($params['token']))
+      || ($this->_paymentProcessor['payment_processor_type'] == 'PayPal' && !empty($params['token']))
     ) {
       $this->_component = $component;
       return $this->doExpressCheckout($params);
diff --git a/CRM/Event/Form/Registration/Register.php b/CRM/Event/Form/Registration/Register.php
index e847a312c9..24b22cf286 100644
--- a/CRM/Event/Form/Registration/Register.php
+++ b/CRM/Event/Form/Registration/Register.php
@@ -892,9 +892,6 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration {
       ) {
         return empty($errors) ? TRUE : $errors;
       }
-      if (!empty($self->_paymentFields)) {
-        CRM_Core_Form::validateMandatoryFields($self->_paymentFields, $fields, $errors);
-      }
       CRM_Core_Payment_Form::validatePaymentInstrument($self->_paymentProcessorID, $fields, $errors, $self);
     }