-
Notifications
You must be signed in to change notification settings - Fork 54
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 V2] Moves validation logic to the Loading phase #2007
Conversation
# Conflicts: # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentViewWindowTests.kt
# Conflicts: # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentViewWindowTests.kt
📸 Snapshot TestSome snapshots failed to render
🛸 Powered by Emerge Tools |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2007 +/- ##
=======================================
Coverage 81.90% 81.90%
=======================================
Files 260 260
Lines 8516 8516
Branches 1226 1226
=======================================
Hits 6975 6975
Misses 1042 1042
Partials 499 499 ☔ View full report in Codecov by Sentry. |
# Conflicts: # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/LoadedPaywallComponents.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/button/ButtonComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/StyleFactory.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/TextComponentStyle.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentView.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/button/ButtonComponentViewTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentViewTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentViewWindowTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/StyleFactoryTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentViewTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentViewWindowTests.kt
# Conflicts: # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/LoadedPaywallComponents.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/LocalizedTextPartial.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/button/ButtonComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/ktx/Localization.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/StyleFactory.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/TextComponentStyle.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentState.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/helpers/Result.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/BuildPresentedPartialTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/LocalizedTextPartialTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/ToPresentedOverridesTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/button/ButtonComponentViewTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentViewTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentViewWindowTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/StyleFactoryTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentViewTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentViewWindowTests.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/helpers/FakePaywallState.kt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was scared, but as you said, it's not that complex lol
I am just a bit worried about that chain of functions. It took me a lot of focus to fully grasp what's going on
locales = localizations.keys, | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chain is not complex, but it's a bit hard to read, it takes some time to fully understand what's going on. I think extracting into some private functions could help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I agree. And we'll have to spend that time every time we need to read this code. Private functions is a good idea. I was also thinking of adding some early returns. I'll have a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to do this separately to minimize the conflicts, as I ended up making some changes to these lines in follow-up PRs. Have a look here: #2017
# Conflicts: # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/LoadedPaywallComponents.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/button/ButtonComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/text/TextComponentView.kt # ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/helpers/NonEmptyMap.kt # ui/revenuecatui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/helpers/ResultTests.kt
**This is an automatic release.** ## RevenueCat SDK ### 🐞 Bugfixes * improvements for fr translations (#2019) via Andy Boedo (@aboedo) ### 🔄 Other Changes * Feedback Surveys (#2010) via Cesar de la Vega (@vegaro) * [Paywalls V2] Fixes previews (#2015) via JayShortway (@JayShortway) * [Paywalls V2] Moves validation logic to the Loading phase (#2007) via JayShortway (@JayShortway) * Add Cancel subscriptions support (#2008) via Cesar de la Vega (@vegaro) * [Paywalls V2] Localizations are kept in a NonEmptyMap (#2001) via JayShortway (@JayShortway) * [Paywalls V2] `TextComponentState` handles locale changes (#2000) via JayShortway (@JayShortway) * [Paywalls V2] Some minor cleanup (#1994) via JayShortway (@JayShortway) * [Paywalls V2] `StackComponentView` handles overrides with `StackComponentState` (#1993) via JayShortway (@JayShortway) * Customer Center restores [CC-2] (#1999) via Cesar de la Vega (@vegaro) * [Paywalls V2] Add more image component previews to test parent being smaller than image size (#2004) via Toni Rico (@tonidero) * [Paywalls V2] Add `PurchaseButtonComponent` support (#2002) via Toni Rico (@tonidero) * Build Customer Center from JSON (#1998) via Cesar de la Vega (@vegaro) * Fix missing import after PR merge conflict (#1997) via Toni Rico (@tonidero) * [Paywalls V2] Add `StickyFooterComponentView` (#1991) via Toni Rico (@tonidero) * [Paywalls V2] `TextComponentView` handles overrides with `TextComponentState` (#1989) via JayShortway (@JayShortway) Co-authored-by: revenuecat-ops <[email protected]>
Description
This PR moves
StyleFactory.create()
to the Loading phase, and adds some missing validation pieces. Don't be scared by the large diff. 😄 A great deal of that is previews and other non-consequential changes that rippled out from the main changes.Main changes
OfferingToStateMapper.kt
. When validation fails, we return all the errors plus the existing legacy fallback paywall. The legacy paywall validation logic should behave the same.PaywallValidationResult
changed somewhat. Its errors are now in aNonEmptySet
. A Legacy paywall still only has 1 error at most.PaywallState.Loaded.Components
no longer has the network response (PaywallComponentsData
). Instead, it only has what it needs, which is now validated.Other changes
NonEmptySet
collection type.PaywallComponentDataValidationTests
.