mariusbackes / cordova-plugin-sumup

:pager: :moneybag: Cordova plugin for SumUp SDK integration
5 stars 18 forks source link

iOS support #35

Closed petermeester closed 2 years ago

petermeester commented 2 years ago

34 #12 #2

Support for iOS

For iOS it is important to first run the prepare function so that the native SumUP SDK is being initiated as needed. After that, run the login function and prepare function again to initiate a payment using the pay function.

mariusbackes commented 2 years ago

Hi there,

first of all, thank you very much for your request and the added support for iOS.

I reviewed your request and saw a few things, that need to be changed.

  1. Please remove the print statements and return a specific message with a code, if an error has occured.
  2. In the pay method, there is still the following line of code:
let obj = createReturnObject(code: 117, message: "Pay method will be implemented soon");
returnCordovaPluginResult(status: CDVCommandStatus_OK, obj: obj, command: command);
  1. In line 171, the title is hardcoded with test as value. Please get this value from an optional parameter which can be passed from JavaScript. You should also update the sumup.d.ts and add there an optional title parameter. These changes should be implemented for android, as well.

  2. Make currencycode an optional parameter in sumup.d.ts. If the parameter is passed, use this. Otherwise use the currency from currentMerchant, like it's already done. Implement this logic for android as well.

Thank your very much for your help!

petermeester commented 2 years ago

Hi Marius,

Can you please review again? I've made the required changes.

Thank you, Peter

mariusbackes commented 2 years ago

Hello, thanks again for your changes. I've found 2 more little issues to fix:

  1. Please use the constant values instead of returning the code directly

Instead of

let obj = self.createReturnObject(code: 117, message: "General error");

implement it like this

let obj = self.createReturnObject(code: PAYMENT_ERROR, message: "General error");
  1. The title and currency parameters are marked as optional, but the payment is not done if they are not provided on Android:
String title;
try {
    title = new String(args.get(1).toString());
} catch (Exception e) {
    JSONObject obj = createReturnObject(CANT_PARSE_CURRENCY, "Can't parse title");
    returnCordovaPluginResult(PluginResult.Status.ERROR, obj, true);
    return false;
}

SumUpPayment.Currency currency;
try {
    currency = SumUpPayment.Currency.valueOf(args.get(2).toString());
} catch (Exception e) {
    JSONObject obj = createReturnObject(CANT_PARSE_CURRENCY, "Can't parse currency");
    returnCordovaPluginResult(PluginResult.Status.ERROR, obj, true);
    return false;
}

Either we have to think about, to make them mandatory or find a better solution if they are not provided. Moreover, you are returning the CANT_PARSE_CURRENCY error code if the title couldn't be parsed. Please create a seperat error code and add them to the README as well.

The logic for the missing title parameter has to be implemented for iOS as well.

Thank you very much!

petermeester commented 2 years ago

Hi Marius,

I've fixed the issues. Both title and currency parameters are still optional. If no currency is given, it will be fetched from the SumUp Merchant account.

Thank you, Peter