From 6a77b4c9e43956819626e3eedf856496728fb029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Thu, 4 May 2023 13:23:57 +0200 Subject: [PATCH] push: Remove the DontNotify and Coalesce variants of push::Action According to MSC3987. --- crates/ruma-common/CHANGELOG.md | 3 +++ crates/ruma-common/src/events/push_rules.rs | 8 ++------ crates/ruma-common/src/push.rs | 15 +++++++++------ crates/ruma-common/src/push/action.rs | 13 +------------ crates/ruma-common/src/push/predefined.rs | 11 +++++------ 5 files changed, 20 insertions(+), 30 deletions(-) diff --git a/crates/ruma-common/CHANGELOG.md b/crates/ruma-common/CHANGELOG.md index 392a0e00..41ac4a5b 100644 --- a/crates/ruma-common/CHANGELOG.md +++ b/crates/ruma-common/CHANGELOG.md @@ -10,6 +10,9 @@ Breaking changes: - Make `in_reply_to` field of `Thread` optional - It was wrong to be mandatory, spec was unclear (clarified [here](https://github.com/matrix-org/matrix-spec/pull/1439)) - `FlattenedJson::get` returns a `FlattenedJsonValue` instead of a string +- Remove the `DontNotify` and `Coalesce` variants of `push::Action` according to MSC3987 + - Old push rules will still deserialize successfully but the `Coalesce` variant will not return + `true` for `Action::should_notify()` anymore Improvements: diff --git a/crates/ruma-common/src/events/push_rules.rs b/crates/ruma-common/src/events/push_rules.rs index be179074..96a3c80a 100644 --- a/crates/ruma-common/src/events/push_rules.rs +++ b/crates/ruma-common/src/events/push_rules.rs @@ -66,18 +66,14 @@ mod tests { ], "override": [ { - "actions": [ - "dont_notify" - ], + "actions": [], "conditions": [], "default": true, "enabled": false, "rule_id": ".m.rule.master" }, { - "actions": [ - "dont_notify" - ], + "actions": [], "conditions": [ { "key": "content.msgtype", diff --git a/crates/ruma-common/src/push.rs b/crates/ruma-common/src/push.rs index 5741ecbc..f0222e98 100644 --- a/crates/ruma-common/src/push.rs +++ b/crates/ruma-common/src/push.rs @@ -999,7 +999,7 @@ mod tests { key: "room_id".into(), pattern: "!roomid:matrix.org".into(), }], - actions: vec![Action::DontNotify], + actions: vec![], rule_id: "!roomid:matrix.org".into(), enabled: true, default: false, @@ -1097,7 +1097,7 @@ mod tests { #[test] fn serialize_simple_push_rule() { let rule = SimplePushRule { - actions: vec![Action::DontNotify], + actions: vec![Action::Notify], default: false, enabled: false, rule_id: room_id!("!roomid:server.name").to_owned(), @@ -1108,7 +1108,7 @@ mod tests { rule_value, json!({ "actions": [ - "dont_notify" + "notify" ], "rule_id": "!roomid:server.name", "default": false, @@ -1492,7 +1492,7 @@ mod tests { }"#, ) .unwrap(); - assert_matches!(set.get_actions(¬ice, context_one_to_one), [Action::DontNotify]); + assert_matches!(set.get_actions(¬ice, context_one_to_one), []); let at_room = serde_json::from_str::>( r#"{ @@ -1583,7 +1583,7 @@ mod tests { assert_matches!(test_set.get_actions(&message, context_one_to_one), [Action::Notify]); let room = SimplePushRule { - actions: vec![Action::DontNotify], + actions: vec![Action::SetTweak(Tweak::Highlight(true))], default: false, enabled: true, rule_id: room_id!("!dm:server.name").to_owned(), @@ -1591,7 +1591,10 @@ mod tests { set.room.insert(room); let test_set = set.clone(); - assert_matches!(test_set.get_actions(&message, context_one_to_one), [Action::DontNotify]); + assert_matches!( + test_set.get_actions(&message, context_one_to_one), + [Action::SetTweak(Tweak::Highlight(true))] + ); let content = PatternedPushRule { actions: vec![Action::SetTweak(Tweak::Sound("content".into()))], diff --git a/crates/ruma-common/src/push/action.rs b/crates/ruma-common/src/push/action.rs index b47e5921..4d535d88 100644 --- a/crates/ruma-common/src/push/action.rs +++ b/crates/ruma-common/src/push/action.rs @@ -15,13 +15,6 @@ pub enum Action { /// Causes matching events to generate a notification. Notify, - /// Prevents matching events from generating a notification. - DontNotify, - - /// Behaves like notify but homeservers may choose to coalesce multiple events - /// into a single notification. - Coalesce, - /// Sets an entry in the 'tweaks' dictionary sent to the push gateway. SetTweak(Tweak), @@ -38,7 +31,7 @@ impl Action { /// Whether this action should trigger a notification. pub fn should_notify(&self) -> bool { - matches!(self, Action::Notify | Action::Coalesce) + matches!(self, Action::Notify) } /// The sound that should be played with this action, if any. @@ -91,8 +84,6 @@ impl<'de> Deserialize<'de> for Action { match &custom { CustomAction::String(s) => match s.as_str() { "notify" => Ok(Action::Notify), - "dont_notify" => Ok(Action::DontNotify), - "coalesce" => Ok(Action::Coalesce), _ => Ok(Action::_Custom(custom)), }, CustomAction::Object(o) => { @@ -113,8 +104,6 @@ impl Serialize for Action { { match self { Action::Notify => serializer.serialize_unit_variant("Action", 0, "notify"), - Action::DontNotify => serializer.serialize_unit_variant("Action", 1, "dont_notify"), - Action::Coalesce => serializer.serialize_unit_variant("Action", 2, "coalesce"), Action::SetTweak(kind) => kind.serialize(serializer), Action::_Custom(custom) => custom.serialize(serializer), } diff --git a/crates/ruma-common/src/push/predefined.rs b/crates/ruma-common/src/push/predefined.rs index 96f00a58..d0b7faa8 100644 --- a/crates/ruma-common/src/push/predefined.rs +++ b/crates/ruma-common/src/push/predefined.rs @@ -128,7 +128,7 @@ impl ConditionalPushRule { /// generated by override rules set by the user. pub fn master() -> Self { Self { - actions: vec![DontNotify], + actions: vec![], default: true, enabled: false, rule_id: PredefinedOverrideRuleId::Master.to_string(), @@ -139,7 +139,7 @@ impl ConditionalPushRule { /// Matches messages with a `msgtype` of `notice`. pub fn suppress_notices() -> Self { Self { - actions: vec![DontNotify], + actions: vec![], default: true, enabled: true, rule_id: PredefinedOverrideRuleId::SuppressNotices.to_string(), @@ -172,7 +172,7 @@ impl ConditionalPushRule { /// Matches any `m.room.member_event`. pub fn member_event() -> Self { Self { - actions: vec![DontNotify], + actions: vec![], default: true, enabled: true, rule_id: PredefinedOverrideRuleId::MemberEvent.to_string(), @@ -232,7 +232,7 @@ impl ConditionalPushRule { #[cfg(feature = "unstable-msc2677")] pub fn reaction() -> Self { Self { - actions: vec![DontNotify], + actions: vec![], default: true, enabled: true, rule_id: PredefinedOverrideRuleId::Reaction.to_string(), @@ -597,7 +597,6 @@ mod tests { let member_event_rule = ruleset.override_.get(PredefinedOverrideRuleId::MemberEvent.as_str()).unwrap(); assert!(member_event_rule.enabled); - assert_eq!(member_event_rule.actions.len(), 1); - assert_matches!(member_event_rule.actions[0], Action::DontNotify); + assert_eq!(member_event_rule.actions.len(), 0); } }