Fixes Changing Account Settings > Notifications > Advanced Notifications on android causes discrepancies with web

changelog.d/3681.removal

@@ -0,0 +1 @@
+updatePushRuleActions signature has been updated to more explicitly enabled/disable the rule and update the actions. It's behaviour has also been changed to match the web with the enable/disable requests being sent on every invocation and actions sent when needed(not null).

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

@@ -31,7 +31,13 @@ interface PushRuleService {
 
     suspend fun addPushRule(kind: RuleKind, pushRule: PushRule)
 
-    suspend fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule)
+    /**
+     * Enables/Disables a push rule and updates the actions if necessary
+     * @param enable Enables/Disables the rule
+     * @param actions Actions to update if not null
+     */
+
+    suspend fun updatePushRuleActions(kind: RuleKind, ruleId: String, enable: Boolean, actions: List<Action>?)
 
     suspend fun removePushRule(kind: RuleKind, pushRule: PushRule)
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/rest/PushRule.kt

@@ -18,6 +18,7 @@ package org.matrix.android.sdk.api.pushrules.rest
 
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
+import org.matrix.android.sdk.api.extensions.orFalse
 import org.matrix.android.sdk.api.pushrules.Action
 import org.matrix.android.sdk.api.pushrules.getActions
 import org.matrix.android.sdk.api.pushrules.toJson
@@ -100,6 +101,13 @@ data class PushRule(
         )
     }
 
+    /**
+     * Get the highlight status. As spec mentions assume false if no tweak present.
+     */
+    fun getHighlight(): Boolean {
+        return (getActions().firstOrNull { it is Action.Highlight } as? Action.Highlight)?.highlight.orFalse()
+    }
+
     /**
      * Set the notification status.
      *

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

@@ -113,8 +113,8 @@ internal class DefaultPushRuleService @Inject constructor(
         addPushRuleTask.execute(AddPushRuleTask.Params(kind, pushRule))
     }
 
-    override suspend fun updatePushRuleActions(kind: RuleKind, oldPushRule: PushRule, newPushRule: PushRule) {
-        updatePushRuleActionsTask.execute(UpdatePushRuleActionsTask.Params(kind, oldPushRule, newPushRule))
+    override suspend fun updatePushRuleActions(kind: RuleKind, ruleId: String, enable: Boolean, actions: List<Action>?) {
+        updatePushRuleActionsTask.execute(UpdatePushRuleActionsTask.Params(kind, ruleId, enable, actions))
     }
 
     override suspend fun removePushRule(kind: RuleKind, pushRule: PushRule) {

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

@@ -15,8 +15,9 @@
  */
 package org.matrix.android.sdk.internal.session.pushers
 
+import org.matrix.android.sdk.api.pushrules.Action
 import org.matrix.android.sdk.api.pushrules.RuleKind
-import org.matrix.android.sdk.api.pushrules.rest.PushRule
+import org.matrix.android.sdk.api.pushrules.toJson
 import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
 import org.matrix.android.sdk.internal.network.executeRequest
 import org.matrix.android.sdk.internal.task.Task
@@ -25,8 +26,9 @@ import javax.inject.Inject
 internal interface UpdatePushRuleActionsTask : Task<UpdatePushRuleActionsTask.Params, Unit> {
     data class Params(
             val kind: RuleKind,
-            val oldPushRule: PushRule,
-            val newPushRule: PushRule
+            val ruleId: String,
+            val enable: Boolean,
+            val actions: List<Action>?
     )
 }
 
@@ -36,20 +38,14 @@ internal class DefaultUpdatePushRuleActionsTask @Inject constructor(
 ) : UpdatePushRuleActionsTask {
 
     override suspend fun execute(params: UpdatePushRuleActionsTask.Params) {
-        if (params.oldPushRule.enabled != params.newPushRule.enabled) {
-            // First change enabled state
             executeRequest(globalErrorReceiver) {
-                pushRulesApi.updateEnableRuleStatus(params.kind.value, params.newPushRule.ruleId, params.newPushRule.enabled)
+                pushRulesApi.updateEnableRuleStatus(params.kind.value, params.ruleId, enable = params.enable)
             }
-        }
-
-        if (params.newPushRule.enabled) {
-            // Also ensure the actions are up to date
-            val body = mapOf("actions" to params.newPushRule.actions)
-
-            executeRequest(globalErrorReceiver) {
-                pushRulesApi.updateRuleActions(params.kind.value, params.newPushRule.ruleId, body)
+            if (params.actions != null) {
+                val body = mapOf("actions" to params.actions.toJson())
+                executeRequest(globalErrorReceiver) {
+                    pushRulesApi.updateRuleActions(params.kind.value, params.ruleId, body)
+                }
             }
-        }
     }
 }

vector/src/main/java/im/vector/app/core/di/FragmentModule.kt

@@ -111,12 +111,12 @@ import im.vector.app.features.roomprofile.settings.RoomSettingsFragment
 import im.vector.app.features.roomprofile.uploads.RoomUploadsFragment
 import im.vector.app.features.roomprofile.uploads.files.RoomUploadsFilesFragment
 import im.vector.app.features.roomprofile.uploads.media.RoomUploadsMediaFragment
-import im.vector.app.features.settings.VectorSettingsAdvancedNotificationPreferenceFragment
+import im.vector.app.features.settings.notifications.VectorSettingsAdvancedNotificationPreferenceFragment
 import im.vector.app.features.settings.VectorSettingsGeneralFragment
 import im.vector.app.features.settings.VectorSettingsHelpAboutFragment
 import im.vector.app.features.settings.VectorSettingsLabsFragment
-import im.vector.app.features.settings.VectorSettingsNotificationPreferenceFragment
-import im.vector.app.features.settings.VectorSettingsNotificationsTroubleshootFragment
+import im.vector.app.features.settings.notifications.VectorSettingsNotificationPreferenceFragment
+import im.vector.app.features.settings.notifications.VectorSettingsNotificationsTroubleshootFragment
 import im.vector.app.features.settings.VectorSettingsPinFragment
 import im.vector.app.features.settings.VectorSettingsPreferencesFragment
 import im.vector.app.features.settings.VectorSettingsSecurityPrivacyFragment

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

@@ -22,18 +22,23 @@ import android.view.View
 import android.widget.RadioGroup
 import androidx.preference.PreferenceViewHolder
 import im.vector.app.R
-import org.matrix.android.sdk.api.pushrules.Action
-import org.matrix.android.sdk.api.pushrules.RuleIds
-import org.matrix.android.sdk.api.pushrules.RuleSetKey
-import org.matrix.android.sdk.api.pushrules.rest.PushRule
-import org.matrix.android.sdk.api.pushrules.rest.PushRuleAndKind
 
 class PushRulePreference : VectorPreference {
 
+    enum class NotificationIndex(val index: Int) {
+        OFF(0),
+        SILENT(1),
+        NOISY(2);
+
+        companion object {
+            fun fromInt(index: Int) = values().first { it.index == index }
+        }
+    }
+
     /**
-     * @return the selected push rule and its kind
+     * @return the selected push rule index
      */
-    var ruleAndKind: PushRuleAndKind? = null
+    var index: NotificationIndex? = null
         private set
 
     constructor(context: Context) : super(context)
@@ -47,119 +52,26 @@ class PushRulePreference : VectorPreference {
     }
 
     /**
-     * @return the bing rule status index
-     */
-    private val ruleStatusIndex: Int
-        get() {
-            val safeRule = ruleAndKind?.pushRule ?: return NOTIFICATION_OFF_INDEX
-
-            if (safeRule.ruleId == RuleIds.RULE_ID_SUPPRESS_BOTS_NOTIFICATIONS) {
-                if (safeRule.shouldNotNotify()) {
-                    return if (safeRule.enabled) {
-                        NOTIFICATION_OFF_INDEX
-                    } else {
-                        NOTIFICATION_SILENT_INDEX
-                    }
-                } else if (safeRule.shouldNotify()) {
-                    return NOTIFICATION_NOISY_INDEX
-                }
-            }
-
-            if (safeRule.enabled) {
-                return if (safeRule.shouldNotNotify()) {
-                    NOTIFICATION_OFF_INDEX
-                } else if (safeRule.getNotificationSound() != null) {
-                    NOTIFICATION_NOISY_INDEX
-                } else {
-                    NOTIFICATION_SILENT_INDEX
-                }
-            }
-
-            return NOTIFICATION_OFF_INDEX
-        }
-
-    /**
-     * Update the push rule.
+     * Update the notification index.
      *
      * @param pushRule
      */
-    fun setPushRule(pushRuleAndKind: PushRuleAndKind?) {
-        ruleAndKind = pushRuleAndKind
+    fun setIndex(notificationIndex: NotificationIndex?) {
+        index = notificationIndex
         refreshSummary()
     }
 
     /**
      * Refresh the summary
      */
     private fun refreshSummary() {
-        summary = context.getString(when (ruleStatusIndex) {
-            NOTIFICATION_OFF_INDEX    -> R.string.notification_off
-            NOTIFICATION_SILENT_INDEX -> R.string.notification_silent
-            else                      -> R.string.notification_noisy
+        summary = context.getString(when (index) {
+            NotificationIndex.OFF           -> R.string.notification_off
+            NotificationIndex.SILENT      -> R.string.notification_silent
+            NotificationIndex.NOISY, null -> R.string.notification_noisy
         })
     }
 
-    /**
-     * Create a push rule with the updated required at index.
-     *
-     * @param index index
-     * @return a push rule with the updated flags / null if there is no update
-     */
-    fun createNewRule(index: Int): PushRule? {
-        val safeRule = ruleAndKind?.pushRule ?: return null
-        val safeKind = ruleAndKind?.kind ?: return null
-
-        return if (index != ruleStatusIndex) {
-            if (safeRule.ruleId == RuleIds.RULE_ID_SUPPRESS_BOTS_NOTIFICATIONS) {
-                when (index) {
-                    NOTIFICATION_OFF_INDEX    -> {
-                        safeRule.copy(enabled = true)
-                                .setNotify(false)
-                                .removeNotificationSound()
-                    }
-                    NOTIFICATION_SILENT_INDEX -> {
-                        safeRule.copy(enabled = false)
-                                .setNotify(false)
-                    }
-                    NOTIFICATION_NOISY_INDEX  -> {
-                        safeRule.copy(enabled = true)
-                                .setNotify(true)
-                                .setNotificationSound()
-                    }
-                    else                      -> safeRule
-                }
-            } else {
-                if (NOTIFICATION_OFF_INDEX == index) {
-                    if (safeKind == RuleSetKey.UNDERRIDE || safeRule.ruleId == RuleIds.RULE_ID_SUPPRESS_BOTS_NOTIFICATIONS) {
-                        safeRule.setNotify(false)
-                    } else {
-                        safeRule.copy(enabled = false)
-                    }
-                } else {
-                    val newRule = safeRule.copy(enabled = true)
-                            .setNotify(true)
-                            .setHighlight(safeKind != RuleSetKey.UNDERRIDE
-                                    && safeRule.ruleId != RuleIds.RULE_ID_INVITE_ME
-                                    && NOTIFICATION_NOISY_INDEX == index)
-
-                    if (NOTIFICATION_NOISY_INDEX == index) {
-                        newRule.setNotificationSound(
-                                if (safeRule.ruleId == RuleIds.RULE_ID_CALL) {
-                                    Action.ACTION_OBJECT_VALUE_VALUE_RING
-                                } else {
-                                    Action.ACTION_OBJECT_VALUE_VALUE_DEFAULT
-                                }
-                        )
-                    } else {
-                        newRule.removeNotificationSound()
-                    }
-                }
-            }
-        } else {
-            safeRule
-        }
-    }
-
     override fun onBindViewHolder(holder: PreferenceViewHolder) {
         super.onBindViewHolder(holder)
 
@@ -170,38 +82,30 @@ class PushRulePreference : VectorPreference {
         val radioGroup = holder.findViewById(R.id.bingPreferenceRadioGroup) as? RadioGroup
         radioGroup?.setOnCheckedChangeListener(null)
 
-        when (ruleStatusIndex) {
-            NOTIFICATION_OFF_INDEX    -> {
+        when (index) {
+            NotificationIndex.OFF       -> {
                 radioGroup?.check(R.id.bingPreferenceRadioBingRuleOff)
             }
-            NOTIFICATION_SILENT_INDEX -> {
+            NotificationIndex.SILENT -> {
                 radioGroup?.check(R.id.bingPreferenceRadioBingRuleSilent)
             }
-            else                      -> {
+            NotificationIndex.NOISY  -> {
                 radioGroup?.check(R.id.bingPreferenceRadioBingRuleNoisy)
             }
         }
 
         radioGroup?.setOnCheckedChangeListener { _, checkedId ->
             when (checkedId) {
                 R.id.bingPreferenceRadioBingRuleOff    -> {
-                    onPreferenceChangeListener?.onPreferenceChange(this, NOTIFICATION_OFF_INDEX)
+                    onPreferenceChangeListener?.onPreferenceChange(this, NotificationIndex.OFF)
                 }
                 R.id.bingPreferenceRadioBingRuleSilent -> {
-                    onPreferenceChangeListener?.onPreferenceChange(this, NOTIFICATION_SILENT_INDEX)
+                    onPreferenceChangeListener?.onPreferenceChange(this, NotificationIndex.SILENT)
                 }
                 R.id.bingPreferenceRadioBingRuleNoisy  -> {
-                    onPreferenceChangeListener?.onPreferenceChange(this, NOTIFICATION_NOISY_INDEX)
+                    onPreferenceChangeListener?.onPreferenceChange(this, NotificationIndex.NOISY)
                 }
             }
         }
     }
-
-    companion object {
-
-        // index in mRuleStatuses
-        private const val NOTIFICATION_OFF_INDEX = 0
-        private const val NOTIFICATION_SILENT_INDEX = 1
-        private const val NOTIFICATION_NOISY_INDEX = 2
-    }
 }

vector/src/main/java/im/vector/app/features/settings/VectorSettingsActivity.kt

@@ -27,6 +27,7 @@ import im.vector.app.core.extensions.replaceFragment
 import im.vector.app.core.platform.VectorBaseActivity
 import im.vector.app.databinding.ActivityVectorSettingsBinding
 import im.vector.app.features.settings.devices.VectorSettingsDevicesFragment
+import im.vector.app.features.settings.notifications.VectorSettingsNotificationPreferenceFragment
 
 import org.matrix.android.sdk.api.failure.GlobalError
 import org.matrix.android.sdk.api.session.Session