[in_app_purchase] payment queue dart ios by cyanglaz · Pull Request #1249 · flutter/plugins

cyanglaz · 2019-02-21T18:45:02Z

Dart side for add payment in StoreKit.

Fixed some bug in the Objective-C code.

Removed add payment with only product identifier in Objective-C since it seems unnecessary.

Part of flutter/flutter#26328

…ment_objc

mklim

Sorry for the delay! Basically looks solid, some questions.

mklim · 2019-02-28T00:25:48Z

packages/in_app_purchase/ios/Classes/FIAPaymentQueueHandler.m

    updatedTransactions:(NSArray<SKPaymentTransaction *> *)transactions {
  for (SKPaymentTransaction *transaction in transactions) {
-    [self.transactionsSetter setObject:transaction forKey:transaction.transactionIdentifier];
+    if (transaction.transactionIdentifier) {


Would this ever be null? Should we handle it like an error if/when it is?

Apple handles the transaction identifier strangely, the identifier is null when the state the purchasing. I should probably put a comment on it.

mklim · 2019-02-28T00:27:56Z

packages/in_app_purchase/ios/Classes/InAppPurchasePlugin.m

+                          stringWithFormat:@"The transaction with transactionIdentifer:%@ does not "
+                                           @"exsit. Note that if the transactionState is "
+                                           @"purchasing, the transactionIdentifier will be "
+                                           @"nil(null). And you should not finish this transaction",


And you should not finish this transaction

Can you go into more detail about this?

Sure, by apple's document finish transaction with transactionIdentifier of purchasing will throw exception; I will put something similar here.https://developer.apple.com/documentation/storekit/skpaymentqueue/1506003-finishtransaction?language=objc

mklim · 2019-02-28T00:29:31Z

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

+  /// payment follow.
+  /// You must set the observer right when App launches to avoid missing callback when your user
+  /// started a purchase flow from the App Store.
+  void setTransactionObserver(SKTransactionObserverWrapper observer) {


Is there any way we could take this as a param at construction time instead? It would force users to set this as soon as this object is instantiated.

Since the payment queue is a singleton, there is no actually a construction time. User can technically call SKPaymentQueue() everywhere to refer the defaultPaymentQueue. So I didn't want user to have to put the observer each time when they refer to the singleton.

Right. Hmm. I think it may be worth converting this from a singleton into a constructed class just to enforce that this is configured ASAP. What do you think? Could be done in a followup PR too.

Let's do more offline discussion on this and will create another PR if we reach a different approach.

mklim · 2019-02-28T00:30:16Z

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

+  /// the transaction that is generated by the payment.
+  /// The [productIdentifier] must match one of the product that is returned in [SKRequestMaker.startProductRequest].
+  /// to test the payment in the [sandbox](https://developer.apple.com/apple-pay/sandbox-testing/).
+  Future<void> addPayment(SKPaymentWrapper payment) async {


We should link to the official dev docs on the related methods for all of these too.

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

mklim · 2019-02-28T00:32:16Z

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

+    );
+  }
+
+  /// Finishes a transaction, remove it from the queue.


Suggested change

/// Finishes a transaction, remove it from the queue.

/// Finishes a transaction and removes it from the queue.

mklim · 2019-02-28T00:48:55Z

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

+  ///
+  /// This method should be called from a observer callback when receiving notification from the payment queue. You should only
+  /// call this method after the transaction is successfully processed and the functionality purchased by the user is unlocked.
+  /// Itt will throw a Platform exception if the [SKPaymentTransactionWrapper.transactionState] is [SKPaymentTransactionStateWrapper.purchasing].


Suggested change

/// Itt will throw a Platform exception if the [SKPaymentTransactionWrapper.transactionState] is [SKPaymentTransactionStateWrapper.purchasing].

/// It will throw a Platform exception if the [SKPaymentTransactionWrapper.transactionState] is [SKPaymentTransactionStateWrapper.purchasing].

mklim · 2019-02-28T00:49:42Z

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

+  // Triage a method channel call from the platform and triggers the correct observer method.
+  Future<dynamic> _handleObserverCallbacks(MethodCall call) {
+    assert(_observer != null,
+        'in_app_purchase]: (Fatal)The observer has not been set but we received a purchase transaction notification. Please ensure the observer has been set using `setTransactionObserver`. Make sure the observer is added right at the App Launch.');


Suggested change

'in_app_purchase]: (Fatal)The observer has not been set but we received a purchase transaction notification. Please ensure the observer has been set using `setTransactionObserver`. Make sure the observer is added right at the App Launch.');

'[in_app_purchase]: (Fatal)The observer has not been set but we received a purchase transaction notification. Please ensure the observer has been set using `setTransactionObserver`. Make sure the observer is added right at the App Launch.');

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

mklim · 2019-02-28T00:53:04Z

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

+
+/// This class is Dart wrapper around [SKTransactionObserver](https://developer.apple.com/documentation/storekit/skpaymenttransactionobserver?language=objc).
+///
+/// Must be subclassed and


nit: doc missing.

OMG lol! Sorry.

googlebot · 2019-02-28T05:25:34Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-02-28T05:25:37Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2019-02-28T05:27:04Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

mklim

LGTM

mklim · 2019-02-28T19:26:09Z

packages/in_app_purchase/ios/Classes/InAppPurchasePlugin.m

+                                    @"The transaction with transactionIdentifer:%@ does not "
+                                    @"exsit. Note that if the transactionState is "
+                                    @"purchasing, the transactionIdentifier will be "
+                                    @"nil(null). And you should not finish this transaction. "


Nit: Sorry, on second thought and after getting the full info I think everything after the sentence about transactionIdentifier being null isn't needed here. It's helpful to know but it's not necessarily relevant, and we don't want to clog up the error logs with extraneous info.

mklim · 2019-02-28T19:27:14Z

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

+  /// payment follow.
+  /// You must set the observer right when App launches to avoid missing callback when your user
+  /// started a purchase flow from the App Store.
+  void setTransactionObserver(SKTransactionObserverWrapper observer) {


Right. Hmm. I think it may be worth converting this from a singleton into a constructed class just to enforce that this is configured ASAP. What do you think? Could be done in a followup PR too.

Dart side for add payment in StoreKit. Fixed some bug in the Objective-C code. Removed add payment with only product identifier in Objective-C since it seems unnecessary. Part of flutter/flutter#26328

Chris Yang added 30 commits February 15, 2019 10:20

draft

3d21006

second draft

73caea7

finish transaction

9420db0

adding more comments

62a9974

more comments

2ec9c58

let dart intercept the payment from store

e75411a

create payment method

824eee8

rearrange method order

98f8d03

formatting

e1176e5

draft

6878875

draft 2

8feddbf

Merge branch 'master' of github.com:flutter/plugins into iap_make_pay…

2ccfe7e

…ment_objc

Merge branch 'iap_make_payment_objc' into iap_payment_queue_dart_ios

0e8de06

add payment

ec177e3

typo fix

ab75985

Merge branch 'iap_make_payment_objc' into iap_payment_queue_dart_ios

5edec3f

more comments

ed11706

remove unused code

564d13b

Merge branch 'iap_make_payment_objc' into iap_payment_queue_dart_ios

330dc3d

update to weakself

b11e3c5

review fixes

7f9e5a9

formatting

0187f9a

merge the objective-c branch

ec1f0bb

rename callback methods

6b2d7ea

nit fix

e7da33a

rearrange payment object handling code

b756e70

fix unit test, remove nil check for callback blocks

4dd5b14

formatting

f3cdb83

merge and resolve conflicts

7ddbc6c

observer update

18f1e82

remove unnecessary method

80fcac1

bparrishMines added bugfix feature labels Feb 22, 2019

Chris Yang added 4 commits February 21, 2019 21:51

Merge branch 'master' into iap_payment_queue_dart_ios

670bc84

remove unnessary addpayment logic

5c8b3c5

formatting and renaming

8132c22

fix transform map to transactions and downloads

ffaf35c

cyanglaz force-pushed the iap_payment_queue_dart_ios branch from de3a84b to ffaf35c Compare February 22, 2019 22:34

Chris Yang added 4 commits February 22, 2019 14:54

refresh serializer

6989936

formatting

693ed90

more comments

57fc26f

merge origin

810fb8b

mklim reviewed Feb 28, 2019

View reviewed changes

Chris Yang added 2 commits February 27, 2019 21:21

formating

711c437

more error message and comments

237bf2c

googlebot added cla: no and removed cla: yes labels Feb 28, 2019

cyanglaz force-pushed the iap_payment_queue_dart_ios branch from b615650 to 237bf2c Compare February 28, 2019 05:26

googlebot added cla: yes and removed cla: no labels Feb 28, 2019

more review fixes

de1dd3c

mklim approved these changes Feb 28, 2019

View reviewed changes

Chris Yang added 2 commits February 28, 2019 13:39

review fix

6a4b1b3

formatting

b163954

cyanglaz merged commit a533148 into flutter:master Feb 28, 2019

cyanglaz deleted the iap_payment_queue_dart_ios branch February 28, 2019 23:28

	/// Finishes a transaction, remove it from the queue.
	/// Finishes a transaction and removes it from the queue.

	/// Itt will throw a Platform exception if the [SKPaymentTransactionWrapper.transactionState] is [SKPaymentTransactionStateWrapper.purchasing].
	/// It will throw a Platform exception if the [SKPaymentTransactionWrapper.transactionState] is [SKPaymentTransactionStateWrapper.purchasing].

	'in_app_purchase]: (Fatal)The observer has not been set but we received a purchase transaction notification. Please ensure the observer has been set using `setTransactionObserver`. Make sure the observer is added right at the App Launch.');
	'[in_app_purchase]: (Fatal)The observer has not been set but we received a purchase transaction notification. Please ensure the observer has been set using `setTransactionObserver`. Make sure the observer is added right at the App Launch.');

Conversation

cyanglaz commented Feb 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

cyanglaz Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

googlebot commented Feb 28, 2019

Uh oh!

googlebot commented Feb 28, 2019

Uh oh!

googlebot commented Feb 28, 2019

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

mklim Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cyanglaz commented Feb 21, 2019 •

edited

Loading

cyanglaz Feb 28, 2019 •

edited

Loading

mklim Feb 28, 2019 •

edited

Loading