From 3aaee9a452d470751140bad6d869bfae836897ac Mon Sep 17 00:00:00 2001 From: NachoSoto Date: Wed, 2 Aug 2023 16:28:36 -0700 Subject: [PATCH] `Paywalls`: added support for custom and lifetime products (#2941) The main change is that `VariableHandler` no longer cashes when trying to determine price per month for non-subscriptions. I added a test to cover this behavior. --- RevenueCatUI/Data/Localization.swift | 3 +- RevenueCatUI/Data/Strings.swift | 5 +++ .../Data/TemplateViewConfiguration.swift | 4 +-- RevenueCatUI/Data/TestData.swift | 16 +++++++++ RevenueCatUI/Data/Variables.swift | 3 +- .../Package+VariableDataProvider.swift | 7 +++- .../Resources/en.lproj/Localizable.strings | 1 + .../Resources/es.lproj/Localizable.strings | 1 + .../Data/TemplateViewConfigurationTests.swift | 35 ++++++++++++++++--- .../Data/VariablesTests.swift | 26 ++++++++++++++ .../RevenueCatUITests/LocalizationTests.swift | 6 ++-- .../SimpleApp/SimpleApp/SamplePaywalls.swift | 21 +++++++++-- 12 files changed, 112 insertions(+), 16 deletions(-) diff --git a/RevenueCatUI/Data/Localization.swift b/RevenueCatUI/Data/Localization.swift index bb9463e70f..0f40bb9e19 100644 --- a/RevenueCatUI/Data/Localization.swift +++ b/RevenueCatUI/Data/Localization.swift @@ -219,8 +219,9 @@ private extension PackageType { case .twoMonth: return "\(keyPrefix)twoMonth" case .monthly: return "\(keyPrefix)monthly" case .weekly: return "\(keyPrefix)weekly" + case .lifetime: return "\(keyPrefix)lifetime" - case .unknown, .custom, .lifetime: + case .unknown, .custom: return nil } } diff --git a/RevenueCatUI/Data/Strings.swift b/RevenueCatUI/Data/Strings.swift index 3e52482146..b02fb735fb 100644 --- a/RevenueCatUI/Data/Strings.swift +++ b/RevenueCatUI/Data/Strings.swift @@ -12,6 +12,7 @@ import RevenueCat enum Strings { + case package_not_subscription(Package) case found_multiple_packages_of_same_type(PackageType) case could_not_find_content_for_variable(variableName: String) @@ -25,6 +26,10 @@ extension Strings: CustomStringConvertible { var description: String { switch self { + case let .package_not_subscription(package): + return "Expected package '\(package.identifier)' to be a subscription. " + + "Type: \(package.packageType.debugDescription)" + case let .found_multiple_packages_of_same_type(type): return "Found multiple \(type) packages. Will use the first one." diff --git a/RevenueCatUI/Data/TemplateViewConfiguration.swift b/RevenueCatUI/Data/TemplateViewConfiguration.swift index eb718e9087..dbbe65fbf2 100644 --- a/RevenueCatUI/Data/TemplateViewConfiguration.swift +++ b/RevenueCatUI/Data/TemplateViewConfiguration.swift @@ -173,9 +173,7 @@ extension TemplateViewConfiguration { /// Filters `packages`, extracting only the values corresponding to `list`. static func filter(packages: [RevenueCat.Package], with list: [PackageType]) -> [RevenueCat.Package] { - // Only subscriptions are supported at the moment - let subscriptions = packages.filter { $0.storeProduct.productCategory == .subscription } - let map = Dictionary(grouping: subscriptions) { $0.packageType } + let map = Dictionary(grouping: packages) { $0.packageType } return list.compactMap { type in if let packages = map[type] { diff --git a/RevenueCatUI/Data/TestData.swift b/RevenueCatUI/Data/TestData.swift index 1cbfd91cfa..356bb53cba 100644 --- a/RevenueCatUI/Data/TestData.swift +++ b/RevenueCatUI/Data/TestData.swift @@ -47,6 +47,16 @@ internal enum TestData { subscriptionPeriod: .init(value: 1, unit: .year), introductoryDiscount: Self.intro(14, .day) ) + static let lifetimeProduct = TestStoreProduct( + localizedTitle: "Lifetime", + price: 119.49, + localizedPriceString: "$119.49", + productIdentifier: "com.revenuecat.product_lifetime", + productType: .consumable, + localizedDescription: "Lifetime purchase", + subscriptionGroupIdentifier: "group", + subscriptionPeriod: nil + ) static let productWithIntroOffer = TestStoreProduct( localizedTitle: "PRO monthly", price: 3.99, @@ -110,6 +120,12 @@ internal enum TestData { storeProduct: productWithNoIntroOffer.toStoreProduct(), offeringIdentifier: Self.offeringIdentifier ) + static let lifetimePackage = Package( + identifier: "lifetime", + packageType: .lifetime, + storeProduct: Self.lifetimeProduct.toStoreProduct(), + offeringIdentifier: Self.offeringIdentifier + ) static let packages = [ Self.packageWithIntroOffer, diff --git a/RevenueCatUI/Data/Variables.swift b/RevenueCatUI/Data/Variables.swift index 1bce91ba67..d31526f7e6 100644 --- a/RevenueCatUI/Data/Variables.swift +++ b/RevenueCatUI/Data/Variables.swift @@ -16,6 +16,7 @@ protocol VariableDataProvider { var applicationName: String { get } + var isSubscription: Bool { get } var isMonthly: Bool { get } var localizedPrice: String { get } @@ -92,7 +93,7 @@ private extension VariableDataProvider { case "price": return self.localizedPrice case "price_per_month": return self.localizedPricePerMonth case "total_price_and_per_month": - if self.isMonthly { + if !self.isSubscription || self.isMonthly { return self.localizedPrice } else { let unit = Localization.abbreviatedUnitLocalizedString(for: .month, locale: locale) diff --git a/RevenueCatUI/Helpers/Package+VariableDataProvider.swift b/RevenueCatUI/Helpers/Package+VariableDataProvider.swift index 34752e9a33..f4fc170b7a 100644 --- a/RevenueCatUI/Helpers/Package+VariableDataProvider.swift +++ b/RevenueCatUI/Helpers/Package+VariableDataProvider.swift @@ -8,6 +8,10 @@ extension Package: VariableDataProvider { return Bundle.main.applicationDisplayName } + var isSubscription: Bool { + return self.storeProduct.productCategory == .subscription + } + var isMonthly: Bool { return self.packageType == .monthly } @@ -46,7 +50,8 @@ private extension Package { var pricePerMonth: NSDecimalNumber { guard let price = self.storeProduct.pricePerMonth else { - fatalError("Unexpectedly found a package which is not a subscription: \(self)") + Logger.warning(Strings.package_not_subscription(self)) + return self.storeProduct.priceDecimalNumber } return price diff --git a/RevenueCatUI/Resources/en.lproj/Localizable.strings b/RevenueCatUI/Resources/en.lproj/Localizable.strings index 6b2e53694d..2321577f2f 100644 --- a/RevenueCatUI/Resources/en.lproj/Localizable.strings +++ b/RevenueCatUI/Resources/en.lproj/Localizable.strings @@ -13,5 +13,6 @@ "PackageType.twoMonth" = "2 month"; "PackageType.monthly" = "Monthly"; "PackageType.weekly" = "Weekly"; +"PackageType.lifetime" = "Lifetime"; "%d%% off" = "%d%% off"; diff --git a/RevenueCatUI/Resources/es.lproj/Localizable.strings b/RevenueCatUI/Resources/es.lproj/Localizable.strings index 1bafeb9eab..8303af207c 100644 --- a/RevenueCatUI/Resources/es.lproj/Localizable.strings +++ b/RevenueCatUI/Resources/es.lproj/Localizable.strings @@ -13,5 +13,6 @@ "PackageType.twoMonth" = "2 meses"; "PackageType.monthly" = "Mensual"; "PackageType.weekly" = "Semanal"; +"PackageType.lifetime" = "Vitalicio"; "%d%% off" = "Ahorra %d%%"; diff --git a/Tests/RevenueCatUITests/Data/TemplateViewConfigurationTests.swift b/Tests/RevenueCatUITests/Data/TemplateViewConfigurationTests.swift index 41bb3267e8..96820b5195 100644 --- a/Tests/RevenueCatUITests/Data/TemplateViewConfigurationTests.swift +++ b/Tests/RevenueCatUITests/Data/TemplateViewConfigurationTests.swift @@ -61,10 +61,31 @@ class TemplateViewConfigurationCreationTests: BaseTemplateViewConfigurationTests } } + func testCreateOnlyLifetime() throws { + let result = try Config.create( + with: [TestData.lifetimePackage], + filter: [.lifetime], + default: nil, + localization: Self.localization, + setting: .single + ) + + switch result { + case let .single(package): + expect(package.content) === TestData.lifetimePackage + Self.verifyLocalizationWasProcessed(package.localization, for: TestData.lifetimePackage) + case .multiple: + fail("Invalid result: \(result)") + } + } + func testCreateMultiplePackage() throws { let result = try Config.create( - with: [TestData.monthlyPackage, TestData.annualPackage, TestData.weeklyPackage], - filter: [.annual, .monthly], + with: [TestData.monthlyPackage, + TestData.annualPackage, + TestData.weeklyPackage, + TestData.lifetimePackage], + filter: [.annual, .monthly, .lifetime], default: .monthly, localization: Self.localization, setting: .multiple @@ -77,7 +98,7 @@ class TemplateViewConfigurationCreationTests: BaseTemplateViewConfigurationTests expect(first.content) === TestData.annualPackage expect(defaultPackage.content) === TestData.monthlyPackage - expect(packages).to(haveCount(2)) + expect(packages).to(haveCount(3)) let annual = packages[0] expect(annual.content) === TestData.annualPackage @@ -89,6 +110,10 @@ class TemplateViewConfigurationCreationTests: BaseTemplateViewConfigurationTests expect(monthly.content) === TestData.monthlyPackage expect(monthly.discountRelativeToMostExpensivePerMonth).to(beNil()) Self.verifyLocalizationWasProcessed(monthly.localization, for: TestData.monthlyPackage) + + let lifetime = packages[2] + expect(lifetime.content) === TestData.lifetimePackage + Self.verifyLocalizationWasProcessed(lifetime.localization, for: TestData.lifetimePackage) } } @@ -119,8 +144,8 @@ class TemplateViewConfigurationFilteringTests: BaseTemplateViewConfigurationTest expect(TemplateViewConfiguration.filter(packages: [TestData.monthlyPackage], with: [.annual])) == [] } - func testFilterOutNonSubscriptions() { - expect(TemplateViewConfiguration.filter(packages: [Self.consumable], with: [.custom])) == [] + func testConsumablesAreIncluded() { + expect(TemplateViewConfiguration.filter(packages: [Self.consumable], with: [.custom])) == [Self.consumable] } func testFilterByPackageType() { diff --git a/Tests/RevenueCatUITests/Data/VariablesTests.swift b/Tests/RevenueCatUITests/Data/VariablesTests.swift index 77005662de..bd38cd6c10 100644 --- a/Tests/RevenueCatUITests/Data/VariablesTests.swift +++ b/Tests/RevenueCatUITests/Data/VariablesTests.swift @@ -47,6 +47,13 @@ class VariablesTests: TestCase { expect(self.process("{{ total_price_and_per_month }}")) == "$49.99 ($4.16/mo)" } + func testTotalPriceAndPerMonthForNonSubscriptions() { + self.provider.isSubscription = false + self.provider.isMonthly = false + self.provider.localizedPrice = "$49.99" + expect(self.process("{{ total_price_and_per_month }}")) == "$49.99" + } + func testTotalPriceAndPerMonthWithDifferentPricesSpanish() { self.provider.localizedPrice = "49,99€" self.provider.localizedPricePerMonth = "4,16€" @@ -137,6 +144,24 @@ class VariablesTests: TestCase { ] } + // Note: this isn't perfect, but a warning is logged + // and it's better than crashing. + func testPricePerMonthForLifetimeProductsReturnsPrice() { + let result = VariableHandler.processVariables( + in: "{{ price_per_month }}", + with: TestData.lifetimePackage + ) + expect(result) == "$119.49" + } + + func testTotalPriceAndPerMonthForLifetimeProductsReturnsPrice() { + let result = VariableHandler.processVariables( + in: "{{ total_price_and_per_month }}", + with: TestData.lifetimePackage + ) + expect(result) == "$119.49" + } + } // MARK: - Private @@ -153,6 +178,7 @@ private extension VariablesTests { private struct MockVariableProvider: VariableDataProvider { var applicationName: String = "" + var isSubscription: Bool = true var isMonthly: Bool = false var localizedPrice: String = "" var localizedPricePerMonth: String = "" diff --git a/Tests/RevenueCatUITests/LocalizationTests.swift b/Tests/RevenueCatUITests/LocalizationTests.swift index 55541ad664..658bbfa7fe 100644 --- a/Tests/RevenueCatUITests/LocalizationTests.swift +++ b/Tests/RevenueCatUITests/LocalizationTests.swift @@ -128,11 +128,11 @@ class PackageTypeEnglishLocalizationTests: BaseLocalizationTests { verify(.twoMonth, "2 month") verify(.monthly, "Monthly") verify(.weekly, "Weekly") + verify(.lifetime, "Lifetime") } func testOtherValues() { verify(.custom, "") - verify(.lifetime, "") verify(.unknown, "") } @@ -150,11 +150,11 @@ class PackageTypeSpanishLocalizationTests: BaseLocalizationTests { verify(.twoMonth, "2 meses") verify(.monthly, "Mensual") verify(.weekly, "Semanal") + verify(.lifetime, "Vitalicio") } func testOtherValues() { verify(.custom, "") - verify(.lifetime, "") verify(.unknown, "") } } @@ -171,11 +171,11 @@ class PackageTypeOtherLanguageLocalizationTests: BaseLocalizationTests { verify(.twoMonth, "2 month") verify(.monthly, "Monthly") verify(.weekly, "Weekly") + verify(.lifetime, "Lifetime") } func testOtherValues() { verify(.custom, "") - verify(.lifetime, "") verify(.unknown, "") } } diff --git a/Tests/TestingApps/SimpleApp/SimpleApp/SamplePaywalls.swift b/Tests/TestingApps/SimpleApp/SimpleApp/SamplePaywalls.swift index b606c3ec03..2ce64e45d3 100644 --- a/Tests/TestingApps/SimpleApp/SimpleApp/SamplePaywalls.swift +++ b/Tests/TestingApps/SimpleApp/SimpleApp/SamplePaywalls.swift @@ -16,7 +16,8 @@ final class SamplePaywallLoader { self.packages = [ Self.weeklyPackage, Self.monthlyPackage, - Self.annualPackage + Self.annualPackage, + Self.lifetimePackage ] } @@ -75,6 +76,12 @@ private extension SamplePaywallLoader { storeProduct: annualProduct.toStoreProduct(), offeringIdentifier: offeringIdentifier ) + static let lifetimePackage = Package( + identifier: "lifetime", + packageType: .lifetime, + storeProduct: lifetimeProduct.toStoreProduct(), + offeringIdentifier: offeringIdentifier + ) static let weeklyProduct = TestStoreProduct( localizedTitle: "Weekly", @@ -124,6 +131,16 @@ private extension SamplePaywallLoader { type: .introductory ) ) + static let lifetimeProduct = TestStoreProduct( + localizedTitle: "Lifetime", + price: 119.49, + localizedPriceString: "$119.49", + productIdentifier: "com.revenuecat.product_lifetime", + productType: .consumable, + localizedDescription: "Lifetime purchase", + subscriptionGroupIdentifier: "group", + subscriptionPeriod: nil + ) } @@ -171,7 +188,7 @@ private extension SamplePaywallLoader { return .init( template: .multiPackageBold, config: .init( - packages: [.weekly, .monthly, .annual], + packages: [.weekly, .monthly, .annual, .lifetime], images: Self.images, colors: .init( light: .init(