-
Notifications
You must be signed in to change notification settings - Fork 317
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Introduce PurchaseParams to allow passing extra configuration info when making a purchase #4400
base: main
Are you sure you want to change the base?
Conversation
if let metadata = metadata { | ||
message += " with metadata: \(metadata)" | ||
} | ||
return message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took the opportunity to simplify the log message to it can be conditionally composed depending on the optional parameters instead of creating a new message for every combination.
@@ -61,6 +61,9 @@ extension CustomerInfoResponse { | |||
@IgnoreDecodeErrors<PurchaseOwnershipType> | |||
var ownershipType: PurchaseOwnershipType | |||
var productPlanIdentifier: String? | |||
#if ENABLE_PURCHASE_PARAMS | |||
var metadata: [String: String]? | |||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the metadata information is only present in the Subscription
object in the customerInfo.subscriber.subscriptions
array. This is not exposed as public API, just there so we can write the integration test.
The way we will expose the metadata in the SDK is to be decided.
* Purchases.shared.purchase(package: package, params: params) | ||
* ``` | ||
*/ | ||
@objc(RCPurchaseParams) public final class PurchaseParams: NSObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opted for using the builder pattern instead of using a Set like in StoreKit's PurchaseOption as well as calling it PurchaseParams
so it more closely resembles our Android APIs. Additionally, we already use the builder pattern in the Configuration
class so it's consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builder pattern++ 😄
@@ -315,43 +315,16 @@ final class PurchasesOrchestrator { | |||
|
|||
func purchase(product: StoreProduct, | |||
package: Package?, | |||
completion: @escaping PurchaseCompletedBlock) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
took the opportunity to simplify a bit the purchase methods in PurchaseOrchestrator
and removed the method to purchase a package without promotionalOffer
, instead opting to make promotionalOffer
and metadata
optional in the method below and handling the routing to SK1 and SK2 appropriately.
@RCGitBot please test |
#if ENABLE_PURCHASE_PARAMS | ||
|
||
@objc(purchaseProduct:params:withCompletion:) | ||
func purchase(product: StoreProduct, params: PurchaseParams, completion: @escaping PurchaseCompletedBlock) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering, should the StoreProduct
/Package
be part of the PurchaseParams
? We can create multiple Builder
constructors, one for each of these, with the mandatory parameters. FWIW, this is how it works in Android.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of simplifying the purchase methods and including the StoreProduct/Package
be part of the PurchaseParams
. However I don't think there are any mandatory parameters beyond the product/package itself. All the rest are optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that maybe we will need a third once we support win back offers? cc @fire-at-will. And mostly this allows us to have a single purchase entry point and the rest being implementation details inside the PurchaseParams
. If there is a reason to want to keep different purchase methods I would be ok, but otherwise I feel that's a cleaner API IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added an initialiser with product and another with package to PurchaseParams.Builder
and removed it from the purchase method. Makes it much cleaner and can help us add better validation inside PurchaseParams
👍
*/ | ||
@objc(RCPurchaseParams) public final class PurchaseParams: NSObject { | ||
|
||
let promotionalOffer: PromotionalOffer? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how I feel about having each offer type as a separate purchase param here because it allows people to potentially attach multiple offers to a purchase. Technically, SK2's purchaseOptions Set API allows for this, but I feel like it'd probably throw an error. I wouldn't want someone to do something like this:
let params = PurchaseParams.Builder()
.with(winBackOffer: winBackOffer)
.with(promotionalOffer: promotionalOffer)
.build()
Purchases.shared.purchase(package: package, params: params)
Perhaps it might be better to prohibit this at the API level by having some sort of wrapping Offer
struct or enum that only allows you to provide one? Of course, this has its downsides:
- Another layer of abstraction
- Enums with associated values are really tricky to deal with in the hybrids
If we want to avoid this additional struct/enum, maybe we could write a validate()
function on PurchaseParams that checks that only one offer is included, and call that before making the purchase.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have StoreProductDiscount which wraps StoreKit's SubcriptionOffer. We will probably have to add for the winBack
type to it when we add manual winBackOffer
purchasing.
Unfortunately, the SubscriptionOffer
object does not contain the signed data required to purchase a promo offer. That's why we have a method to create a PromotionalOffer
from a StoreProductDiscount
and we need a separate .with(promotionalOffer: promotionalOffer)
method that accepts it.
It would be great if we could have just a .with(storeProductDiscount: storeProductDiscount)
method that could work for all types of offers tho.
- For intro offers, it would be applied automatically, so we wouldn't need to do anything with it.
- For promo offers, we would generate the signature and perform the purchase, or throw an error if the signature is not valid or the customer is not eligible.
- For wingback offers, we would have already validated eligibility via
eligibleWinBackOfferIDs
, or otherwise the purchase method will just return an error.
Another option could be just specifying the priority order in which the offers will be applied, so e.g. if you include a winback offer and a promo offer, the winback will be applied and the promo offer will be ignored.
* Purchases.shared.purchase(package: package, params: params) | ||
* ``` | ||
*/ | ||
@objc(RCPurchaseParams) public final class PurchaseParams: NSObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Builder pattern++ 😄
|
||
@objc(purchasePackage:params:withCompletion:) | ||
func purchase(package: Package, params: PurchaseParams, completion: @escaping PurchaseCompletedBlock) { | ||
purchasesOrchestrator.purchase(product: package.storeProduct, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose this would be the best place to switch to the different purchase() functions based on the present subscription offer once we support different offer types in the purchase params?
Just thinking ahead to when we support win-back offers :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just add an extra optional parameter to the existing purchase methods in PurchasesOrchestrator like I did here for metadata:
…en making a purchase
…rom Local.xcconfig
2130c42
to
b27e5af
Compare
Current status:
Basically I focused on implementing the public API and making it possible to test the end-to-end flow using PurchaseTester.
The API looks like this:
Checklist
purchases-android
and hybridsMotivation
Description