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

This commit is contained in:
Yoan Pintas 2023-06-16 11:13:13 +02:00 committed by GitHub
parent ce80d7ff2f
commit 710d21f6a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 11 additions and 15 deletions

1
changelog.d/8503.misc Normal file
View File

@ -0,0 +1 @@
MSC3987 implementation: the 'dont_notify' action for a push_rule is now deprecated and replaced by an empty action list.

View File

@ -20,7 +20,6 @@ import timber.log.Timber
sealed class Action { sealed class Action {
object Notify : Action() object Notify : Action()
object DoNotNotify : Action()
data class Sound(val sound: String = ACTION_OBJECT_VALUE_VALUE_DEFAULT) : Action() data class Sound(val sound: String = ACTION_OBJECT_VALUE_VALUE_DEFAULT) : Action()
data class Highlight(val highlight: Boolean) : Action() data class Highlight(val highlight: Boolean) : Action()
@ -72,7 +71,6 @@ fun List<Action>.toJson(): List<Any> {
return map { action -> return map { action ->
when (action) { when (action) {
is Action.Notify -> Action.ACTION_NOTIFY is Action.Notify -> Action.ACTION_NOTIFY
is Action.DoNotNotify -> Action.ACTION_DONT_NOTIFY
is Action.Sound -> { is Action.Sound -> {
mapOf( mapOf(
Action.ACTION_OBJECT_SET_TWEAK_KEY to Action.ACTION_OBJECT_SET_TWEAK_VALUE_SOUND, 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 -> actions.forEach { actionStrOrObj ->
when (actionStrOrObj) { when (actionStrOrObj) {
Action.ACTION_NOTIFY -> Action.Notify Action.ACTION_NOTIFY -> Action.Notify
Action.ACTION_DONT_NOTIFY -> Action.DoNotNotify Action.ACTION_DONT_NOTIFY -> return@forEach
is Map<*, *> -> { is Map<*, *> -> {
when (actionStrOrObj[Action.ACTION_OBJECT_SET_TWEAK_KEY]) { when (actionStrOrObj[Action.ACTION_OBJECT_SET_TWEAK_KEY]) {
Action.ACTION_OBJECT_SET_TWEAK_VALUE_SOUND -> { Action.ACTION_OBJECT_SET_TWEAK_VALUE_SOUND -> {

View File

@ -121,8 +121,6 @@ data class PushRule(
if (notify) { if (notify) {
mutableActions.add(Action.ACTION_NOTIFY) mutableActions.add(Action.ACTION_NOTIFY)
} else {
mutableActions.add(Action.ACTION_DONT_NOTIFY)
} }
return copy(actions = mutableActions) return copy(actions = mutableActions)
@ -140,5 +138,5 @@ data class PushRule(
* *
* @return true if the rule should not play sound * @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)
} }

View File

@ -63,7 +63,7 @@ internal fun RoomNotificationState.toRoomPushRule(roomId: String): RoomPushRule?
pattern = roomId pattern = roomId
) )
val rule = PushRule( val rule = PushRule(
actions = listOf(Action.DoNotNotify).toJson(), actions = emptyList<Action>().toJson(),
enabled = true, enabled = true,
ruleId = roomId, ruleId = roomId,
conditions = listOf(condition) conditions = listOf(condition)
@ -81,7 +81,7 @@ internal fun RoomNotificationState.toRoomPushRule(roomId: String): RoomPushRule?
internal fun RoomPushRule.toRoomNotificationState(): RoomNotificationState { internal fun RoomPushRule.toRoomNotificationState(): RoomNotificationState {
return if (rule.enabled) { return if (rule.enabled) {
val actions = rule.getActions() val actions = rule.getActions()
if (actions.contains(Action.DoNotNotify)) { if (actions.isEmpty()) {
if (kind == RuleSetKey.OVERRIDE) { if (kind == RuleSetKey.OVERRIDE) {
RoomNotificationState.MUTE RoomNotificationState.MUTE
} else { } else {

View File

@ -30,7 +30,6 @@ fun List<Action>.toNotificationAction(): NotificationAction {
forEach { action -> forEach { action ->
when (action) { when (action) {
is Action.Notify -> shouldNotify = true is Action.Notify -> shouldNotify = true
is Action.DoNotNotify -> shouldNotify = false
is Action.Highlight -> highlight = action.highlight is Action.Highlight -> highlight = action.highlight
is Action.Sound -> sound = action.sound is Action.Sound -> sound = action.sound
} }

View File

@ -26,6 +26,6 @@ sealed class StandardActions(
object NotifyRingSound : StandardActions(actions = listOf(Action.Notify, Action.Sound(sound = Action.ACTION_OBJECT_VALUE_VALUE_RING))) 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 Highlight : StandardActions(actions = listOf(Action.Notify, Action.Highlight(highlight = true)))
object HighlightDefaultSound : StandardActions(actions = listOf(Action.Notify, Action.Highlight(highlight = true), Action.Sound())) 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) object Disabled : StandardActions(actions = null)
} }

View File

@ -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`() { fun `given a list of push rules with children not matching their parent when execute then returns the list of not matching rules`() {
// Given // Given
val firstActions = listOf(Action.Notify) val firstActions = listOf(Action.Notify)
val secondActions = listOf(Action.DoNotNotify) val secondActions = emptyList<Action>()
givenARuleList( givenARuleList(
listOf( listOf(
// first set of related rules // first set of related rules
givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, true, firstActions), 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_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, false, listOf(Action.Notify)), // diff
givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)), givenARuleId(RuleIds.RULE_ID_POLL_END_ONE_TO_ONE_UNSTABLE, true, listOf(Action.Notify)),
// second set of related rules // second set of related rules
givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, false, secondActions), 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, 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, false, listOf(Action.Notify)), // diff
givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff givenARuleId(RuleIds.RULE_ID_POLL_END_UNSTABLE, true, listOf()), // diff
// Another rule // Another rule

View File

@ -54,12 +54,12 @@ internal class UpdatePushRulesIfNeededUseCaseTest {
val firstParentActions = listOf(Action.Notify) val firstParentActions = listOf(Action.Notify)
val firstParent = givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, firstParentEnabled, firstParentActions) val firstParent = givenARuleId(RuleIds.RULE_ID_ONE_TO_ONE_ROOM, firstParentEnabled, firstParentActions)
val secondParentEnabled = false 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 secondParent = givenARuleId(RuleIds.RULE_ID_ALL_OTHER_MESSAGES_ROOMS, secondParentEnabled, secondParentActions)
val rulesOnError = listOf( val rulesOnError = listOf(
// first set of related rules // first set of related rules
firstParent, 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_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, false, listOf(Action.Notify)), // diff
// second set of related rules // second set of related rules