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 StoreKit flow. #423

Closed
hyochan opened this issue Mar 14, 2019 · 9 comments · Fixed by #510
Closed

Better StoreKit flow. #423

hyochan opened this issue Mar 14, 2019 · 9 comments · Fixed by #510
Labels
🍗 enhancement New feature or request 🥺 feature request Request a new feature 👣 waiting for response Need feedback to continue

Comments

@hyochan
Copy link
Member

hyochan commented Mar 14, 2019

Moving discussion from #307 (comment).

We are willing to handle better store kit flow.

@hyochan
Copy link
Member Author

hyochan commented Mar 14, 2019

@Edgpaez I think I can just remove RNIap.addAdditionalSuccessPurchaseListenerIOS which comes after failure just by encapsulating. Followed by PR I've made in flutter_inapp_purchase.

Would this be enough?

@hyochan hyochan added the 👣 waiting for response Need feedback to continue label Mar 14, 2019
@hyochan
Copy link
Member Author

hyochan commented Mar 15, 2019

@Edgpaez Actually you were right. Today, I just reviewed flutter_inapp_purchase and tried to integrate to react-native-iap, and realized that I was using the stream in dart so I didn't have to expose addAdditionalSuccessPurchaseIOS listener to production code.
Therefore, your idea of using RxJS would be really great to have it here.

@cayasso
Copy link

cayasso commented Mar 19, 2019

Hi guys, thanks for this great discussion? any update on this, we are experiencing the same issue mentioned in #307 pretty bad user experience, I would contribute but have no experience in ios native language whatsoever :(

@hyochan
Copy link
Member Author

hyochan commented Mar 20, 2019

@cayasso Currently, this discussion is about javascript side and not related to native part. Originally @Edgpaez issued that we can encapsulate addAdditionalSuccessPurchaseListenerIOS in index.js and not expose to actual usage so the user can end up with buyProduct method without calling extra listener. This could be done using rxjs. If there is no contribution to this for a while, I'll have to think on different structures.

@Edgpaez
Copy link
Contributor

Edgpaez commented Mar 20, 2019

Hi @hyochan, I'll be happy to take a look. I think your proposal:

RNIap.buyProduct('com.example.coins100');
RNIap.buyProduct('com.example.coins200');

// receiving events
const subs =  RNIap.purchaseStatusListener(purchase => {
  ...
});

might be easier to reason about without prior experience in Rx. So I'll try to implement it. AFAICT, we only need JS changes since purchase has a productId property that would let users figure out which product was "purchased".

@cayasso: you can use the flow described here. No need to wait for the change in the API

@chr4ss1
Copy link

chr4ss1 commented May 10, 2019

I see we are trying to move away from finishTransaction and clearTransaction, which is fine, we could simplify that logic, but we need to be more strict when we finish transactions. In my opinion, we can only finish a successful purchase transaction when client side (JS) has acknownledged that it received & processed it.

This flow is incorrect:

  try {
    // Will return a purchase object with a receipt which can be used to validate on your server.
    const purchase = await RNIap.buyProduct('com.example.coins100');
    this.setState({
      receipt: purchase.transactionReceipt, // save the receipt if you need it, whether locally, or to your server.
    });
  } catch(err) {
    // standardized err.code and err.message available
    console.warn(err.code, err.message);
    const subscription = RNIap.addAdditionalSuccessPurchaseListenerIOS(async (purchase) => {
      this.setState({ receipt: purchase.transactionReceipt }, () => this.goToNext());
      subscription.remove();
    });
  }

There could be multiple scenarios where this code can miss successful purchases and not credit user account. Having addAdditionalSuccessPurchaseListenerIOS there for sure minimizes that chance, but you still need to "re-play" the transaction queue when a new listener is added.

The correct flow would be:

  1. Register event handlers ONCE when the app bootstraps, and don't unsubscribe.
  2. All the application logic will be in these handlers.
  3. These handlers will decide if we should de-queue/finish the transactions or not.

Of course you don't need to leak StoreKit concepts in to this, you can always simplify it further, but the point is, a client is going to be the one who will decide if we de-queue the transactions or not, otherwise there will be scenarios where we will miss the successful purchase transactions.

For example, this could be a idea:

  1. Once app bootstraps:
RNIap.addAdditionalSuccessPurchaseListenerIOS(purchase => {
    _purchaseService.addSuccessfulPurchase(purchase); // this purchase will be delivered to back end at some point, it's cached locally at first

   // because we returned true, it means
   // that we have processed it successfully,
   // thus transaction QUEUE can be emptied
   // note that we should empty ONLY the transactions in the queue that have the same 
originalTransactionId.
    return true; 
}

Note; all the transaction queue has to be "re-played" against RNIap.addAdditionalSuccessPurchaseListenerIOS when someone actually subscribes to it, so he wouldn't miss old successful transactions.

  1. If you actually want to buy new product:
try {
     if(!_purchaseService.hasPendingSuccessfulPurchases()) {
        await RNIap buyProductWithoutAutoConfirm(productId)
    }
catch (e) {
   // Do not show any dialogs here
   // The only place you can show successful dialog is when _purchaseService.addSuccessfulPurchase is successful.
   // You have no idea if purchase process fails or not.
}

So essentially these are the changes I propose (I Have no Swift experience, but can stitch something together if it makes sense):

-(void)purchaseProcess:(SKPaymentTransaction *)transaction {
  if (autoReceiptConform) {
    [[SKPaymentQueue defaultQueue] finishTransaction:transaction];
    currentTransaction = nil;
  } else {
    currentTransaction = transaction;
  }
  NSURL *receiptUrl = [[NSBundle mainBundle] appStoreReceiptURL];
  NSDictionary* purchase = [self getPurchaseData:transaction];
  [self resolvePromisesForKey:RCTKeyForInstance(transaction.payment.productIdentifier) value:purchase];

  // additionally send event & finish transaction when listener processes it
  if (hasListeners) {
    const status = [self sendEventWithName:@"iap-purchase-event" body: purchase];
    if (status) {
          [[SKPaymentQueue defaultQueue] finishTransaction:transaction];
    }
  }
}

and

- (void)addListener:(NSString *)eventName {
  [super addListener:eventName];

  SKPayment *promotedPayment = [IAPPromotionObserver sharedObserver].payment;
  if ([eventName isEqualToString:@"iap-promoted-product"] && promotedPayment != nil) {
    [self sendEventWithName:@"iap-promoted-product" body:promotedPayment.productIdentifier];
  }

   if ([eventName isEqualToString:@"iap-purchase-event"]) {
       NSArray *pendingTrans = [[SKPaymentQueue defaultQueue] transactions];
       for (int k = 0; k < pendingTrans.count; k++) {
          // Check if trans is actually purchase event one before sending it over
          if (pendingTrans[k] is purchase event) {
            const status = [self sendEventWithName:@"iap-purchase-event" pendingTrans[k]:
            if (status) {
               [[SKPaymentQueue defaultQueue] finishTransaction:pendingTrans[k]];
            }
          }
       }   
  }
}

The transaction queue should be accessible for all the consumers, especially since it is possible to for example, downgrade or upgrade the subscription plan. (In which case I believe transaction queue will have more events in it?)

@vafada
Copy link

vafada commented May 11, 2019

The suggestion in the README:

  try {
    // Will return a purchase object with a receipt which can be used to validate on your server.
    const purchase = await RNIap.buyProduct('com.example.coins100');
    this.setState({
      receipt: purchase.transactionReceipt, // save the receipt if you need it, whether locally, or to your server.
    });
  } catch(err) {
    // standardized err.code and err.message available
    console.warn(err.code, err.message);
    const subscription = RNIap.addAdditionalSuccessPurchaseListenerIOS(async (purchase) => {
      this.setState({ receipt: purchase.transactionReceipt }, () => this.goToNext());
      subscription.remove();
    });
  }

doesn't look right. When the user cancels the purchase (user decided not to buy), StoreKit throws a paymentCancelled error

In this scenario, the flow will enter the catch block but the callback will never get executed.

Here are more details about this StoreKit flow: chirag04/react-native-in-app-utils#202 (comment)

Maybe we can get an idea how to solve this issue from @superandrew213 fork:

https://github.com/superandrew213/react-native-in-app-utils/tree/listen-for-purchase-event#helpers

@hyochan
Copy link
Member Author

hyochan commented May 12, 2019

@vafada Wow that is a good point we should handle this. Actually, I'm really thinking of redesigning react-native-iap and it would take around 1 week of my working day. However, I'm currently so busy on my main job (Really sorry for this). I'd love to handle this together before July. Hope you could wait.

@Edgpaez Edgpaez mentioned this issue May 13, 2019
@Edgpaez
Copy link
Contributor

Edgpaez commented May 13, 2019

Based on @ChrisEelmaa's comment, I realized the addAdditionalSuccessPurchaseListenerIOS doesn't work unless you register the callback before buyProduct. I tried to address it at #486

I'm not sure I follow @vafada's comment. Is it a problem that can be prevented by clients of this library? if so, maybe we can write JS code that handles this completely and integrate it with the current native implementation?

@hyochan hyochan mentioned this issue Jun 9, 2019
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍗 enhancement New feature or request 🥺 feature request Request a new feature 👣 waiting for response Need feedback to continue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants