Merge pull request #8141 from vector-im/feature/fre/poll_sync_error_h…

…andling

[Poll] Error handling for push rules synchronization

changelog.d/8141.feature

@@ -0,0 +1 @@
+[Poll] Error handling for push rules synchronization

library/ui-strings/src/main/res/values/strings.xml

@@ -862,6 +862,8 @@
     <string name="settings_notification_keyword_contains_dot">Keywords cannot start with \'.\'</string>
     <string name="settings_notification_keyword_contains_invalid_character">Keywords cannot contain \'%s\'</string>
 
+    <string name="settings_notification_error_on_update">An error occurred when updating your notification preferences. Please try again.</string>
+
     <string name="settings_notification_troubleshoot">Troubleshoot Notifications</string>
     <string name="settings_troubleshoot_diagnostic">Troubleshooting diagnostics</string>
     <string name="settings_troubleshoot_diagnostic_run_button_title">Run Tests</string>

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/events/model/Event.kt

@@ -488,7 +488,7 @@ fun Event.getPollContent(): MessagePollContent? {
 }
 
 fun Event.supportsNotification() =
-        this.getClearType() in EventType.MESSAGE + EventType.POLL_START.values + EventType.STATE_ROOM_BEACON_INFO.values
+        this.getClearType() in EventType.MESSAGE + EventType.POLL_START.values + EventType.POLL_END.values + EventType.STATE_ROOM_BEACON_INFO.values
 
 fun Event.isContentReportable() =
         this.getClearType() in EventType.MESSAGE + EventType.STATE_ROOM_BEACON_INFO.values

vector/src/main/java/im/vector/app/core/preference/VectorCheckboxPreference.kt

@@ -32,6 +32,8 @@ class VectorCheckboxPreference : CheckBoxPreference {
 
     constructor(context: Context) : super(context)
 
+    var summaryTextColor: Int? = null
+
     init {
         // Set to false to remove the space when there is no icon
         isIconSpaceReserved = true
@@ -42,4 +44,9 @@ class VectorCheckboxPreference : CheckBoxPreference {
         (holder.findViewById(android.R.id.title) as? TextView)?.isSingleLine = false
         super.onBindViewHolder(holder)
     }
+
+    override fun syncSummaryView(holder: PreferenceViewHolder) {
+        super.syncSummaryView(holder)
+        summaryTextColor?.let { (holder.findViewById(android.R.id.summary) as? TextView)?.setTextColor(it) }
+    }
 }

vector/src/main/java/im/vector/app/features/settings/notifications/PushRuleDefinitions.kt

@@ -42,8 +42,7 @@ fun getStandardAction(ruleId: String, index: NotificationIndex): StandardActions
         RuleIds.RULE_ID_POLL_START_ONE_TO_ONE,
         RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE,
         RuleIds.RULE_ID_POLL_END_ONE_TO_ONE,
-        RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE,
-        ->
+        RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE ->
             when (index) {
                 NotificationIndex.OFF -> StandardActions.DontNotify
                 NotificationIndex.SILENT -> StandardActions.Notify

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationFragment.kt

@@ -21,71 +21,92 @@ import android.view.View
 import androidx.preference.Preference
 import com.airbnb.mvrx.fragmentViewModel
 import com.airbnb.mvrx.withState
+import im.vector.app.R
 import im.vector.app.core.preference.VectorCheckboxPreference
 import im.vector.app.features.settings.VectorSettingsBaseFragment
+import im.vector.app.features.themes.ThemeUtils
 
 abstract class VectorSettingsPushRuleNotificationFragment :
         VectorSettingsBaseFragment() {
 
-    private val viewModel: VectorSettingsPushRuleNotificationViewModel by fragmentViewModel()
+    protected val viewModel: VectorSettingsPushRuleNotificationViewModel by fragmentViewModel()
 
     abstract val prefKeyToPushRuleId: Map<String, String>
 
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
         observeViewEvents()
-        viewModel.onEach(VectorSettingsPushRuleNotificationViewState::isLoading) { isLoading ->
-            if (isLoading) {
-                displayLoadingView()
-            } else {
-                hideLoadingView()
-            }
-        }
+        viewModel.onEach(VectorSettingsPushRuleNotificationViewState::allRules) { refreshPreferences() }
+        viewModel.onEach(VectorSettingsPushRuleNotificationViewState::isLoading) { updateLoadingView(it) }
+        viewModel.onEach(VectorSettingsPushRuleNotificationViewState::rulesOnError) { refreshErrors(it) }
     }
 
     private fun observeViewEvents() {
         viewModel.observeViewEvents {
             when (it) {
-                is VectorSettingsPushRuleNotificationViewEvent.Failure -> refreshDisplay()
-                is VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated -> updatePreference(it.ruleId, it.checked)
+                is VectorSettingsPushRuleNotificationViewEvent.Failure -> onFailure(it.ruleId)
+                is VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated -> {
+                    updatePreference(it.ruleId, it.checked)
+                    if (it.failure != null) {
+                        onFailure(it.ruleId)
+                    }
+                }
             }
         }
     }
 
     override fun bindPref() {
-        for (preferenceKey in prefKeyToPushRuleId.keys) {
-            val preference = findPreference<VectorCheckboxPreference>(preferenceKey)!!
-            preference.isIconSpaceReserved = false
-            val ruleAndKind = prefKeyToPushRuleId[preferenceKey]?.let { viewModel.getPushRuleAndKind(it) }
-            if (ruleAndKind == null) {
-                // The rule is not defined, hide the preference
-                preference.isVisible = false
-            } else {
-                preference.isVisible = true
-                updatePreference(ruleAndKind.pushRule.ruleId, viewModel.isPushRuleChecked(ruleAndKind.pushRule.ruleId))
-                preference.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue ->
-                    viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(ruleAndKind, newValue as Boolean))
+        prefKeyToPushRuleId.forEach { (preferenceKey, ruleId) ->
+            findPreference<VectorCheckboxPreference>(preferenceKey)?.apply {
+                isIconSpaceReserved = false
+                onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue ->
+                    viewModel.handle(VectorSettingsPushRuleNotificationViewAction.UpdatePushRule(ruleId, newValue as Boolean))
                     false
                 }
             }
         }
     }
 
-    override fun invalidate() = withState(viewModel) { state ->
-        if (state.isLoading) {
+    private fun updateLoadingView(isLoading: Boolean) {
+        if (isLoading) {
             displayLoadingView()
         } else {
             hideLoadingView()
         }
     }
 
+    private fun refreshPreferences() {
+        prefKeyToPushRuleId.values.forEach { ruleId -> updatePreference(ruleId, viewModel.isPushRuleChecked(ruleId)) }
+    }
+
+    private fun refreshErrors(rulesWithError: Set<String>) {
+        if (withState(viewModel, VectorSettingsPushRuleNotificationViewState::isLoading)) return
+        prefKeyToPushRuleId.forEach { (preferenceKey, ruleId) ->
+            findPreference<VectorCheckboxPreference>(preferenceKey)?.apply {
+                if (ruleId in rulesWithError) {
+                    summaryTextColor = ThemeUtils.getColor(context, R.attr.colorError)
+                    setSummary(R.string.settings_notification_error_on_update)
+                } else {
+                    summaryTextColor = null
+                    summary = null
+                }
+            }
+        }
+    }
+
     protected fun refreshDisplay() {
         listView?.adapter?.notifyDataSetChanged()
     }
 
     private fun updatePreference(ruleId: String, checked: Boolean) {
         val preferenceKey = prefKeyToPushRuleId.entries.find { it.value == ruleId }?.key ?: return
         val preference = findPreference<VectorCheckboxPreference>(preferenceKey) ?: return
+        val ruleIds = withState(viewModel) { state -> state.allRules.map { it.ruleId } }
+        preference.isVisible = ruleId in ruleIds
         preference.isChecked = checked
     }
+
+    protected open fun onFailure(ruleId: String) {
+        refreshDisplay()
+    }
 }

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewAction.kt

@@ -17,8 +17,7 @@
 package im.vector.app.features.settings.notifications
 
 import im.vector.app.core.platform.VectorViewModelAction
-import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind
 
 sealed interface VectorSettingsPushRuleNotificationViewAction : VectorViewModelAction {
-    data class UpdatePushRule(val pushRuleAndKind: PushRuleAndKind, val checked: Boolean) : VectorSettingsPushRuleNotificationViewAction
+    data class UpdatePushRule(val ruleId: String, val checked: Boolean) : VectorSettingsPushRuleNotificationViewAction
 }

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewEvent.kt

@@ -31,7 +31,8 @@ sealed interface VectorSettingsPushRuleNotificationViewEvent : VectorViewEvents
     /**
      * A failure has occurred.
      *
+     * @property ruleId the global rule id related to the failure.
      * @property throwable the related exception, if any.
      */
-    data class Failure(val throwable: Throwable?) : VectorSettingsPushRuleNotificationViewEvent
+    data class Failure(val ruleId: String, val throwable: Throwable?) : VectorSettingsPushRuleNotificationViewEvent
 }

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewModel.kt

@@ -20,26 +20,31 @@ import com.airbnb.mvrx.MavericksViewModelFactory
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
-import im.vector.app.core.di.ActiveSessionHolder
 import im.vector.app.core.di.MavericksAssistedViewModelFactory
 import im.vector.app.core.di.hiltMavericksViewModelFactory
 import im.vector.app.core.platform.VectorViewModel
 import im.vector.app.features.settings.notifications.VectorSettingsPushRuleNotificationViewEvent.Failure
 import im.vector.app.features.settings.notifications.VectorSettingsPushRuleNotificationViewEvent.PushRuleUpdated
+import im.vector.app.features.settings.notifications.usecase.GetPushRulesOnInvalidStateUseCase
 import kotlinx.coroutines.launch
 import org.matrix.android.sdk.api.failure.Failure.ServerError
 import org.matrix.android.sdk.api.failure.MatrixError
+import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.session.accountdata.UserAccountDataTypes
 import org.matrix.android.sdk.api.session.pushrules.Action
 import org.matrix.android.sdk.api.session.pushrules.RuleIds
 import org.matrix.android.sdk.api.session.pushrules.RuleKind
 import org.matrix.android.sdk.api.session.pushrules.rest.PushRuleAndKind
+import org.matrix.android.sdk.flow.flow
+import org.matrix.android.sdk.flow.unwrap
 
 private typealias ViewModel = VectorSettingsPushRuleNotificationViewModel
 private typealias ViewState = VectorSettingsPushRuleNotificationViewState
 
 class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor(
         @Assisted initialState: ViewState,
-        private val activeSessionHolder: ActiveSessionHolder,
+        private val session: Session,
+        private val getPushRulesOnInvalidStateUseCase: GetPushRulesOnInvalidStateUseCase,
 ) : VectorViewModel<VectorSettingsPushRuleNotificationViewState,
         VectorSettingsPushRuleNotificationViewAction,
         VectorSettingsPushRuleNotificationViewEvent>(initialState) {
@@ -51,28 +56,42 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor(
 
     companion object : MavericksViewModelFactory<ViewModel, ViewState> by hiltMavericksViewModelFactory()
 
+    init {
+        session.flow()
+                .liveUserAccountData(UserAccountDataTypes.TYPE_PUSH_RULES)
+                .unwrap()
+                .setOnEach {
+                    val allRules = session.pushRuleService().getPushRules().getAllRules()
+                    val rulesOnError = getPushRulesOnInvalidStateUseCase.execute(session).map { it.ruleId }.toSet()
+                    copy(
+                            allRules = allRules,
+                            rulesOnError = rulesOnError
+                    )
+                }
+    }
+
     override fun handle(action: VectorSettingsPushRuleNotificationViewAction) {
         when (action) {
-            is VectorSettingsPushRuleNotificationViewAction.UpdatePushRule -> handleUpdatePushRule(action.pushRuleAndKind, action.checked)
+            is VectorSettingsPushRuleNotificationViewAction.UpdatePushRule -> handleUpdatePushRule(action.ruleId, action.checked)
         }
     }
 
     fun getPushRuleAndKind(ruleId: String): PushRuleAndKind? {
-        return activeSessionHolder.getSafeActiveSession()?.pushRuleService()?.getPushRules()?.findDefaultRule(ruleId)
+        return session.pushRuleService().getPushRules().findDefaultRule(ruleId)
     }
 
     fun isPushRuleChecked(ruleId: String): Boolean {
         val rulesGroup = listOf(ruleId) + RuleIds.getSyncedRules(ruleId)
         return rulesGroup.mapNotNull { getPushRuleAndKind(it) }.any { it.pushRule.notificationIndex != NotificationIndex.OFF }
     }
 
-    private fun handleUpdatePushRule(pushRuleAndKind: PushRuleAndKind, checked: Boolean) {
-        val ruleId = pushRuleAndKind.pushRule.ruleId
-        val kind = pushRuleAndKind.kind
+    private fun handleUpdatePushRule(ruleId: String, checked: Boolean) {
+        val kind = getPushRuleAndKind(ruleId)?.kind ?: return
         val newIndex = if (checked) NotificationIndex.NOISY else NotificationIndex.OFF
         val standardAction = getStandardAction(ruleId, newIndex) ?: return
         val enabled = standardAction != StandardActions.Disabled
         val newActions = standardAction.actions
+
         setState { copy(isLoading = true) }
 
         viewModelScope.launch {
@@ -82,28 +101,37 @@ class VectorSettingsPushRuleNotificationViewModel @AssistedInject constructor(
                     updatePushRule(kind, ruleId, enabled, newActions)
                 }
             }
-            setState { copy(isLoading = false) }
-            val failure = results.firstNotNullOfOrNull { result ->
+
+            val failures = results.mapNotNull { result ->
                 // If the failure is a rule not found error, do not consider it
                 result.exceptionOrNull()?.takeUnless { it is ServerError && it.error.code == MatrixError.M_NOT_FOUND }
             }
-            val newChecked = if (checked) {
-                // If any rule is checked, the global rule is checked
-                results.any { it.isSuccess }
+            val hasSuccess = results.any { it.isSuccess }
+            val hasFailures = failures.isNotEmpty()
+
+            // Any rule has been checked or some rules have not been unchecked
+            val newChecked = (checked && hasSuccess) || (!checked && hasFailures)
+            if (hasSuccess) {
+                _viewEvents.post(PushRuleUpdated(ruleId, newChecked, failures.firstOrNull()))
             } else {
-                // If any rule has not been unchecked, the global rule remains checked
-                failure != null
+                _viewEvents.post(Failure(ruleId, failures.firstOrNull()))
             }
-            if (results.any { it.isSuccess }) {
-                _viewEvents.post(PushRuleUpdated(ruleId, newChecked, failure))
-            } else {
-                _viewEvents.post(Failure(failure))
+
+            setState {
+                copy(
+                        isLoading = false,
+                        rulesOnError = when {
+                            hasSuccess && hasFailures -> rulesOnError.plus(ruleId) // some failed
+                            hasSuccess -> rulesOnError.minus(ruleId) // all succeed
+                            else -> rulesOnError // all failed
+                        }
+                )
             }
         }
     }
 
     private suspend fun updatePushRule(kind: RuleKind, ruleId: String, enable: Boolean, newActions: List<Action>?) {
-        activeSessionHolder.getSafeActiveSession()?.pushRuleService()?.updatePushRuleActions(
+        session.pushRuleService().updatePushRuleActions(
                 kind = kind,
                 ruleId = ruleId,
                 enable = enable,

vector/src/main/java/im/vector/app/features/settings/notifications/VectorSettingsPushRuleNotificationViewState.kt

@@ -17,7 +17,10 @@
 package im.vector.app.features.settings.notifications
 
 import com.airbnb.mvrx.MavericksState
+import org.matrix.android.sdk.api.session.pushrules.rest.PushRule
 
 data class VectorSettingsPushRuleNotificationViewState(
         val isLoading: Boolean = false,
+        val allRules: List<PushRule> = emptyList(),
+        val rulesOnError: Set<String> = emptySet(),
 ) : MavericksState

vector/src/main/java/im/vector/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCase.kt

@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 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.
+ */
+
+package im.vector.app.features.settings.notifications.usecase
+
+import im.vector.app.features.settings.notifications.getParentRule
+import im.vector.app.features.settings.notifications.getSyncedRules
+import org.matrix.android.sdk.api.extensions.orTrue
+import org.matrix.android.sdk.api.session.Session
+import org.matrix.android.sdk.api.session.pushrules.RuleIds
+import org.matrix.android.sdk.api.session.pushrules.getActions
+import org.matrix.android.sdk.api.session.pushrules.rest.PushRule
+import javax.inject.Inject
+
+class GetPushRulesOnInvalidStateUseCase @Inject constructor() {
+
+    fun execute(session: Session): List<PushRule> {
+        val allRules = session.pushRuleService().getPushRules().getAllRules()
+        return allRules.filter { it.isOnInvalidState(allRules) }
+    }
+
+    private fun PushRule.isOnInvalidState(allRules: List<PushRule>): Boolean {
+        val parent = RuleIds.getParentRule(ruleId)?.let { parentId -> allRules.find { it.ruleId == parentId } }
+        val children = RuleIds.getSyncedRules(ruleId).mapNotNull { childId -> allRules.find { it.ruleId == childId } }
+        val isAlignedWithParent = parent?.let { isAlignedWithParentRule(it) }.orTrue()
+        return !isAlignedWithParent || !isAlignedWithChildrenRules(children)
+    }
+
+    private fun PushRule.isAlignedWithParentRule(parent: PushRule) = this.getActions() == parent.getActions() && this.enabled == parent.enabled
+    private fun PushRule.isAlignedWithChildrenRules(children: List<PushRule>) = children.all { it.isAlignedWithParentRule(this) }
+}