Fixes a Paywall Template 7 crash when none of the tiers have any available products. #4243
Conversation
| locale: locale, | ||
| showZeroDecimalPlacePrices: showZeroDecimalPlacePrices | ||
| ) | ||
| let filteredTiers: [(PaywallData.Tier, (MultiPackage, String))] = try tiers.compactMap { tier in |
There was a problem hiding this comment.
I recommend hiding whitespace when reviewing this file.
Instead of filtering the tiers inline while creating the Dictionary, it is now filtering the tiers separately first. Then we can use the filtered list to grab the first item, for our firstTier. Then the filtered list is passed into the Dictionary constructor as before.
There was a problem hiding this comment.
Nice fix to catch the invalid configuration when it's constructed.
The initializer where this is used has a fatalError and a ! - in my opinion it's better if at all possible to avoid these unless there truly is no other choice. Some other undetected error upstream could cause the same crash, and if we can bow out more gracefully at that point, we ought to. Even if that were done, this change of course should still be added!
It all looks good to me, commented on some small nits.
| ) | ||
| } | ||
|
|
||
| let filteredTiersMap: [PaywallData.Tier: (package: MultiPackage, tierName: String)] = Dictionary( |
There was a problem hiding this comment.
Referring to a dictionary as a "map" isn't idiomatic swift.... our style guide would recommend something like packageAndTierNameByTier. Or you could use filteredTiersDict...
There was a problem hiding this comment.
Ah right, thanks for pointing me to the style guide. Done in 24c1f68.
| locale: locale, | ||
| showZeroDecimalPlacePrices: showZeroDecimalPlacePrices | ||
| ) | ||
| let filteredTiers: [(PaywallData.Tier, (MultiPackage, String))] = try tiers.compactMap { tier in |
There was a problem hiding this comment.
I think it's helpful to maintain the labels on the tuple, (package: MultiPackage, tierName: String) rather than (MultiPackage, String).
| guard let firstTier = tiers.first else { | ||
| guard let (firstTier, _) = filteredTiers.first else { | ||
| throw TemplateError.noTiers | ||
| } |
There was a problem hiding this comment.
This could be moved above the dictionary construction.
There was a problem hiding this comment.
Good one! Done in 24c1f68.
|
Waiting for #4252 to be merged to fix lint. |
Note
There's an equivalent Android fix here: RevenueCat/purchases-android#1834.
Bug
If a Template 7 Paywall is configured to use packages for which the products are not available, it crashes in
Template7View.swift, line 74:The SDK prints these logs:
After that, no tiers are left anymore, but this case isn't considered.
Stack trace
Reproducer
In PurchaseTester, using the Main RevenueCat Sample App RevenueCat project, there's a Paywall configured called
template7_crash_ios_pr_4243. Trying to show this Paywall reproduces the crash. This is because the tiers are configured to show the weekly, monthly and lifetime packages, but only the yearly package is correctly configured for iOS in this offering.Fix
The fix implemented in this PR is to check whether we have any tiers left after filtering out those without available products, and throwing the existing
TemplateError.noTierserror.