[in_app_purchase] Add support for promotional offers through Store-Kit wrappers #4458

mvanbeusekom · 2021-10-29T09:39:17Z

This PR adds support for promotional offers on iOS by exposing the SKProduct.discounts (via the SKProductWrapper Dart class) and the SKMutablePayment.paymentDiscount (via the SKPaymentWrapper Dart class) properties.

Resolves flutter/flutter#92231

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

cyanglaz

Thanks for the quick PR! I left some comments and questions.

cyanglaz · 2021-10-29T18:55:33Z

packages/in_app_purchase/in_app_purchase_ios/example/ios/RunnerTests/Stubs.m

@@ -61,6 +61,14 @@ - (instancetype)initWithMap:(NSDictionary *)map {
      [self setValue:map[@"subscriptionGroupIdentifier"] ?: [NSNull null]
              forKey:@"subscriptionGroupIdentifier"];
    }
+    if (@available(iOS 12.2, *)) {
+      NSMutableArray *discounts = [NSMutableArray new];


nits: Do now use new per google objc style guide: https://google.github.io/styleguide/objcguide.html#do-not-use-new

cyanglaz · 2021-10-29T18:58:32Z

packages/in_app_purchase/in_app_purchase_ios/example/ios/RunnerTests/TranslatorTests.m

@@ -14,6 +14,7 @@ @interface TranslatorTest : XCTestCase
 @property(strong, nonatomic) NSMutableDictionary *productMap;
 @property(strong, nonatomic) NSDictionary *productResponseMap;
 @property(strong, nonatomic) NSDictionary *paymentMap;
+@property(strong, nonatomic) NSDictionary *paymentDiscountMap;


NSDictionary should be copy: https://google.github.io/styleguide/objcguide.html#setters-copy-nsstrings

(I know some old codes use strong, which should also be updated to copy is possible, in a separate PR)

cyanglaz · 2021-10-29T19:00:02Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.h

@@ -47,6 +51,9 @@ NS_ASSUME_NONNULL_BEGIN
                 andSKPaymentTransaction:(SKPaymentTransaction *)transaction
    API_AVAILABLE(ios(13), macos(10.15), watchos(6.2));

+// Creates an instance of the SKPaymentDiscount class based on the supplied disctionary.


nits: dictionary

Suggested change

// Creates an instance of the SKPaymentDiscount class based on the supplied disctionary.

// Creates an instance of the SKPaymentDiscount class based on the supplied dictionary.

cyanglaz · 2021-10-29T19:00:17Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

@@ -24,6 +24,7 @@ + (NSDictionary *)getMapFromSKProduct:(SKProduct *)product {
  // https://github.com/flutter/flutter/issues/26610
  [map setObject:[FIAObjectTranslator getMapFromNSLocale:product.priceLocale] ?: [NSNull null]
          forKey:@"priceLocale"];
+


nits: revert

cyanglaz · 2021-10-29T19:00:40Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

@@ -49,6 +54,17 @@ + (NSDictionary *)getMapFromSKProductSubscriptionPeriod:(SKProductSubscriptionPe
  return @{@"numberOfUnits" : @(period.numberOfUnits), @"unit" : @(period.unit)};
 }

+ (nonnull NSArray *)getMapArrayFromSKProductDiscounts:
+    (nonnull NSArray<SKProductDiscount *> *)productDiscounts {
+  NSMutableArray *discountsMapArray = [NSMutableArray new];


use alloc] init]; instead of new

cyanglaz · 2021-10-29T19:07:10Z

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

+  /// Constructs an instance of this from a key value map of data.
+  ///
+  /// The map needs to have named string keys with values matching the names and
+  /// types of all of the members on this class. The `map` parameter must not be


nits: "The map parameter must not be null." is not necessary as it is a NNBD supported file.

cyanglaz · 2021-10-29T19:09:07Z

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

+
+  /// A string identifying the key that is used to generate the signature.
+  ///
+  /// Keys are generated and downloaded from App Store Connect. See the "KEY ID"


Maybe a link to some official guide might help explain this better? I feel like this is hard to maintain. For example, if App Store Connect changed their UI, this has to be updated to match.

cyanglaz · 2021-10-29T19:10:18Z

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

+  /// The unique nonce should be generated on your server when it creates the
+  /// `signature` for the payment discount. The nonce can be used once, a new
+  /// nonce should be created for each payment request.
+  /// The string representation of the nonce must be lowercase.


Is there an official guide on how to generate this from Apple, if so, we can add a link here too.

Yes there is, I have added an extra link.

cyanglaz · 2021-10-29T19:11:30Z

packages/in_app_purchase/in_app_purchase_ios/lib/src/store_kit_wrappers/sk_product_wrapper.dart

@@ -241,7 +241,8 @@ class SKProductWrapper {
    required this.price,
    this.subscriptionPeriod,
    this.introductoryPrice,
-  });
+    List<SKProductDiscountWrapper>? discounts,


Suggested change

List<SKProductDiscountWrapper>? discounts,

this.discounts = <SKProductDiscountWrapper>[],

And remove the next line.

I remember there's a way to indicate default value to be empty array in jsonSerializable.

Updating List<SKProductDiscountWrapper>? discounts, to this.discounts = <SKProductDiscountWrapper>[], will not work as the default value is not a constant. The following error is generated:

The default value of an optional parameter must be constant. dart(non_constant_default_value)

That is why I resolved it like this. The field is already configured to use an empty array as default value in jsonSerializable, however this is done on field level not in the constructor:

@JsonKey(defaultValue: <SKProductDiscountWrapper>[]) final List<SKProductDiscountWrapper> discounts;

I am not aware you can add additional attributes on constructor parameters.

now you've updated the filed to be none-optional, will this.discounts = const <SKProductDiscountWrapper>[], not work? The parameter doesn't have to be optional, right?

cyanglaz · 2021-10-29T19:11:56Z

packages/in_app_purchase/in_app_purchase_ios/lib/src/store_kit_wrappers/sk_product_wrapper.dart

+  /// An array of subscription offers available for the auto-renewable subscription (available on iOS 12.2 and higher).
+  ///
+  /// This property lists all promotional offers set up in App Store Connect. If
+  /// no promotional offers have been set up this field returns an empty list.


nits:

Suggested change

/// no promotional offers have been set up this field returns an empty list.

/// no promotional offers have been set up, this field returns an empty list.

cyanglaz · 2021-11-03T19:53:30Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

+    return nil;
+  }
+
+  NSString *identifier = map[@"identifier"];


what happens if the map is empty, or some of the values are NSNull?

I haven't added specific check for these cases because the fields are mandatory which is enforced on the Dart side. Since this is only called from Dart side I skipped doing additional tests.

If you feel it is better to do so I will expand the logic.

Yes let's add logic for these values, if any of the critical values is nil, we should return nil.

My rational is that if for some reason, the dart api changes so that these values are nullable, dart tests will get updated along with the change; however, there are no tests on the platform side to catch this.

We can also add an NSAssert to make sure these values are non-null.

Thanks! the asserts are for testings, for prod app, should we also prevent nil values?

cyanglaz · 2021-11-03T19:56:32Z

packages/in_app_purchase/in_app_purchase_ios/lib/src/store_kit_wrappers/sk_product_wrapper.dart

@@ -241,7 +241,8 @@ class SKProductWrapper {
    required this.price,
    this.subscriptionPeriod,
    this.introductoryPrice,
-  });
+    List<SKProductDiscountWrapper>? discounts,


now you've updated the filed to be none-optional, will this.discounts = const <SKProductDiscountWrapper>[], not work? The parameter doesn't have to be optional, right?

cyanglaz · 2021-11-04T15:58:29Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

+    return nil;
+  }
+
+  NSString *identifier = map[@"identifier"];


Yes let's add logic for these values, if any of the critical values is nil, we should return nil.

My rational is that if for some reason, the dart api changes so that these values are nullable, dart tests will get updated along with the change; however, there are no tests on the platform side to catch this.

We can also add an NSAssert to make sure these values are non-null.

mvanbeusekom · 2021-11-04T16:38:19Z

@cyanglaz thanks for the review. I have added NSAssert statements to validate if all required paymentDiscount fields are supplied. I have also added additional unit tests to ensure the asserts are there (so somebody removing them will be warned).

cyanglaz · 2021-11-05T17:18:40Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

+  NSString *signature = map[@"signature"];
+  NSNumber *timestamp = map[@"timestamp"];
+
+  SKPaymentDiscount *discount = [[SKPaymentDiscount alloc] initWithIdentifier:identifier


what happens to the SKPaymentDiscount object if any of the values are nil? Will it crash or throw some iap server error?

If the SKPaymentDiscount object it self is nil the payment will continue without discount. This is also documented as part of the Dart API.

The values of a SKPaymentDiscount should never be nil as this would indicate an invalid discount offer. Apples documentation is not very clear on this but it should result into a server error.

I have restructured the code and added additional validation to ensure empty and nil values are detected and returns a PlatformError in case invalid values are supplied. This to make it more clear to developers what went wrong.

P.S. I also removed the NSAssert lines since they are now obsolete.

cyanglaz · 2021-11-05T17:19:32Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

+
+  NSString *identifier = map[@"identifier"];
+  NSString *keyIdentifier = map[@"keyIdentifier"];
+  NSUUID *nonce = [[NSUUID alloc] initWithUUIDString:map[@"nonce"]];


will initWithUUIDString crash if map[@"nonce"] is nil?

According to Apple's documentation the initWithUUIDString method returns nil for invalid strings. I have made a quick test and confirmed this also includes nil values passed in.

I did restructure the code to improve error handling (see earlier comment).

cyanglaz · 2021-11-05T17:20:02Z

packages/in_app_purchase/in_app_purchase_ios/ios/Classes/FIAObjectTranslator.m

+    return nil;
+  }
+
+  NSString *identifier = map[@"identifier"];


Thanks! the asserts are for testings, for prod app, should we also prevent nil values?

mvanbeusekom · 2021-11-15T09:44:32Z

Hi @cyanglaz,

Last week I processed your feedback but forgot to request another review. Would you mind having another look? Many thanks in advance.

cyanglaz

LGTM

…Store-Kit wrappers (flutter/plugins#4458)

…t wrappers (flutter#4458)

mvanbeusekom added 2 commits October 28, 2021 21:38

Support promotional offers

020800b

Support promotional offers

d89dee5

mvanbeusekom requested a review from cyanglaz October 29, 2021 09:39

mvanbeusekom requested a review from LHLL as a code owner October 29, 2021 09:39

google-cla bot added the cla: yes label Oct 29, 2021

github-actions bot added p: in_app_purchase platform-ios labels Oct 29, 2021

mvanbeusekom force-pushed the issue/92231 branch from 2659f4e to 24b4f1e Compare October 29, 2021 09:40

Merge branch 'temp' into issue/92231

38c5dbc

mvanbeusekom force-pushed the issue/92231 branch from 24b4f1e to 38c5dbc Compare October 29, 2021 09:40

cyanglaz suggested changes Oct 29, 2021

View reviewed changes

mvanbeusekom added 3 commits October 29, 2021 21:59

Merge remote-tracking branch 'upstream/master' into issue/92231

f3e8ba9

Add paymentDiscount to payment on native side

6abe36d

Added feedback on PR

ffc2ca5

mvanbeusekom requested a review from cyanglaz November 2, 2021 10:56

mvanbeusekom changed the title ~~[webview_flutter] Add support for promotional offers through Store-Kit wrappers~~ [in_app_purchase] Add support for promotional offers through Store-Kit wrappers Nov 3, 2021

Merge remote-tracking branch 'upstream/master' into issue/92231

907dd52

cyanglaz suggested changes Nov 3, 2021

View reviewed changes

Use empty const array to initialize discounts

d8b6533

mvanbeusekom requested a review from cyanglaz November 4, 2021 10:42

cyanglaz suggested changes Nov 4, 2021

View reviewed changes

mvanbeusekom requested a review from cyanglaz November 4, 2021 16:34

Guard against null values for paymentDiscount

ba6dfba

mvanbeusekom force-pushed the issue/92231 branch from af8cf8c to ba6dfba Compare November 5, 2021 07:39

cyanglaz reviewed Nov 5, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into issue/92231

87fc1fd

mvanbeusekom requested a review from cyanglaz November 8, 2021 20:34

Improved testing on illegal values

1867ff4

mvanbeusekom force-pushed the issue/92231 branch from 108cb53 to 1867ff4 Compare November 8, 2021 20:39

Fix analysis warnings

490009b

cyanglaz approved these changes Nov 15, 2021

View reviewed changes

cyanglaz added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Nov 15, 2021

fluttergithubbot merged commit 45f87b9 into flutter:master Nov 15, 2021

engine-flutter-autoroll mentioned this pull request Nov 16, 2021

Roll Plugins from 31268aabbaa9 to 1e85f320695d (9 revisions) flutter/flutter#93694

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 16, 2021

45f87b9 [in_app_purchase] Add support for promotional offers through …

ea1c157

…Store-Kit wrappers (flutter/plugins#4458)

mvanbeusekom deleted the issue/92231 branch November 17, 2021 14:00

mvanbeusekom mentioned this pull request Nov 17, 2021

[in_app_purchase] Fix CHANGELOG for 0.2.0. #4517

Merged

11 tasks

amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021

[in_app_purchase] Add support for promotional offers through Store-Ki…

e2b178c

…t wrappers (flutter#4458)

KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021

[in_app_purchase] Add support for promotional offers through Store-Ki…

eba4077

…t wrappers (flutter#4458)

This was referenced Jul 13, 2022

[in-app-purchase][iOS] Promotional Offers in iOS not working flutter/flutter#107533

Closed

fix payment discount (iOS Promotional Offers) into being actually usable #6099

Closed

huycozy mentioned this pull request Mar 29, 2024

[in_app_purchase] How to Use Promotional Offers for iOS Subscriptions flutter/flutter#145898

Closed

	// Creates an instance of the SKPaymentDiscount class based on the supplied disctionary.
	// Creates an instance of the SKPaymentDiscount class based on the supplied dictionary.

	List<SKProductDiscountWrapper>? discounts,
	this.discounts = <SKProductDiscountWrapper>[],

	/// no promotional offers have been set up this field returns an empty list.
	/// no promotional offers have been set up, this field returns an empty list.

[in_app_purchase] Add support for promotional offers through Store-Kit wrappers #4458

[in_app_purchase] Add support for promotional offers through Store-Kit wrappers #4458

Uh oh!

Conversation

mvanbeusekom commented Oct 29, 2021 • edited by cyanglaz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

cyanglaz 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!

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvanbeusekom Nov 3, 2021 • 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!

mvanbeusekom commented Nov 4, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvanbeusekom Nov 8, 2021 • 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

mvanbeusekom commented Oct 29, 2021 •

edited by cyanglaz

Loading

mvanbeusekom Nov 3, 2021 •

edited

Loading

mvanbeusekom Nov 8, 2021 •

edited

Loading