Right now the error message returned by accept.js is not being displayed to the user. Only a console log is happening and alert('acceptJS library error!');. Here is a patch which make use of Drupal.theme('commerceAuthorizeNetError', errorMsgs) to display the error.

CommentFileSizeAuthor
#14 Order_information___My_Store.png37.17 KBmglaman
#14 2922360-14.patch6.96 KBmglaman
#12 2922360-12.patch6.75 KBmglaman
#5 authnet-errors.patch3.31 KBbradjones1
#2 display_accept_js_error-2922360-2.patch1.65 KBnikathone

Comments

nikathone created an issue. See original summary.

nikathone’s picture

nikathone
Primary language Swahili
Location Ontario
commented
Status: Active » Needs review
StatusFileSize
new display_accept_js_error-2922360-2.patch1.65 KB

And here is the patch.

nikathone’s picture

nikathone
Primary language Swahili
Location Ontario
commented

To be accurate this patch also will enable the submit button back. Otherwise the user is forced to reload the form in order to submit again in case there is an error.

nikathone’s picture

nikathone
Primary language Swahili
Location Ontario
commented
Parent issue: » #2813401: Use Accept.js to ease PCI requirements.

Adding parent issue.

bradjones1’s picture

bradjones1
Primary language English
commented
StatusFileSize
new authnet-errors.patch3.31 KB

Here's a similar, but different, approach which more closely follows the example in Stripe. Same idea though... perhaps Matt/maintainers have a preference.

nikathone’s picture

nikathone
Primary language Swahili
Location Ontario
commented

@bradjones1 noticed that you did removed the 

+++ b/js/commmerce_authnet.form.js
@@ -95,14 +95,10 @@
-          event.preventDefault();

This will cause the form to submit though and we don't want that cause of the error.

bradjones1’s picture

bradjones1
Primary language English
commented

event isn't in scope, so it was erroring, at least in Firefox. The default submit functionality is short-circuited already by the library.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Assigned: Unassigned » mglaman

Giving this a look over.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

First realization:

Braintree, Square, Authorize.net, and Stripe are having to rethink the way to display errors. We need an error handling library in Commerce itself which renders the display message.

+++ b/js/commmerce_authnet.form.js
@@ -126,6 +122,17 @@
+        $('html, body').animate({ scrollTop: errors.offset().top });

I think this is really important, making sure that the screen scrolls to errors.

I think both solutions are viable for people to use. They're pretty close in implementation. I'd like to talk to bojanz to see if we could make a core fix in Commerce that could be part of 2.2, which is slated to release December 6th.

bradjones1’s picture

bradjones1
Primary language English
commented

Most of this was lifted from Stripe, apparently, so abstracting this to something in payments core would probably make sense.

Glad you agree on the scrolling piece; I wondered if that was too bespoke but there's nothing else to indicate to the user that the submission a) did anything and b) resulted in an error.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Assigned: mglaman » Unassigned

I like #9. I'd like to get feedback from some others. I do not have the time to manually test right now.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
StatusFileSize
new 2922360-12.patch6.75 KB

Neither patch applied nor provided same error response handling for eCheck. Rerolled. Testing

nikathone’s picture

nikathone
Primary language Swahili
Location Ontario
commented
Status: Needs review » Needs work 
+++ b/js/commmerce_authnet.accept.form.js
@@ -52,13 +52,10 @@
+          Drupal.behaviors.commerceAuthorizeNetForm.errorDisplay(response.messages.message[i].text);

+++ b/js/commmerce_authnet.echeck.form.js
@@ -45,13 +45,10 @@
+          Drupal.behaviors.commerceAuthorizeNetForm.errorDisplay(response.messages.message[i].text);

+++ b/js/commmerce_authnet.form.js
@@ -30,6 +30,16 @@
+    errorDisplay: function (code, error_message) {

First argument is suppose to be the error code but we are sending error text message.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new 2922360-14.patch6.96 KB
new Order_information___My_Store.png37.17 KB

Looks good! Thanks for catching that. Addressed.

  • mglaman committed ec051d4 on 8.x-1.x authored by bradjones1 
    Issue #2922360 by mglaman, nikathone, bradjones1: Display accept.js...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Fixed

Fixed! Thank you both.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.