From ce895597e56bcd6b94e99e58ee126bfa6ff1fc97 Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 2 Aug 2018 23:13:37 +1200 Subject: [PATCH] CRM-21311 Credit card type is unset on submission if labels are customised. We have been following a strange pattern where we take the label of a credit card field and munge it into a css compatible string and then expect that to be a match with an array based on lcasing the name field for the credit card. This means that if label !== name the house of cards falls. This is particularly a problem for Amex which needs to be set correctly for the 4 digit cvv not to trigger a validation fail. Notably Amex is probably also the most likely for people to want to rename it since Amex is an abbreviation. This PR backs out of all of that and assigns an array of data to js - including the pattern itself and then updates the js script to refer to the right variable name for each purpose and to stop munging --- CRM/Core/Payment/Form.php | 21 ------- CRM/Financial/Form/Payment.php | 59 ++++++++++++++++++- templates/CRM/Core/BillingBlock.js | 47 ++++----------- .../Financial/BAO/PaymentProcessorTest.php | 34 ----------- 4 files changed, 71 insertions(+), 90 deletions(-) diff --git a/CRM/Core/Payment/Form.php b/CRM/Core/Payment/Form.php index 60e946d7dd..54fe05bf54 100644 --- a/CRM/Core/Payment/Form.php +++ b/CRM/Core/Payment/Form.php @@ -279,27 +279,6 @@ class CRM_Core_Payment_Form { $payment->validatePaymentInstrument($values, $errors); } - /** - * The credit card pseudo constant results only the CC label, not the key ID - * So we normalize the name to use it as a CSS class. - */ - public static function getCreditCardCSSNames($creditCards = array()) { - $creditCardTypes = array(); - if (empty($creditCards)) { - $creditCards = CRM_Contribute_PseudoConstant::creditCard(); - } - foreach ($creditCards as $key => $name) { - // Replace anything not css-friendly by an underscore - // Non-latin names will not like this, but so many things are wrong with - // the credit-card type configurations already. - $key = str_replace(' ', '', $key); - $key = preg_replace('/[^a-zA-Z0-9]/', '_', $key); - $key = strtolower($key); - $creditCardTypes[$key] = $name; - } - return $creditCardTypes; - } - /** * Set default values for the form. * diff --git a/CRM/Financial/Form/Payment.php b/CRM/Financial/Form/Payment.php index f96398b51d..1d3ab86044 100644 --- a/CRM/Financial/Form/Payment.php +++ b/CRM/Financial/Form/Payment.php @@ -124,7 +124,19 @@ class CRM_Financial_Form_Payment extends CRM_Core_Form { */ public static function addCreditCardJs($paymentProcessorID = NULL, $region = 'billing-block') { $creditCards = CRM_Financial_BAO_PaymentProcessor::getCreditCards($paymentProcessorID); - $creditCardTypes = CRM_Core_Payment_Form::getCreditCardCSSNames($creditCards); + if (empty($creditCards)) { + $creditCards = CRM_Contribute_PseudoConstant::creditCard(); + } + $creditCardTypes = []; + foreach ($creditCards as $name => $label) { + $creditCardTypes[$name] = [ + 'label' => $label, + 'name' => $name, + 'css_key' => self::getCssLabelFriendlyName($name), + 'pattern' => self::getCardPattern($name), + ]; + } + CRM_Core_Resources::singleton() // CRM-20516: add BillingBlock script on billing-block region // to support this feature in payment form snippet too. @@ -134,4 +146,49 @@ class CRM_Financial_Form_Payment extends CRM_Core_Form { ->addScript('CRM.config.creditCardTypes = ' . json_encode($creditCardTypes) . ';', '-9999', $region); } + /** + * Get css friendly labels for credit cards. + * + * We add the icons based on these css names which are lower cased + * and only AlphaNumeric (+ _). + * + * @param $key + * + * @return string + */ + protected static function getCssLabelFriendlyName($key) { + $key = str_replace(' ', '', $key); + $key = preg_replace('/[^a-zA-Z0-9]/', '_', $key); + $key = strtolower($key); + + return $key; + } + + /** + * Get the pattern that can be used to determine the card type. + * + * We do a strotolower comparison as we don't know what case people might have if they + * are using a non-std one like dinersclub. + * + * @param $key + * + * Based on http://davidwalsh.name/validate-credit-cards + * See also https://en.wikipedia.org/wiki/Credit_card_numbers + * + * @return string + */ + protected static function getCardPattern($key) { + $cardMappings = [ + 'mastercard' => '(5[1-5][0-9]{2}|2[3-6][0-9]{2}|22[3-9][0-9]|222[1-9]|27[0-1][0-9]|2720)[0-9]{12}', + 'visa' => '4(?:[0-9]{12}|[0-9]{15})', + 'amex' => '3[47][0-9]{13}', + 'dinersclub' => '3(?:0[0-5][0-9]{11}|[68][0-9]{12})', + 'carteblanche' => '3(?:0[0-5][0-9]{11}|[68][0-9]{12})', + 'discover' => '6011[0-9]{12}', + 'jcb' => '(?:3[0-9]{15}|(2131|1800)[0-9]{11})', + 'unionpay' => '62(?:[0-9]{14}|[0-9]{17})', + ]; + return isset($cardMappings[strtolower($key)]) ? $cardMappings[strtolower($key)] : ''; + } + } diff --git a/templates/CRM/Core/BillingBlock.js b/templates/CRM/Core/BillingBlock.js index 3883948497..98469c9a75 100644 --- a/templates/CRM/Core/BillingBlock.js +++ b/templates/CRM/Core/BillingBlock.js @@ -10,13 +10,13 @@ function civicrm_billingblock_creditcard_helper() { $(function() { $.each(CRM.config.creditCardTypes, function(key, val) { - var html = '' + val + ''; + var html = '' + val.label + ''; $('.crm-credit_card_type-icons').append(html); - $('.crm-credit_card_type-icon-' + key).click(function() { - $('#credit_card_type').val(val); + $('.crm-credit_card_type-icon-' + val.css_key).click(function() { + $('#credit_card_type').val(key); $('.crm-container .credit_card_type-section a').css('opacity', 0.25); - $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + key).css('opacity', 1); + $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + val.css_key).css('opacity', 1); return false; }); }); @@ -27,13 +27,13 @@ // set the card type value as default if any found var cardtype = $('#credit_card_type').val(); if (cardtype) { - $.each(CRM.config.creditCardTypes, function(key, value) { + $.each(CRM.config.creditCardTypes, function(key, val) { // highlight the selected card type icon - if (value == cardtype) { - $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + key).css('opacity', 1); + if (key === cardtype) { + $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + val.css_key).css('opacity', 1); } else { - $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + key).css('opacity', 0.25); + $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + val.css_key).css('opacity', 0.25); } }); } @@ -56,33 +56,12 @@ } function civicrm_billingblock_set_card_type(ccnumber) { - // Based on http://davidwalsh.name/validate-credit-cards - // See also https://en.wikipedia.org/wiki/Credit_card_numbers - var card_types = { - 'mastercard': '(5[1-5][0-9]{2}|2[3-6][0-9]{2}|22[3-9][0-9]|222[1-9]|27[0-1][0-9]|2720)[0-9]{12}', - 'visa': '4(?:[0-9]{12}|[0-9]{15})', - 'amex': '3[47][0-9]{13}', - 'dinersclub': '3(?:0[0-5][0-9]{11}|[68][0-9]{12})', - 'carteblanche': '3(?:0[0-5][0-9]{11}|[68][0-9]{12})', - 'discover': '6011[0-9]{12}', - 'jcb': '(?:3[0-9]{15}|(2131|1800)[0-9]{11})', - 'unionpay': '62(?:[0-9]{14}|[0-9]{17})' - }; - var card_values = CRM.config.creditCardTypes; - - $.each(card_types, function(key, pattern) { - if (ccnumber.match('^' + pattern + '$')) { - var value = card_values[key]; - //$.each(CRM.config.creditCardTypes, function(key2, val) { - // if (value == val) { - $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + key).css('opacity', 1); - $('select#credit_card_type').val(value); - return false; - // } - // else { - // $ - // }); + $.each(card_values, function(key, spec) { + if (ccnumber.match('^' + spec.pattern + '$')) { + $('.crm-container .credit_card_type-section .crm-credit_card_type-icon-' + spec.css_key).css('opacity', 1); + $('select#credit_card_type').val(key); + return false; } }); } diff --git a/tests/phpunit/CRM/Financial/BAO/PaymentProcessorTest.php b/tests/phpunit/CRM/Financial/BAO/PaymentProcessorTest.php index 928961d167..a986375855 100644 --- a/tests/phpunit/CRM/Financial/BAO/PaymentProcessorTest.php +++ b/tests/phpunit/CRM/Financial/BAO/PaymentProcessorTest.php @@ -62,38 +62,4 @@ class CRM_Financial_BAO_PaymentProcessorTest extends CiviUnitTestCase { $this->assertEquals($cards, $expectedCards, 'Verify correct credit card types are returned'); } - public function testCreditCardCSSName() { - $params = array( - 'name' => 'API_Test_PP_Type', - 'title' => 'API Test Payment Processor Type', - 'class_name' => 'CRM_Core_Payment_APITest', - 'billing_mode' => 'form', - 'payment_processor_type_id' => 1, - 'is_recur' => 0, - 'domain_id' => 1, - 'accepted_credit_cards' => json_encode(array( - 'Visa' => 'Visa', - 'Mastercard' => 'Mastercard', - 'Amex' => 'Amex', - )), - ); - $paymentProcessor = CRM_Financial_BAO_PaymentProcessor::create($params); - $cards = CRM_Financial_BAO_PaymentProcessor::getCreditCards($paymentProcessor->id); - $CSSCards = CRM_Core_Payment_Form::getCreditCardCSSNames($cards); - $expectedCSSCards = array( - 'visa' => 'Visa', - 'mastercard' => 'Mastercard', - 'amex' => 'Amex', - ); - $this->assertEquals($CSSCards, $expectedCSSCards, 'Verify correct credit card types are returned'); - $CSSCards2 = CRM_Core_Payment_Form::getCreditCardCSSNames(array()); - $allCards = array( - 'visa' => 'Visa', - 'mastercard' => 'MasterCard', - 'amex' => 'Amex', - 'discover' => 'Discover', - ); - $this->assertEquals($CSSCards2, $allCards, 'Verify correct credit card types are returned'); - } - } -- 2.25.1