Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[in_app_purchase][iOS] fix iOS promotional offers (SKPaymentDiscountWrapper was not used properly) #6541

Merged
merged 12 commits into from
Oct 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.3.3

* Supports adding discount information to AppStorePurchaseParam.
* Fixes iOS Promotional Offers bug which prevents them from working.

## 0.3.2+2

* Updates imports for `prefer_relative_imports`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -390,4 +390,27 @@ - (void)testSKPaymentDiscountFromMapMissingTimestamp {
}
}

- (void)testSKPaymentDiscountFromMapOverflowingTimestamp {
if (@available(iOS 12.2, *)) {
NSDictionary *discountMap = @{
@"identifier" : @"payment_discount_identifier",
@"keyIdentifier" : @"payment_discount_key_identifier",
@"nonce" : @"d18981e0-9003-4365-98a2-4b90e3b62c52",
@"signature" : @"this is a encrypted signature",
@"timestamp" : @1665044583595, // timestamp 2022 Oct
};
NSString *error = nil;
SKPaymentDiscount *paymentDiscount =
[FIAObjectTranslator getSKPaymentDiscountFromMap:discountMap withError:&error];
XCTAssertNil(error);
XCTAssertNotNil(paymentDiscount);
XCTAssertEqual(paymentDiscount.identifier, discountMap[@"identifier"]);
XCTAssertEqual(paymentDiscount.keyIdentifier, discountMap[@"keyIdentifier"]);
XCTAssertEqualObjects(paymentDiscount.nonce,
[[NSUUID alloc] initWithUUIDString:discountMap[@"nonce"]]);
XCTAssertEqual(paymentDiscount.signature, discountMap[@"signature"]);
XCTAssertEqual(paymentDiscount.timestamp, discountMap[@"timestamp"]);
}
}

@end
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ + (SKPaymentDiscount *)getSKPaymentDiscountFromMap:(NSDictionary *)map
return nil;
}

if (!timestamp || ![timestamp isKindOfClass:NSNumber.class] || [timestamp intValue] <= 0) {
if (!timestamp || ![timestamp isKindOfClass:NSNumber.class] || [timestamp longLongValue] <= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch. Can we add an XCUnitTest for this? Maybe have the timestamp to be a very big long long number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've added XCUnitTest using current timestamp number value. It's enough to cause the intValue overflow in negative number. If I use really big long long number, the intValue overflow goes positive again and passes the test. I'm not sure of the usefulness of this "not negative int" check in the original code.

if (error) {
*error = @"When specifying a payment discount the 'timestamp' field is mandatory.";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,11 @@ - (void)addPayment:(FlutterMethodCall *)call result:(FlutterResult)result {
: [simulatesAskToBuyInSandbox boolValue];

if (@available(iOS 12.2, *)) {
NSDictionary *paymentDiscountMap = [self getNonNullValueFromDictionary:paymentMap
forKey:@"paymentDiscount"];
NSString *error = nil;
SKPaymentDiscount *paymentDiscount = [FIAObjectTranslator
getSKPaymentDiscountFromMap:[paymentMap objectForKey:@"paymentDiscount"]
withError:&error];
SKPaymentDiscount *paymentDiscount =
[FIAObjectTranslator getSKPaymentDiscountFromMap:paymentDiscountMap withError:&error];

if (error) {
result([FlutterError
Expand Down Expand Up @@ -367,6 +368,11 @@ - (void)showPriceConsentIfNeeded:(FlutterResult)result {
result(nil);
}

- (id)getNonNullValueFromDictionary:(NSDictionary *)dictionary forKey:(NSString *)key {
id value = dictionary[key];
return [value isKindOfClass:[NSNull class]] ? nil : value;
}

#pragma mark - transaction observer:

- (void)handleTransactionsUpdated:(NSArray<SKPaymentTransaction *> *)transactions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ class InAppPurchaseStoreKitPlatform extends InAppPurchasePlatform {
purchaseParam is AppStorePurchaseParam ? purchaseParam.quantity : 1,
applicationUsername: purchaseParam.applicationUserName,
simulatesAskToBuyInSandbox: purchaseParam is AppStorePurchaseParam &&
purchaseParam.simulatesAskToBuyInSandbox));
purchaseParam.simulatesAskToBuyInSandbox,
paymentDiscount: purchaseParam is AppStorePurchaseParam
Copy link
Contributor

Choose a reason for hiding this comment

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

We should test that the paymentDiscount is encoded and sent to method channel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see your point, that in fact the bug was that the discount parameter was not sent trough the method channel. But I am not sure on how to construct such a test?
In my understanding: now the discount is send trough AppStorePurchaseParam in the buyNonConsumable method, which is not returning a useful result to test. This method constructs SKPaymentWrapper with a discount parameter and sends it to _skPaymentQueueWrapper.addPayment which turns the SKPaymentWrapper to a map (which we test in the newly created test for the toMap method) and sends it trough the method channel. But the channel is not returning useful result for testing either. The encoding of the paymentDiscount is in the toMap method which we test now.
Can you elaborate please?

Copy link
Contributor

Choose a reason for hiding this comment

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

You might be able to intercept the method call in the FakeStoreKitPlatform and inspect the value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got your idea. Thanks! Done in the last commit.

? purchaseParam.discount
: null));

return true; // There's no error feedback from iOS here to return.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ class SKPaymentWrapper {
'applicationUsername': applicationUsername,
'requestData': requestData,
'quantity': quantity,
'simulatesAskToBuyInSandbox': simulatesAskToBuyInSandbox
'simulatesAskToBuyInSandbox': simulatesAskToBuyInSandbox,
'paymentDiscount': paymentDiscount?.toMap(),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have some test for the toMap method, can we update the test to include the discount? If we don't have an existing test, do you mind adding a test for the toMap method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've added a new test checking the values of the SKPaymentWrapper object and the result of the toMap method explicitly.

};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class AppStorePurchaseParam extends PurchaseParam {
String? applicationUserName,
this.quantity = 1,
this.simulatesAskToBuyInSandbox = false,
this.discount,
}) : super(
productDetails: productDetails,
applicationUserName: applicationUserName,
Expand All @@ -32,4 +33,7 @@ class AppStorePurchaseParam extends PurchaseParam {

/// Quantity of the product user requested to buy.
final int quantity;

/// Discount applied to the product. The value is `null` when the product does not have a discount.
final SKPaymentDiscountWrapper? discount;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: in_app_purchase_storekit
description: An implementation for the iOS platform of the Flutter `in_app_purchase` plugin. This uses the StoreKit Framework.
repository: https://github.com/flutter/plugins/tree/main/packages/in_app_purchase/in_app_purchase_storekit
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+in_app_purchase%22
version: 0.3.2+2
version: 0.3.3
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bugfix, so the version should be 0.3.2+3.

Copy link
Contributor

Choose a reason for hiding this comment

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

This adds a new field to the AppStorePurchaseParam so it should be a new feature. The PR description or CHANGELOG should probably be something like "Supports adding discount information to AppStorePurchaseParam", instead of "fixing".

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Oct 12, 2022

Choose a reason for hiding this comment

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

Thanks for the correction there; I didn't look closely enough at the details and thought it was just populating something that was previously added but not wired up.


environment:
sdk: ">=2.14.0 <3.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class FakeStoreKitPlatform {
PlatformException? restoreException;
SKError? testRestoredError;
bool queueIsActive = false;
Map<String, dynamic> discountReceived = <String, dynamic>{};

void reset() {
transactions = <SKPaymentTransactionWrapper>[];
Expand All @@ -54,6 +55,7 @@ class FakeStoreKitPlatform {
restoreException = null;
testRestoredError = null;
queueIsActive = false;
discountReceived = <String, dynamic>{};
}

SKPaymentTransactionWrapper createPendingTransaction(String id,
Expand Down Expand Up @@ -169,6 +171,18 @@ class FakeStoreKitPlatform {
case '-[InAppPurchasePlugin addPayment:result:]':
final String id = call.arguments['productIdentifier'] as String;
final int quantity = call.arguments['quantity'] as int;

// Keep the received paymentDiscount parameter when testing payment with discount.
if (call.arguments['applicationUsername'] == 'userWithDiscount') {
if (call.arguments['paymentDiscount'] != null) {
final Map<dynamic, dynamic> discountArgument =
call.arguments['paymentDiscount'];
discountReceived = discountArgument.cast<String, dynamic>();
} else {
discountReceived = <String, dynamic>{};
}
}

final SKPaymentTransactionWrapper transaction =
createPendingTransaction(id, quantity: quantity);
transactions.add(transaction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,38 @@ void main() {
expect(
fakeStoreKitPlatform.finishedTransactions.first.payment.quantity, 5);
});

test(
'buying non consumable with discount, should get purchase objects in the purchase update callback',
() async {
final List<PurchaseDetails> details = <PurchaseDetails>[];
final Completer<List<PurchaseDetails>> completer =
Completer<List<PurchaseDetails>>();
final Stream<List<PurchaseDetails>> stream =
iapStoreKitPlatform.purchaseStream;

late StreamSubscription<List<PurchaseDetails>> subscription;
subscription = stream.listen((List<PurchaseDetails> purchaseDetailsList) {
details.addAll(purchaseDetailsList);
if (purchaseDetailsList.first.status == PurchaseStatus.purchased) {
completer.complete(details);
subscription.cancel();
}
});
final AppStorePurchaseParam purchaseParam = AppStorePurchaseParam(
productDetails:
AppStoreProductDetails.fromSKProduct(dummyProductWrapper),
applicationUserName: 'userWithDiscount',
discount: dummyPaymentDiscountWrapper,
);
await iapStoreKitPlatform.buyNonConsumable(purchaseParam: purchaseParam);

final List<PurchaseDetails> result = await completer.future;
expect(result.length, 2);
expect(result.first.productID, dummyProductWrapper.productIdentifier);
expect(fakeStoreKitPlatform.discountReceived,
dummyPaymentDiscountWrapper.toMap());
});
});

group('complete purchase', () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ void main() {
expect(payment, equals(dummyPayment));
});

test('SKPaymentWrapper should have propery values consistent with .toMap()',
() {
final Map<String, dynamic> mapResult = dummyPaymentWithDiscount.toMap();
expect(mapResult['productIdentifier'],
dummyPaymentWithDiscount.productIdentifier);
expect(mapResult['applicationUsername'],
dummyPaymentWithDiscount.applicationUsername);
expect(mapResult['requestData'], dummyPaymentWithDiscount.requestData);
expect(mapResult['quantity'], dummyPaymentWithDiscount.quantity);
expect(mapResult['simulatesAskToBuyInSandbox'],
dummyPaymentWithDiscount.simulatesAskToBuyInSandbox);
expect(mapResult['paymentDiscount'],
equals(dummyPaymentWithDiscount.paymentDiscount?.toMap()));
});

test('Should construct correct SKError from json', () {
final SKError error = SKError.fromJson(buildErrorMap(dummyError));
expect(error, equals(dummyError));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ const SKPaymentWrapper dummyPayment = SKPaymentWrapper(
requestData: 'fake-data-utf8',
quantity: 2,
simulatesAskToBuyInSandbox: true);

final SKPaymentWrapper dummyPaymentWithDiscount = SKPaymentWrapper(
productIdentifier: 'prod-id',
applicationUsername: 'app-user-name',
requestData: 'fake-data-utf8',
quantity: 2,
simulatesAskToBuyInSandbox: true,
paymentDiscount: dummyPaymentDiscountWrapper);

const SKError dummyError = SKError(
code: 111,
domain: 'dummy-domain',
Expand Down Expand Up @@ -186,3 +195,12 @@ Map<String, dynamic> buildTransactionMap(
};
return map;
}

final SKPaymentDiscountWrapper dummyPaymentDiscountWrapper =
SKPaymentDiscountWrapper.fromJson(const <String, dynamic>{
'identifier': 'dummy-discount-identifier',
'keyIdentifier': 'KEYIDTEST1',
'nonce': '00000000-0000-0000-0000-000000000000',
'signature': 'dummy-signature-string',
'timestamp': 1231231231,
});