Alert the user about configuration mismatch in notification settings (#1543)

2026-11-02 21:33:26 +00:00 · 2023-08-23 16:01:10 +02:00
parent 276adb6084
commit e0f885c590
19 changed files with 272 additions and 19 deletions
@@ -264,6 +264,8 @@
 "screen_migration_title" = "Setting up your account.";
 "screen_notification_settings_additional_settings_section_title" = "Additional settings";
 "screen_notification_settings_calls_label" = "Audio and video calls";
+"screen_notification_settings_configuration_mismatch" = "Configuration mismatch";
+"screen_notification_settings_configuration_mismatch_description" = "We’ve simplified Notifications Settings to make options easier to find.\n\nSome custom settings you’ve chosen in the past are not shown here, but they’re still active.\n\nIf you proceed, some of your settings may change.";
 "screen_notification_settings_direct_chats" = "Direct chats";
 "screen_notification_settings_edit_custom_settings_section_title" = "Custom setting per chat";
 "screen_notification_settings_edit_failed_updating_default_mode" = "An error occurred while updating the notification setting.";
@@ -272,6 +274,7 @@
 "screen_notification_settings_edit_screen_direct_section_header" = "On direct chats, notify me for";
 "screen_notification_settings_edit_screen_group_section_header" = "On group chats, notify me for";
 "screen_notification_settings_enable_notifications" = "Enable notifications on this device";
+"screen_notification_settings_failed_fixing_configuration" = "The configuration has not been corrected, please try again.";
 "screen_notification_settings_group_chats" = "Group chats";
 "screen_notification_settings_mentions_section_title" = "Mentions";
 "screen_notification_settings_mode_all" = "All";
@@ -686,6 +686,14 @@ public enum L10n {
  public static var screenNotificationSettingsAdditionalSettingsSectionTitle: String { return L10n.tr("Localizable", "screen_notification_settings_additional_settings_section_title") }
  /// Audio and video calls
  public static var screenNotificationSettingsCallsLabel: String { return L10n.tr("Localizable", "screen_notification_settings_calls_label") }
+  /// Configuration mismatch
+  public static var screenNotificationSettingsConfigurationMismatch: String { return L10n.tr("Localizable", "screen_notification_settings_configuration_mismatch") }
+  /// We’ve simplified Notifications Settings to make options easier to find.
+  /// 
+  /// Some custom settings you’ve chosen in the past are not shown here, but they’re still active.
+  /// 
+  /// If you proceed, some of your settings may change.
+  public static var screenNotificationSettingsConfigurationMismatchDescription: String { return L10n.tr("Localizable", "screen_notification_settings_configuration_mismatch_description") }
  /// Direct chats
  public static var screenNotificationSettingsDirectChats: String { return L10n.tr("Localizable", "screen_notification_settings_direct_chats") }
  /// Custom setting per chat
@@ -702,6 +710,8 @@ public enum L10n {
  public static var screenNotificationSettingsEditScreenGroupSectionHeader: String { return L10n.tr("Localizable", "screen_notification_settings_edit_screen_group_section_header") }
  /// Enable notifications on this device
  public static var screenNotificationSettingsEnableNotifications: String { return L10n.tr("Localizable", "screen_notification_settings_enable_notifications") }
+  /// The configuration has not been corrected, please try again.
+  public static var screenNotificationSettingsFailedFixingConfiguration: String { return L10n.tr("Localizable", "screen_notification_settings_failed_fixing_configuration") }
  /// Group chats
  public static var screenNotificationSettingsGroupChats: String { return L10n.tr("Localizable", "screen_notification_settings_group_chats") }
  /// Mentions
@@ -38,6 +38,8 @@ extension NotificationSettingsProxyMock {
        getDefaultRoomNotificationModeIsEncryptedIsOneToOneReturnValue = configuration.defaultRoomMode
        getUserDefinedRoomNotificationModeRoomIdReturnValue = configuration.roomMode.isDefault ? nil : configuration.roomMode.mode
        getRoomsWithUserDefinedRulesReturnValue = []
+        isRoomMentionEnabledReturnValue = true
+        isCallEnabledReturnValue = true
        
        setNotificationModeRoomIdModeClosure = { [weak self] _, mode in
            guard let self else { return }
@@ -36,6 +36,7 @@ struct A11yIdentifiers {
    static let invitesScreen = InvitesScreen()
    static let welcomeScreen = WelcomeScreen()
    static let migrationScreen = MigrationScreen()
+    static let notificationSettingsScreen = NotificationSettingsScreen()
    static let notificationSettingsEditScreen = NotificationSettingsEditScreen()

    struct AnalyticsPromptScreen {
@@ -163,6 +164,10 @@ struct A11yIdentifiers {
        let message = "migration_screen-message"
    }
    
+    struct NotificationSettingsScreen {
+        let fixMismatchConfiguration = "notification_settings_screen-fix_mismatch_configuration"
+    }
+    
    struct NotificationSettingsEditScreen {
        let roomNamePrefix = "notification_settings_edit_screen-room_name"
        func roomName(_ name: String) -> String {
@@ -28,6 +28,7 @@ struct NotificationSettingsScreenViewState: BindableState {
    let isModallyPresented: Bool
    var isUserPermissionGranted: Bool?
    var allowedNotificationModes: [RoomNotificationModeProxy] = [.allMessages, .mentionsAndKeywordsOnly]
+    var fixingConfigurationMismatch = false
    
    var showSystemNotificationsAlert: Bool {
        bindings.enableNotifications && isUserPermissionGranted == false
@@ -51,7 +52,12 @@ struct NotificationSettingsScreenSettings {
    let callsEnabled: Bool?
    // Old clients were having specific settings for encrypted and unencrypted rooms,
    // so it's possible for `group chats` and `direct chats` settings to be inconsistent (e.g. encrypted `direct chats` can have a different mode that unencrypted `direct chats`)
-    let inconsistentSettings: Bool
+    let inconsistentSettings: [NotificationSettingsScreenInvalidSetting]
+}
+
+struct NotificationSettingsScreenInvalidSetting: Equatable {
+    let chatType: NotificationSettingsChatType
+    let isEncrypted: Bool
 }

 struct NotificationSettingsScreenStrings {
@@ -87,9 +93,11 @@ enum NotificationSettingsScreenViewAction {
    case roomMentionChanged
    case callsChanged
    case close
+    case fixConfigurationMismatchTapped
 }

 enum NotificationSettingsScreenErrorType: Hashable {
    /// A specific error message shown in an alert.
    case alert
+    case fixMismatchConfigurationFailed
 }
@@ -77,6 +77,8 @@ class NotificationSettingsScreenViewModel: NotificationSettingsScreenViewModelTy
            Task { await enableCalls(state.bindings.callsEnabled) }
        case .close:
            actionsSubject.send(.close)
+        case .fixConfigurationMismatchTapped:
+            Task { await fixConfigurationMismatch() }
        }
    }
    
@@ -121,6 +123,7 @@ class NotificationSettingsScreenViewModel: NotificationSettingsScreenViewModelTy
    
    private func fetchSettings() {
        fetchSettingsTask = Task {
+            var inconsistentSettings: [NotificationSettingsScreenInvalidSetting] = []
            // Group chats
            var groupChatsMode = await notificationSettingsProxy.getDefaultRoomNotificationMode(isEncrypted: false, isOneToOne: false)
            let encryptedGroupChatsMode = await notificationSettingsProxy.getDefaultRoomNotificationMode(isEncrypted: true, isOneToOne: false)
@@ -131,14 +134,15 @@ class NotificationSettingsScreenViewModel: NotificationSettingsScreenViewModelTy
                        
            // Old clients were having specific settings for encrypted and unencrypted rooms,
            // so it's possible for `group chats` and `direct chats` settings to be inconsistent (e.g. encrypted `direct chats` can have a different mode that unencrypted `direct chats`)
-            var inconsistencyDetected = false
            if groupChatsMode != encryptedGroupChatsMode {
                groupChatsMode = .allMessages
-                inconsistencyDetected = true
+                // a default setting for a chat can only be `.allMessages` or `.mentionsAndKeywordsOnly`.
+                inconsistentSettings.append(.init(chatType: .groupChat, isEncrypted: encryptedGroupChatsMode != .allMessages))
            }
            if directChatsMode != encryptedDirectChatsMode {
                directChatsMode = .allMessages
-                inconsistencyDetected = true
+                // a default setting for a chat can only be `.allMessages` or `.mentionsAndKeywordsOnly`.
+                inconsistentSettings.append(.init(chatType: .oneToOneChat, isEncrypted: encryptedDirectChatsMode != .allMessages))
            }
            
            // The following calls may fail if the associated push rule doesn't exist
@@ -151,7 +155,7 @@ class NotificationSettingsScreenViewModel: NotificationSettingsScreenViewModelTy
                                                                          directChatsMode: directChatsMode,
                                                                          roomMentionsEnabled: roomMentionsEnabled,
                                                                          callsEnabled: callEnabled,
-                                                                          inconsistentSettings: inconsistencyDetected)
+                                                                          inconsistentSettings: inconsistentSettings)

            state.settings = notificationSettings
            state.bindings.roomMentionsEnabled = notificationSettings.roomMentionsEnabled ?? false
@@ -159,6 +163,28 @@ class NotificationSettingsScreenViewModel: NotificationSettingsScreenViewModelTy
        }
    }
    
+    private func fixConfigurationMismatch() async {
+        guard let settings = state.settings, !settings.inconsistentSettings.isEmpty, !state.fixingConfigurationMismatch else { return }
+        state.fixingConfigurationMismatch = true
+        
+        var failures: [NotificationSettingsScreenInvalidSetting] = []
+        for inconsistentSetting in settings.inconsistentSettings {
+            do {
+                try await notificationSettingsProxy.setDefaultRoomNotificationMode(isEncrypted: inconsistentSetting.isEncrypted, isOneToOne: inconsistentSetting.chatType == .oneToOneChat, mode: .allMessages)
+            } catch {
+                failures.append(inconsistentSetting)
+            }
+        }
+        
+        if !failures.isEmpty {
+            state.bindings.alertInfo = AlertInfo(id: .fixMismatchConfigurationFailed,
+                                                 title: L10n.commonError,
+                                                 message: L10n.screenNotificationSettingsFailedFixingConfiguration,
+                                                 primaryButton: .init(title: L10n.actionOk, action: nil))
+        }
+        state.fixingConfigurationMismatch = false
+    }
+    
    private func enableRoomMention(_ enable: Bool) async {
        guard let notificationSettings = state.settings else { return }
        do {
@@ -22,17 +22,21 @@ struct NotificationSettingsScreen: View {
    
    var body: some View {
        Form {
-            if context.viewState.showSystemNotificationsAlert {
-                userPermissionSection
-            }
-            enableNotificationSection
-            if context.enableNotifications {
-                roomsNotificationSection
-                if context.viewState.settings?.roomMentionsEnabled != nil {
-                    mentionsSection
+            if context.viewState.settings?.inconsistentSettings.isEmpty == false {
+                configurationMismatchSection
+            } else {
+                if context.viewState.showSystemNotificationsAlert {
+                    userPermissionSection
                }
-                if context.viewState.settings?.callsEnabled != nil {
-                    callsSection
+                enableNotificationSection
+                if context.enableNotifications {
+                    roomsNotificationSection
+                    if context.viewState.settings?.roomMentionsEnabled != nil {
+                        mentionsSection
+                    }
+                    if context.viewState.settings?.callsEnabled != nil {
+                        callsSection
+                    }
                }
            }
        }
@@ -152,6 +156,33 @@ struct NotificationSettingsScreen: View {
                .compoundListSectionHeader()
        }
    }
+    
+    private var configurationMismatchSection: some View {
+        Section {
+            ListRow(kind: .custom {
+                VStack(alignment: .leading, spacing: 12) {
+                    Image(systemName: "gearshape")
+                        .font(.compound.headingSMSemibold)
+                    VStack(alignment: .leading, spacing: 4) {
+                        Text(L10n.screenNotificationSettingsConfigurationMismatch)
+                            .font(.compound.headingSMSemibold)
+                        Text(L10n.screenNotificationSettingsConfigurationMismatchDescription)
+                            .foregroundColor(.compound.textSecondary)
+                    }
+                    Button {
+                        context.send(viewAction: .fixConfigurationMismatchTapped)
+                    } label: {
+                        Text(L10n.actionContinue)
+                    }
+                    .buttonStyle(.elementCapsuleProminent)
+                    .disabled(context.viewState.fixingConfigurationMismatch)
+                    .accessibilityIdentifier(A11yIdentifiers.notificationSettingsScreen.fixMismatchConfiguration)
+                }
+                .padding(.horizontal, ListRowPadding.horizontal)
+                .padding(.vertical, ListRowPadding.vertical)
+            })
+        }
+    }
 }

 // MARK: - Previews
@@ -183,8 +214,39 @@ struct NotificationSettingsScreen_Previews: PreviewProvider {
        viewModel.fetchInitialContent()
        return viewModel
    }()
+    
+    static let viewModelConfigurationMismatch: NotificationSettingsScreenViewModel = {
+        let appSettings = AppSettings()
+        let notificationCenter = UserNotificationCenterMock()
+        notificationCenter.authorizationStatusReturnValue = .notDetermined
+        let notificationSettingsProxy = NotificationSettingsProxyMock(with: .init())
+        notificationSettingsProxy.getDefaultRoomNotificationModeIsEncryptedIsOneToOneClosure = { isEncrypted, isOneToOne in
+            switch (isEncrypted, isOneToOne) {
+            case (true, true):
+                return .allMessages
+            case (false, true):
+                return .mute
+            default:
+                return .mentionsAndKeywordsOnly
+            }
+        }
+        notificationSettingsProxy.isRoomMentionEnabledReturnValue = true
+        notificationSettingsProxy.isCallEnabledReturnValue = false
+        
+        let userSession = MockUserSession(clientProxy: MockClientProxy(userID: "John Doe"), mediaProvider: MockMediaProvider())
+
+        var viewModel = NotificationSettingsScreenViewModel(userSession: userSession,
+                                                            appSettings: appSettings,
+                                                            userNotificationCenter: notificationCenter,
+                                                            notificationSettingsProxy: notificationSettingsProxy,
+                                                            isModallyPresented: true)
+        viewModel.fetchInitialContent()
+        return viewModel
+    }()

    static var previews: some View {
        NotificationSettingsScreen(context: viewModel.context)
+        NotificationSettingsScreen(context: viewModelConfigurationMismatch.context)
+            .previewDisplayName("Configuration mismatch")
    }
 }
@@ -204,6 +204,25 @@ class MockScreen: Identifiable {
                                                                             notificationSettings: NotificationSettingsProxyMock(with: .init()),
                                                                             isModallyPresented: false)
            return NotificationSettingsScreenCoordinator(parameters: parameters)
+        case .notificationSettingsScreenMismatchConfiguration:
+            let userNotificationCenter = UserNotificationCenterMock()
+            userNotificationCenter.authorizationStatusReturnValue = .denied
+            let session = MockUserSession(clientProxy: MockClientProxy(userID: "@mock:matrix.org"),
+                                          mediaProvider: MockMediaProvider())
+            let notificationSettings = NotificationSettingsProxyMock(with: .init())
+            notificationSettings.getDefaultRoomNotificationModeIsEncryptedIsOneToOneClosure = { isEncrypted, isOneToOne in
+                switch (isEncrypted, isOneToOne) {
+                case (true, _):
+                    return .allMessages
+                case (false, _):
+                    return .mentionsAndKeywordsOnly
+                }
+            }
+            let parameters = NotificationSettingsScreenCoordinatorParameters(userSession: session,
+                                                                             userNotificationCenter: userNotificationCenter,
+                                                                             notificationSettings: notificationSettings,
+                                                                             isModallyPresented: false)
+            return NotificationSettingsScreenCoordinator(parameters: parameters)
        case .onboarding:
            return OnboardingCoordinator()
        case .roomPlainNoAvatar:
@@ -34,6 +34,7 @@ enum UITestsScreenIdentifier: String {
    case bugReport
    case bugReportWithScreenshot
    case notificationSettingsScreen
+    case notificationSettingsScreenMismatchConfiguration
    case onboarding
    case roomPlainNoAvatar
    case roomEncryptedWithAvatar
@@ -18,4 +18,14 @@ import ElementX
 import XCTest

@MainActor
-class NotificationSettingsScreenUITests: XCTestCase { }
+class NotificationSettingsScreenUITests: XCTestCase {
+    func testRegularScreen() async throws {
+        let app = Application.launch(.notificationSettingsScreen)
+        try await app.assertScreenshot(.notificationSettingsScreen)
+    }
+    
+    func testMismatchConfigurationScreen() async throws {
+        let app = Application.launch(.notificationSettingsScreenMismatchConfiguration)
+        try await app.assertScreenshot(.notificationSettingsScreenMismatchConfiguration)
+    }
+}
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:ba5e1710d50a76d3581f946c77e0736ca8274e2198e28b69794a3b4851fdc2c3
+size 115177
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:12aa24cb445649e4b6520e960667a92c8166bf84418e24306380d63aee6c03a7
+size 95563
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:79c131aa3127ebfa5737ef286e550925c601f493368083ba3c034763d3cd4ab7
+size 145299
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:9319b24a7b129121d5e6bf8a14fae1b9700df5b44ddea3e0be03872602480578
+size 129165
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:c92c4baf77cb2ad25f07aeea790075400dc2ecd57a6e44b523193bd616f6190b
+size 134108
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:a339016cc249bc0f2014b7f65d084213115a2ceac30c7b7df65379ab47ea1512
+size 124169
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:570373515f3a642836661d2f549d3f93c914245f333114640339402ae42ef9bb
+size 189090
@@ -0,0 +1,3 @@
+version https://git-lfs.github.com/spec/v1
+oid sha256:69fc8921f9bd24b07eeb042da2b6b4dc85861dda6a62f94133fa136c5bfb498a
+size 197579
@@ -82,7 +82,7 @@ class NotificationSettingsScreenViewModelTests: XCTestCase {
        
        XCTAssertEqual(context.viewState.settings?.groupChatsMode, .mentionsAndKeywordsOnly)
        XCTAssertEqual(context.viewState.settings?.directChatsMode, .allMessages)
-        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, false)
+        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, [])
        XCTAssertNil(context.viewState.bindings.alertInfo)
    }
        
@@ -104,7 +104,7 @@ class NotificationSettingsScreenViewModelTests: XCTestCase {
        try await deferred.fulfill()
        
        XCTAssertEqual(context.viewState.settings?.groupChatsMode, .allMessages)
-        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, true)
+        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, [.init(chatType: .groupChat, isEncrypted: false)])
    }
    
    func testInconsistentDirectChatsSettings() async throws {
@@ -125,7 +125,90 @@ class NotificationSettingsScreenViewModelTests: XCTestCase {
        try await deferred.fulfill()
        
        XCTAssertEqual(context.viewState.settings?.directChatsMode, .allMessages)
-        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, true)
+        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, [.init(chatType: .oneToOneChat, isEncrypted: false)])
+    }
+    
+    func testFixInconsistentSettings() async throws {
+        // Initialize with a configuration mismatch where encrypted one-to-one chats is `.allMessages` and unencrypted one-to-one chats is `.mentionsAndKeywordsOnly`
+        notificationSettingsProxy.getDefaultRoomNotificationModeIsEncryptedIsOneToOneClosure = { isEncrypted, isOneToOne in
+            switch (isEncrypted, isOneToOne) {
+            case (true, true):
+                return .allMessages
+            case (false, true):
+                return .mentionsAndKeywordsOnly
+            default:
+                return .allMessages
+            }
+        }
+                
+        let deferred = deferFulfillment(viewModel.context.$viewState.map(\.settings)
+            .first(where: { $0 != nil }))
+        notificationSettingsProxy.callbacks.send(.settingsDidChange)
+        try await deferred.fulfill()
+        
+        XCTAssertEqual(context.viewState.settings?.directChatsMode, .allMessages)
+        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, [.init(chatType: .oneToOneChat, isEncrypted: false)])
+        
+        let deferredState = deferFulfillment(viewModel.context.$viewState
+            .map(\.fixingConfigurationMismatch)
+            .removeDuplicates()
+            .collect(3)
+            .first())
+        context.send(viewAction: .fixConfigurationMismatchTapped)
+        let fixingStates = try await deferredState.fulfill()
+        
+        XCTAssertEqual(fixingStates, [false, true, false])
+        
+        // Ensure we only fix the invalid setting: unencrypted one-to-one chats should be set to `.allMessages` (to match encrypted one-to-one chats)
+        XCTAssertEqual(notificationSettingsProxy.setDefaultRoomNotificationModeIsEncryptedIsOneToOneModeCallsCount, 1)
+        let callArguments = notificationSettingsProxy.setDefaultRoomNotificationModeIsEncryptedIsOneToOneModeReceivedArguments
+        XCTAssertEqual(callArguments?.isEncrypted, false)
+        XCTAssertEqual(callArguments?.isOneToOne, true)
+        XCTAssertEqual(callArguments?.mode, .allMessages)
+    }
+    
+    func testFixAllInconsistentSettings() async throws {
+        // Initialize with a configuration mismatch where
+        // - encrypted one-to-one chats is `.allMessages` and unencrypted one-to-one chats is `.mentionsAndKeywordsOnly`
+        // - encrypted group chats is `.allMessages` and unencrypted group chats is `.mentionsAndKeywordsOnly`
+        notificationSettingsProxy.getDefaultRoomNotificationModeIsEncryptedIsOneToOneClosure = { isEncrypted, isOneToOne in
+            switch (isEncrypted, isOneToOne) {
+            case (true, _):
+                return .allMessages
+            case (false, _):
+                return .mentionsAndKeywordsOnly
+            }
+        }
+        
+        let deferred = deferFulfillment(viewModel.context.$viewState.map(\.settings)
+            .first(where: { $0 != nil }))
+        notificationSettingsProxy.callbacks.send(.settingsDidChange)
+        try await deferred.fulfill()
+        
+        XCTAssertEqual(context.viewState.settings?.directChatsMode, .allMessages)
+        XCTAssertEqual(context.viewState.settings?.inconsistentSettings, [.init(chatType: .groupChat, isEncrypted: false), .init(chatType: .oneToOneChat, isEncrypted: false)])
+        
+        let deferredState = deferFulfillment(viewModel.context.$viewState
+            .map(\.fixingConfigurationMismatch)
+            .removeDuplicates()
+            .collect(3)
+            .first())
+        context.send(viewAction: .fixConfigurationMismatchTapped)
+        let fixingStates = try await deferredState.fulfill()
+        
+        XCTAssertEqual(fixingStates, [false, true, false])
+        
+        // All problems should be fixed
+        XCTAssertEqual(notificationSettingsProxy.setDefaultRoomNotificationModeIsEncryptedIsOneToOneModeCallsCount, 2)
+        let callArguments = notificationSettingsProxy.setDefaultRoomNotificationModeIsEncryptedIsOneToOneModeReceivedInvocations
+        // Ensure we fix the invalid unencrypted group chats setting (it should be set to `.allMessages` to match encrypted group chats)
+        XCTAssertEqual(callArguments[0].isEncrypted, false)
+        XCTAssertEqual(callArguments[0].isOneToOne, false)
+        XCTAssertEqual(callArguments[0].mode, .allMessages)
+        // Ensure we fix the invalid unencrypted one-to-one chats setting (it should be set to `.allMessages` to match encrypted one-to-one chats)
+        XCTAssertEqual(callArguments[1].isEncrypted, false)
+        XCTAssertEqual(callArguments[1].isOneToOne, true)
+        XCTAssertEqual(callArguments[1].mode, .allMessages)
    }
    
    func testToggleRoomMentionOff() async throws {