Restore push rule settings - fix issues

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/PushRuleService.kt

@@ -22,7 +22,6 @@ import im.vector.matrix.android.api.session.events.model.Event
 import im.vector.matrix.android.api.util.Cancelable
 
 interface PushRuleService {
-
     /**
      * Fetch the push rules from the server
      */
@@ -30,12 +29,12 @@ interface PushRuleService {
 
     fun getPushRules(scope: String = RuleScope.GLOBAL): RuleSet
 
-    // TODO update rule
-
     fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean, callback: MatrixCallback<Unit>): Cancelable
 
     fun addPushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable
 
+    fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable
+
     fun removePushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable
 
     fun addPushRuleListener(listener: PushRuleListener)

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt

@@ -32,6 +32,7 @@ import im.vector.matrix.android.internal.session.SessionScope
 import im.vector.matrix.android.internal.session.pushers.AddPushRuleTask
 import im.vector.matrix.android.internal.session.pushers.GetPushRulesTask
 import im.vector.matrix.android.internal.session.pushers.RemovePushRuleTask
+import im.vector.matrix.android.internal.session.pushers.UpdatePushRuleActionsTask
 import im.vector.matrix.android.internal.session.pushers.UpdatePushRuleEnableStatusTask
 import im.vector.matrix.android.internal.task.TaskExecutor
 import im.vector.matrix.android.internal.task.configureWith
@@ -43,6 +44,7 @@ internal class DefaultPushRuleService @Inject constructor(
         private val getPushRulesTask: GetPushRulesTask,
         private val updatePushRuleEnableStatusTask: UpdatePushRuleEnableStatusTask,
         private val addPushRuleTask: AddPushRuleTask,
+        private val updatePushRuleActionsTask: UpdatePushRuleActionsTask,
         private val removePushRuleTask: RemovePushRuleTask,
         private val taskExecutor: TaskExecutor,
         private val monarchy: Monarchy
@@ -117,6 +119,14 @@ internal class DefaultPushRuleService @Inject constructor(
                 .executeBy(taskExecutor)
     }
 
+    override fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable {
+        return updatePushRuleActionsTask
+                .configureWith(UpdatePushRuleActionsTask.Params(kind, oldPushRule, newPushRule)) {
+                    this.callback = callback
+                }
+                .executeBy(taskExecutor)
+    }
+
     override fun removePushRule(kind: RuleKind, pushRule: PushRule, callback: MatrixCallback<Unit>): Cancelable {
         return removePushRuleTask
                 .configureWith(RemovePushRuleTask.Params(kind, pushRule)) {

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/pushers/PushRulesApi.kt

@@ -47,6 +47,7 @@ internal interface PushRulesApi {
 
     /**
      * Update the ruleID action
+     * Ref: https://matrix.org/docs/spec/client_server/latest#put-matrix-client-r0-pushrules-scope-kind-ruleid-actions
      *
      * @param kind    the notification kind (sender, room...)
      * @param ruleId  the ruleId

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/pushers/PushersModule.kt

@@ -72,6 +72,9 @@ internal abstract class PushersModule {
     @Binds
     abstract fun bindAddPushRuleTask(task: DefaultAddPushRuleTask): AddPushRuleTask
 
+    @Binds
+    abstract fun bindUpdatePushRuleActionTask(task: DefaultUpdatePushRuleActionsTask): UpdatePushRuleActionsTask
+
     @Binds
     abstract fun bindRemovePushRuleTask(task: DefaultRemovePushRuleTask): RemovePushRuleTask
 

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/pushers/UpdatePushRuleActionsTask.kt

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2020 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.matrix.android.internal.session.pushers
+
+import im.vector.matrix.android.api.pushrules.RuleKind
+import im.vector.matrix.android.api.pushrules.rest.PushRule
+import im.vector.matrix.android.internal.network.executeRequest
+import im.vector.matrix.android.internal.task.Task
+import org.greenrobot.eventbus.EventBus
+import javax.inject.Inject
+
+internal interface UpdatePushRuleActionsTask : Task<UpdatePushRuleActionsTask.Params, Unit> {
+    data class Params(
+            val kind: RuleKind,
+            val oldPushRule: PushRule,
+            val newPushRule: PushRule
+    )
+}
+
+internal class DefaultUpdatePushRuleActionsTask @Inject constructor(
+        private val pushRulesApi: PushRulesApi,
+        private val eventBus: EventBus
+) : UpdatePushRuleActionsTask {
+
+    override suspend fun execute(params: UpdatePushRuleActionsTask.Params) {
+        if (params.oldPushRule.enabled != params.newPushRule.enabled) {
+            // First change enabled state
+            executeRequest<Unit>(eventBus) {
+                apiCall = pushRulesApi.updateEnableRuleStatus(params.kind.value, params.newPushRule.ruleId, params.newPushRule.enabled)
+            }
+        }
+
+        if (params.newPushRule.enabled) {
+            // Also ensure the actions are up to date
+            val body = mapOf("actions" to params.newPushRule.actions)
+
+            executeRequest<Unit>(eventBus) {
+                apiCall = pushRulesApi.updateRuleActions(params.kind.value, params.newPushRule.ruleId, body)
+            }
+        }
+    }
+}

vector/src/main/java/im/vector/riotx/core/preference/BingRulePreference.kt

@@ -67,7 +67,7 @@ class BingRulePreference : VectorPreference {
             if (safeRule.enabled) {
                 return if (safeRule.shouldNotNotify()) {
                     NOTIFICATION_OFF_INDEX
-                } else if (null != safeRule.getNotificationSound()) {
+                } else if (safeRule.getNotificationSound() != null) {
                     NOTIFICATION_NOISY_INDEX
                 } else {
                     NOTIFICATION_SILENT_INDEX
@@ -104,7 +104,7 @@ class BingRulePreference : VectorPreference {
      * @param index index
      * @return a push rule with the updated flags / null if there is no update
      */
-    fun createRule(index: Int): PushRule? {
+    fun createNewRule(index: Int): PushRule? {
         val safeRule = ruleAndKind?.pushRule ?: return null
         val safeKind = ruleAndKind?.kind ?: return null
 
@@ -114,6 +114,7 @@ class BingRulePreference : VectorPreference {
                     NOTIFICATION_OFF_INDEX    -> {
                         safeRule.copy(enabled = true)
                                 .setNotify(false)
+                                .removeNotificationSound()
                     }
                     NOTIFICATION_SILENT_INDEX -> {
                         safeRule.copy(enabled = false)

vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsAdvancedNotificationPreferenceFragment.kt

@@ -23,11 +23,13 @@ import android.os.Parcelable
 import androidx.core.content.edit
 import androidx.preference.Preference
 import androidx.preference.PreferenceManager
+import im.vector.matrix.android.api.MatrixCallback
 import im.vector.matrix.android.api.pushrules.rest.PushRule
 import im.vector.matrix.android.api.pushrules.rest.PushRuleAndKind
 import im.vector.riotx.R
 import im.vector.riotx.core.preference.BingRulePreference
 import im.vector.riotx.core.preference.VectorPreference
+import im.vector.riotx.core.utils.toast
 import im.vector.riotx.features.notifications.NotificationUtils
 import javax.inject.Inject
 
@@ -112,29 +114,27 @@ class VectorSettingsAdvancedNotificationPreferenceFragment @Inject constructor(
                     preference.isVisible = true
                     preference.setPushRule(ruleAndKind)
                     preference.onPreferenceChangeListener = Preference.OnPreferenceChangeListener { _, newValue ->
-                        val rule2 = preference.createRule(newValue as Int)
+                        val newRule = preference.createNewRule(newValue as Int)
-                        if (null != rule2) {
+                        if (newRule != null) {
-                            /*
+                            displayLoadingView()
-                              TODO
-                           displayLoadingView()
-                            session.dataHandler.bingRulesManager.updateRule(preference.rule,
-                                    rule,
-                                    object : BingRulesManager.onBingRuleUpdateListener {
-                                        private fun onDone() {
-                                            refreshDisplay()
-                                            hideLoadingView()
-                                        }
 
-                                        override fun onBingRuleUpdateSuccess() {
+                            session.updatePushRuleActions(ruleAndKind.kind, preference.ruleAndKind?.pushRule ?: ruleAndKind.pushRule, newRule, object : MatrixCallback<Unit> {
-                                            onDone()
+                                override fun onSuccess(data: Unit) {
-                                        }
+                                    if (!isAdded) {
+                                        return
+                                    }
+                                    preference.setPushRule(ruleAndKind.copy(pushRule = newRule))
+                                    hideLoadingView()
+                                }
 
-                                        override fun onBingRuleUpdateFailure(errorMessage: String) {
+                                override fun onFailure(failure: Throwable) {
-                                            activity?.toast(errorMessage)
+                                    if (!isAdded) {
-                                            onDone()
+                                        return
-                                        }
+                                    }
-                                    })
+                                    hideLoadingView()
-                                    */
+                                    activity?.toast(errorFormatter.toHumanReadable(failure))
+                                }
+                            })
                         }
                         false
                     }

vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsBaseFragment.kt

@@ -26,6 +26,7 @@ import im.vector.riotx.R
 import im.vector.riotx.core.di.DaggerScreenComponent
 import im.vector.riotx.core.di.HasScreenInjector
 import im.vector.riotx.core.di.ScreenComponent
+import im.vector.riotx.core.error.ErrorFormatter
 import im.vector.riotx.core.platform.VectorBaseActivity
 import im.vector.riotx.core.utils.toast
 import timber.log.Timber
@@ -40,6 +41,7 @@ abstract class VectorSettingsBaseFragment : PreferenceFragmentCompat(), HasScree
 
     // members
     protected lateinit var session: Session
+    protected lateinit var errorFormatter: ErrorFormatter
     private lateinit var screenComponent: ScreenComponent
 
     abstract val preferenceXmlRes: Int
@@ -54,6 +56,7 @@ abstract class VectorSettingsBaseFragment : PreferenceFragmentCompat(), HasScree
         screenComponent = DaggerScreenComponent.factory().create(vectorActivity.getVectorComponent(), vectorActivity)
         super.onAttach(context)
         session = screenComponent.activeSessionHolder().getActiveSession()
+        errorFormatter = screenComponent.errorFormatter()
         injectWith(injector())
     }
 

vector/src/main/java/im/vector/riotx/features/settings/VectorSettingsNotificationPreferenceFragment.kt

@@ -114,7 +114,7 @@ class VectorSettingsNotificationPreferenceFragment @Inject constructor(
                             !switchPref.isChecked,
                             object : MatrixCallback<Unit> {
                                 override fun onSuccess(data: Unit) {
-                                    // Push rules will be updated form the sync
+                                    // Push rules will be updated from the sync
                                 }
 
                                 override fun onFailure(failure: Throwable) {