`PaymentQueueWrapper`: handle promotional purchase requests from App Store when SK1 is disabled by NachoSoto · Pull Request #1901 · RevenueCat/purchases-ios

NachoSoto · 2022-09-10T18:35:59Z

Follow up to #1882. After that PR, StoreKit1Wrapper was no longer listening to SKPaymentQueueDelegate.paymentQueue(:shouldAddStorePayment:for:), which means that purchases initiated from the App Store were being ignored by the SDK if SK2 is enabled.

To fix this, PaymentQueueWrapper now becomes the SKPaymentQueueDelegate only when StoreKit1Wrapper is not initialized, and PurchasesOrchestrator initiates an SK2 purchase instead.

Other changes:

PurchasesOrchestrator purchase methods no longer need the whole PromotionalOffer, only the SignedData. This allows the new implementation to create that data from only the SKPaymentDiscount provided from the SKPayment.
Added tests for PromotionalOffer.SignedData

TODO:

Log warning if fetching product failed.
Handle discounts
Test that StoreKit1Wrapper is the delegate if SK1 is enabled
Test that PaymentQueueWrapper is the delegate otherwise
Test PromotionalOffer.SignedData initialization from SK1

NachoSoto · 2022-09-10T18:39:13Z

This is not a precondition so it will be ignored in release builds. Still useful to catch potential misconfigurations in tests.

Should we also log an error if this gets called for any reason with SK1 in release builds?

Yeah that's a good idea.

NachoSoto · 2022-09-12T16:47:33Z

I'll finish this today.

tonidero

LGTM but will leave to people more familiar with this to do another review.

tonidero · 2022-09-13T08:31:49Z

Should we also log an error if this gets called for any reason with SK1 in release builds?

…Store when SK1 is disabled Follow up to #1882. After that PR, `StoreKit1Wrapper` was no longer listening to `SKPaymentQueueDelegate.paymentQueue(:shouldAddStorePayment:for:)`, which means that purchases initiated from the App Store were being ignored by the SDK if SK2 is enabled. To fix this, `PaymentQueueWrapper` now becomes the `SKPaymentQueueDelegate` only when `StoreKit1Wrapper` is not initialized, and `PurchasesOrchestrator` initiates an SK2 purchase instead. - `PurchasesOrchestrator` purchase methods no longer need the whole `PromotionalOffer`, only the `SignedData`. This allows the new implementation to create that data from only the `SKPaymentDiscount` provided from the `SKPayment`. - [ ] Test that `StoreKit1Wrapper` is the delegate if SK1 is enabled - [ ] Test that `PaymentQueueWrapper` is the delegate otherwise - [ ] Test `PromotionalOffer.SignedData` initialization from SK1 - [ ] Test conversion of SK1 product to SK2.

@NachoSoto

**This is an automatic release.** ### New Features * 🚨 `StoreKit 2` is now enabled by default 🚨 (#1922) via NachoSoto (@NachoSoto) * Extracted `PurchasesType` and `PurchasesSwiftType` (#1912) via NachoSoto (@NachoSoto) ### Bugfixes * `StoreKit 1`: changed result of cancelled purchases to be consistent with `StoreKit 2` (#1910) via NachoSoto (@NachoSoto) * `PaymentQueueWrapper`: handle promotional purchase requests from App Store when SK1 is disabled (#1901) via NachoSoto (@NachoSoto) ### Other Changes * Fixed iOS 12 tests (#1936) via NachoSoto (@NachoSoto) * `CacheableNetworkOperation`: fixed race condition in new test (#1932) via NachoSoto (@NachoSoto) * `BasePurchasesTests`: changed default back to SK1 (#1935) via NachoSoto (@NachoSoto) * `Logger`: refactored default `LogLevel` definition (#1934) via NachoSoto (@NachoSoto) * `AppleReceipt`: refactored declarations into nested types (#1933) via NachoSoto (@NachoSoto) * `Integration Tests`: relaunch tests when retrying failures (#1925) via NachoSoto (@NachoSoto) * `CircleCI`: downgraded release jobs to Xcode 13.x (#1927) via NachoSoto (@NachoSoto) * `ErrorUtils`: added test to verify that `PublicError`s can be `catch`'d as `ErrorCode` (#1924) via NachoSoto (@NachoSoto) * `StoreKitIntegrationTests`: print `AppleReceipt` data whenever `verifyEntitlementWentThrough` fails (#1929) via NachoSoto (@NachoSoto) * `OperationQueue`: log debug message when requests are found in cache and skipped (#1926) via NachoSoto (@NachoSoto) * `GetCustomerInfoAPI`: avoid making a request if there's any `PostReceiptDataOperation` in progress (#1911) via NachoSoto (@NachoSoto) * `PurchaseTester`: allow HTTP requests and enable setting `ProxyURL` (#1917) via NachoSoto (@NachoSoto)

…aymentQueueDelegate` implementations This way it's more clear which methods belong to which protocol. This confusion is what lead to #1901 being wrong.

… to fix promoted purchases Follow up to #1901. That fix was incomplete. I missed it because the protocols were mixed up (fixed in #1961). Now `PaymentQueueWrapper`, when set up as the delegate (only when SK2 is enabled and SK1 is disabled) will be added as a transaction observer.

…aymentQueueDelegate` implementations (#1961) This way it's more clear which methods belong to which protocol. This confusion is what lead to #1901 being wrong (will fix in a separate PR).

… to fix promoted purchases Follow up to #1901. That fix was incomplete. I missed it because the protocols were mixed up (fixed in #1961). Now `PaymentQueueWrapper`, when set up as the delegate (only when SK2 is enabled and SK1 is disabled) will be added as a transaction observer.

… to fix promoted purchases (#1962) Follow up to #1901. That fix was incomplete. I missed it because the protocols were mixed up (fixed in #1961). Now `PaymentQueueWrapper`, when set up as the delegate (only when SK2 is enabled and SK1 is disabled) will be added as a transaction observer.

…e` wrapper This is a follow up to #1901 and #1882. Essentially before this change, `Purchases` had both `StoreKit1Wrapper?` and a `PaymentQueueWrapper`. The later wasn't actually needed when the first was not-nil, so it was essentially storing `(a?, b?)`. What we really wanted to express is that it could have one OR the other. This introduces `Either` to express this requirement in the type system, as well as `PaymentQueueWrapperType` that will be used to fix #1964.

…e` wrapper (#1968) This is a follow up to #1901 and #1882. Depends on #1967. Essentially before this change, `Purchases` had both `StoreKit1Wrapper?` and a `PaymentQueueWrapper`. The later wasn't actually needed when the first was not-nil, so it was essentially storing `(a?, b?)`. What we really wanted to express is that it could have one OR the other. This introduces `Either` to express this requirement in the type system, as well as `PaymentQueueWrapperType` that will be used to fix #1964.

NachoSoto added the pr:fix A bug fix label Sep 10, 2022

NachoSoto requested review from a team, aboedo and joshdholtz September 10, 2022 18:35

NachoSoto force-pushed the promotional-app-store-purchase-sk1 branch from 3e896bb to e5119ec Compare September 10, 2022 18:38

NachoSoto commented Sep 10, 2022

View reviewed changes

NachoSoto force-pushed the promotional-app-store-purchase-sk1 branch from 87d8b60 to 1d9cfc3 Compare September 13, 2022 00:14

NachoSoto marked this pull request as ready for review September 13, 2022 00:14

NachoSoto force-pushed the promotional-app-store-purchase-sk1 branch 2 times, most recently from 5bec134 to 0cae177 Compare September 13, 2022 00:22

tonidero reviewed Sep 13, 2022

View reviewed changes

NachoSoto force-pushed the promotional-app-store-purchase-sk1 branch 2 times, most recently from b9623f8 to 4e2f0d9 Compare September 15, 2022 18:01

NachoSoto added 6 commits September 19, 2022 11:02

Added tests

42c4325

Fixed build

afa91a7

Documented return false

2a014af

Removed unused method

e4d513e

Fixed release build

804f0ac

NachoSoto force-pushed the promotional-app-store-purchase-sk1 branch from 4e2f0d9 to 804f0ac Compare September 19, 2022 18:03

Added log

f21674b

aboedo approved these changes Sep 21, 2022

View reviewed changes

NachoSoto merged commit 72a9f57 into main Sep 21, 2022

NachoSoto deleted the promotional-app-store-purchase-sk1 branch September 21, 2022 19:03

NachoSoto mentioned this pull request Sep 23, 2022

[AUTOMATIC] Release/4.13.0 #1938

Merged

NachoSoto mentioned this pull request Sep 30, 2022

StoreKit1Wrapper: separated SKPaymentTransactionObserver and SKPaymentQueueDelegate implementations #1961

Merged

NachoSoto mentioned this pull request Sep 30, 2022

PaymentQueueWrapper: also conform to SKPaymentTransactionObserver to fix promoted purchases #1962

Merged

NachoSoto mentioned this pull request Oct 7, 2022

PaymentQueueWrapper: improved abstraction for active SKPaymentQueue wrapper #1968

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`PaymentQueueWrapper`: handle promotional purchase requests from App Store when SK1 is disabled#1901

`PaymentQueueWrapper`: handle promotional purchase requests from App Store when SK1 is disabled#1901
NachoSoto merged 7 commits into
mainfrom
promotional-app-store-purchase-sk1

NachoSoto commented Sep 10, 2022 •

edited

Loading

Uh oh!

NachoSoto Sep 10, 2022

Uh oh!

tonidero Sep 13, 2022

Uh oh!

NachoSoto Sep 15, 2022

Uh oh!

NachoSoto commented Sep 12, 2022

Uh oh!

tonidero left a comment

Uh oh!

tonidero Sep 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

NachoSoto commented Sep 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Other changes:

TODO:

Uh oh!

NachoSoto Sep 10, 2022

Choose a reason for hiding this comment

Uh oh!

tonidero Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

NachoSoto Sep 15, 2022

Choose a reason for hiding this comment

Uh oh!

NachoSoto commented Sep 12, 2022

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

tonidero Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented Sep 10, 2022 •

edited

Loading