From ded8f3d349d8481d1c9a48835cde0b94f785e371 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Thu, 4 May 2023 07:54:13 -0400
Subject: [PATCH] Update the base rules to remove the dont_notify action.
 (MSC3987) (#15534)

A dont_notify action is a no-op (and coalesce is undefined). These are
both considered no-ops by the spec, per MSC3987 and the predefined
push rules were updated to remove dont_notify from the list of actions.
---
 changelog.d/15534.misc         | 1 +
 rust/src/push/base_rules.rs    | 6 +++---
 rust/src/push/evaluator.rs     | 7 ++++---
 rust/src/push/mod.rs           | 6 ++++--
 synapse/handlers/push_rules.py | 2 ++
 5 files changed, 14 insertions(+), 8 deletions(-)
 create mode 100644 changelog.d/15534.misc

diff --git a/changelog.d/15534.misc b/changelog.d/15534.misc
new file mode 100644
index 0000000000..fd9ba2a6e1
--- /dev/null
+++ b/changelog.d/15534.misc
@@ -0,0 +1 @@
+Implement [MSC3987](https://github.com/matrix-org/matrix-spec-proposals/pull/3987) by removing `"dont_notify"` from the list of actions in default push rules.
diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs
index d7c73c1f25..51372e1553 100644
--- a/rust/src/push/base_rules.rs
+++ b/rust/src/push/base_rules.rs
@@ -57,7 +57,7 @@ pub const BASE_PREPEND_OVERRIDE_RULES: &[PushRule] = &[PushRule {
     rule_id: Cow::Borrowed("global/override/.m.rule.master"),
     priority_class: 5,
     conditions: Cow::Borrowed(&[]),
-    actions: Cow::Borrowed(&[Action::DontNotify]),
+    actions: Cow::Borrowed(&[]),
     default: true,
     default_enabled: false,
 }];
@@ -88,7 +88,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
                 pattern: Cow::Borrowed("m.notice"),
             },
         ))]),
-        actions: Cow::Borrowed(&[Action::DontNotify]),
+        actions: Cow::Borrowed(&[]),
         default: true,
         default_enabled: true,
     },
@@ -122,7 +122,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[
                 pattern: Cow::Borrowed("m.room.member"),
             },
         ))]),
-        actions: Cow::Borrowed(&[Action::DontNotify]),
+        actions: Cow::Borrowed(&[]),
         default: true,
         default_enabled: true,
     },
diff --git a/rust/src/push/evaluator.rs b/rust/src/push/evaluator.rs
index 6941c61ea4..2d7c4c06be 100644
--- a/rust/src/push/evaluator.rs
+++ b/rust/src/push/evaluator.rs
@@ -140,7 +140,7 @@ impl PushRuleEvaluator {
     /// name.
     ///
     /// Returns the set of actions, if any, that match (filtering out any
-    /// `dont_notify` actions).
+    /// `dont_notify` and `coalesce` actions).
     pub fn run(
         &self,
         push_rules: &FilteredPushRules,
@@ -198,8 +198,9 @@ impl PushRuleEvaluator {
             let actions = push_rule
                 .actions
                 .iter()
-                // Filter out "dont_notify" actions, as we don't store them.
-                .filter(|a| **a != Action::DontNotify)
+                // Filter out "dont_notify" and "coalesce" actions, as we don't store them
+                // (since they result in no action by the pushers).
+                .filter(|a| **a != Action::DontNotify && **a != Action::Coalesce)
                 .cloned()
                 .collect();
 
diff --git a/rust/src/push/mod.rs b/rust/src/push/mod.rs
index 42c7c84132..f19d3c739f 100644
--- a/rust/src/push/mod.rs
+++ b/rust/src/push/mod.rs
@@ -164,11 +164,13 @@ impl PushRule {
 /// The "action" Synapse should perform for a matching push rule.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Action {
-    DontNotify,
     Notify,
-    Coalesce,
     SetTweak(SetTweak),
 
+    // Legacy actions that should be understood, but are equivalent to no-ops.
+    DontNotify,
+    Coalesce,
+
     // An unrecognized custom action.
     Unknown(Value),
 }
diff --git a/synapse/handlers/push_rules.py b/synapse/handlers/push_rules.py
index 1219672a59..813f3aa2d5 100644
--- a/synapse/handlers/push_rules.py
+++ b/synapse/handlers/push_rules.py
@@ -129,6 +129,8 @@ def check_actions(actions: List[Union[str, JsonDict]]) -> None:
         raise InvalidRuleException("No actions found")
 
     for a in actions:
+        # "dont_notify" and "coalesce" are legacy actions. They are allowed, but
+        # ignored (resulting in no action from the pusher).
         if a in ["notify", "dont_notify", "coalesce"]:
             pass
         elif isinstance(a, dict) and "set_tweak" in a: