PaymentQueueWrapper: handle promotional purchase requests from App Store when SK1 is disabled
#1901
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NachoSoto
force-pushed
the
promotional-app-store-purchase-sk1
branch
from
3e896bb
to
e5119ec
Compare
NachoSoto
commented
Sep 10, 2022
| // when `StoreKit1Wrapper` is not initialized, which means that promoted purchases | ||
| // need to be handled as a SK2 purchase. | ||
| // This method converts the `SKPayment` into an SK2 purchase by fetching the product again. | ||
| assert(self.storeKit1Wrapper == nil, "This method should not be invoked if SK1 is enabled") |
There was a problem hiding this comment.
This is not a precondition so it will be ignored in release builds. Still useful to catch potential misconfigurations in tests.
There was a problem hiding this comment.
Should we also log an error if this gets called for any reason with SK1 in release builds?
There was a problem hiding this comment.
Yeah that's a good idea.
|
I'll finish this today. |
NachoSoto
force-pushed
the
promotional-app-store-purchase-sk1
branch
from
87d8b60
to
1d9cfc3
Compare
NachoSoto
force-pushed
the
promotional-app-store-purchase-sk1
branch
2 times, most recently
from
5bec134
to
0cae177
Compare
tonidero
reviewed
Sep 13, 2022
| // when `StoreKit1Wrapper` is not initialized, which means that promoted purchases | ||
| // need to be handled as a SK2 purchase. | ||
| // This method converts the `SKPayment` into an SK2 purchase by fetching the product again. | ||
| assert(self.storeKit1Wrapper == nil, "This method should not be invoked if SK1 is enabled") |
There was a problem hiding this comment.
Should we also log an error if this gets called for any reason with SK1 in release builds?
Tests/UnitTests/Purchasing/StoreKitAbstractions/PromotionalOfferTests.swift
Outdated
Show resolved
Hide resolved
NachoSoto
force-pushed
the
promotional-app-store-purchase-sk1
branch
2 times, most recently
from
b9623f8
to
4e2f0d9
Compare
…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
force-pushed
the
promotional-app-store-purchase-sk1
branch
from
4e2f0d9
to
804f0ac
Compare
aboedo
approved these changes
Sep 21, 2022
NachoSoto
added a commit
that referenced
this pull request
Sep 27, 2022
**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)
NachoSoto
added a commit
that referenced
this pull request
Sep 30, 2022
…aymentQueueDelegate` implementations This way it's more clear which methods belong to which protocol. This confusion is what lead to #1901 being wrong.
NachoSoto
added a commit
that referenced
this pull request
Oct 7, 2022
…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.
NachoSoto
added a commit
that referenced
this pull request
Oct 10, 2022
…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.
NachoSoto
added a commit
that referenced
this pull request
Oct 11, 2022
…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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow up to #1882. After that PR,
StoreKit1Wrapperwas no longer listening toSKPaymentQueueDelegate.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,
PaymentQueueWrappernow becomes theSKPaymentQueueDelegateonly whenStoreKit1Wrapperis not initialized, andPurchasesOrchestratorinitiates an SK2 purchase instead.Other changes:
PurchasesOrchestratorpurchase methods no longer need the wholePromotionalOffer, only theSignedData. This allows the new implementation to create that data from only theSKPaymentDiscountprovided from theSKPayment.PromotionalOffer.SignedDataTODO:
StoreKit1Wrapperis the delegate if SK1 is enabledPaymentQueueWrapperis the delegate otherwisePromotionalOffer.SignedDatainitialization from SK1