Support not showing Contact Customer Support if the user isn't on the most recent app version #4567
Conversation
| @@ -550,6 +555,7 @@ extension CustomerCenterConfigData.Support { | |||
|
|
|||
| init(from response: CustomerCenterConfigResponse.Support) { | |||
| self.email = response.email | |||
| self.shouldWarnCustomerToUpdate = response.shouldWarnCustomerToUpdate ?? false | |||
There was a problem hiding this comment.
Defaults to false if it isn't included in the response from the backend since the checkbox's default value in the dashboard is false.
|
|
||
| /// Whether or not the user needs to update their app version to contact support. | ||
| var appUpdateRequiredToContactSupport: Bool { | ||
| // For now, we're using the same flag as the app update warning. |
There was a problem hiding this comment.
I'd remove the "for now", since it assumes some kind of temporal nature to it, and if we never change it later on it will look to a maintainer like this was temporary or hacky code
| expect(viewModel.appUpdateRequiredToContactSupport).to(beFalse()) | ||
| } | ||
|
|
||
| func testAppUpdateRequiredToContactSupport_false_blocked_by_config() { |
There was a problem hiding this comment.
the mix of casing here looks a little odd to my eye
There was a problem hiding this comment.
I would maybe go with something more like
testAppUpdateRequiredToContactSupportReturnsFalseIfBlockedByConfig
There was a problem hiding this comment.
Ah, sometimes I mix them when the test names get super long in my apps - I'll change them to use the normal style here :)
| } | ||
|
|
||
| /// Whether or not the user needs to update their app version to contact support. | ||
| var appUpdateRequiredToContactSupport: Bool { |
There was a problem hiding this comment.
we don't entirely require but rather suggest, right?
There was a problem hiding this comment.
With this implementation, I built it the way the copy for the checkbox on the dashboard is worded and made it required to contact support in the WrongPlatformView & the Restore Purchases flow.
The checkbox's copy reads:
Require users to update to the latest version of your app before contacting support
That said, I don't think it's great to prevent people from contacting support and would suggest that we instead show a message or button in the alert urging the user to update in addition to letting them contact support.
| @@ -132,6 +132,7 @@ struct CustomerCenterConfigResponse { | |||
| struct Support { | |||
|
|
|||
| let email: String | |||
| let shouldWarnCustomerToUpdate: Bool? | |||
There was a problem hiding this comment.
Hmm I'm thinking... the original idea was that, if the backend sent the lastPublishedAppVersion, the SDK would consider it "enabled" implicitly. The backend shouldn't send that version if it's disabled. So this property would be unnecessary I think?
| @@ -83,7 +83,7 @@ struct RestorePurchasesAlert: ViewModifier { | |||
|
|
|||
| case .purchasesNotFound: | |||
| let message = Text(localization.commonLocalizedString(for: .purchasesNotRecovered)) | |||
| if let url = supportURL { | |||
| if let url = supportURL, !customerCenterViewModel.appUpdateRequiredToContactSupport { | |||
There was a problem hiding this comment.
Hmm do we need to show the AppUpdateWarningView in the else case? As in, right now we won't be able to contact support if the app update is required right? Which I think we didn't want to do? Same for the WrongPlatformView.
|
Closing in favor of #4571 |
Description
This PR updates the Customer Center to support the "Require users to update to the latest version of your app before contacting support" checkbox in the dashboard. The checkbox now controls:
AppUpdateWarningViewis shown when the Customer Center is shown and the user is on an old app versionTesting
CustomerCenterViewModel.appUpdateRequiredToContactSupport