Sync polls' push rules (PSG-77, PSG-1097)

RiotSwiftUI/Modules/Settings/Notifications/Model/Mock/MockNotificationPushRule.swift

@@ -19,7 +19,9 @@ import Foundation
 struct MockNotificationPushRule: NotificationPushRuleType {
     var ruleId: String!
     var enabled: Bool
+    var actions: NotificationActions? = NotificationStandardActions.notifyDefaultSound.actions
+
     func matches(standardActions: NotificationStandardActions?) -> Bool {
-        false
+        standardActions?.actions == actions
     }
 }

RiotSwiftUI/Modules/Settings/Notifications/Model/NotificationActions.swift

@@ -17,7 +17,7 @@
 import Foundation
 
 /// The actions defined on a push rule, used in the static push rule definitions.
-struct NotificationActions {
+struct NotificationActions: Equatable {
     let notify: Bool
     let highlight: Bool
     let sound: String?

RiotSwiftUI/Modules/Settings/Notifications/Model/NotificationPushRuleDefinitions.swift

@@ -22,7 +22,7 @@ extension NotificationPushRuleId {
     /// It is defined similarly across Web and Android.
     /// - Parameter index: The notification index for which to get the actions for.
     /// - Returns: The associated `NotificationStandardActions`.
-    func standardActions(for index: NotificationIndex) -> NotificationStandardActions? {
+    func standardActions(for index: NotificationIndex) -> NotificationStandardActions {
         switch self {
         case .containDisplayName:
             switch index {
@@ -42,7 +42,7 @@ extension NotificationPushRuleId {
             case .silent: return .notify
             case .noisy: return .highlight
             }
-        case .oneToOneRoom:
+        case .oneToOneRoom, .oneToOnePollStart, .msc3930oneToOnePollStart, .oneToOnePollEnd, .msc3930oneToOnePollEnd:
             switch index {
             case .off: return .dontNotify
             case .silent: return .notify
@@ -54,7 +54,7 @@ extension NotificationPushRuleId {
             case .silent: return .notify
             case .noisy: return .notifyDefaultSound
             }
-        case .allOtherMessages:
+        case .allOtherMessages, .pollStart, .msc3930pollStart, .pollEnd, .msc3930pollEnd:
             switch index {
             case .off: return .dontNotify
             case .silent: return .notify

RiotSwiftUI/Modules/Settings/Notifications/Model/NotificationPushRuleIds.swift

@@ -30,6 +30,18 @@ enum NotificationPushRuleId: String {
     case allOtherMessages = ".m.rule.message"
     case encrypted = ".m.rule.encrypted"
     case keywords = "_keywords"
+    // poll started event
+    case pollStart = ".m.rule.poll_start"
+    case msc3930pollStart = ".org.matrix.msc3930.rule.poll_start"
+    // poll started event (one to one)
+    case oneToOnePollStart = ".m.rule.poll_start_one_to_one"
+    case msc3930oneToOnePollStart = ".org.matrix.msc3930.rule.poll_start_one_to_one"
+    // poll ended event
+    case pollEnd = ".m.rule.poll_end"
+    case msc3930pollEnd = ".org.matrix.msc3930.rule.poll_end"
+    // poll ended event (one to one)
+    case oneToOnePollEnd = ".m.rule.poll_end_one_to_one"
+    case msc3930oneToOnePollEnd = ".org.matrix.msc3930.rule.poll_end_one_to_one"
 }
 
 extension NotificationPushRuleId: Identifiable {
@@ -65,6 +77,20 @@ extension NotificationPushRuleId {
             return VectorL10n.settingsEncryptedGroupMessages
         case .keywords:
             return VectorL10n.settingsMessagesContainingKeywords
+        case .pollStart, .msc3930pollStart, .oneToOnePollStart, .msc3930oneToOnePollStart, .pollEnd, .msc3930pollEnd, .oneToOnePollEnd, .msc3930oneToOnePollEnd:
+            // They don't need to be rendered on the UI
+            return ""
+        }
+    }
+
+    var syncedRules: [NotificationPushRuleId] {
+        switch self {
+        case .oneToOneRoom:
+            return [.oneToOnePollStart, .msc3930oneToOnePollStart, .oneToOnePollEnd, .msc3930oneToOnePollEnd]
+        case .allOtherMessages:
+            return [.pollStart, .msc3930pollStart, .pollEnd, .msc3930pollEnd]
+        default:
+            return []
         }
     }
 }

RiotSwiftUI/Modules/Settings/Notifications/Service/MatrixSDK/MXNotificationSettingsService.swift

@@ -44,7 +44,9 @@ class MXNotificationSettingsService: NotificationSettingsServiceType {
 
         // Observe future updates to content rules
         rulesUpdated
-            .compactMap { _ in self.session.notificationCenter.rules.global.content as? [MXPushRule] }
+            .compactMap { [weak self] _ in
+                self?.session.notificationCenter.rules.global.content as? [MXPushRule]
+            }
             .assign(to: &$contentRules)
 
         // Set initial value of rules
@@ -53,14 +55,15 @@ class MXNotificationSettingsService: NotificationSettingsServiceType {
         }
         // Observe future updates to rules
         rulesUpdated
-            .compactMap { _ in self.session.notificationCenter.flatRules as? [MXPushRule] }
+            .compactMap { [weak self] _ in
+                self?.session.notificationCenter.flatRules as? [MXPushRule]
+            }
             .assign(to: &$rules)
     }
 
     func add(keyword: String, enabled: Bool) {
         let index = NotificationIndex.index(when: enabled)
-        guard let actions = NotificationPushRuleId.keywords.standardActions(for: index)?.actions
-        else {
+        guard let actions = NotificationPushRuleId.keywords.standardActions(for: index).actions else {
             return
         }
         session.notificationCenter.addContentRuleWithRuleId(matchingPattern: keyword, notify: actions.notify, sound: actions.sound, highlight: actions.highlight)
@@ -71,16 +74,52 @@ class MXNotificationSettingsService: NotificationSettingsServiceType {
         session.notificationCenter.removeRule(rule)
     }
 
-    func updatePushRuleActions(for ruleId: String, enabled: Bool, actions: NotificationActions?) {
-        guard let rule = session.notificationCenter.rule(byId: ruleId) else { return }
-        session.notificationCenter.enableRule(rule, isEnabled: enabled)
+    func updatePushRuleActions(for ruleId: String,
+                               enabled: Bool,
+                               actions: NotificationActions?) async throws {
 
-        if let actions = actions {
-            session.notificationCenter.updatePushRuleActions(ruleId,
-                                                             kind: rule.kind,
-                                                             notify: actions.notify,
-                                                             soundName: actions.sound,
-                                                             highlight: actions.highlight)
+        guard let rule = session.notificationCenter.rule(byId: ruleId) else {
+            return
+        }
+
+        guard let actions = actions else {
+            try await session.notificationCenter.enableRule(pushRule: rule, isEnabled: enabled)
+            return
+        }
+
+        // Updating the actions before enabling the rule allows the homeserver to triggers just one sync update
+        try await session.notificationCenter.updatePushRuleActions(ruleId,
+                                                         kind: rule.kind,
+                                                         notify: actions.notify,
+                                                         soundName: actions.sound,
+                                                         highlight: actions.highlight)
+
+        try await session.notificationCenter.enableRule(pushRule: rule, isEnabled: enabled)
+    }
+}
+
+private extension MXNotificationCenter {
+    func enableRule(pushRule: MXPushRule, isEnabled: Bool) async throws {
+        try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
+            enableRule(pushRule, isEnabled: isEnabled) { error in
+                if let error = error {
+                    continuation.resume(with: .failure(error))
+                } else {
+                    continuation.resume()
+                }
+            }
+        }
+    }
+
+    func updatePushRuleActions(ruleId: String, kind: __MXPushRuleKind, notify: Bool, soundName: String, highlight: Bool) async throws {
+        try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
+            updatePushRuleActions(ruleId, kind: kind, notify: notify, soundName: soundName, highlight: highlight) { error in
+                if let error = error {
+                    continuation.resume(with: .failure(error))
+                } else {
+                    continuation.resume()
+                }
+            }
         }
     }
 }

RiotSwiftUI/Modules/Settings/Notifications/Service/Mock/MockNotificationSettingsService.swift

@@ -44,5 +44,11 @@ class MockNotificationSettingsService: NotificationSettingsServiceType, Observab
         keywords.remove(keyword)
     }
 
-    func updatePushRuleActions(for ruleId: String, enabled: Bool, actions: NotificationActions?) { }
+    func updatePushRuleActions(for ruleId: String, enabled: Bool, actions: NotificationActions?) async throws {
+        guard let ruleIndex = rules.firstIndex(where: { $0.ruleId == ruleId }) else {
+            return
+        }
+
+        rules[ruleIndex] = MockNotificationPushRule(ruleId: ruleId, enabled: enabled, actions: actions)
+    }
 }

RiotSwiftUI/Modules/Settings/Notifications/Service/NotificationSettingsServiceType.swift

@@ -40,5 +40,5 @@ protocol NotificationSettingsServiceType {
     ///   - ruleId: The id of the rule.
     ///   - enabled: Whether the rule should be enabled or disabled.
     ///   - actions: The actions to update with.
-    func updatePushRuleActions(for ruleId: String, enabled: Bool, actions: NotificationActions?)
+    func updatePushRuleActions(for ruleId: String, enabled: Bool, actions: NotificationActions?) async throws
 }

RiotSwiftUI/Modules/Settings/Notifications/Test/Unit/NotificationSettingsViewModelTests.swift

@@ -0,0 +1,136 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+@testable import RiotSwiftUI
+import XCTest
+
+final class NotificationSettingsViewModelTests: XCTestCase {
+    private var viewModel: NotificationSettingsViewModel!
+    private var notificationService: MockNotificationSettingsService!
+
+    override func setUpWithError() throws {
+        notificationService = .init()
+    }
+
+    func testAllTheRulesAreChecked() throws {
+        viewModel = .init(notificationSettingsService: notificationService, ruleIds: .default)
+
+        XCTAssertEqual(viewModel.viewState.selectionState.count, 4)
+        XCTAssertTrue(viewModel.viewState.selectionState.values.allSatisfy { $0 })
+    }
+
+    func testUpdateRule() async {
+        viewModel = .init(notificationSettingsService: notificationService, ruleIds: .default)
+        notificationService.rules = [MockNotificationPushRule].default
+
+        await viewModel.update(ruleID: .encrypted, isChecked: false)
+        XCTAssertEqual(viewModel.viewState.selectionState.count, 4)
+        XCTAssertEqual(viewModel.viewState.selectionState[.encrypted], false)
+    }
+
+    func testUpdateOneToOneRuleAlsoUpdatesPollRules() async {
+        setupWithPollRules()
+
+        await viewModel.update(ruleID: .oneToOneRoom, isChecked: false)
+
+        XCTAssertEqual(viewModel.viewState.selectionState.count, 8)
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOneRoom], false)
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOnePollStart], false)
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOnePollEnd], false)
+
+        // unrelated poll rules stay the same
+        XCTAssertEqual(viewModel.viewState.selectionState[.allOtherMessages], true)
+        XCTAssertEqual(viewModel.viewState.selectionState[.pollStart], true)
+        XCTAssertEqual(viewModel.viewState.selectionState[.pollEnd], true)
+    }
+
+    func testUpdateMessageRuleAlsoUpdatesPollRules() async {
+        setupWithPollRules()
+
+        await viewModel.update(ruleID: .allOtherMessages, isChecked: false)
+        XCTAssertEqual(viewModel.viewState.selectionState.count, 8)
+        XCTAssertEqual(viewModel.viewState.selectionState[.allOtherMessages], false)
+        XCTAssertEqual(viewModel.viewState.selectionState[.pollStart], false)
+        XCTAssertEqual(viewModel.viewState.selectionState[.pollEnd], false)
+
+        // unrelated poll rules stay the same
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOneRoom], true)
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOnePollStart], true)
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOnePollEnd], true)
+    }
+
+    func testMismatchingRulesAreHandled() async {
+        setupWithPollRules()
+
+        await viewModel.update(ruleID: .allOtherMessages, isChecked: false)
+
+        // simulating a "mismatch" on the poll started rule
+        await viewModel.update(ruleID: .pollStart, isChecked: true)
+
+        XCTAssertEqual(viewModel.viewState.selectionState.count, 8)
+
+        // The other messages rule ui flag should match the loudest related poll rule
+        XCTAssertEqual(viewModel.viewState.selectionState[.allOtherMessages], true)
+    }
+
+    func testMismatchingOneToOneRulesAreHandled() async {
+        setupWithPollRules()
+
+        await viewModel.update(ruleID: .oneToOneRoom, isChecked: false)
+        // simulating a "mismatch" on the one to one poll started rule
+        await viewModel.update(ruleID: .oneToOnePollStart, isChecked: true)
+
+        XCTAssertEqual(viewModel.viewState.selectionState.count, 8)
+
+        // The one to one room rule ui flag should match the loudest related poll rule
+        XCTAssertEqual(viewModel.viewState.selectionState[.oneToOneRoom], true)
+    }
+}
+
+private extension NotificationSettingsViewModelTests {
+    func setupWithPollRules() {
+        viewModel = .init(notificationSettingsService: notificationService, ruleIds: .default + .polls)
+        notificationService.rules = [MockNotificationPushRule].default + [MockNotificationPushRule].polls
+    }
+}
+
+private extension Array where Element == NotificationPushRuleId {
+    static var `default`: [NotificationPushRuleId] {
+        [.oneToOneRoom, .allOtherMessages, .oneToOneEncryptedRoom, .encrypted]
+    }
+
+    static var polls: [NotificationPushRuleId] {
+        [.pollStart, .pollEnd, .oneToOnePollStart, .oneToOnePollEnd]
+    }
+}
+
+private extension Array where Element == MockNotificationPushRule {
+    static var `default`: [MockNotificationPushRule] {
+        [NotificationPushRuleId]
+            .default
+            .map { ruleId in
+                MockNotificationPushRule(ruleId: ruleId.rawValue, enabled: true)
+            }
+    }
+
+    static var polls: [MockNotificationPushRule] {
+        [NotificationPushRuleId]
+            .polls
+            .map { ruleId in
+                MockNotificationPushRule(ruleId: ruleId.rawValue, enabled: true)
+            }
+    }
+}

RiotSwiftUI/Modules/Settings/Notifications/View/NotificationSettings.swift

@@ -33,13 +33,16 @@ struct NotificationSettings<BottomSection: View>: View {
                 ForEach(viewModel.viewState.ruleIds) { ruleId in
                     let checked = viewModel.viewState.selectionState[ruleId] ?? false
                     FormPickerItem(title: ruleId.title, selected: checked) {
-                        viewModel.update(ruleID: ruleId, isChecked: !checked)
+                        Task {
+                            await viewModel.update(ruleID: ruleId, isChecked: !checked)
+                        }
                     }
                 }
             }
             bottomSection
         }
         .activityIndicator(show: viewModel.viewState.saving)
+        .disabled(viewModel.viewState.saving)
     }
 }