Skip to content
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

Created PaywallExtensions: StoreView and SubscriptionStoreView overloads for Offering #2593

Merged
merged 3 commits into from
Jun 9, 2023

Conversation

NachoSoto
Copy link
Contributor

@NachoSoto NachoSoto commented Jun 7, 2023

Description

This adds overloads, but there's still a few things pending for the SDK to fully support these:

  • Set presentedOfferingIdentifier when displaying StoreKit2 paywalls: SDK-3176
  • Fix preventPurchasePopupCallFromTriggeringCacheRefresh: SDK-3172
  • Always initialize StoreKit2TransactionListener even on SK1 mode: SDK-3170

Example:

struct App: View {

    var offering: Offering

    var body: some View {
        Content()
            .sheet(isPresented: self.$showingSubscriptionSheet) {
                SubscriptionStoreView(offering: self.offering)
            }
    }

}

@NachoSoto NachoSoto added pr:feat A new feature iOS 17 labels Jun 7, 2023
@NachoSoto NachoSoto requested a review from a team June 7, 2023 16:59
@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Merging #2593 (5ba1822) into main (9494f2e) will decrease coverage by 0.05%.
The diff coverage is n/a.

❗ Current head 5ba1822 differs from pull request most recent head d747701. Consider uploading reports for the commit d747701 to get more accurate results

@@            Coverage Diff             @@
##             main    #2593      +/-   ##
==========================================
- Coverage   86.29%   86.24%   -0.05%     
==========================================
  Files         205      205              
  Lines       14334    14334              
==========================================
- Hits        12369    12362       -7     
- Misses       1965     1972       +7     

see 4 files with indirect coverage changes

Sources/Support/PaywallExtensions.swift Show resolved Hide resolved
}
} else {
ProgressView()
.progressViewStyle(.circular)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 questions:

  • Will the ProgressView use the full available space? Or more importantly, will the transition between this and the SubscriptionStoreView once it's loaded look good?
  • Did you/should we get a design review on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is very WIP.
I'm going to extract CurrentOfferingSubscriptionStoreView into a separate PR so we can merge the rest, and handle this and the offering failure separately.

}
}
.task {
if let offering = try? await Purchases.shared.offerings().current {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if getting offerings fail, we just keep the loading state? I think we should show some error UI. Thoughts?

SubscriptionStoreView(offering: currentOffering,
marketingContent: marketingContent)
} else {
SubscriptionStoreView(offering: currentOffering)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple more questions about these views:

  • When purchasing through this view, does it automatically finish the transaction? Or does the developer still have to do it?
  • If the user purchases an offering through this view, we won't be getting the presentedOfferingIdentifier setup in the SDK... We need to figure out a way to set that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add to this, that anything we do in the purchasePackage/Product methods won't happen when they use this view. We should check if there are any other unintended side effects of this. Another one I see is that we would not be calling preventPurchasePopupCallFromTriggeringCacheRefresh so we would never set the offerings cache as stale I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When purchasing through this view, does it automatically finish the transaction? Or does the developer still have to do it?

Our SDK handles that :) (though we do need SDK-3170)

If the user purchases an offering through this view, we won't be getting the presentedOfferingIdentifier setup in the SDK... We need to figure out a way to set that.

Oh great call. Just filed SDK-3176 for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add to this, that anything we do in the purchasePackage/Product methods won't happen when they use this view. We should check if there are any other unintended side effects of this.

I think that's fine. This is basically like our SDK acting in observer mode for a purchase being done from outside.

Another one I see is that we would not be calling preventPurchasePopupCallFromTriggeringCacheRefresh so we would never set the offerings cache as stale I think.

Another reason to do SDK-3172 and get rid of that method.

@NachoSoto
Copy link
Contributor Author

@tonidero updated this to leave just the overloads, and listed the 3 tasks that need to be done (working on those today)
This is just overloads for SwiftUI so they don't have to wait for those fixes.

Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! As you said we can work on future improvements in other PRs

@NachoSoto NachoSoto enabled auto-merge (squash) June 9, 2023 17:02
@NachoSoto NachoSoto merged commit 3032007 into main Jun 9, 2023
@NachoSoto NachoSoto deleted the xcode-15-paywall branch June 9, 2023 17:18
NachoSoto added a commit that referenced this pull request Jun 9, 2023
NachoSoto added a commit that referenced this pull request Jun 9, 2023
This was referenced Jun 13, 2023
NachoSoto pushed a commit that referenced this pull request Jun 13, 2023
**This is an automatic release.**

### New Features
* New `DebugViewController`: UIKit counterpart for SwiftUI's
`debugRevenueCatOverlay` (#2631) via NachoSoto (@NachoSoto)
* Created `PaywallExtensions`: `StoreView` and `SubscriptionStoreView`
overloads for `Offering` (#2593) via NachoSoto (@NachoSoto)
* Introduced `debugRevenueCatOverlay()`: new SwiftUI debug overlay
(#2567) via NachoSoto (@NachoSoto)
### Bugfixes
* Removed `preventPurchasePopupCallFromTriggeringCacheRefresh`, update
caches on `willEnterForeground` (#2623) via NachoSoto (@NachoSoto)
* Fixed `Catalyst` build with `Xcode 15 beta 1` (#2586) via NachoSoto
(@NachoSoto)
### Dependency Updates
* Bump danger from 9.3.0 to 9.3.1 (#2592) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* `StoreTransaction`: added new `Storefront` to API testers (#2634) via
NachoSoto (@NachoSoto)
* `DebugView`: added snapshot tests (#2630) via NachoSoto (@NachoSoto)
* `verifyNoUnfinishedTransactions`/`verifyUnfinishedTransaction`: added
missing `#file` parameter (#2625) via NachoSoto (@NachoSoto)
* `PostReceiptDataOperation`: clean up cache key (#2628) via NachoSoto
(@NachoSoto)
* `PurchasesOrchestrator`: also get `Storefront` from SK1 (#2629) via
NachoSoto (@NachoSoto)
* `CI`: disable iOS 17 for now (#2627) via NachoSoto (@NachoSoto)
* `Tests`: fixed crash on iOS 13 (#2624) via NachoSoto (@NachoSoto)
* `StoreTransaction`: read `Storefront` from `StoreKit.Transaction`
(#2611) via NachoSoto (@NachoSoto)
* `StoreKitConfigTestCase`/`BaseStoreKitIntegrationTests`: also clear
transactions after every test (#2616) via NachoSoto (@NachoSoto)
* `ErrorCode.networkError`: improved description (#2610) via NachoSoto
(@NachoSoto)
* `PurchaseTester`: make CI job always point to current version (#2622)
via NachoSoto (@NachoSoto)
* Improved `finishAllUnfinishedTransactions` (#2615) via NachoSoto
(@NachoSoto)
* `StoreKitConfigTestCase`: improved `waitForStoreKitTestIfNeeded`
(#2614) via NachoSoto (@NachoSoto)
* `StoreKitConfigTestCase`: set `continueAfterFailure` to `false`
(#2617) via NachoSoto (@NachoSoto)
* `PaywallExtensions`: fixed compilation (#2613) via NachoSoto
(@NachoSoto)
* `CI`: added `iOS 17` job (#2591) via NachoSoto (@NachoSoto)
* `Encodable.jsonEncodedData`: fixed tests on iOS 17 due to inconsistent
key ordering (#2607) via NachoSoto (@NachoSoto)
* `debugRevenueCatOverlay`: added ability to display new
`SubscriptionStoreView` (#2595) via NachoSoto (@NachoSoto)
* Refactor: extracted all log strings (#2600) via NachoSoto (@NachoSoto)
* Changed tests to work around `URL` decoding differences in `iOS 17`
(#2605) via NachoSoto (@NachoSoto)
* Removed unnecessary `Strings.trimmedOrError` (#2601) via NachoSoto
(@NachoSoto)
* Fixed test compilation with `Xcode 15` (#2602) via NachoSoto
(@NachoSoto)
* Tests: added `iOS 17` snapshots (#2603) via NachoSoto (@NachoSoto)
* `StoreProductDiscount`: added `description` (#2604) via NachoSoto
(@NachoSoto)
* `debugRevenueCatOverlay` improvements (#2594) via NachoSoto
(@NachoSoto)
* `Xcode 15`: fixed all documentation warnings (#2596) via NachoSoto
(@NachoSoto)
* `StoreKitObserverModeIntegrationTests`: fixed and disabled SK2
`testPurchaseInDevicePostsReceipt` (#2589) via NachoSoto (@NachoSoto)
* `StoreKit2TransactionListener`: added log when receiving
`Transactions.Updates` (#2588) via NachoSoto (@NachoSoto)
* `Dictionary.MergeStrategy`: simplify implementation (#2587) via
NachoSoto (@NachoSoto)
* `Configuration.Builder`: fixed doc reference (#2583) via NachoSoto
(@NachoSoto)
* `APITesters`: available since iOS 11 (#2581) via NachoSoto
(@NachoSoto)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS 17 pr:feat A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants