Paywalls: new .onPurchaseStarted(package) modifier
#3693
Conversation
vegaro
changed the title
Paywalls: new .onPurchaseStarted(package) modifier
vegaro
force-pushed
the
purchase-started
branch
from
216b956
to
c8e9e65
Compare
While working on #3693 I noticed we were missing some API tests that would be helpful for that PR
vegaro
force-pushed
the
purchase-started
branch
2 times, most recently
from
3cd5f28
to
d3b028d
Compare
Since we are deprecating the old function in https://github.com/RevenueCat/purchases-ios/pull/3693/files I thought it will be simpler to just add both `restoreStarted` and the new `purchaseStarted(package:)` in the same release. We haven't made a release since I merged #3694 so these are safe to remove for now. I open this PR in case we want to make a release before #3693 is finished.
vegaro
force-pushed
the
purchase-started
branch
3 times, most recently
from
41d9b2d
to
d13cc83
Compare
vegaro
force-pushed
the
purchase-started
branch
from
ca20c3f
to
2cccdc3
Compare
| public func presentPaywallIfNeeded( | ||
| requiredEntitlementIdentifier: String, | ||
| offering: Offering? = nil, | ||
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
Made this non nullable so the function wouldn't collide if someone was only passing requiredEntitlementIdentifier. I also had to made it @escaping. Is that considered a breaking change?
There was a problem hiding this comment.
@vegaro I'm not certain if the @escaping is breaking changing (I'd have to experiment) but making this non-nullable is a breaking change, isn't it? 😬
If somebody was using this function without purchaseStarted, it will now require them to have purchaseStarted?
There was a problem hiding this comment.
Did the API tester for this break? That would really let us know, wouldn't it? 🤷♂️
There was a problem hiding this comment.
It didn't fail, that's why I am not sure 🤷♂️
If somebody was using this function without purchaseStarted, it will now require them to have purchaseStarted?
If they were not passing it, it will use the new non-deprecated function automatically (that one has it as nullable)
There was a problem hiding this comment.
Ahhh, okay! That makes sense then 😊 I think this should be good then!
| public func presentPaywallIfNeeded( | ||
| offering: Offering? = nil, | ||
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| shouldDisplay: @escaping @Sendable (CustomerInfo) -> Bool, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
Same here. If someone was not passing anything or passing let's say just offering, it would collide with the new version
There was a problem hiding this comment.
LGTM! I don't think the escaping change is breaking but might be good to confirm with @joshdholtz and/or @MarkVillacampa
| return self.modifier(OnPurchaseStartedModifier(handler: { _ in | ||
| handler() | ||
| })) | ||
| } |
There was a problem hiding this comment.
Should we mark this one as deprecated as well?
| public func presentPaywallIfNeeded( | ||
| requiredEntitlementIdentifier: String, | ||
| offering: Offering? = nil, | ||
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
@vegaro I'm not certain if the @escaping is breaking changing (I'd have to experiment) but making this non-nullable is a breaking change, isn't it? 😬
If somebody was using this function without purchaseStarted, it will now require them to have purchaseStarted?
| @@ -35,13 +36,17 @@ class PurchaseCompletedHandlerTests: TestCase { | |||
| .onPurchaseStarted { | |||
| started = true | |||
| } | |||
| .onPurchaseStarted { package in | |||
There was a problem hiding this comment.
Does this work for a variable name?! Even though package is a reserved word? 😅
There was a problem hiding this comment.
I know, it doesn't complain so I think so?
| public func presentPaywallIfNeeded( | ||
| requiredEntitlementIdentifier: String, | ||
| offering: Offering? = nil, | ||
| fonts: PaywallFontProvider = DefaultPaywallFontProvider(), | ||
| presentationMode: PaywallPresentationMode = .default, | ||
| purchaseStarted: PurchaseStartedHandler? = nil, | ||
| purchaseStarted: @escaping PurchaseStartedHandler, |
There was a problem hiding this comment.
Did the API tester for this break? That would really let us know, wouldn't it? 🤷♂️
Similar to #3627.
This API is available in:
PaywallView().presentPaywallIfNeeded.paywallFooterPaywallViewControllerDelegate