Always returning AuthorizationCodeForNoNetworkEnvironment

[geniuscd]

I'm using react-native-paypal-wrapper: ^1.3.0,react-native: 0.53.0,iOS

I followed the instruction that you specified in the README file

import PayPal from 'react-native-paypal-wrapper';
const options = {
    merchantName : "COMPANY NME",
    merchantPrivacyPolicyUri: "https://link.com/privacy",
    merchantUserAgreementUri: "https://link.com/terms",
}
PayPal.initializeWithOptions(PayPal.SANDBOX, "<client_id here>", options);

Calling the future payment to get the authorization code

PayPal
 .obtainConsent()
 .then(authorization => console.log(authorization))
 .catch(error => console.log(error));

will always return the results of the NO NETWORK environment.

Client:
 environment: "mock"
 paypal_sdk_version : "2.16.1"
 platform : "iOS"
 product_name : "PayPal iOS SDK"
response:
 code : "AuthorizationCodeForNoNetworkEnvironment"
 response_type : "authorization_code"

any thoughts?

[alvinthen]

We currently have no resources to work on this, but we're happy to accept PR or takeovers.

[seanyu4296]

@geniuscd have you figured out why it only calls no network?

[seanyu4296]

I found the issue https://github.com/taessina/react-native-paypal-wrapper/blob/master/android/src/main/java/com/taessina/paypal/RNPaypalWrapperModule.java#L128 the environment being passed here is wrong if you compare it https://github.com/paypal/PayPal-Android-SDK/blob/master/SampleApp/src/main/java/com/paypal/example/paypalandroidsdkexample/SampleActivity.java#L64 it uses PayPalConfiguration. Do you guys still accept PRs @alvinthen ?

[alvinthen]

Yes we're happy to accept PR anytime.

[seanyu4296]

I noticed that the environment actually works fine, but you need to restart the whole application if you plan to "reinitialize" the Paypal service. A javascript reload wouldn't correctly reinitialize the paypal service I think.

but if i add a stop service on PaypalService it will work fine

 @ReactMethod
  public void initializeWithOptions(String environment, String clientId, ReadableMap params) {
    Log.d("ENV", environment);
    Log.d("CLIENTID", clientId);
    this.config = new PayPalConfiguration().environment(environment).clientId(clientId);

    if(params.hasKey("merchantName") && params.hasKey("merchantPrivacyPolicyUri") && params.hasKey("merchantUserAgreementUri")) {
      String merchantName = params.getString("merchantName");
      String merchantPrivacyPolicyUri = params.getString("merchantPrivacyPolicyUri");
      String merchantUserAgreementUri = params.getString("merchantUserAgreementUri");

      config = config.merchantName(merchantName)
        .merchantPrivacyPolicyUri(Uri.parse(merchantPrivacyPolicyUri))
        .merchantUserAgreementUri(Uri.parse(merchantUserAgreementUri));

    }
    reactContext.stopService(new Intent(reactContext, PayPalService.class)); // add this to properly initialize
    Intent intent = new Intent(reactContext, PayPalService.class);
    intent.putExtra(PayPalService.EXTRA_PAYPAL_CONFIGURATION, config);
    reactContext.startService(intent);
  }

Do you think it's safe to add stopServices? I'm not an Android Java dev expert, so will need your opinion @alvinthen

One thing i noticed also is in the examples in the official android sdk of paypal. The PaypalConfig is set to private static not something getting reinitialized everytime

[alvinthen]

I'm afraid I have no more knowledge than you on this. But here's what I found https://gist.github.com/feelinc/e29a11f3b19a6e2c848f9a01826e1a65#file-rnpaypalwrappermodule-java-L128 which uses the stopService in onHostDestroy. I figured it's better to this than stopping the service in initialization. Could you test it out? We're happy to accept PRs.

[seanyu4296]

There is already a stopService in onHostDestroy in the current codebase https://github.com/taessina/react-native-paypal-wrapper/blob/master/android/src/main/java/com/taessina/paypal/RNPaypalWrapperModule.java#L198. The effect of putting stopService in onHostDestroy is needing to kill the app i think to reinitialize correct paypal config.

[seanyu4296]

I guess we have two options:

1. If we really want to expose the initialization of the paypal to be exposed to the JS side, It's possible that no code changes are needed for this. It's either README.md should specify that you can only initialise the PaypalConfig once, and if you need to reinitialize you need to restart the application.
   or
2. We follow the sample https://github.com/paypal/PayPal-Android-SDK/blob/master/SampleApp/src/main/java/com/paypal/example/paypalandroidsdkexample/SampleActivity.java#L64 here in the Paypal-Android-SDK that it should be initialized in the Native side as a private static.

By the way, from my usage, iOS works fine. Changing initialization -> Reloading metro bundler -> results to proper change in initialization

[alvinthen]

1. If we really want to expose the initialization of the paypal to be exposed to the JS side, It's possible that no code changes are needed for this. It's either README.md should specify that you can only initialise the PaypalConfig once, and if you need to reinitialize you need to restart the application.

Another way is to check if the PayPalService is already running (assuming starting the PayPalService more than once will ruin things up). Start the service if it's not. I dug a bit of react-native repo and there are a few issues regarding reloading native modules, but unfortunately are not responded by the devs.

2. We follow the sample https://github.com/paypal/PayPal-Android-SDK/blob/master/SampleApp/src/main/java/com/paypal/example/paypalandroidsdkexample/SampleActivity.java#L64 here in the Paypal-Android-SDK that it should be initialized in the Native side as a private static.

I find this to be unnecessary, as we're exposing the available constants to JS side. As long as we're using those constants when initializing PayPal, it will work the same. Although I may be wrong.

I have no objection to add stopService as you have suggested earlier, my only concern is that it will not break others that are already using this package.

[seanyu4296]

Another way is to check if the PayPalService is already running (assuming starting the PayPalService more than once will ruin things up). Start the service if it's not. I dug a bit of react-native repo and there are a few issues regarding reloading native modules, but unfortunately are not responded by the devs.

Hmmm I think it's okay to startService even if another one is running (see: https://stackoverflow.com/questions/10739413/how-to-prevent-service-to-run-again-if-already-running-android). I think the problem is more around the behavior of iOS and Android is not the same. On iOS, a Javascript Reload correctly reloads Paypal Config. However, on Android, it does not reload because the PayPalService.class is still running.

To make it both behave the same way, I think I can add something like

 @ReactMethod
public void initializeWithOptions(String environment, String clientId, ReadableMap params) {
  // some code ...
  if(isPaypalServiceRunning()) {
    reactContext.stopService(new Intent(reactContext, PayPalService.class)); 
  }
Intent intent = new Intent(reactContext, PayPalService.class);
    intent.putExtra(PayPalService.EXTRA_PAYPAL_CONFIGURATION, config);
    reactContext.startService(intent);
}

but i am quite hesitant on implementing isPaypalServiceRunning since there are a lot of discussions around what's the right way to check if a service is running. (see: https://stackoverflow.com/questions/600207/how-to-check-if-a-service-is-running-on-android?noredirect=1&lq=1) It might not be worth it.

What do you think @alvinthen ? Really appreciate you responding quickly and giving feedback

[alvinthen]

I'd say let's go with your initial proposal, to stop the service every time during the initialization. I'm giving you the access rights as I trusted you on this. I hardly have any time to test this on.

[seanyu4296]

Alright. Thank you!

A few more questions. Do I create a PR or do I merge it straight to master? How do you do releases to npm? @alvinthen

[alvinthen]

You have the permission to put it straight to master :)
I'll do the release. I'm not sure to do it on npm.