Remove present code redemption sheet from paymentqueuewrapper #2
Conversation
…per' of https://github.com/RevenueCat/purchases-ios into remove-presentCodeRedemptionSheet-from-paymentqueuewrapper
…d for ipad on macos
…app_extension message
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR moves the code redemption sheet functionality from PaymentQueueWrapper to a dedicated OfferCodeRedemptionSheetPresenter class, modernizing the API with async/await support and improving platform-specific handling.
- Added new
OfferCodeRedemptionSheetPresenterin/Sources/Purchasing/StoreKitAbstractions/OfferCodeRedemptionSheetPresenter.swiftto handle sheet presentation with proper platform checks - Updated
Purchases.swiftwith new asyncpresentCodeRedemptionSheet(uiWindowScene:)method and deprecated sync version - Added comprehensive error cases in
StoreKitStrings.swiftfor sheet presentation failures (macOS, missing window scene, app extensions) - Removed
presentCodeRedemptionSheetfromPaymentQueueWrapperand updated mocks/tests to use new presenter pattern
12 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
| Task { | ||
| #if !targetEnvironment(macCatalyst) | ||
| try await self.presentOfferCodeRedemptionSheet(uiWindowScene: nil) | ||
| #endif | ||
| } |
There was a problem hiding this comment.
logic: Silently swallowing errors from the async call could cause issues. Consider adding error handling or logging.
| internal func presentOfferCodeRedemptionSheet(uiWindowScene: UIWindowScene?) async throws { | ||
| var windowScene = uiWindowScene | ||
| if windowScene == nil { | ||
| windowScene = try await systemInfo.currentWindowScene | ||
| } |
There was a problem hiding this comment.
style: Consider using early return pattern here to reduce nesting and improve readability
| @available(macCatalyst 16.0, *) | ||
| @objc func presentCodeRedemptionSheet( |
There was a problem hiding this comment.
logic: macCatalyst availability should be 14.0 to match iOS version, not 16.0
| if #available(iOS 16.0, iOSApplicationExtension 16.0, *) { | ||
| try await AppStore.presentOfferCodeRedeemSheet(in: windowScene) | ||
| } else { |
There was a problem hiding this comment.
logic: Redundant availability check - this block is already guarded by the osMajorVersion check above
| #if !targetEnvironment(macCatalyst) | ||
| self.sk1PresentCodeRedemptionSheet() | ||
| #else | ||
| Logger.warn(Strings.storeKit.error_displaying_offer_code_redemption_sheet_unavailable_in_app_extension) | ||
| #endif | ||
| } |
There was a problem hiding this comment.
logic: This else block is unreachable due to the osMajorVersion check above
| } | ||
| #endif | ||
|
|
||
| #if os(iOS) || VISION_OS |
There was a problem hiding this comment.
syntax: Missing VISION_OS availability annotation that matches the protocol
| func sk1PresentCodeRedemptionSheet() { | ||
| self.paymentQueue.presentCodeRedemptionSheet() | ||
| } |
There was a problem hiding this comment.
style: Consider handling potential errors from presentCodeRedemptionSheet() call
| Task { | ||
| try await purchases.presentCodeRedemptionSheet(uiWindowScene: nil) | ||
| } |
There was a problem hiding this comment.
logic: passing nil for uiWindowScene parameter may cause issues in production - should document or validate this parameter
| @@ -298,7 +300,8 @@ class BasePurchasesTests: TestCase { | |||
| purchasesOrchestrator: self.purchasesOrchestrator, | |||
| purchasedProductsFetcher: self.mockPurchasedProductsFetcher, | |||
| trialOrIntroPriceEligibilityChecker: self.cachingTrialOrIntroPriceEligibilityChecker, | |||
| storeMessagesHelper: self.mockStoreMessagesHelper) | |||
| storeMessagesHelper: self.mockStoreMessagesHelper, | |||
| offerCodeRedemptionSheetPresenter: mockOfferCodeSheetPresenter) | |||
There was a problem hiding this comment.
style: mockOfferCodeSheetPresenter is passed without self, which is inconsistent with other parameters in this initializer
Checklist
purchases-androidand hybridsMotivation
Description