From 26f94f67faafffd95658087674305bb60fda7fa4 Mon Sep 17 00:00:00 2001
From: Matthew Wire <mjw@mjwconsult.co.uk>
Date: Sun, 8 Oct 2017 10:26:37 +0100
Subject: [PATCH] Code cleanup
---
CRM/Core/Form/Stripe.php | 16 ----
CRM/Core/Payment/Stripe.php | 70 +++++---------
README.md | 8 ++
js/civicrm_stripe.js | 176 ++++++++++++++++++------------------
stripe.php | 8 +-
5 files changed, 123 insertions(+), 155 deletions(-)
delete mode 100644 CRM/Core/Form/Stripe.php
diff --git a/CRM/Core/Form/Stripe.php b/CRM/Core/Form/Stripe.php
deleted file mode 100644
index 828e2ea5..00000000
--- a/CRM/Core/Form/Stripe.php
+++ /dev/null
@@ -1,16 +0,0 @@
-<?php
-
-/*
- * Form Class for Stripe
- */
-
-class CRM_Core_Form_Stripe extends CRM_Core_Form {
-
- /**
- * Function to access protected payProcessors array in event registraion forms
- */
- public static function get_ppids(&$form) {
- $payprocessorIds = $form->_paymentProcessors;
- return $payprocessorIds;
- }
-}
diff --git a/CRM/Core/Payment/Stripe.php b/CRM/Core/Payment/Stripe.php
index f938097b..74bfc693 100644
--- a/CRM/Core/Payment/Stripe.php
+++ b/CRM/Core/Payment/Stripe.php
@@ -208,7 +208,7 @@ class CRM_Core_Payment_Stripe extends CRM_Core_Payment {
* @param $form - reference to the form object
*/
public function buildForm(&$form) {
- $stripe_ppid = self::get_stripe_ppid($form);
+ $stripe_ppid = CRM_Utils_Array::value('id', $form->_paymentProcessor);
// Add the ID to our form so our js can tell if Stripe has been selected.
$form->addElement('hidden', 'stripe_id', $stripe_ppid, array('id' => 'stripe-id'));
@@ -221,59 +221,19 @@ class CRM_Core_Payment_Stripe extends CRM_Core_Payment {
if (!empty($params[0]['stripe_token'])) {
$params = $params[0];
}
- $stripe_token = (empty($params['stripe_token']) ? NULL : $params['stripe_token']);
+ $stripeToken = (empty($params['stripetoken']) ? NULL : $params['stripetoken']);
// Add some hidden fields for Stripe.
- if (!$form->elementExists('stripe_token')) {
- $form->setAttribute('class', $form->getAttribute('class') . ' stripe-payment-form');
- $form->addElement('hidden', 'stripe_token', $stripe_token, array('id' => 'stripe-token'));
+ if (!empty($stripeToken) && !$form->elementExists('stripetoken')) {
+ $form->addElement('hidden', 'stripetoken', $stripeToken, array('id' => 'stripe-token'));
}
- // Add the Civi version so we can accommodate different versions in civicrm_stripe.js.
- if (self::get_civi_version() <= '4.7.0') {
- $ext_mode = 1;
- }
- else {
- $ext_mode = 2;
- }
- $form->addElement('hidden', 'ext_mode', $ext_mode, array('id' => 'ext-mode'));
-
// Add email field as it would usually be found on donation forms.
if (!isset($form->_elementIndex['email']) && !empty($form->userEmail)) {
$form->addElement('hidden', 'email', $form->userEmail, array('id' => 'user-email'));
}
}
- public static function get_stripe_ppid($form) {
- if (empty($form->_paymentProcessor)) {
- return;
- }
-
- // When called from admin backend (eg via CRM_Contribute_Form_Contribution, CRM_Member_Form_Membership)
- // the isBackOffice flag will be set to true.
- // But if called via webform in CiviCRM 4.7: isBackOffice=NULL and for is of class CRM_Financial_Form_Payment or CRM_Contribute_Form_Contribution
- // Those don't have a _paymentProcessors array and only have one payprocesssor.
- if (!empty($form->isBackOffice)
- || (in_array(get_class($form), array('CRM_Financial_Form_Payment', 'CRM_Contribute_Form_Contribution')))) {
- return $stripe_ppid = $form->_paymentProcessor['id'];
- }
- else {
- // Find a Stripe pay processor ascociated with this Civi form and find the ID.
- // $payProcessors = $form->_paymentProcessors;
- $payProcessors = CRM_Core_Form_Stripe::get_ppids($form);
- foreach ($payProcessors as $payProcessor) {
- if ($payProcessor['class_name'] == 'Payment_Stripe') {
- return $stripe_ppid = $payProcessor['id'];
- break;
- }
- }
- }
- // None of the payprocessors are Stripe.
- if (empty($stripe_ppid)) {
- return;
- }
- }
-
/**
* Given a payment processor id, return the pub key.
*/
@@ -342,8 +302,9 @@ class CRM_Core_Payment_Stripe extends CRM_Core_Payment {
$amount = (int) preg_replace('/[^\d]/', '', strval($amount));
// Use Stripe.js instead of raw card details.
- if (!empty($params['stripe_token'])) {
- $card_details = $params['stripe_token'];
+ if (!empty($params['credit_card_number']) && (substr($params['credit_card_number'], 0, 4) === 'tok_')) {
+ $card_details = $params['credit_card_number'];
+ $params['credit_card_number'] = '';
}
else {
CRM_Core_Error::fatal(ts('Stripe.js token was not passed! Report this message to the site administrator.'));
@@ -816,4 +777,21 @@ class CRM_Core_Payment_Stripe extends CRM_Core_Payment {
public function doTransferCheckout(&$params, $component) {
CRM_Core_Error::fatal(ts('Use direct billing instead of Transfer method.'));
}
+
+ /**
+ * Default payment instrument validation.
+ *
+ * Implement the usual Luhn algorithm via a static function in the CRM_Core_Payment_Form if it's a credit card
+ * Not a static function, because I need to check for payment_type.
+ *
+ * @param array $values
+ * @param array $errors
+ */
+ public function validatePaymentInstrument($values, &$errors) {
+ CRM_Core_Form::validateMandatoryFields($this->getMandatoryFields(), $values, $errors);
+ if ($this->_paymentProcessor['payment_type'] == 1) {
+ // Don't validate credit card details as they are not passed (and stripe does this for us)
+ //CRM_Core_Payment_Form::validateCreditCard($values, $errors, $this->_paymentProcessor['id']);
+ }
+ }
}
diff --git a/README.md b/README.md
index df41ab0e..3ddf345e 100644
--- a/README.md
+++ b/README.md
@@ -66,3 +66,11 @@ OTHER CREDITS
-------------
For bug fixes, new features, and documentiation, thanks to:
rgburton, Swingline0, BorislavZlatanov, agh1, & jmcclelland
+
+TESTING
+--------
+1. Test webform submission with payment and user-select, single processor.
+2. Test online contribution page with single processor, multi-processor (stripe default, stripe non-default).
+3. Test offline contribution page with single processor, multi-processor (stripe default, stripe non-default).
+4. Test event registration.
+5. Test event registration (cart checkout).
diff --git a/js/civicrm_stripe.js b/js/civicrm_stripe.js
index d42de560..c8dbb85b 100644
--- a/js/civicrm_stripe.js
+++ b/js/civicrm_stripe.js
@@ -24,68 +24,73 @@
+ '</div>');
$submit.removeAttr('disabled').attr('value', buttonText);
-
}
else {
var token = response['id'];
// Update form with the token & submit.
- $form.find("input#stripe-token").val(token);
- $form.find("input#credit_card_number").removeAttr('name');
- $form.find("input#cvv2").removeAttr('name');
- $submit.prop('disabled', false);
+ removeCCDetails($form);
+ // We use the credit_card_number field to pass token as this is reliable.
+ // Inserting an input field is unreliable on ajax forms and often gets missed from POST request for some reason.
+ $form.find("input#credit_card_number").val(token);
+
+ // Disable unload event handler
window.onbeforeunload = null;
+ // This triggers submit without generating a submit event (so we don't run submit handler again)
$form.get(0).submit();
}
}
// Prepare the form.
$(document).ready(function() {
- Stripe.setPublishableKey($('#stripe-pub-key').val());
loadStripeBillingBlock();
});
- CRM.$('input[name="payment_processor_id"]').change(function() {
+ // On the frontend, we have a set of radio buttons. Trigger on change.
+ $('input[name="payment_processor_id"]').change(function() {
+ loadStripeBillingBlock();
+ });
+
+ // On the backend, we have a select. Trigger on change.
+ $('select#payment_processor_id').change(function() {
loadStripeBillingBlock();
});
function loadStripeBillingBlock() {
- // Check for form marked as a stripe-payment-form by the server.
- if (!($('form.stripe-payment-form').length)) {
- // If there isn't one look for it.
- if ($('.webform-client-form').length) {
- isWebform = true;
- $('form.webform-client-form').addClass('stripe-payment-form');
- }
- else if ($('#crm-container form').length) {
- $('#crm-container form').addClass('stripe-payment-form');
- }
- else {
- return;
+ var $stripePubKey = $('#stripe-pub-key');
+ if ($stripePubKey.length) {
+ if (!$().Stripe) {
+ $.getScript('https://js.stripe.com/v2/', function () {
+ Stripe.setPublishableKey($('#stripe-pub-key').val());
+ });
}
}
- $form = $('form.stripe-payment-form');
+ if ($('.webform-client-form').length) {
+ isWebform = true;
+ }
+
+ // Get the form containing payment details
+ $form = CRM.$('input#stripe-pub-key').closest('form');
+
if (isWebform) {
$submit = $form.find('.button-primary');
}
else {
$submit = $form.find('input[type="submit"][formnovalidate!="1"]');
- // If CiviDiscount button or field is submitted, flag the form.
- $form.data('cividiscount-dont-handle', '0');
- // This is an ugly hack. Really, the code should positively identify the
- // "real" submit button(s) and only respond to them. Otherwise, we're
- // chasing down a potentially endless number of exceptions. The problem
- // is that it's unclear if CiviCRM consistently names its submit buttons.
+ // If another submit button on the form is pressed (eg. apply discount)
+ // add a flag that we can set to stop payment submission
+ $form.data('submit-dont-process', '0');
+ // Find submit buttons with formnovalidate=1 and add an onclick handler to set flag
$form.find('input[type="submit"][formnovalidate="1"], input[type="submit"].cancel').click( function() {
- $form.data('cividiscount-dont-handle', 1);
+ $form.data('submit-dont-process', 1);
});
+ // Add a keypress handler to set flag if enter is pressed
$form.find('input#discountcode').keypress( function(e) {
- if (e.which == 13) {
- $form.data('cividiscount-dont-handle', 1);
+ if (e.which === 13) {
+ $form.data('submit-dont-process', 1);
}
});
- $submit;
}
// For CiviCRM Webforms.
@@ -94,9 +99,10 @@
$form.append('<input type="hidden" name="op" id="action" />');
}
$(document).keypress(function(event) {
- if (event.which == 13) {
+ if (event.which === 13) {
+ // Enter was pressed
event.preventDefault();
- $submit.click();
+ submit(event);
}
});
$(":submit").click(function() {
@@ -108,49 +114,62 @@
var webformPrevious = $('input.webform-previous').first().val();
}
else {
- // This is native civicrm form - check for existing token.
- if ($form.find("input#stripe-token").val()) {
+ // CiviCRM form
+ // If we already have a token hide CC details
+ if ($form.find("input#credit_card_number").val()) {
$('.credit_card_info-group').hide();
$('#billing-payment-block').append('<input type="button" value="Edit CC details" id="ccButton" />');
$('#ccButton').click(function() {
+ // Clear token and show CC details if edit button was clicked
+ // As we use credit_card_number to pass token, make sure it is empty when shown
+ $form.find("input#credit_card_number").val('');
$('.credit_card_info-group').show();
$('#ccButton').hide();
- $form.find('input#stripe-token').val('');
});
}
+ else {
+ // As we use credit_card_number to pass token, make sure it is empty when shown
+ $form.find("input#credit_card_number").val('');
+ }
}
$submit.removeAttr('onclick');
-
$form.unbind('submit');
// Intercept form submission.
$form.submit(function (event) {
+ event.preventDefault();
+ submit(event);
+ });
+
+ function submit(event) {
+ // Don't handle submits generated by non-stripe processors
+ if (!$('input#stripe-pub-key').length) {
+ debugging('submit missing stripe-pub-key element');
+ return true;
+ }
// Don't handle submits generated by the CiviDiscount button.
- if ($form.data('cividiscount-dont-handle') == 1) {
+ if ($form.data('submit-dont-process') === 1) {
debugging('debug: pvjwy (Discount is in play)');
return true;
}
if (isWebform) {
var $processorFields = $('.civicrm-enabled[name$="civicrm_1_contribution_1_contribution_payment_processor_id]"]');
- if ($('#action').attr('value') == webformPrevious) {
- debugging('wmlfp');
+ if ($('#action').attr('value') === webformPrevious) {
+ // Don't submit if the webform back button was pressed
+ debugging('webform back button');
return true;
}
if ($('#wf-crm-billing-total').length) {
- if ($('#wf-crm-billing-total').data('data-amount') == '0') {
- debugging('qplfr');
+ if ($('#wf-crm-billing-total').data('data-amount') === '0') {
+ debugging('webform total is 0');
return true;
}
}
if ($processorFields.length) {
- if ($processorFields.filter(':checked').val() == '0') {
- debugging('evxyh');
- return true;
- }
- if (!($form.find('input[name="stripe_token"]').length)) {
- debugging('irjfg');
+ if ($processorFields.filter(':checked').val() === '0') {
+ debugging('no payment processor selected');
return true;
}
}
@@ -159,68 +178,39 @@
buttonText = $submit.attr('value');
$submit.prop('disabled', true).attr('value', 'Processing');
- // Hide payment if total is 0 and no more participants.
- if ($('#priceset').length) {
- additionalParticipants = cj("#additional_participants").val();
- // The currentTotal is already being calculated in Form/Contribution/Main.tpl.
- if(typeof currentTotal !== 'undefined') {
- if (currentTotal == 0 && !additionalParticipants) {
- // This is also hit when "Going back", but we already have stripe_token.
- debugging('ozlkf');
- // This should not happen on Confirm Contribution, but seems to on 4.6 for some reason.
- //return true;
- }
- }
- }
-
// Handle multiple payment options and Stripe not being chosen.
if ($form.find(".crm-section.payment_processor-section").length > 0) {
var extMode = $('#ext-mode').val();
var stripeProcessorId = $('#stripe-id').val();
- // Support for CiviCRM 4.6 and 4.7 multiple payment options
- if (extMode == 1) {
- var chosenProcessorId = $form.find('input[name="payment_processor"]:checked').val();
- }
- else if (extMode == 2) {
- var chosenProcessorId = $form.find('input[name="payment_processor_id"]:checked').val();
- }
+ var chosenProcessorId = $form.find('input[name="payment_processor_id"]:checked').val();
+
// Bail if we're not using Stripe or are using pay later (option value '0' in payment_processor radio group).
- if ((chosenProcessorId != stripeProcessorId) || (chosenProcessorId == 0)) {
- debugging('debug: kfoej (Not a Stripe transaction, or pay-later)');
+ if ((chosenProcessorId !== stripeProcessorId) || (chosenProcessorId === 0)) {
+ debugging('debug: Not a Stripe transaction, or pay-later');
return true;
}
}
else {
- debugging('debug: qlmvy (Stripe is the only payprocessor here)');
+ debugging('debug: Stripe is the selected payprocessor');
}
// Handle reuse of existing token
- if ($form.find("input#stripe-token").val()) {
- $form.find("input#credit_card_number").removeAttr('name');
- $form.find("input#cvv2").removeAttr('name');
- debugging('debug: zpqef (Re-using Stripe token)');
+ if ($form.find("input#credit_card_number").val()) {
+ removeCCDetails($form);
+ debugging('debug: Re-using Stripe token');
return true;
}
// If there's no credit card field, no use in continuing (probably wrong
// context anyway)
if (!$form.find('#credit_card_number').length) {
- debugging('debug: gvzod (No credit card field)');
+ debugging('debug: No credit card field');
return true;
}
- event.preventDefault();
- event.stopPropagation();
+ var cc_month = $form.find('#credit_card_exp_date_M').val();
+ var cc_year = $form.find('#credit_card_exp_date_Y').val();
- // Handle changes introduced in CiviCRM 4.3.
- if ($form.find('#credit_card_exp_date_M').length > 0) {
- var cc_month = $form.find('#credit_card_exp_date_M').val();
- var cc_year = $form.find('#credit_card_exp_date_Y').val();
- }
- else {
- var cc_month = $form.find('#credit_card_exp_date\\[M\\]').val();
- var cc_year = $form.find('#credit_card_exp_date\\[Y\\]').val();
- }
Stripe.card.createToken({
name: $form.find('#billing_first_name').val() + ' ' + $form.find('#billing_last_name').val(),
address_zip: $form.find('#billing_postal_code-5').val(),
@@ -230,14 +220,20 @@
exp_year: cc_year
}, stripeResponseHandler);
- debugging('debug: ywkvh (Getting Stripe token)');
+ debugging('debug: Getting Stripe token');
return false;
- });
+ }
}
}(cj, CRM));
+function removeCCDetails($form) {
+ // Remove the "name" attribute so params are not submitted
+ $form.find("input#credit_card_number").val('0000000000000000');
+ $form.find("input#cvv2").val('000');
+}
+
function debugging (errorCode) {
// Uncomment the following to debug unexpected returns.
- // console.log(errorCode);
+ console.log(errorCode);
}
diff --git a/stripe.php b/stripe.php
index f8555526..162a4dd4 100644
--- a/stripe.php
+++ b/stripe.php
@@ -3,6 +3,8 @@
require_once 'stripe.civix.php';
require_once __DIR__.'/vendor/autoload.php';
+use CRM_Stripe_ExtensionUtil as E;
+
/**
* Implementation of hook_civicrm_config().
*/
@@ -194,7 +196,7 @@ function stripe_civicrm_managed(&$entities) {
* @return void
*/
function stripe_civicrm_alterContent( &$content, $context, $tplName, &$object ) {
- if($context == 'form' && !empty($object->_paymentProcessor['class_name'])) {
+ if ($context == 'form' && !empty($object->_paymentProcessor['class_name'])) {
$stripeJSURL = CRM_Core_Resources::singleton()->getUrl('com.drastikbydesign.stripe', 'js/civicrm_stripe.js');
$content .= "<script src='{$stripeJSURL}'></script>";
}
@@ -207,9 +209,9 @@ function stripe_civicrm_managed(&$entities) {
*/
function stripe_civicrm_buildForm($formName, &$form) {
if (!empty($form->_paymentProcessor['class_name'])) {
- CRM_Core_Resources::singleton()->addScriptUrl('https://js.stripe.com/v2/');
+ //CRM_Core_Resources::singleton()->addScriptUrl('https://js.stripe.com/v2/', 10, 'page-body');
}
- if (($formName == 'CRM_Member_Form_MembershipRenewal') && !empty($form->_paymentProcessor['class_name'])) {
+ if (/*$form->isBackOffice &&*/ !empty($form->_paymentProcessor['class_name'])) {
// civicrm_stripe.js is not included on backend form renewal unless we add it here.
CRM_Core_Resources::singleton()->addScriptFile('com.drastikbydesign.stripe', 'js/civicrm_stripe.js');
}
--
GitLab