jenkins-bot has submitted this change and it was merged.

Change subject: Replace alert with red errors for Mustache forms
......................................................................


Replace alert with red errors for Mustache forms

No more embarassing 90s validation messages!

Uses validate_personal (like GC forms) rather than validate_form.
Renders initial errors server side and clears fixed errors on
validation.

Lots of repetition rendering the error messages - mustache's
lack of logic makes the only other alternative setting dummy
errors for correct fields (a la getEmptyErrorArray).

Red highlight around incorrect fields is not yet working for
Mustache but can be fixed in css.

Bug: T107363
Change-Id: I09aece9fa6bb992af42bea739c4b7eb642b2eab6
---
M DonationInterface.php
M adyen_gateway/forms/js/adyen.js
M astropay_gateway/astropay.js
M gateway_common/DonationData.php
M gateway_forms/Mustache.php
A gateway_forms/mustache/error_message.html.mustache
M gateway_forms/mustache/index.html.mustache
M gateway_forms/mustache/payment_amount.html.mustache
M gateway_forms/mustache/personal_info.html.mustache
M globalcollect_gateway/forms/css/gc.css
M modules/css/gateway.css
M modules/validate_input.js
M worldpay_gateway/forms/js/esop.js
13 files changed, 185 insertions(+), 104 deletions(-)

Approvals:
  Awight: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/DonationInterface.php b/DonationInterface.php
index e8237ea..febb7ad 100644
--- a/DonationInterface.php
+++ b/DonationInterface.php
@@ -1020,6 +1020,7 @@
                'donate_interface-error-msg-discover',
                'donate_interface-error-msg-amount',
                'donate_interface-error-msg-email',
+               'donate_interface-error-msg-fiscal_number',
                'donate_interface-error-msg-fname',
                'donate_interface-error-msg-lname',
                'donate_interface-error-msg-street',
@@ -1052,6 +1053,7 @@
                'donate_interface-donor-postal',
                'donate_interface-donor-country',
                'donate_interface-donor-email',
+               'donate_interface-donor-fiscal_number',
                'donate_interface-state-province',
                'donate_interface-cvv-explain',
        )
diff --git a/adyen_gateway/forms/js/adyen.js b/adyen_gateway/forms/js/adyen.js
index bdccacb..78513dc 100644
--- a/adyen_gateway/forms/js/adyen.js
+++ b/adyen_gateway/forms/js/adyen.js
@@ -100,7 +100,7 @@
        }
 
        $( 'input[name="payment_submethod"]' ).on( 'change', function () {
-               if ( window.validateAmount() && window.validate_form( 
document.payment ) ) {
+               if ( window.validateAmount() && window.validate_personal( 
document.payment ) ) {
                        window.displayCreditCardForm();
                } else {
                        $( '#paymentContinue' ).show();
@@ -108,7 +108,7 @@
        } );
 
        $( '#paymentContinueBtn' ).on( 'click', function () {
-               if ( window.validateAmount() && window.validate_form( 
document.payment ) ) {
+               if ( window.validateAmount() && window.validate_personal( 
document.payment ) ) {
                        window.displayCreditCardForm();
                        // hide the continue button so that people don't get 
confused with two of them
                        $( '#paymentContinue' ).hide();
diff --git a/astropay_gateway/astropay.js b/astropay_gateway/astropay.js
index 3a6437b..b16e778 100644
--- a/astropay_gateway/astropay.js
+++ b/astropay_gateway/astropay.js
@@ -9,7 +9,7 @@
        // Submit on submethod click if valid,
        // otherwise show continue button.
        $( 'input[name="payment_submethod"]' ).on( 'click', function() {
-               if ( window.validateAmount() && window.validate_form( form ) ) {
+               if ( window.validateAmount() && window.validate_personal( form 
) ) {
                        submitForm();
                } else {
                        $( '#paymentContinue' ).show();
@@ -17,7 +17,7 @@
        } );
 
        $( '#paymentContinueBtn' ).click( function() {
-               if ( window.validateAmount() && window.validate_form( form ) ) {
+               if ( window.validateAmount() && window.validate_personal( form 
) ) {
                        submitForm();
                }
        });
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 06bdf0f..b38c167 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -24,6 +24,83 @@
        protected $logger;
 
        /**
+        * $fieldNames now just contains all the vars we want to
+        * get poked through to gateways in some form or other,
+        * from a get or post. We handle the actual
+        * normalization in normalize() helpers, below.
+        * @TODO: It would be really neat if the gateways kept
+        * track of all the things that ***only they will ever
+        * need***, and could interject those needs here...
+        * Then we could really clean up.
+        * @TODO also: Think about putting log alarms on the
+        * keys we want to see disappear forever, complete with
+        * ffname and referrer for easy total destruction.
+        */
+       protected static $fieldNames = array(
+               'amount',
+               'amountGiven',
+               'amountOther',
+               'appeal',
+               'email',
+               'emailAdd',
+               'fname',
+               'lname',
+               'street',
+               'street_supplemental',
+               'city',
+               'state',
+               'zip',
+               'country',
+               'card_num',
+               'card_type',
+               'expiration',
+               'cvv',
+               'currency',
+               'currency_code',
+               'payment_method',
+               'payment_submethod',
+               'issuer_id',
+               'order_id',
+               'subscr_id',
+               'referrer',
+               'utm_source',
+               'utm_source_id',
+               'utm_medium',
+               'utm_campaign',
+               'utm_key',
+               'language',
+               'uselang',
+               'token',
+               'contribution_tracking_id',
+               'data_hash',
+               'action',
+               'gateway',
+               'owa_session',
+               'owa_ref',
+               'descriptor',
+               'account_name',
+               'account_number',
+               'authorization_id',
+               'bank_check_digit',
+               'bank_name',
+               'bank_code',
+               'branch_code',
+               'country_code_bank',
+               'date_collect',
+               'direct_debit_text',
+               'iban',
+               'fiscal_number',
+               'transaction_type',
+               'form_name',
+               'ffname',
+               'recurring',
+               'recurring_paypal',
+               'redirect',
+               'user_ip',
+               'server_ip',
+       );
+
+       /**
         * DonationData constructor
         * @param GatewayAdapter $gateway
         * @param mixed $data An optional array of donation data that will, if
@@ -54,85 +131,7 @@
                        //I don't care if you're a test or not. At all.
                        $this->normalized = $external_data;
                } else {
-
-                       /**
-                        * $varNames now just contains all the vars we want to
-                        * get poked through to gateways in some form or other,
-                        * from a get or post. We handle the actual
-                        * normalization in normalize() helpers, below.
-                        * @TODO: It would be really neat if the gateways kept
-                        * track of all the things that ***only they will ever
-                        * need***, and could interject those needs here...
-                        * Then we could really clean up.
-                        * @TODO also: Think about putting log alarms on the
-                        * keys we want to see disappear forever, complete with
-                        * ffname and referrer for easy total destruction.
-                        */
-                       $varNames = array (
-                               'amount',
-                               'amountGiven',
-                               'amountOther',
-                               'appeal',
-                               'email',
-                               'emailAdd',
-                               'fname',
-                               'lname',
-                               'street',
-                               'street_supplemental',
-                               'city',
-                               'state',
-                               'zip',
-                               'country',
-                               'card_num',
-                               'card_type',
-                               'expiration',
-                               'cvv',
-                               'currency',
-                               'currency_code',
-                               'payment_method',
-                               'payment_submethod',
-                               'issuer_id',
-                               'order_id',
-                               'subscr_id',
-                               'referrer',
-                               'utm_source',
-                               'utm_source_id',
-                               'utm_medium',
-                               'utm_campaign',
-                               'utm_key',
-                               'language',
-                               'uselang',
-                               'token',
-                               'contribution_tracking_id',
-                               'data_hash',
-                               'action',
-                               'gateway',
-                               'owa_session',
-                               'owa_ref',
-                               'descriptor',
-                               'account_name',
-                               'account_number',
-                               'authorization_id',
-                               'bank_check_digit',
-                               'bank_name',
-                               'bank_code',
-                               'branch_code',
-                               'country_code_bank',
-                               'date_collect',
-                               'direct_debit_text',
-                               'iban',
-                               'fiscal_number',
-                               'transaction_type',
-                               'form_name',
-                               'ffname',
-                               'recurring',
-                               'recurring_paypal',
-                               'redirect',
-                               'user_ip',
-                               'server_ip',
-                       );
-
-                       foreach ( $varNames as $var ) {
+                       foreach ( self::$fieldNames as $var ) {
                                $this->normalized[$var] = $this->sourceHarvest( 
$var );
                        }
 
@@ -140,7 +139,7 @@
                                $this->setVal( 'posted', false );
                        }
                }
-               
+
                //if we have saved any donation data to the session, pull them 
in as well.
                $this->integrateDataFromSession();
 
@@ -151,6 +150,10 @@
                }
        }
 
+       public static function getFieldNames() {
+               return self::$fieldNames;
+       }
+
        /**
         * Harvest a varname from its source - post, get, maybe even session 
eventually.
         * @TODO: Provide a way that gateways can override default behavior 
here for individual keys.
diff --git a/gateway_forms/Mustache.php b/gateway_forms/Mustache.php
index d3855e8..de81aca 100644
--- a/gateway_forms/Mustache.php
+++ b/gateway_forms/Mustache.php
@@ -166,10 +166,19 @@
                }
        }
 
+       /**
+        * Get errors, sorted into two buckets - 'general' errors to display at
+        * the top of the form, and field errors to display inline.
+        * Also get some error-related flags.
+        * @return array with all general errors under the 'general' key, and
+        *               field errors each keyed with their own field.
+        */
        protected function getErrors() {
                $errors = $this->gateway->getAllErrors();
-               $return = array();
-               $return['errors'] = array();
+               $return = array( 'errors' => array(
+                       'general' => array(),
+               ) );
+               $fieldNames = DonationData::getFieldNames();
                foreach( $errors as $key => $error ) {
                        if ( is_array( $error ) ) {
                                // TODO: set errors consistently
@@ -177,10 +186,15 @@
                        } else {
                                $message = $error;
                        }
-                       $return['errors'][] = array(
+                       $errorContext = array(
                                'key' => $key,
                                'message' => $message,
                        );
+                       if ( in_array( $key, $fieldNames ) ) {
+                               $return['errors'][$key] = $errorContext;
+                       } else {
+                               $return['errors']['general'][] = $errorContext;
+                       }
                        $return["{$key}_error"] = true;
                        if ( $key === 'currency_code' || $key === 'amount' ) {
                                $return['show_amount_input'] = true;
diff --git a/gateway_forms/mustache/error_message.html.mustache 
b/gateway_forms/mustache/error_message.html.mustache
new file mode 100644
index 0000000..2ce91d8
--- /dev/null
+++ b/gateway_forms/mustache/error_message.html.mustache
@@ -0,0 +1 @@
+<span id="{{ key }}Msg" class="errorMsg" >{{{ message }}}</span>
diff --git a/gateway_forms/mustache/index.html.mustache 
b/gateway_forms/mustache/index.html.mustache
index 309ca82..3ff715d 100644
--- a/gateway_forms/mustache/index.html.mustache
+++ b/gateway_forms/mustache/index.html.mustache
@@ -8,7 +8,7 @@
                <td id="donate" valign="top">
                        <div id="greenBackground" class="">
                                {{> no_script }}
-                               <div id="topError" 
class="creditcard-error-msg">{{# errors }}<p class="error">{{{ message 
}}}</p>{{/ errors }}</div>
+                               <div id="topError" 
class="creditcard-error-msg">{{# errors/general }}<p class="error">{{{ message 
}}}</p>{{/ errors/general }}</div>
 
                        <form name="payment" id="payment-form" method="post" 
action="{{ action }}">
                                <div id="payment_gateway-personal-info">
diff --git a/gateway_forms/mustache/payment_amount.html.mustache 
b/gateway_forms/mustache/payment_amount.html.mustache
index de36ada..e1db96c 100644
--- a/gateway_forms/mustache/payment_amount.html.mustache
+++ b/gateway_forms/mustache/payment_amount.html.mustache
@@ -1,7 +1,7 @@
 <h3 class="amount_header">{{# recurring }}{{ l10n 
"donate_interface-monthlybox-amount" }}{{/ recurring }}{{^ recurring }}{{ l10n 
"donate_interface-amount-legend" }}{{/ recurring }}:
        <span id="amount_input" {{^ show_amount_input }}class="hidden"{{/ 
show_amount_input }} >
-               <input type="number" step="any" name="amount" id="amount" 
value="{{ amount }}" />
-               <select name="currency_code" id="currency_code" {{^ 
show_currency_selector }}class="hidden"{{/ show_currency_selector }} >
+               <input type="number" step="any" name="amount" id="amount" 
title="{{ donate_interface-amount }}" value="{{ amount }}" />
+               <select name="currency_code" id="currency_code" title="{{ 
donate_interface-currency }}" {{^ show_currency_selector }}class="hidden"{{/ 
show_currency_selector }} >
                {{# currencies }}
                        <option value="{{ code }}" {{# selected }}selected{{/ 
selected }} >{{ code }}</option>
                {{/ currencies }}
@@ -14,3 +14,4 @@
                {{ amount }} {{ currency_code }}
        </span>
 </h3>
+{{# errors/amount }}{{> error_message }}{{/ errors/amount }}{{^ errors/amount 
}}<span id="amountMsg" class="errorMsgHide"></span>{{/ errors/amount }}
diff --git a/gateway_forms/mustache/personal_info.html.mustache 
b/gateway_forms/mustache/personal_info.html.mustache
index bde7d5d..3ce9a89 100644
--- a/gateway_forms/mustache/personal_info.html.mustache
+++ b/gateway_forms/mustache/personal_info.html.mustache
@@ -4,6 +4,16 @@
                                                                <input 
class="halfwidth" name="fname" value="{{ fname }}" type="text" placeholder="{{ 
l10n "donate_interface-donor-fname" }}" id="fname" required><input 
class="halfwidth" name="lname" value="{{ lname }}" type="text" placeholder="{{ 
l10n "donate_interface-donor-lname" }}" id="lname" required>
                                                        </td>
                                                </tr>
+                        <tr>
+                            <td>
+                                {{# errors/fname }}{{> error_message }}{{/ 
errors/fname }}{{^ errors/fname }}<span id="fnameMsg" 
class="errorMsgHide"></span>{{/ errors/fname }}
+                            </td>
+                        </tr>
+                        <tr>
+                            <td>
+                                {{# errors/lname }}{{> error_message }}{{/ 
errors/lname }}{{^ errors/lname }}<span id="lnameMsg" 
class="errorMsgHide"></span>{{/ errors/lname }}
+                            </td>
+                        </tr>
 {{/ fname_required }}
 {{# address_required }}
                                                <tr>
@@ -28,6 +38,28 @@
 {{/ postal_code_required }}
                                                        </td>
                                                </tr>
+
+    {{# city_required }}
+                        <tr>
+                            <td>
+                                {{# errors/city }}{{> error_message }}{{/ 
errors/city }}{{^ errors/city }}<span id="cityMsg" 
class="errorMsgHide"></span>{{/ errors/city }}
+                            </td>
+                        </tr>
+    {{/ city_required }}
+    {{# state_required }}
+                        <tr>
+                            <td>
+                                {{# errors/state }}{{> error_message }}{{/ 
errors/state }}{{^ errors/state }}<span id="stateMsg" 
class="errorMsgHide"></span>{{/ errors/state }}
+                            </td>
+                        </tr>
+    {{/ state_required }}
+    {{# postal_code_required }}
+                        <tr>
+                            <td>
+                                {{# errors/postal_code }}{{> error_message 
}}{{/ errors/postal_code }}{{^ errors/postal_code }}<span id="postal_codeMsg" 
class="errorMsgHide"></span>{{/ errors/postal_code }}
+                            </td>
+                        </tr>
+    {{/ postal_code_required }}
 {{/ address_required }}
 {{# fiscal_number_required }}
                                                <tr>
@@ -35,6 +67,11 @@
                                                                <input 
class="fullwidth" name="fiscal_number" value="{{ fiscal_number }}" type="text" 
title="{{ l10n "donate_interface-donor-fiscal_number" }}" placeholder="{{ l10n 
"donate_interface-donor-fiscal_number" }}" id="fiscal_number" required >
                                                        </td>
                                                </tr>
+                        <tr>
+                            <td>
+                                {{# errors/fiscal_number }}{{> error_message 
}}{{/ errors/fiscal_number }}{{^ errors/fiscal_number }}<span 
id="fiscal_numberMsg" class="errorMsgHide"></span>{{/ errors/fiscal_number }}
+                            </td>
+                        </tr>
 {{/ fiscal_number_required }}
 {{# email_required }}
                                                <tr>
@@ -42,4 +79,9 @@
                                                                <input 
class="fullwidth" name="email" value="{{ email }}" type="email" title="{{ l10n 
"donate_interface-donor-email" }}" placeholder="{{ l10n 
"donate_interface-donor-email" }}" id="email" required>
                                                        </td>
                                                </tr>
+                        <tr>
+                            <td>
+                                {{# errors/email }}{{> error_message }}{{/ 
errors/email }}{{^ errors/email }}<span id="emailMsg" 
class="errorMsgHide"></span>{{/ errors/email }}
+                            </td>
+                        </tr>
 {{/ email_required }}
diff --git a/globalcollect_gateway/forms/css/gc.css 
b/globalcollect_gateway/forms/css/gc.css
index 6be8c97..1cfbbf4 100644
--- a/globalcollect_gateway/forms/css/gc.css
+++ b/globalcollect_gateway/forms/css/gc.css
@@ -276,12 +276,3 @@
   -webkit-box-shadow: inset 0 1px 4px rgba(0,0,1,.5);
   box-shadow: inset 0 1px 4px rgba(0,0,1,.5);
 }
-.errorHighlight {
-  border: 2px solid red !important;
-}
-.errorMsg {
-  color: red;
-  display: inherit;
-}
-.errorMsgHide {
-  display: none;
diff --git a/modules/css/gateway.css b/modules/css/gateway.css
index b05a7a7..9a53336 100644
--- a/modules/css/gateway.css
+++ b/modules/css/gateway.css
@@ -219,6 +219,19 @@
  border: none;
 }
 
+.errorHighlight {
+    border: 2px solid red !important;
+}
+
+.errorMsg {
+    color: red;
+    display: inherit;
+}
+
+.errorMsgHide {
+    display: none;
+}
+
 /* Respond to small viewport */
 @media screen and (max-width: 981px) {
     /* Overall layout */
diff --git a/modules/validate_input.js b/modules/validate_input.js
index a106614..990c922 100644
--- a/modules/validate_input.js
+++ b/modules/validate_input.js
@@ -45,7 +45,10 @@
                amount = $( 'input[name="amount"]' ).val(), // get the amount
                currency_code = '',
                rate,
-               minUsd = mw.config.get( 'wgDonationInterfacePriceFloor' );
+               minUsd = mw.config.get( 'wgDonationInterfacePriceFloor' ),
+               minDisplay,
+               message = mediaWiki.msg( 'donate_interface-smallamount-error' ),
+               $amountMsg = $( '#amountMsg' );
 
        // Normalize weird amount formats.
        // Don't mess with these unless you know what you're doing.
@@ -73,8 +76,19 @@
        } else {
                rate = wgCurrencyMinimums[ currency_code ];
        }
+       // if we're on a new form, clear existing amount error
+       $amountMsg.removeClass( 'errorMsg' ).addClass( 'errorMsgHide' ).text( 
'' );
        if ( ( amount < minUsd * rate ) || error ) {
-               alert( mediaWiki.msg( 'donate_interface-smallamount-error' 
).replace( '$1', minUsd * rate + ' ' + currency_code ) );
+               // Round to two decimal places (TODO: no decimals for some 
currencies)
+               minDisplay = Math.round( minUsd * rate * 100 ) / 100;
+               message = message.replace( '$1', minDisplay + ' ' + 
currency_code );
+               if ( $amountMsg.length > 0 ) {
+                       // newness
+                       $amountMsg.removeClass( 'errorMsgHide' ).addClass( 
'errorMsg' ).text( message );
+               } else {
+                       // ugliness
+                       alert( message );
+               }
                error = true;
                // See if we're on a webitects accordian form
                if ( $( '#step1wrapper' ).length ) {
@@ -102,7 +116,7 @@
                errorsPresent = false,
                currField = '',
                i,
-               fields = [ 'fname', 'lname', 'street', 'city', 'zip', 'email' ],
+               fields = [ 'fname', 'lname', 'street', 'city', 'zip', 'email', 
'fiscal_number' ],
                errorTemplate = mediaWiki.msg( 'donate_interface-error-msg' ),
                numFields = fields.length,
                invalids = [ '..', '/', '\\', ',', '<', '>' ];
diff --git a/worldpay_gateway/forms/js/esop.js 
b/worldpay_gateway/forms/js/esop.js
index f6b7a90..caa77ef 100644
--- a/worldpay_gateway/forms/js/esop.js
+++ b/worldpay_gateway/forms/js/esop.js
@@ -9,14 +9,14 @@
                if ( !$( this ).hasClass( 'enabled' ) ) {
                        return false;
                }
-               if ( window.validateAmount() && window.validate_form( form ) ) {
+               if ( window.validateAmount() && window.validate_personal( form 
) ) {
                        submitForm();
                }
        } );
 
        // Submit on submethod selection if valid, otherwise show continute 
button.
        $( 'input[name="payment_submethod"]' ).on( 'change', function () {
-               if ( window.validateAmount() && window.validate_form( form ) ) {
+               if ( window.validateAmount() && window.validate_personal( form 
) ) {
                        submitForm();
                } else {
                        $( '#paymentContinue' ).show();

-- 
To view, visit https://gerrit.wikimedia.org/r/284835
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I09aece9fa6bb992af42bea739c4b7eb642b2eab6
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com>
Gerrit-Reviewer: Awight <awi...@wikimedia.org>
Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org>
Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org>
Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org>
Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to