[In_app_purchase] getproductlist basic draft #1169

cyanglaz · 2019-02-06T21:58:37Z

This is a very basic first draft for combining android and iOS get product list.
Would like this to be solid before working into more details.

cyanglaz · 2019-02-07T18:08:55Z

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

+  ///
+  /// Returns a List of [Product] that each [Product] uniquely matches one valid identifier in [identifiers].
+  /// Any item in the `identifiers` that is not specified in App Store Connect or Google Play Console will be ignored.
+  Future<List<Product>> queryProductDetails(List<String> identifiers);


An alternative could be returning a custom 'Response' class something like ProductResponse which contains the list of the product and some errors if any. Any thoughts on this?

I like that idea but I don't know what information we would be setting as the response code.

BillingClient returns an error code sometimes, and StoreKit returns a list of invalidProductIdentifiers. How would we combine that in a way that makes sense?

That's also what blocking me to introduce the response class.

Discussed offline, going to add a response with a list of the not found product IDs.

mklim

Some API feedback but the underlying code is solid.

I also think now would be a good time to update the example app to query the products and list them. Proof of concept here.

mklim · 2019-02-08T23:10:15Z

packages/in_app_purchase/lib/src/billing_client_wrappers/sku_details_wrapper.g.dart

      subscriptionPeriod: json['subscriptionPeriod'] as String,
      title: json['title'] as String,
-      type: const SkuTypeConverter().fromJson(json['type'] as String),
+      type: _$enumDecode(_$SkuTypeEnumMap, json['type']),


For some reason this generated with extra enum boilerplate and it shouldn't have.

oh yeah I didn't know why this piece of code got changed, I'll discuss with you offline.

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

mklim · 2019-02-08T23:55:54Z

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

+  ///
+  /// Returns a List of [Product] that each [Product] uniquely matches one valid identifier in [identifiers].
+  /// Any item in the `identifiers` that is not specified in App Store Connect or Google Play Console will be ignored.
+  Future<List<Product>> queryProductDetails(List<String> identifiers);


I like that idea but I don't know what information we would be setting as the response code.

BillingClient returns an error code sometimes, and StoreKit returns a list of invalidProductIdentifiers. How would we combine that in a way that makes sense?

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

cyanglaz · 2019-02-12T02:00:10Z

@mklim Updated with example app. Please take another look.

mklim · 2019-02-12T21:01:42Z

packages/in_app_purchase/example/lib/main.dart

+                  'gas',
+                  'premium',
+                  'upgrade',
+                  'somethingNotValid'


Ideally the example app should just be demoing the right way to use the API. I don't think it's worth deliberately checking for invalid product IDs here. All we're asking people to set up in the README so far is consumable, upgrade, and subscription.

mklim · 2019-02-12T21:11:06Z

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

+}
+
+/// The response returned by [InAppPurchaseConnection.queryProductDetails]
+class QueryProductDetailsResponse {


nit: I think it would be a little clearer and more concise to name this ProductDetailsResponse. The "Query" part is already implied by "Response" (it's a response to a query).

mklim · 2019-02-12T21:12:01Z

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

+  QueryProductDetailsResponse(
+      {@required this.productDetails, @required this.notFoundIDs});
+
+  ///Each [ProductDetails] uniquely matches one valid identifier in [identifiers] of [InAppPurchaseConnection.queryProductDetails].


nit: Extra whitespace before each sentence in the dartdoc here and below.

mklim · 2019-02-12T21:20:58Z

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

+
+  ///The list of identifiers that are in the `identifiers` of [InAppPurchaseConnection.queryProductDetails] but failed to be fetched.
+  ///
+  ///These are the identifiers not matching the `productIdentifer` or `sku` in any product on the App Store Connect or Google Play Console.


I'd also avoid mentioning the underlying platforms here. "failed to be fetched" is about as specific as we can safely get at this layer without going into a bunch of platform specific details I think. For example I know in the App Store case invalidIdentifiers will be full of "valid" products in a lot of cases where the IDs do match whatever is in App Store connect and the problem is really in some other configuration in either the app or its store listing.

Maybe we could broadly point the users to look into the underlying platforms if they're seeing this failing? Like, "There's multiple platform specific reasons that product information could fail to be fetched, ranging from products not being correctly configured in the storefront to the queried IDs not existing." That's unfortunately not very helpful on its own, but I think that's really all the information we have at this level.

packages/in_app_purchase/example/lib/main.dart

mklim · 2019-02-12T21:27:33Z

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

+
+  /// Query the product detail list.
+  ///
+  /// This method only returns [QueryProductDetailsResponse].


FYI I don't think this or GooglePlayConnection really need dart doc, since we're not exposing either of them as public API classes and they're overriding the documented public interface. You also don't need to delete documentation you've already written though.

mklim · 2019-02-12T21:29:57Z

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

+  Future<QueryProductDetailsResponse> queryProductDetails(
+      Set<String> identifiers) async {
+    SkuDetailsResponseWrapper inappResponse =
+        await _billingClient.querySkuDetails(


nit: It would be a little faster if these two queries happened in parallel and we blocked on them both completing in general, instead of blocking on both of them sequentially.

I think you can use Future.wait to block on multiple futures resolving.

mklim · 2019-02-12T21:32:25Z

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

+            skuType: SkuType.inapp, skusList: identifiers.toList());
+    SkuDetailsResponseWrapper subResponse = await _billingClient
+        .querySkuDetails(skuType: SkuType.subs, skusList: identifiers.toList());
+    List<ProductDetails> inappProducts = inappResponse.skuDetailsList


nit, subjective: instead of writing this conversion twice what about combining these lists first?

Something like:

List<ProductDetails> products = (inappResponse.skuDetailsList + subResponse.skuDetailsList) .map((SkuDetailsWrapper wrapper) => //etc

mklim · 2019-02-12T21:41:54Z

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

  /// Returns true if the payment platform is ready and available.
  Future<bool> isAvailable();

+  /// Query the product details list.


nit: I don't think we're really querying from a pre-existing list of product details here. It's more like we're querying for product details that match the given set of identifiers.

mklim · 2019-02-12T21:42:31Z

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

+
+/// The class represents the information of a product.
+///
+/// A list of [ProductDetails] can be obtained from the [QueryProductDetailsResponse].


nit, subjective: I think it would be more helpful to link to Connection.queryProductDetails here than the response data class specifically.

cyanglaz · 2019-02-13T18:50:20Z

@mklim Updated with suggested reviews.

mklim

Contains the basic draft of the combined api to get product details. Add an example app to demonstrate the usage.

This reverts commit 35da9be.

Contains the basic draft of the combined api to get product details. Add an example app to demonstrate the usage.

Chris Yang added 11 commits February 4, 2019 16:52

first draft

abc5aed

expose nslocale

9abd172

fixing test

36fdd7f

avoid ios 10 limitation

cb1d9f0

revert testing code

892d78e

format

d698b35

Merge branch 'iap_expose_nslocale' into iap_getproductlist

b125c01

Merge branch 'iap_expose_nslocale' into iap_getproductlist

0c8e70f

add consolidated product object

efa1506

unit test cases

0a2ce52

adding top level abstract method

3bf78ff

cyanglaz requested a review from mklim February 6, 2019 21:58

googlebot added the cla: yes label Feb 6, 2019

cyanglaz commented Feb 7, 2019

View reviewed changes

mklim reviewed Feb 8, 2019

View reviewed changes

Chris Yang added 8 commits February 8, 2019 17:01

merge master

1d0b8c9

format

2d3effc

update return value for queryProduct to be a response object

d147242

example showing producgts

883afdf

separates the product list and the connection tile

8e9ae77

finishes main.dart

d12c0c4

Merge branch 'master' into iap_getproductlist

d46c251

format

ba5f467

cyanglaz mentioned this pull request Feb 12, 2019

[In_app_purchase] fix requesthandler crash #1199

Merged

mklim reviewed Feb 12, 2019

View reviewed changes

review fixes

023e97b

mklim approved these changes Feb 13, 2019

View reviewed changes

analyzer fix

c2b275e

cyanglaz merged commit 81dff2b into flutter:master Feb 13, 2019

cyanglaz deleted the iap_getproductlist branch February 14, 2019 01:31

This was referenced Feb 15, 2019

IAP: Get a list of all available products flutter/flutter#26325

Closed

IAP: Create an example app with getting started documentation flutter/flutter#25987

Closed

andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019

Iap getproductlist basic draft (flutter#1169)

35da9be

Contains the basic draft of the combined api to get product details. Add an example app to demonstrate the usage.

andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019

Revert "Iap getproductlist basic draft (flutter#1169)"

d23890c

This reverts commit 35da9be.

karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019

Iap getproductlist basic draft (flutter#1169)

cd6bbce

Contains the basic draft of the combined api to get product details. Add an example app to demonstrate the usage.

karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019

Iap getproductlist basic draft (flutter#1169)

365964b

Contains the basic draft of the combined api to get product details. Add an example app to demonstrate the usage.

cyanglaz changed the title ~~Iap getproductlist basic draft~~ [In_app_purchase] getproductlist basic draft Feb 22, 2019

[In_app_purchase] getproductlist basic draft #1169

[In_app_purchase] getproductlist basic draft #1169

Uh oh!

Conversation

cyanglaz commented Feb 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cyanglaz commented Feb 12, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cyanglaz commented Feb 13, 2019

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants