[in_app_purchase]load purchase #1380

cyanglaz · 2019-03-22T00:35:38Z

Description

Adding unified IAP load purchase api.
Updated the example app to have indicators on already purchased items.
No pubspec and CHANGELOG.md is updated since this plugin is still under development.

Related Issues

flutter/flutter#26326

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

mklim

Thanks for this!

mklim · 2019-03-26T19:59:20Z

packages/firebase_crashlytics/example/README.md

@@ -10,7 +10,7 @@ This example initially uses a default project for CI purposes. You must
 replace the default project with your own so that you can review the error
 reports submitted to the Firebase console.

-See [docs](https://firebase.google.com/docs/flutter/setup) for how to add 
+See [docs](https://firebase.google.com/docs/flutter/setup) for how to add


I think all the changes under packages/firebase_crashlytics should be split out into their own changes.

mklim · 2019-03-26T20:00:01Z

packages/in_app_purchase/example/lib/main.dart

@@ -118,6 +117,31 @@ class _MyAppState extends State<MyApp> {
              'This app needs special configuration to run. Please see example/README.md for instructions.')));
    }

+    print('xyzzy about to query past purchases');


These debug logs should be removed.

packages/in_app_purchase/lib/src/billing_client_wrappers/purchase_wrapper.dart

packages/in_app_purchase/example/lib/main.dart

mklim · 2019-03-26T20:27:43Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/app_store_connection.dart

+
+  /// Triggered when any transactions are updated.
+  void updatedTransactions({List<SKPaymentTransactionWrapper> transactions}) {
+    assert(_restoreCompleter != null);


What happens if we call cleanUpRestoredTransactions() and then this triggers from a new purchase?

mklim · 2019-03-26T20:44:48Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

+
+  /// The original purchase data of this purchase.
+  ///
+  /// It is only available when this purchase is a restored purchase in iOS.


Do we need to nest the originalPurchase here? I think it would be cleaner if the structure of this was the same across all our platforms.

mklim · 2019-03-26T20:45:49Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

+/// The response object for fetching the past purchases.
+///
+/// An instance of this class is returned in [InAppPurchaseConnection.queryPastPurchases].
+class QueryPastPurchaseResponse {


nit: I think QueryPurchaseDetails makes a little more sense as a name here since it matches what this is returning.

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

mklim · 2019-03-26T20:48:21Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

@@ -16,6 +128,18 @@ abstract class InAppPurchaseConnection {
  /// Query product details list that match the given set of identifiers.
  Future<ProductDetailsResponse> queryProductDetails(Set<String> identifiers);

+  /// Query all the past purchases.
+  ///
+  /// The `applicationUserName` is used for iOS only and it is optional. It does not have any effects on Android.


Same thoughts above about asking people to case on this. Even if this isn't really used in Play I think it would probably be cleaner for users to just always pass this in if they also passed it in for originally making a purchase. I think it would be clearer here if we said that it is required in cases where this was passed in with the original transaction.

mklim · 2019-03-26T20:49:09Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

+  Future<QueryPastPurchaseResponse> queryPastPurchases(
+      {String applicationUserName});
+
+  /// Get a refreshed purchase verification data.


This sort of rephrases the method name. Maybe mention that this is a utility in case there's an issue with getting the verification data originally, and as long as that's not null this doesn't need to be called?

…ase_wrapper.dart Co-Authored-By: cyanglaz <ychris@google.com>

…ogle_play_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

mklim

Looking pretty good. The only thing left that I'd really like to see updated is the param type on restoreCompletedTransactionsFailed.

packages/in_app_purchase/example/lib/main.dart

mklim · 2019-03-27T19:30:43Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/app_store_connection.dart

-      return wrapper.transactionState ==
-          SKPaymentTransactionStateWrapper.restored;
-    }).toList();
+    if (_restoreCompleter != null) {


subjective: I think an early return/guard clause would make this more readable.

https://testing.googleblog.com/2017/06/code-health-reduce-nesting-reduce.html

mklim · 2019-03-27T19:32:16Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

-/// on Android, all purchase information should also be verified manually, with your
-/// server if at all possible. See [`Verify a purchase`](https://developer.android.com/google/play/billing/billing_library_overview#Verify).
+/// On Android, all purchase information should also be verified manually. See [`Verify a purchase`](https://developer.android.com/google/play/billing/billing_library_overview#Verify).
+///


nit: This line can be deleted.

mklim · 2019-03-27T19:40:46Z

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

+  final Map<String, dynamic> message;
+}
+
+/// Represents the transaction details of a purchase.


Still think this one is worth adding. "Transactions" on iOS can have multiple statuses so I think it's worth pointing out which one this is.

packages/in_app_purchase/lib/src/in_app_purchase_connection/in_app_purchase_connection.dart

mklim · 2019-03-27T19:44:26Z

packages/in_app_purchase/lib/src/store_kit_wrappers/sk_payment_transaction_wrappers.dart

-  void restoreCompletedTransactions({Error error});
+  ///
+  /// The error is represented in a Map. The map contains `errorCode` and `message`
+  void restoreCompletedTransactionsFailed({Map<String, String> error});


We should probably define a class with these props on it instead of using a map. The Map leaves this open to a bunch of dynamic typing kinds of bugs.

Co-Authored-By: cyanglaz <ychris@google.com>

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

…iap_load_purchase

mklim

LGTM

Chris Yang added 6 commits March 21, 2019 10:18

init patch

6f56042

fix crash on restore failed

7ee4bca

to purchase details tests, and dart doc

2a37e9f

android query purchases

d707573

load purchase android

ea59c6a

doc updates

0f82217

cyanglaz added the feature label Mar 22, 2019

cyanglaz requested a review from mklim as a code owner March 22, 2019 00:35

googlebot added the cla: yes label Mar 22, 2019

Chris Yang added 6 commits March 21, 2019 18:03

remove extra code

d493b7c

remove more extra code

c193a19

remove more

134cf56

merge refactor branch

4fc0f35

formatting

7e856f5

merge refactor branch

60b5caa

cyanglaz closed this Mar 22, 2019

Chris Yang added 9 commits March 22, 2019 11:01

fake ios platform helps testing app store connection methods

6d5ee05

make the ci happy

cbc230d

merge factor branch

922921b

remove from testing

4489d4f

Merge branch 'iap_refactor' into iap_load_purchase

533f7cc

adding applcation user name to queryPastPurchase

01e7e23

more comments

33c37b9

refresh verification data call

7da8cdf

merge master

324c7ec

cyanglaz reopened this Mar 22, 2019

Chris Yang added 4 commits March 25, 2019 13:58

renaming, and test null case

db8365a

add a response object for queryPastPurchases

f802c5a

compile error fix

f59e395

adding purchase error object

5fb4b40

revert unnecessary extra product_list file

d5cec73

mklim reviewed Mar 26, 2019

View reviewed changes

Chris Yang and others added 8 commits March 26, 2019 14:09

Merge branch 'master' into iap_load_purchase

a6a9b28

Update packages/in_app_purchase/lib/src/billing_client_wrappers/purch…

363bd2b

…ase_wrapper.dart Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/go…

3362ad1

…ogle_play_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/in…

da3a52d

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/in…

f0bc753

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/in…

9de51e8

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

review fixes

cc09eda

merge website review commits

f6a0425

mklim reviewed Mar 27, 2019

View reviewed changes

Michael Klimushyn and others added 9 commits March 27, 2019 13:57

Update packages/in_app_purchase/example/lib/main.dart

23e9896

Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/example/lib/main.dart

47b713c

Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/example/lib/main.dart

048d2da

Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/in…

78b07e2

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/in…

3dd155e

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

Update packages/in_app_purchase/lib/src/in_app_purchase_connection/in…

a8d5917

…_app_purchase_connection.dart Co-Authored-By: cyanglaz <ychris@google.com>

review fixes

27b8b71

Merge branch 'iap_load_purchase' of github.com:cyanglaz/plugins into …

84b5328

…iap_load_purchase

revert line removal

4bdcdc7

mklim approved these changes Mar 27, 2019

View reviewed changes

make the ci happy

86cb3a4

cyanglaz closed this Mar 28, 2019

cyanglaz deleted the iap_load_purchase branch March 28, 2019 18:49

cyanglaz restored the iap_load_purchase branch April 4, 2019 23:19

cyanglaz reopened this Apr 4, 2019

cyanglaz merged commit 8c180f0 into flutter:master Apr 4, 2019

cyanglaz deleted the iap_load_purchase branch April 4, 2019 23:21

cyanglaz mentioned this pull request Apr 4, 2019

[in_app_purchase] Add references to the original object for PurchaseDetails and ProductDetails #1448

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[in_app_purchase]load purchase #1380

[in_app_purchase]load purchase #1380

cyanglaz commented Mar 22, 2019

mklim left a comment

mklim Mar 26, 2019

mklim Mar 26, 2019

mklim Mar 26, 2019

mklim Mar 26, 2019

mklim Mar 26, 2019

mklim Mar 26, 2019

mklim Mar 26, 2019

mklim left a comment

mklim Mar 27, 2019

mklim Mar 27, 2019

mklim Mar 27, 2019

mklim Mar 27, 2019

mklim left a comment

[in_app_purchase]load purchase #1380

[in_app_purchase]load purchase #1380

Conversation

cyanglaz commented Mar 22, 2019

Description

Related Issues

Checklist

Breaking Change

mklim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mklim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mklim left a comment

Choose a reason for hiding this comment