Saferpay JSON API support (PSD 2 compliance)

No, this is not the same module. But we are just about to start implementing the new api and the plan is to make it in time before 14 September. One thing that needs to be pointed out though is that the new module version will only support payment page (and not business).

The authentication and other configurations required for the json api are not compatible with the existing one (json api basic auth for example). This means that if I understand that correctly we cannot have a drop-in replacement.

So the plan is:

• Create a new 7.x-2.x branch
• Create a new payment method inside of the PP submodule

First iteration. I added a new module, although there already are 3. We discussed with @Berdir how to approach this and he suggested it might make sense to have this in the main module. In light of EOL of the old api, we might delete the submodules anyway.

Moved to main module.

1. +++ b/commerce_saferpay.json_pp.inc
   @@ -0,0 +1,324 @@
   +/**
   + * @file
   + * Process payments using Saferpay Payment Page usin JSON api.
   

   usin => using, although we have two using's then, maybe "through the JSON API"?

   We should also prepare a snippet that we can put into the release notes that explains that the old modules are now basically deprecated and no longer supported. And that everyone should update to the new integration, with short instructions on how to do that.

2. +++ b/commerce_saferpay.json_pp.inc
   @@ -0,0 +1,324 @@
   +  $form['payment_methods']['#access'] = FALSE;
   

   we need support for this, we have projects that specifcally rely on it. We added that feature for those :)

3. +++ b/commerce_saferpay.json_pp.inc
   @@ -0,0 +1,324 @@
   +function commerce_saferpay_json_pp_redirect_form_validate($order, $payment_method) {
   +  // @todo Add debug option and log it here.
   +  if (!empty($payment_method['settings']['debug'])) {
   +    watchdog('commerce_paypal_wps', 'Customer returned from Saferpay.', WATCHDOG_DEBUG);
   +  }
   

   looks like we have a debug option and are logging, just nothing useful yet? :) also wrong type.

   and, don't we need to assert that this is valid here? otherwise you could just visit the redirect page directly and we'd think it has been paid?

4. +++ b/commerce_saferpay.json_pp.inc
   @@ -0,0 +1,324 @@
   + */
   +function commerce_saferpay_json_pp_redirect_form_submit($order, $payment_method) {
   +  // @todo Don't think we can do anything here.
   +  return;
   

   does that means we only process the payment async? This might be a problem with one of our sites that we'll need to test and deal with.

5. +++ b/commerce_saferpay.json_pp.inc
   @@ -0,0 +1,324 @@
   +  if (!empty($response->data)) {
   +    $return_data = json_decode($response->data);
   +  }
   +
   +  if (isset($response->error)) {
   +    $log[] = "Error: {$response->error}.";
   +
   +    if (isset($return_data)) {
   +      // @see https://saferpay.github.io/jsonapi/#errorhandling
   +      $log[] = "Error name: {$return_data->ErrorName}";
   +      $log[] = "Behavior: {$return_data->Behavior}";
   +      $log[] = "Error message: {$return_data->ErrorMessage}";
   +    }
   +
   +    watchdog('commerce_saferpay_pp', implode(' / ', $log));
   

   maybe an existing pattern, but this is problematic as watchdog() expects a string that is passed to t() and shouldn't have dynamic values.

   you can set arguments to NULL to disable translation, but I think we could easily use placeholders here? We could likely even drop the imploding then and use short placeholders on a single line?

6. +++ b/commerce_saferpay.json_pp_pages.inc
   @@ -0,0 +1,105 @@
   +      // This was already payed. We can exit.
   

   https://www.grammarly.com/blog/paid-payed/, I'm pretty sure we're not doing anything with ships here :)

7. +++ b/commerce_saferpay.module
   @@ -5,6 +5,9 @@
    
   +
   +module_load_include('inc', 'commerce_saferpay', 'commerce_saferpay.json_pp');
   +
   

   not a fan of this pattern. this add a function call to load that file dynamically during an early bootstrap phase.

   I'd rather add some of the methods that we call ourself to a class with static methods or just put it all in the .module file.. I don't really care about another 300 loc in there. And if we ever do a 7.x-2.x branch without the old methods then it will be easier to clean up, would be weird to have basically nothing but the include then.

Thanks for the review.

1. Removed anyway.

DEPRECATION NOTICE
The old api will not be supported after 14 September 2019. Due to that the old submodules will be deprecated (and deleted) and manual configuration (see below) of the new payment method that uses the new json api is needed. Until EOL of the old api, both the new payment method and the old ones will be available.

CONFIGURATION
1. The new payment method lives in the main module, so no additional module needs to be enabled
2. Add the payment method as usual (admin/commerce/config/payment-methods)
3. Configure the payment method (admin/commerce/config/payment-methods/manage/commerce_payment_commerce_saferpay_json_pp/edit/[ID])

Because the new api uses a different type of authentication. This needs to be configured on the Saferpay backoffice
* You can get the Account ID as per instructions on the field
* The url of the api now must point to the /api folder. Ie: https://test.saferpay.com/api
* The Order identifier now only supports alphanumeric characters.
* The Terminal id can be derived from as described on the field description
* Basic auth username and Basic auth password must be set in the Saferpay backoffice Settings > JSON API basic authentication)

2. Added support for that

3. As discussed, AFAIK we don't get anything from Saferpay at that point

4. Yes

5. Fixed that

6. Fixed that :)

7. Moved to the main file.

Will be afk for a while, unassigning :)

Lots of small improvements, in no specific order:

* Improvements on the settings form, keeping related settings together, using customer id and not account id
* Improved default settings
* Notify callback improvements, year is now 4-digit, so we no longer need the 20 prefix, also card types are completely different now, so new mapping is needed. Also the same callback url was defined commerce_saferpay_pp, so that overlapped and the other one won, resulting in a lot of confusion.
* Error handling improved to have more useful errors

interdiff is large because a lot of code was slightly touched.

This fixes the alias stuff, wasn't actually setting the correct keys before to request it and saving the wrong data. Also passes through the language like before and some other fixes.

Was fighting with race conditions that resulted in losing the information in $order->data, so I changed the API to return arrays which are easier to store in $transaction->payload and instead storing the cardonfile ID in there.

Committed.