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

method to retrieve pending transactions #663

Merged
merged 6 commits into from
Aug 28, 2019
Merged

Conversation

emilioicai
Copy link
Contributor

We are having several customers complaining about their purchases not to be confirmed even though they are payed. We identify this might happen when something wrong happens between finishing the payment in the appstore and confirming the purchase. That's why we came with this method: it retrieves the pending transactions (those who are payed but not yet finished), so they can be processed and finished once the user receives his products. This can be very useful as a complement to a 'Restore Purchases' button, which can be pressed when the user has issues with his purchases.

@hyochan hyochan added the 🍗 enhancement New feature or request label Aug 24, 2019
@hyochan hyochan requested a review from solidfox August 24, 2019 04:20
@hyochan
Copy link
Owner

hyochan commented Aug 24, 2019

@emilioicai Thanks for coming up with this! @solidfox could you also review this if you are available? I think you can be a good reviewer on the feature.

@hyochan hyochan added the 🎯 feature New feature label Aug 24, 2019
@hyochan
Copy link
Owner

hyochan commented Aug 26, 2019

@solifox I am sorry but I don't quite get what you mean by here. I've just wanted you to respond on this PR if you have and thoughts. If you don't have any opinion I'll merge this!

@solidfox
Copy link
Contributor

Sorry, github didn't make it very clear that I commented on a line in the code. The block gets fed an NSError and so one would expect there to be cases where the promise would get rejected with that potential error. Not sure how relevant any errors may be though.

@solidfox
Copy link
Contributor

@emilioicai May I ask what kind of purchases you are seeing this bug for? (Consumable or Non-consumable?)

It seems to me that rather than manually requesting unfinished purchases you should simply register your purchaseUpdatedListener when the app launches. If there are any unfinished purchases when you register your listener the listener should be called with those purchases so you can deliver them to your user and finish them.

@emilioicai
Copy link
Contributor Author

We've seen this in Non-consumables. We already have a listener purchaseUpdatedListener which works fine for 95% of the cases but we have a large number of purchases so our customer support asked to have a nicer way for users to recover from these errors. Hopefully by using this functionallity we can ensure 100% of the purchases are eventually fulfilled.

@solidfox
Copy link
Contributor

@emilioicai I see. Have you evaluated the possibility of correcting purchaseUpdatedListener to include what it seems to be missing for your use case? I have never worked with non-consumables so it's possible that I'm missing something but if purchases are not being passed by purchaseUpdatedListener I seem to prefer to see that fixed rather than making extra checks on the javascript side.

@emilioicai
Copy link
Contributor Author

@solidfox the range of issues is too wide (app crashing during the purchase process, lack of connectivity, user closing the app,...). On top of that, I think this method can be useful for several other use cases, what we are doing here is merely mapping a method which already exists in StoreKit so users of react-native-iap can use it as well.

@solidfox
Copy link
Contributor

@emilioicai indeed, having that method mapped might make sense either way.

Nonetheless, none of the cases you are listing should need anything more than your purchaseUpdatedListener. Are you setting andDangerouslyFinishTransactionAutomatically to false when requesting purchases as recommended?

Copy link
Contributor

@solidfox solidfox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly a sensible function to have mapped. Thanks for the help!

@solidfox
Copy link
Contributor

solidfox commented Aug 26, 2019

@emilioicai Actually I just noticed you're using the request verb in the naming of the function. In react-native-iap this is used for mutating functions like requestPurchase. Would it be ok to rename the function with the get verb so we have consistency?

@emilioicai
Copy link
Contributor Author

Yes, I'm passing andDangerouslyFinishTransactionAutomatically to false. What we realised is sometimes StoreKit fails to emit the purchase-updated event for several reasons so the listener is not called. Of course, when the app is re-initialised, the event is successfully emitted but this only happens when the app is killed and re-opened. In our app we need a way to let users run validations and finish transactions in a manual way so they don't need to be asked to kill the app and open it again.

@emilioicai
Copy link
Contributor Author

@emilioicai Actually I just noticed you're using the request verb in the naming of the function. In react-native-iap this is used for mutating functions like requestPurchase. Would it be ok to rename the function with the get verb so we have consistency?

done!

@solidfox
Copy link
Contributor

@emilioicai I see, so it's actually an Apple bug that we're working around here. I'm now totally confident this should exist. Great work and explanation, thanks!

Copy link
Owner

@hyochan hyochan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! @solidfox for a thorough review. Let's approve this feature! Thank you for your contribution @emilioicai.

@hyochan hyochan merged commit ae9e0b5 into hyochan:master Aug 28, 2019
@hyochan
Copy link
Owner

hyochan commented Aug 28, 2019

@solidfox @emilioicai I've invited all of you as a collaborator with write access to react-native-iap repo since you people seem to have a great understanding of this module. If you don't want it, you may refuse it. Thank you!

@emilioicai
Copy link
Contributor Author

Thanks a lot @hyochan !

@jvandenaardweg
Copy link
Contributor

jvandenaardweg commented Sep 3, 2019

I think the docs become too overwhelming with a variety of methods that do not really explain why they exist and when they should be used.

Maybe good to adjust the docs to explain when you need this method? Or provide an example inside https://github.com/dooboolab/react-native-iap/tree/master/IapExample which shows how to use this correctly.

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 New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants