Check the billing fields returned by each payment processor (rather than a fixed...

Add missing newline after closing brace

Add safer check for non-empty variable

Test if billing fields presence is done respecting payment processors

Pass civilint warnings for test file

Add assertion for payment processor with uncomplete profile

Apply civilint fix (remove extra line ending)

diff --git a/CRM/Core/BAO/UFField.php b/CRM/Core/BAO/UFField.php
index 2e1a4dc56d8aeba39ed7f2bb0416c6d49773d817..965d8c6f2fa3fe9d6c0125dcfec461c84e980331 100644 (file)
--- a/CRM/Core/BAO/UFField.php
+++ b/CRM/Core/BAO/UFField.php
@@ -778,11 +778,13 @@ SELECT  id
    * @param array $profileFilter
    *   Filter to apply to profile fields - expected usage is to only fill based on.
    *   the bottom profile per CRM-13726
+   * @param array $paymentProcessorBillingFields
+   *   Array of billing fields required by the payment processor.
    *
    * @return bool
    *   Can the address block be hidden safe in the knowledge all fields are elsewhere collected (see CRM-15118)
    */
-  public static function assignAddressField($key, &$profileAddressFields, $profileFilter) {
+  public static function assignAddressField($key, &$profileAddressFields, $profileFilter, $paymentProcessorBillingFields = NULL) {
     $billing_id = CRM_Core_BAO_LocationType::getBilling();
     list($prefixName, $index) = CRM_Utils_System::explode('-', $key, 2);
 
@@ -796,17 +798,22 @@ SELECT  id
       ]
     ));
     //check for valid fields ( fields that are present in billing block )
-    $validBillingFields = [
-      'first_name',
-      'middle_name',
-      'last_name',
-      'street_address',
-      'supplemental_address_1',
-      'city',
-      'state_province',
-      'postal_code',
-      'country',
-    ];
+    if (!empty($paymentProcessorBillingFields)) {
+      $validBillingFields = $paymentProcessorBillingFields;
+    }
+    else {
+      $validBillingFields = [
+        'first_name',
+        'middle_name',
+        'last_name',
+        'street_address',
+        'supplemental_address_1',
+        'city',
+        'state_province',
+        'postal_code',
+        'country',
+      ];
+    }
     $requiredBillingFields = array_diff($validBillingFields, ['middle_name', 'supplemental_address_1']);
     $validProfileFields = [];
     $requiredProfileFields = [];
@@ -841,8 +848,10 @@ SELECT  id
     }
 
     $potentiallyMissingRequiredFields = array_diff($requiredBillingFields, $requiredProfileFields);
+    $billingProfileIsHideable = empty($potentiallyMissingRequiredFields);
     CRM_Core_Resources::singleton()
-      ->addSetting(['billing' => ['billingProfileIsHideable' => empty($potentiallyMissingRequiredFields)]]);
+      ->addSetting(['billing' => ['billingProfileIsHideable' => $billingProfileIsHideable]]);
+    return $billingProfileIsHideable;
   }
 
   /**

diff --git a/CRM/Core/Payment/ProcessorForm.php b/CRM/Core/Payment/ProcessorForm.php
index 1649fa333a13808506041219b25f3fdbaef3ce52..a4062aef44b042db5c227928e5aa075d36135f57 100644 (file)
--- a/CRM/Core/Payment/ProcessorForm.php
+++ b/CRM/Core/Payment/ProcessorForm.php
@@ -67,13 +67,15 @@ class CRM_Core_Payment_ProcessorForm {
       $form->_values['cancelSubscriptionUrl'] = $form->_paymentObject->subscriptionURL(NULL, NULL, 'cancel');
     }
 
+    $paymentProcessorBillingFields = array_keys($form->_paymentProcessor['object']->getBillingAddressFields());
+
     if (!empty($form->_values['custom_pre_id'])) {
       $profileAddressFields = [];
       $fields = CRM_Core_BAO_UFGroup::getFields($form->_values['custom_pre_id'], FALSE, CRM_Core_Action::ADD, NULL, NULL, FALSE,
         NULL, FALSE, NULL, CRM_Core_Permission::CREATE, NULL);
 
       foreach ((array) $fields as $key => $value) {
-        CRM_Core_BAO_UFField::assignAddressField($key, $profileAddressFields, ['uf_group_id' => $form->_values['custom_pre_id']]);
+        CRM_Core_BAO_UFField::assignAddressField($key, $profileAddressFields, ['uf_group_id' => $form->_values['custom_pre_id']], $paymentProcessorBillingFields);
       }
       if (count($profileAddressFields)) {
         $form->set('profileAddressFields', $profileAddressFields);

diff --git a/tests/phpunit/CRM/Core/Payment/ProcessorFormTest.php b/tests/phpunit/CRM/Core/Payment/ProcessorFormTest.php
new file mode 100644 (file)
index 0000000..c924c08
--- /dev/null
+++ b/tests/phpunit/CRM/Core/Payment/ProcessorFormTest.php
@@ -0,0 +1,248 @@
+<?php
+/*
+ +--------------------------------------------------------------------+
+ | Copyright CiviCRM LLC. All rights reserved.                        |
+ |                                                                    |
+ | This work is published under the GNU AGPLv3 license with some      |
+ | permitted exceptions and without any warranty. For full license    |
+ | and copyright information, see https://civicrm.org/licensing       |
+ +--------------------------------------------------------------------+
+ */
+
+/**
+ * Class CRM_Core_Payment_ProcessorFormTest
+ * @group headless
+ */
+class CRM_Core_Payment_ProcessorFormTest extends CiviUnitTestCase {
+
+  public function setUp(): void {
+    parent::setUp();
+
+    $this->standardProfile = $this->createStandardBillingProfile();
+    $this->customProfile = $this->createCustomBillingProfile();
+
+    $this->standardProcessorType = $this->paymentProcessorTypeCreate([
+      'class_name' => 'PaymentProcessorWithStandardBillingRequirements',
+      'name' => 'StandardBillingType',
+    ]);
+
+    $this->customProcessorType = $this->paymentProcessorTypeCreate([
+      'class_name' => 'PaymentProcessorWithCustomBillingRequirements',
+      'name' => 'CustomBillingType',
+    ]);
+
+    $this->standardProcessor = $this->paymentProcessorCreate([
+      'name' => 'StandardBilling',
+      'class_name' => 'PaymentProcessorWithStandardBillingRequirements',
+      'payment_processor_type_id' => $this->standardProcessorType,
+      'is_test' => 0,
+    ]);
+
+    $this->customProcessor = $this->paymentProcessorCreate([
+      'name' => 'CustomBilling',
+      'class_name' => 'PaymentProcessorWithCustomBillingRequirements',
+      'payment_processor_type_id' => $this->customProcessorType,
+      'is_test' => 0,
+    ]);
+  }
+
+  public function tearDown(): void {
+    $this->callAPISuccess('PaymentProcessor', 'delete', [
+      'id' => $this->standardProcessor,
+    ]);
+
+    $this->callAPISuccess('PaymentProcessor', 'delete', [
+      'id' => $this->customProcessor,
+    ]);
+
+    $this->callAPISuccess('PaymentProcessorType', 'delete', [
+      'id' => $this->standardProcessorType,
+    ]);
+
+    $this->callAPISuccess('PaymentProcessorType', 'delete', [
+      'id' => $this->customProcessorType,
+    ]);
+
+    $this->quickCleanUpFinancialEntities();
+    $this->quickCleanup(['civicrm_uf_group', 'civicrm_uf_field']);
+
+    parent::tearDown();
+  }
+
+  public function createStandardBillingProfile() {
+    return $this->createTestableBillingProfile('standard', TRUE);
+  }
+
+  public function createCustomBillingProfile() {
+    return $this->createTestableBillingProfile('custom', FALSE);
+  }
+
+  public function createTestableBillingProfile($name, $withState) {
+    $billingId = CRM_Core_BAO_LocationType::getBilling();
+
+    $profile = $this->callAPISuccess('UFGroup', 'create', [
+      'group_type' => 'Contact',
+      'title' => "Billing fields: $name",
+      'name' => "${name}_billing",
+    ]);
+
+    $this->callAPISuccess('UFField', 'create', [
+      'uf_group_id' => $profile['id'],
+      'field_name' => 'first_name',
+      'is_required' => TRUE,
+    ]);
+
+    $this->callAPISuccess('UFField', 'create', [
+      'uf_group_id' => $profile['id'],
+      'field_name' => 'last_name',
+      'is_required' => TRUE,
+    ]);
+
+    $this->callAPISuccess('UFField', 'create', [
+      'uf_group_id' => $profile['id'],
+      'field_name' => 'street_address',
+      'is_required' => TRUE,
+    ]);
+
+    $this->callAPISuccess('UFField', 'create', [
+      'uf_group_id' => $profile['id'],
+      'field_name' => 'city',
+      'location_type_id' => $billingId,
+      'is_required' => TRUE,
+    ]);
+
+    if ($withState) {
+      $this->callAPISuccess('UFField', 'create', [
+        'uf_group_id' => $profile['id'],
+        'field_name' => 'state_province',
+        'location_type_id' => $billingId,
+        'is_required' => TRUE,
+      ]);
+    }
+
+    $this->callAPISuccess('UFField', 'create', [
+      'uf_group_id' => $profile['id'],
+      'field_name' => 'postal_code',
+      'location_type_id' => $billingId,
+      'is_required' => TRUE,
+    ]);
+
+    $this->callAPISuccess('UFField', 'create', [
+      'uf_group_id' => $profile['id'],
+      'field_name' => 'country',
+      'location_type_id' => $billingId,
+      'is_required' => TRUE,
+    ]);
+
+    return $profile;
+  }
+
+  /**
+   * Checks a payment processor with either the standard,
+   * or the custom profile and returns a boolean that
+   * indicates whether the billing block can be hidden, or not.
+   */
+  public function checkPaymentProcessorWithProfile($processorClass, $case) {
+    $whichProcessor = $case . "Processor";
+    $whichProfile = $case . "Profile";
+
+    $processor = new $processorClass();
+    $processor->id = $this->$whichProcessor;
+
+    $missingBillingFields = [];
+
+    $fields = array_column(
+      $this->callAPISuccess('UFField', 'get', ['uf_group_id' => $this->$whichProfile['id']])['values'],
+      'field_name'
+    );
+
+    $fields = array_map(function($field) {
+      if (!isset($field['location_type_id'])) {
+        return "$field";
+      }
+      return $field . "-" . $field['location_type_id'];
+    }, $fields);
+
+    $canBeHidden = FALSE;
+    foreach ((array) $fields as $field) {
+      $canBeHidden = CRM_Core_BAO_UFField::assignAddressField(
+        $field,
+        $missingBillingFields,
+        ['uf_group_id' => $this->$whichProfile['id']],
+        array_keys($processor->getBillingAddressFields())
+      );
+
+      if (!$canBeHidden) {
+        break;
+      }
+    }
+
+    return $canBeHidden;
+  }
+
+  /**
+   * Checks that, if a payment processor declares the standard
+   * billing fields as needed, they must be considered mandatory.
+   */
+  public function testPaymentProcessorWithStandardBillingRequirements() {
+    $canBeHiddenWithTheStandardProfile = $this->checkPaymentProcessorWithProfile(
+      "PaymentProcessorWithStandardBillingRequirements",
+      "standard"
+    );
+
+    $canBeHiddenWithTheCustomProfile = $this->checkPaymentProcessorWithProfile(
+      "PaymentProcessorWithStandardBillingRequirements",
+      "custom"
+    );
+
+    $this->assertEquals(TRUE, $canBeHiddenWithTheStandardProfile);
+    $this->assertEquals(FALSE, $canBeHiddenWithTheCustomProfile);
+  }
+
+  /**
+   * Checks that, if the payment processor doesn't declare a field
+   * as needed, the field shouldn't be considered mandatory.
+   */
+  public function testPaymentProcessorWithCustomRequirements() {
+    $canBeHiddenWithTheCustomProfile = $this->checkPaymentProcessorWithProfile(
+      "PaymentProcessorWithCustomBillingRequirements",
+      "custom"
+    );
+
+    $this->assertEquals(TRUE, $canBeHiddenWithTheCustomProfile);
+  }
+
+}
+
+class PaymentProcessorWithStandardBillingRequirements extends CRM_Core_Payment {
+
+  /**
+   * again, `checkConfig` is abstract in CRM_Core_Payment, so we are forced to implement it
+   */
+  public function checkConfig() {
+  }
+
+}
+
+class PaymentProcessorWithCustomBillingRequirements extends CRM_Core_Payment {
+
+  /**
+   * again, `checkConfig` is abstract in CRM_Core_Payment, so we are forced to implement it
+   */
+  public function checkConfig() {
+  }
+
+  public function getBillingAddressFields($billingLocationID = NULL) {
+    // Note that it intentionally misses the state_province field
+    return [
+      'first_name' => 'billing_first_name',
+      'middle_name' => 'billing_middle_name',
+      'last_name' => 'billing_last_name',
+      'street_address' => "billing_street_address-{$billingLocationID}",
+      'city' => "billing_city-{$billingLocationID}",
+      'country' => "billing_country_id-{$billingLocationID}",
+      'postal_code' => "billing_postal_code-{$billingLocationID}",
+    ];
+  }
+
+}