PurchasesOrchestrator: fix behavior dealing with nil SKPaymentTransaction.productIdentifier during purchase#1680
Conversation
There was a problem hiding this comment.
This currently fails.
There was a problem hiding this comment.
Creating the error already logs it, so this was duplicated. The new code relies on the String without hardcoding another error message.
ff15467 to
dd4d84b
Compare
nil SKPaymentTransaction.productIdentifier during purchasePurchasesOrchestrator: fix behavior dealing with nil SKPaymentTransaction.productIdentifier during purchase
There was a problem hiding this comment.
StoreKitWorkarounds actually made the whole purchase hang.
|
This is ready for review. |
taquitos
left a comment
There was a problem hiding this comment.
Slight wording suggestion, but 👍
aboedo
left a comment
There was a problem hiding this comment.
I left a question, lmk if I'm reading it wrong
|
What's the question? 🤓 |
|
LOL not sure if it didn't get posted or I'm going nuts, but I'll re-post the question, just a second |
There was a problem hiding this comment.
why are we ignoring the product identifier from the SKProduct now?
The order should be:
try getting the product id from the product, then from the payment, and then if we still have nothing, raise an error
There was a problem hiding this comment.
That's explained in the PR description 😇
There was a problem hiding this comment.
Basically that was nice, the problem is when the transaction comes back, we only have that, and if the identifier there is nil we never called the callback. So the test I wrote (for example) hung forever.
So because we didn't have test coverage for that workaround we didn't realize that it was enough to begin the purchase, but then we can't complete it.
There was a problem hiding this comment.
I read the description but my brain didn't process it 🤦♂️ my apologies.
That makes total sense. I keep thinking that there has to be something that we can do to improve this case, but ultimately I feel like the solution will be to start moving people to SK2 as it matures.
In the meantime, I do think we should have an extensive comment here, since it won't be immediately obvious to folks maintaining this
There was a problem hiding this comment.
but ultimately I feel like the solution will be to start moving people to SK2 as it matures.
Agreed.
In the meantime, I do think we should have an extensive comment here, since it won't be immediately obvious to folks maintaining this
Good call, adding.
|
re-added the comment here https://github.com/RevenueCat/purchases-ios/pull/1680/files#r901905200 |
There was a problem hiding this comment.
I read the description but my brain didn't process it 🤦♂️ my apologies.
That makes total sense. I keep thinking that there has to be something that we can do to improve this case, but ultimately I feel like the solution will be to start moving people to SK2 as it matures.
In the meantime, I do think we should have an extensive comment here, since it won't be immediately obvious to folks maintaining this
…ansaction.productIdentifier` during purchase Fixes [CSDK-119]. `PurchasesOrchestrator` uses product identifiers for callback keys. Previously, `extractProductIdentifier` was allowing `SKPayment.productIdentifier` to be `nil`, as long as the product had an identifier. However, as seen in the newly added test, this resulted in a purchase call that never invoked its callback, because when the transaction would come back, it wouldn't know which callback to use. Because of this, there is no point starting the purchase if we can't retrieve the product identifier from the transaction, so now we fail early with an error instead of hanging forever.
a82b8a1 to
4b5b3bf
Compare
### Changes: * Replaced `CustomerInfo.nonSubscriptionTransactions` with a new non-`StoreTransaction` type (#1733) via NachoSoto (@NachoSoto) * `Purchases.configure`: added overload taking a `Configuration.Builder` (#1720) via NachoSoto (@NachoSoto) * Extract Attribution logic out of Purchases (#1693) via Joshua Liebowitz (@taquitos) * Remove create alias (#1695) via Joshua Liebowitz (@taquitos) All attribution APIs can now be accessed from `Purchases.shared.attribution`. ### Improvements: * Improved purchasing logs, added promotional offer information (#1725) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: don't log attribute errors if there are none (#1742) via NachoSoto (@NachoSoto) * `FatalErrorUtil`: don't override `fatalError` on release builds (#1736) via NachoSoto (@NachoSoto) * `SKPaymentTransaction`: added more context to warnings about missing properties (#1731) via NachoSoto (@NachoSoto) * New SwiftUI Purchase Tester example (#1722) via Josh Holtz (@joshdholtz) * update docs for `showManageSubscriptions` (#1729) via aboedo (@aboedo) * `PurchasesOrchestrator`: unify finish transactions between SK1 and SK2 (#1704) via NachoSoto (@NachoSoto) * `SubscriberAttribute`: converted into `struct` (#1648) via NachoSoto (@NachoSoto) * `CacheFetchPolicy.notStaleCachedOrFetched`: added warning to docstring (#1708) via NachoSoto (@NachoSoto) * Clear cached offerings and products after Storefront changes (2/4) (#1583) via Juanpe Catalán (@Juanpe) * `ROT13`: optimized initialization and removed magic numbers (#1702) via NachoSoto (@NachoSoto) ### Fixes: * `logIn`/`logOut`: sync attributes before aliasing (#1716) via NachoSoto (@NachoSoto) * `Purchases.customerInfo(fetchPolicy:)`: actually use `fetchPolicy` parameter (#1721) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: fix behavior dealing with `nil` `SKPaymentTransaction.productIdentifier` during purchase (#1680) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator.handlePurchasedTransaction`: always refresh receipt data (#1703) via NachoSoto (@NachoSoto)
Fixes CSDK-119.
PurchasesOrchestratoruses product identifiers for callback keys.Previously,
extractProductIdentifierwas allowingSKPayment.productIdentifierto benil, as long as the product had an identifier.However, as seen in the newly added test, this resulted in a purchase call that never invoked its callback, because when the transaction would come back, it wouldn't know which callback to use.
Because of this, there is no point starting the purchase if we can't retrieve the product identifier from the transaction, so now we fail early with an error instead of hanging forever.