Implement MSC3987: Push actions clean-up (#8530)

element-hq · Jun 16, 2023 · 710d21f · 710d21f
1 parent ce80d7f
commit 710d21f
Show file tree

Hide file tree

Showing 8 changed files with 11 additions and 15 deletions.
diff --git a/changelog.d/8503.misc b/changelog.d/8503.misc
@@ -0,0 +1 @@
+MSC3987 implementation: the 'dont_notify' action for a push_rule is now deprecated and replaced by an empty action list.
diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/Action.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/Action.kt
@@ -20,7 +20,6 @@ import timber.log.Timber
 
 sealed class Action {
     object Notify : Action()
-    object DoNotNotify : Action()
     data class Sound(val sound: String = ACTION_OBJECT_VALUE_VALUE_DEFAULT) : Action()
     data class Highlight(val highlight: Boolean) : Action()
 
@@ -72,7 +71,6 @@ fun List<Action>.toJson(): List<Any> {
     return map { action ->
         when (action) {
             is Action.Notify -> Action.ACTION_NOTIFY
-            is Action.DoNotNotify -> Action.ACTION_DONT_NOTIFY
             is Action.Sound -> {
                 mapOf(
                         Action.ACTION_OBJECT_SET_TWEAK_KEY to Action.ACTION_OBJECT_SET_TWEAK_VALUE_SOUND,
@@ -95,7 +93,7 @@ fun PushRule.getActions(): List<Action> {
     actions.forEach { actionStrOrObj ->
         when (actionStrOrObj) {
             Action.ACTION_NOTIFY -> Action.Notify
-            Action.ACTION_DONT_NOTIFY -> Action.DoNotNotify
+            Action.ACTION_DONT_NOTIFY -> return@forEach
             is Map<*, *> -> {
                 when (actionStrOrObj[Action.ACTION_OBJECT_SET_TWEAK_KEY]) {
                     Action.ACTION_OBJECT_SET_TWEAK_VALUE_SOUND -> {

diff --git a/...x-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushRule.kt b/...x-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushRule.kt
@@ -121,8 +121,6 @@ data class PushRule(
 
         if (notify) {
             mutableActions.add(Action.ACTION_NOTIFY)
-        } else {
-            mutableActions.add(Action.ACTION_DONT_NOTIFY)
         }
 
         return copy(actions = mutableActions)
@@ -140,5 +138,5 @@ data class PushRule(
      *
      * @return true if the rule should not play sound
      */
-    fun shouldNotNotify() = actions.contains(Action.ACTION_DONT_NOTIFY)
+    fun shouldNotNotify() = actions.isEmpty() || actions.contains(Action.ACTION_DONT_NOTIFY)
 }
diff --git a/...main/java/org/matrix/android/sdk/internal/session/room/notification/RoomPushRuleMapper.kt b/...main/java/org/matrix/android/sdk/internal/session/room/notification/RoomPushRuleMapper.kt
@@ -63,7 +63,7 @@ internal fun RoomNotificationState.toRoomPushRule(roomId: String): RoomPushRule?
                     pattern = roomId
             )
             val rule = PushRule(
-                    actions = listOf(Action.DoNotNotify).toJson(),
+                    actions = emptyList<Action>().toJson(),
                     enabled = true,
                     ruleId = roomId,
                     conditions = listOf(condition)
@@ -81,7 +81,7 @@ internal fun RoomNotificationState.toRoomPushRule(roomId: String): RoomPushRule?
 internal fun RoomPushRule.toRoomNotificationState(): RoomNotificationState {
     return if (rule.enabled) {
         val actions = rule.getActions()
-        if (actions.contains(Action.DoNotNotify)) {
+        if (actions.isEmpty()) {
             if (kind == RuleSetKey.OVERRIDE) {
                 RoomNotificationState.MUTE
             } else {

diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationAction.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationAction.kt
@@ -30,7 +30,6 @@ fun List<Action>.toNotificationAction(): NotificationAction {
     forEach { action ->
         when (action) {
             is Action.Notify -> shouldNotify = true
-            is Action.DoNotNotify -> shouldNotify = false
             is Action.Highlight -> highlight = action.highlight
             is Action.Sound -> sound = action.sound
         }

diff --git a/vector/src/main/java/im/vector/app/features/settings/notifications/StandardActions.kt b/vector/src/main/java/im/vector/app/features/settings/notifications/StandardActions.kt
@@ -26,6 +26,6 @@ sealed class StandardActions(
     object NotifyRingSound : StandardActions(actions = listOf(Action.Notify, Action.Sound(sound = Action.ACTION_OBJECT_VALUE_VALUE_RING)))
     object Highlight : StandardActions(actions = listOf(Action.Notify, Action.Highlight(highlight = true)))
     object HighlightDefaultSound : StandardActions(actions = listOf(Action.Notify, Action.Highlight(highlight = true), Action.Sound()))
-    object DontNotify : StandardActions(actions = listOf(Action.DoNotNotify))
+    object DontNotify : StandardActions(actions = emptyList())
     object Disabled : StandardActions(actions = null)
 }
diff --git a/...ctor/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt b/...ctor/app/features/settings/notifications/usecase/GetPushRulesOnInvalidStateUseCaseTest.kt
@@ -48,19 +48,19 @@ internal class GetPushRulesOnInvalidStateUseCaseTest {
     fun `given a list of push rules with children not matching their parent when execute then returns the list of not matching rules`() {
         // Given
         val firstActions = listOf(Action.Notify)
-        val secondActions = listOf(Action.DoNotNotify)
+        val secondActions = emptyList<Action>()
         givenARuleList(
                 listOf(
                         // first set of related rules
                         givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, firstActions),
-                        givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, listOf(Action.DoNotNotify)), // diff
+                        givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, emptyList()), // diff
                         givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, emptyList()), // diff
                         givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, false, listOf(Action.Notify)), // diff
                         givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)),
                         // second set of related rules
                         givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false, secondActions),
                         givenARuleId(RuleIds.RULE_ID_POLL_START, true, listOf(Action.Notify)), // diff
-                        givenARuleId(RuleIds.RULE_ID_POLL_START_UNSTABLE, false, listOf(Action.DoNotNotify)),
+                        givenARuleId(RuleIds.RULE_ID_POLL_START_UNSTABLE, false, emptyList()),
                         givenARuleId(RuleIds.RULE_ID_POLL_END, false, listOf(Action.Notify)), // diff
                         givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff
                         // Another rule

diff --git a/.../vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt b/.../vector/app/features/settings/notifications/usecase/UpdatePushRulesIfNeededUseCaseTest.kt
@@ -54,12 +54,12 @@ internal class UpdatePushRulesIfNeededUseCaseTest {
         val firstParentActions = listOf(Action.Notify)
         val firstParent = givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, firstParentEnabled, firstParentActions)
         val secondParentEnabled = false
-        val secondParentActions = listOf(Action.DoNotNotify)
+        val secondParentActions = emptyList<Action>()
         val secondParent = givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, secondParentEnabled, secondParentActions)
         val rulesOnError = listOf(
                 // first set of related rules
                 firstParent,
-                givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, listOf(Action.DoNotNotify)), // diff
+                givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE, true, emptyList()), // diff
                 givenARuleId(RuleIds.RULE_ID_POLL_START_ONE_TO_ONE_UNSTABLE, true, emptyList()), // diff
                 givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE, false, listOf(Action.Notify)), // diff
                 // second set of related rules