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

Paywalls: send events to Purchases #3164

Merged
merged 5 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions RevenueCatUI/Data/Strings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ enum Strings {
case not_displaying_paywall
case dismissing_paywall

case attempted_to_track_event_with_missing_data

}

extension Strings: CustomStringConvertible {
Expand Down Expand Up @@ -55,6 +57,9 @@ extension Strings: CustomStringConvertible {

case .dismissing_paywall:
return "Dismissing PaywallView"

case .attempted_to_track_event_with_missing_data:
return "Attempted to track event with missing data"
}
}

Expand Down
24 changes: 14 additions & 10 deletions RevenueCatUI/Data/TestData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,8 @@ internal enum TestData {
colors: .init(light: Self.lightColors, dark: Self.darkColors)
),
localization: Self.localization1,
assetBaseURL: Self.paywallAssetBaseURL
assetBaseURL: Self.paywallAssetBaseURL,
revision: 5
Copy link
Member

Choose a reason for hiding this comment

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

this quickly became a BIG file. Maybe we should move it to a folder and split into smaller things?

Copy link
Member

Choose a reason for hiding this comment

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

it'll only grow over time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup 👍🏻 I'll do it next week.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also done in #3196

)

static let offeringWithIntroOffer = Offering(
Expand Down Expand Up @@ -560,19 +561,19 @@ extension PurchaseHandler {
)
} restorePurchases: {
return TestData.customerInfo
} trackEvent: { event in
Logger.debug("Tracking event: \(event)")
}
}

static func cancelling() -> Self {
return self.init { _ in
return (
transaction: nil,
customerInfo: TestData.customerInfo,
userCancelled: true
)
} restorePurchases: {
return TestData.customerInfo
}
return .mock()
.map { block in {
var result = try await block($0)
result.userCancelled = true
return result
}
} restore: { $0 }
}

/// Creates a copy of this `PurchaseHandler` with a delay.
Expand All @@ -589,8 +590,11 @@ extension PurchaseHandler {
}
}
}

}

// MARK: -

extension PaywallColor: ExpressibleByStringLiteral {

/// Creates a `PaywallColor` with a string literal
Expand Down
53 changes: 53 additions & 0 deletions RevenueCatUI/PaywallView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,9 @@ struct LoadedOfferingPaywallView: View {
private let mode: PaywallViewMode
private let fonts: PaywallFontProvider

@State
private var session: (lastPaywall: DisplayedPaywall, id: PaywallEvent.SessionID)

@StateObject
private var introEligibility: IntroEligibilityViewModel
@ObservedObject
Expand All @@ -195,6 +198,9 @@ struct LoadedOfferingPaywallView: View {
@Environment(\.locale)
private var locale

@Environment(\.colorScheme)
private var colorScheme

init(
offering: Offering,
activelySubscribedProductIdentifiers: Set<String>,
Expand All @@ -215,6 +221,13 @@ struct LoadedOfferingPaywallView: View {
wrappedValue: .init(introEligibilityChecker: introEligibility)
)
self._purchaseHandler = .init(initialValue: purchaseHandler)

// Each `PaywallView` impression gets its own session.
// See also `updateSessionIfNeeded`.
self._session = .init(initialValue: (
Comment on lines +225 to +227
Copy link
Member

Choose a reason for hiding this comment

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

❤️

lastPaywall: .init(offering: offering, paywall: paywall),
id: .init()
))
}

var body: some View {
Expand All @@ -233,6 +246,8 @@ struct LoadedOfferingPaywallView: View {
.preference(key: RestoredCustomerInfoPreferenceKey.self,
value: self.purchaseHandler.restoredCustomerInfo)
.disabled(self.purchaseHandler.actionInProgress)
.onAppear { self.purchaseHandler.trackPaywallView(self.eventData) }
.onDisappear { self.purchaseHandler.trackPaywallClose(self.eventData) }

switch self.mode {
case .fullScreen:
Expand All @@ -245,6 +260,44 @@ struct LoadedOfferingPaywallView: View {
}
}

private var eventData: PaywallEvent.Data {
self.updateSessionIfNeeded()

return .init(
offering: self.offering,
paywall: self.paywall,
sessionID: self.session.id,
displayMode: self.mode,
locale: .current,
darkMode: self.colorScheme == .dark
)
}

private func updateSessionIfNeeded() {
let newPaywall: DisplayedPaywall = .init(offering: self.offering, paywall: self.paywall)
guard self.session.lastPaywall != newPaywall else { return }

self.session.lastPaywall = newPaywall
self.session.id = .init()
NachoSoto marked this conversation as resolved.
Show resolved Hide resolved
}

}

@available(iOS 15.0, macOS 12.0, tvOS 15.0, *)
private extension LoadedOfferingPaywallView {

struct DisplayedPaywall: Equatable {
var offeringIdentifier: String
var paywallTemplate: String
var revision: Int

init(offering: Offering, paywall: PaywallData) {
self.offeringIdentifier = offering.identifier
self.paywallTemplate = paywall.templateName
self.revision = paywall.revision
}
}

}

// MARK: -
Expand Down
81 changes: 76 additions & 5 deletions RevenueCatUI/Purchasing/PurchaseHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@ import RevenueCat
import StoreKit
import SwiftUI

@available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.2, *)
@available(iOS 15.0, macOS 12.0, tvOS 15.0, *)
final class PurchaseHandler: ObservableObject {

typealias PurchaseBlock = @Sendable (Package) async throws -> PurchaseResultData
typealias RestoreBlock = @Sendable () async throws -> CustomerInfo
typealias TrackEventBlock = @Sendable (PaywallEvent) async -> Void
Comment on lines 21 to +23
Copy link
Member

Choose a reason for hiding this comment

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

These are now only needed for testing purposes, right?
Is it worth keeping them around for that? Having them also forces us to have fatalError workarounds in the loading view IIRC?

Would it suffice to have a purchases mock (spy) and pass that in, so that we can directly call the methods from purchases? Seems like we only use them to check that we're calling the right methods with the right params

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These serve several purposes:

  • Abstract implementation from Purchases
  • Previews: we can have custom handlers that don't do anything or that complete a successful purchase
  • Snapshots: same thing
  • Unit tests (PaywallFooterTests for example)
  • LoadingPaywallView: this one doesn't use real blocks for obvious reasons

Having them also forces us to have fatalError workarounds in the loading view IIRC?

We have a fatalError in case you try to purchase from the loading view (which is .disabled, so it's impossible). Other than that, the trackEvent one is a no-op now.

Hopefully that answers your question why we can't just use a Purchases instance. We could create a PurchasesType implementation, but that would require implementing dozens of methods instead of just 3.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's what I was getting at, essentially for DI.

I don't feel strong enough about this to continue to block this PR, but I do think there are alternative approaches worth studying, that wouldn't necessitate building a mock of all methods in PurchasesType from scratch.

I worry about us adding a different approach for testing (instead of DI of a class, DI at the closure level). It makes things more complex for a brand new maintainer. And I'd argue that the closures are harder to read and wrap your head around than just passing in a purchases object, although that bit is subjective.

Alternate approaches that I think would also do the trick:

  • find a way to share the purchases mock instance we use for other tests. We already share some tests code, this wouldn't fall too far from that
  • create a new, much smaller protocol, that only has the methods that we'd need (purchase, restore, track events). Refactor PurchasesType so that it also conforms to this protocol. Then we pass in an instance of this smaller protocol, which is easy to mock. Then PurchasesHandler just takes a purchases: NewSmallType = Purchases.shared as param, and the rest of the class is dead simple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for clarifying what you meant! It does indeed lead to a simpler implementation :)

Done in #3196


/// `false` if this `PurchaseHandler` is not backend by a configured `Purchases`instance.
let isConfigured: Bool

private let purchaseBlock: PurchaseBlock
private let restoreBlock: RestoreBlock
private let trackEventBlock: TrackEventBlock

/// Whether a purchase or restore is currently in progress
@Published
Expand All @@ -47,22 +49,28 @@ final class PurchaseHandler: ObservableObject {
@Published
fileprivate(set) var restoredCustomerInfo: CustomerInfo?

private var eventData: PaywallEvent.Data?

convenience init(purchases: Purchases = .shared) {
self.init(isConfigured: true) { package in
return try await purchases.purchase(package: package)
} restorePurchases: {
return try await purchases.restorePurchases()
} trackEvent: { event in
await purchases.track(paywallEvent: event)
}
}

init(
isConfigured: Bool = true,
purchase: @escaping PurchaseBlock,
restorePurchases: @escaping RestoreBlock
restorePurchases: @escaping RestoreBlock,
trackEvent: @escaping TrackEventBlock
) {
self.isConfigured = isConfigured
self.purchaseBlock = purchase
self.restoreBlock = restorePurchases
self.trackEventBlock = trackEvent
}

static func `default`() -> Self {
Expand All @@ -74,7 +82,7 @@ final class PurchaseHandler: ObservableObject {
throw ErrorCode.configurationError
} restorePurchases: {
throw ErrorCode.configurationError
}
} trackEvent: { _ in }
}

}
Expand All @@ -91,7 +99,9 @@ extension PurchaseHandler {

let result = try await self.purchaseBlock(package)

if !result.userCancelled {
if result.userCancelled {
self.trackCancelledPurchase()
} else {
withAnimation(Constants.defaultAnimation) {
self.purchased = true
self.purchasedCustomerInfo = result.customerInfo
Expand All @@ -116,13 +126,60 @@ extension PurchaseHandler {
return customerInfo
}

func trackPaywallView(_ eventData: PaywallEvent.Data) {
Copy link
Member

Choose a reason for hiding this comment

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

I'm super late to this, but given that we already use "view" and "paywallView" a bunch because SwiftUI, I'd call this a Paywall Impression event

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make a PR renaming this in the code. I'll keep the backend event identifier as "view" unless @joshdholtz is also okay changing that.

self.eventData = eventData
self.track(.view(eventData))
}

func trackPaywallClose(_ eventData: PaywallEvent.Data) {
self.track(.close(eventData))
}

fileprivate func trackCancelledPurchase() {
guard let data = self.eventData else {
Logger.warning(Strings.attempted_to_track_event_with_missing_data)
return
}

self.track(.cancel(data.withCurrentDate()))
}

}

@available(iOS 15.0, macOS 12.0, tvOS 15.0, *)
extension PurchaseHandler {

/// Creates a copy of this `PurchaseHandler` wrapping the purchase and restore blocks.
func map(
purchase: @escaping (@escaping PurchaseBlock) -> PurchaseBlock,
restore: @escaping (@escaping RestoreBlock) -> RestoreBlock
) -> Self {
return .init(purchase: purchase(self.purchaseBlock),
restorePurchases: restore(self.restoreBlock))
restorePurchases: restore(self.restoreBlock),
trackEvent: self.trackEventBlock)
}

func map(
trackEvent: @escaping (@escaping TrackEventBlock) -> TrackEventBlock
) -> Self {
return .init(
purchase: self.purchaseBlock,
restorePurchases: self.restoreBlock,
trackEvent: trackEvent(self.trackEventBlock)
)
}

}

// MARK: - Private

@available(iOS 15.0, macOS 12.0, tvOS 15.0, *)
private extension PurchaseHandler {

func track(_ event: PaywallEvent) {
Task.detached(priority: .background) { [block = self.trackEventBlock] in
await block(event)
}
}

}
Expand Down Expand Up @@ -150,3 +207,17 @@ struct RestoredCustomerInfoPreferenceKey: PreferenceKey {
}

}

// MARK: -

private extension PaywallEvent.Data {

@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
func withCurrentDate() -> Self {
var copy = self
copy.date = .now

return copy
}

}
3 changes: 3 additions & 0 deletions RevenueCatUI/Views/LoadingPaywallView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ private extension LoadingPaywallView {
},
restorePurchases: {
fatalError("Should not be able to purchase")
},
trackEvent: { _ in
// Ignoring events from loading paywall view
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I love that the compiler made me implement this. And realized this was the best solution.

}
)

Expand Down
Loading