Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better JS wrapper #17

Open
judgej opened this issue Sep 17, 2015 · 14 comments
Open

Better JS wrapper #17

judgej opened this issue Sep 17, 2015 · 14 comments

Comments

@judgej
Copy link
Member

judgej commented Sep 17, 2015

Create a wrapper script for sagepay.js to cater for a few use-cases a bit more elegantly. It should not make any assumptions about:

  • Selectors for the form.
  • HTML layout of the form.
  • Method of displaying messages connected to form items.
  • Other items (not related to card details) that may be in the form.

It should be able to:

  • Handle form item errors.
  • Handle non-form item errors.
  • Handle an expired session token.
  • Handle form submission on successfully obtaining a form token.

This would be ideally implemented using callbacks, so the application can decide what to do about each of these circumstances.

It can be based on jQuery, since that is already a dependency of sagepay.js (in the beta v1 release at least, maybe not long-term).

@judgej
Copy link
Member Author

judgej commented Sep 17, 2015

The v1 beta demo script, on successfully obtaining a card token, creates a new form containing just that token and submits it. That's a little bizarre IMO. Instead, it should create or update the card token field in the same form as the card details and submit that, since that form is likely to contain all the other user address details too. However, that's all an implementation detail - the wrapper script should not care how it's handled, but getting a good use-case demo together is important.

@judgej
Copy link
Member Author

judgej commented Sep 20, 2015

The sagepay.js script has been updated a number of times during the course of a single version and release date of the API.

It contains some wrapper/helper functions that are very much dependent on bootstrap being used and the form in a particular HTML structure. I expect that will go eventually. Here is the sagepay.js script today. Understanding it will help to see where this API is heading:

https://gist.github.com/judgej/5a75e540a477a0fe0db0

(code moved to a gist, being quite long and all that)

Functions to note towards the bottom are sagepayResponseHandler and generatePaymentForm. The 3000-odd lines look like they may actually have jQuery embedded within it, but I'm not clear on that. It certainly doesn't need more than a few hundred lines of code to do what it does, so this is all I can conclude.

The Sagepay function starts at line 2994. To note here, is that it looks at the amount, the currency, as well as the documented four main card details (and naturally, the session key). This may be an attempt to put 3DSecure at the front end - we'll see how it pans out.

@judgej
Copy link
Member Author

judgej commented Sep 28, 2015

The gist has been updated to version 2015-09-16:

https://gist.github.com/judgej/5a75e540a477a0fe0db0

It looks like some of the 3DSecure code has been taken out. I understand 3DSecure will be available in the next release of this beta, so that should be a good step forward.

@judgej
Copy link
Member Author

judgej commented Nov 13, 2015

The JS library, along with form rendering, should be done in another package. We'll keep this package for messages only.

@judgej
Copy link
Member Author

judgej commented Nov 24, 2015

I've updated the gist with the 2015-11-10 version of the Sage Pay JS script. It still contains a full version of jQuery, makes many assumptions about the use of bootstrap, and does not contain an awful lot otherewise. I am very inclined to just write an entire replacement for the official script, without jQuery built in (but keeping jQuery as a dependency, for browser compatibility). There is no reason why the supplied JS needs to be used - the REST API is clear and will talk to any script that wants to. There may be some "same origin" issues to deal with, but that's to find later.

@judgej
Copy link
Member Author

judgej commented Dec 4, 2015

Of the 3000+ lines of code in sagepay.js just these 40 lines are all that is needed to grab a card token. I'm just going to write my own library, starting with this core + jQuery, rather than try to incorporate the official Sage Pay script.

var $acadSagepayJQuery = jQuery.noConflict(),
    cardIdentifierEndpoint = "https://test.sagepay.com/api/v1/card-identifiers",
    Sagepay = {
        tokeniseCardDetails: function(form, responseHandler) {
            //console.log("enter tokeniseCardDetails");
            var c = $acadSagepayJQuery('input[data-sagepay="merchantSessionKey"]', form).val(),
                d = $acadSagepayJQuery('input[data-sagepay="cardholderName"]', form).val(),
                e = $acadSagepayJQuery('input[data-sagepay="cardNumber"]', form).val(),
                f = $acadSagepayJQuery('input[data-sagepay="expiryDate"]', form).val(),
                g = $acadSagepayJQuery('input[data-sagepay="securityCode"]', form).val(),
                h = $acadSagepayJQuery('input[data-sagepay="amount"]', form).val(),
                i = $acadSagepayJQuery('input[data-sagepay="currency"]', form).val(),
                j = {
                    amount: h,
                    currency: i,
                    cardDetails: {
                        cardholderName: d,
                        cardNumber: e,
                        expiryDate: f,
                        securityCode: g
                    }
                };
            $acadSagepayJQuery.ajax({
                type: "POST",
                url: cardIdentifierEndpoint,
                contentType: "application/json",
                dataType: "json",
                data: JSON.stringify(j),
                headers: {
                    Authorization: "Bearer " + c
                },
                async: false
            }).done(function(form, c, d) {
                responseHandler(d.status, form)
            }).fail(function(form, c, d) {
                responseHandler(form.status, form)
            }).always(function() {})
        }
    };
"undefined" != typeof exports && (module.exports = Sagepay);

@judgej
Copy link
Member Author

judgej commented Apr 28, 2016

Sage Pay have a whole load of new JS options to play with now. Go play :-)

@judgej
Copy link
Member Author

judgej commented Jun 5, 2016

This front-end stuff has all moved on lots now. I'll close this issue, and more specific issues can track looking into these new JS scripts.

@judgej judgej closed this as completed Jun 5, 2016
@judgej
Copy link
Member Author

judgej commented Sep 6, 2016

Here's an unminified sagepay-dropin.js (2016-08-31):

https://gist.github.com/judgej/69d23448fc681fe8eb5481dadc99304c

@judgej
Copy link
Member Author

judgej commented Feb 20, 2017

I'm going to reopen this. I have seen a much better implementation on Authorize.Net's Accept.JS, at least for self-hosted CC forms. The key is not to use preventDefault at all - it is far too much of a blunt tool that strips out any other processing that may have been added to the form. We can do better than this, and not have to embed jQuery/bootstrap in the core scripts either (those are a site decision IMO).

@judgej judgej reopened this Feb 20, 2017
@judgej
Copy link
Member Author

judgej commented Aug 9, 2017

Trying to understand how to work around the preventDefault approach this gateway has taken, without having to rewrite all the JS:

https://stackoverflow.com/questions/45536760/doing-ajax-cc-tokenisation-action-in-form-submission-event-and-waiting-for-resul

@judgej
Copy link
Member Author

judgej commented Aug 9, 2017

Some info here:

https://www.reddit.com/r/javascript/comments/2zw0gc/can_we_do_addeventlistener_with_return_false/

  • Returning false from an event listener to prevent the default is deprecated.
  • Use evt.preventDefault() instead of return false.
  • Within an addEventListener handler return false will do absolutely nothing.
  • preventDefault, stopPropagation, and stopImmediatePropagation are explicit means with which to handle these decisions.

@judgej
Copy link
Member Author

judgej commented Aug 11, 2017

PHP Guzzle promises have a wait() method. Once a promise is created, the code can synchronously wait for that promise to be resolved. It appears that JavaScript (ECMAscript) has nothing like this. All promises are asynchronous, always, and there is no way to wait for one to finish. All you can do is fire off a callback when the promise completes.

With ES7 you can chain them together using then(), but even those operate on callbacks - the "then" construct only serves to avoid the "callback hell" nesting in the code, so it looks like synchronous code, but it's not, it's just promises changed together with callbacks internally; just a nicer syntax.

So, what I wanted to do, was allow each submit event on the form to run to completion, any one of which could find an error and issue a preventDefault(). But if none find any errors, then the form should submit. It is important that all the events must run to completion before the form submits. We don't know how many events there are, and how many promises they set in motion; we just want to know they have all finished. I really cannot see a way to do this. All the SO questions state that you CANNOT under any circumstances wait for a promise. You also SHOULD NOT run AJAX synchronously. That leaves a massive hole - how on earth are we supposed to validate and tokenise the card and address details in one form if we cannot link the form submission to the completing of all the events with their callbacks?

Possibly if ALL the events use promises to do their longer processing, and those promises are dumped to a stack, then the last event to run could wait for all those promises to complete (Promise.all([], ...) and check for any that have flagged up errors, then force a form submit if all have completed without error. That would require that ALL the events use the same stack, which I guess is not too unreasonable.

@judgej
Copy link
Member Author

judgej commented Aug 13, 2017

An example of form validation in action:

https://jsfiddle.net/bzy991yz/2/

An overview of the event loop, which explains what "single threaded" really means in the context of JavaScript:

https://developer.mozilla.org/en/docs/Web/JavaScript/EventLoop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant