-
Notifications
You must be signed in to change notification settings - Fork 338
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
Support toggling update warnings & show update in restore flow #4571
Conversation
…rnCustomerToUpdate is true
@@ -117,6 +125,13 @@ import RevenueCat | |||
func loadCustomerCenterConfig() async { | |||
do { | |||
self.configuration = try await Purchases.shared.loadCustomerCenter() | |||
if let productId = configuration?.productId { |
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.
Moved this from the AppUpdateWarningView to centralize the logic into one place since it's also used in the restore flow now
private(set) var onUpdateAppClick: (() -> Void)? | ||
|
||
/// Whether or not the Customer Center should warn the customer that they're on an outdated version of the app. | ||
var shouldShowAppUpdateWarnings: Bool { |
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 think this can be private(var)
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 don't think it can - since it's used in other views it needs to be non-private and it can't be made private(set)
because it's a computed property
Tests/RevenueCatUITests/CustomerCenter/CustomerCenterViewModelTests.swift
Outdated
Show resolved
Hide resolved
@@ -132,6 +132,7 @@ struct CustomerCenterConfigResponse { | |||
struct Support { | |||
|
|||
let email: String | |||
let shouldWarnCustomerToUpdate: Bool? |
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.
is this nullable in the response?
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.
Yeah, some configurations made from before the field was introduced don't have it. It's also nullable in the dashboard!
Description
This PR:
Support.shouldWarnCustomerToUpdate
boolean from the backend to determine if update warnings should be displayed to the userAlert
to aConfirmationDialog
since we have up to 3 actions that a user can take, andAlert
only supports a max of 2Support.shouldWarnCustomerToUpdate
is enabled, and the user is an old app versionWarning
This PR is a breaking change from a UI standpoint because it now requires
Support.shouldWarnCustomerToUpdate
to be set for the AppUpdateWarningView to be displayed. This hasn't been enabled by default yet, so most people using Customer Center so far will need to enable the checkbox to see the warnings.Follow-Up Actions
Screenshots
Restore Flow
iOS
iPadOS
macOS (Designed for iPad)