From d5f43bb7cfad43411d8044456b008043f3c1d1e8 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Fri, 21 Jan 2022 02:44:33 +0100 Subject: [PATCH 01/22] Pushrules for relations Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 proposals/3664-notifications-for-relations.md diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md new file mode 100644 index 0000000000..acd84f3a88 --- /dev/null +++ b/proposals/3664-notifications-for-relations.md @@ -0,0 +1,152 @@ +# MSC3664: Notifications for relations + +Relations are very powerful and are becoming a platform to build new features +like replies, edits, reactions, threads, polls and much more on. + +On the other hand there is currently no way to control, what you are getting +notified for. Some people want to get notified, when someone replies to their +message. Some want to get notified for reactions to their message. Some people +explicitly do not want that. You might want to be able to mute a thread and you +may want to get notified for poll responeses or not. Some people like getting +notified for edits, others prefer to not get notified, when someone fixes typos +20 times in a row for a long message they sent a week ago. + +We should extend push rules so that a server can provide sane defaults and users +can adjust them to their own wishes. + +## Proposal + +### New push rule condition: `related_event_match` + +Notifications for relation based features need to distinguish, what type of +relation was used and potentially match on the content of the related-to event. + +To do that we introduce a new type of condition: `related_event_match`. This is +largely similar to the existing `event_match`, but operates on the related-to +event. Such a condition could look like this: + +```json5 +{ + "kind": "related_event_match", + "rel_type": "m.in_reply_to", + "key": "sender", + "pattern": "@me:my.server" +} +``` + +This condition can be used to notify me whenever someone sends a reply to my +messages. + +- `rel_type` is the relation type. For the sake of compatibility replies + should be matched as if they were sent in the relation format from + [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) with a + `rel_type` of `m.in_reply_to`. If the event has any relation of this type, + the related event should be matched using `pattern` and `key`. +- `key` (optional): The dot-separated field of the event to match, e.g. `content.body` + or `sender`. If it is not present, the condition should match all events, + that have a relation of type `rel_type`. +- `pattern` (optional): The glob-style pattern to match against. Patterns with + no special glob characters should be treated as having asterisks prepended + and appended when testing the condition. If this is not present, it should + match everything if the specific key is present. + +`key` and `pattern` are optional to allow you to enable or suppress all +notifications for a specific event type. For example one could suppress +notifications for all events in +[threads](https://github.com/matrix-org/matrix-doc/pull/3440) and all +[edits](https://github.com/matrix-org/matrix-doc/pull/2676) with the following +two conditions: + +```json5 +{ + "kind": "related_event_match", + "rel_type": "m.replace" +} +``` + +```json5 +{ + "kind": "related_event_match", + "rel_type": "m.thread" +} +``` + +Without a `key` the push rule can be evaluated without fetching the related to +event. + +### A push rule for replies + +To enable notifications for replies without relying on the reply fallback, but +with similar semantics we also define a new default push rule. The proposed +push rule differs slightly from the old behaviour, because it only notifies you +for replies to your events, but it does not notify you for replies to events, +which contained your display name or matrix ID. The rule should look like this: + +```json5 +{ + "rule_id": ".m.rule.reply", + "default": true, + "enabled": true, + "conditions": [ + { + "kind": "related_event_match" + "rel_type": "m.in_reply_to", + "key": "sender", + "pattern": "[the user's Matrix ID]" + } + ], + "actions": [ + "notify", + { + "set_tweak": "sound", + "value": "default" + }, + { + "set_tweak": "highlight" + } + ] +} +``` + +This should be an underride rule, since it can't be a content rule. It ensures +you get notified for replies to all events you sent. The actions are the same as +for `.m.rule.contains_display_name` and `.m.rule.contains_user_name`. + +No other rules are proposed as no other relations are in the specification as of +writing this MSC to decrease dependencies. + +## Potential issues + +- Most push rules for relations will need a lookup into a second event. This + causes additional implementation complexity and can potentially be + expensive. Looking up one event shouldn't be that heavy, but it is overhead, + that wasn't there before and it needs to be evaluated for every event, so it + clearly is somewhat performance sensitive. +- If the related to event is not present on the homeserver, evaluating the + push rule may be delayed or fail completely. For most rules this should not + be an issue. You can assume the event was not sent by a user on your server + if the event is not present on your server. + +## Alternatives + +- One could add an optional `rel_type` key to all existing conditions. This + would allow you to also easily match by `contains_display_name`, + `sender_notification_permission` and `room_member_count`. Out of those + conditions only `contains_display_name` seems to be useful in a related + event context. Having a potentially expensive key like `rel_type` available + for all conditions would also increase implementation complexity. As such + this MSC proposes the minimum amount of conditions to support push rules for + most relations, although allowing `rel_type` on `contains_display_name` and + `event_match` could be a good alternative. + +## Security considerations + +- These pushrules could be used to increase load on the homeserver. Apart from + that there shouldn't be any potential security issues. + +## Unstable prefix + +While this proposal is still in progress, implementations should use the +unstable prefix `im.nheko.msc3664` for the `related_event_match` condition. As +a result it should be called `im.nheko.msc3664.related_event_match`. + From 53a46071f13ad728ac98007bfff0611d2d676a9f Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Fri, 21 Jan 2022 02:52:28 +0100 Subject: [PATCH 02/22] Mention beeper implementation Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index acd84f3a88..a62712e50a 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -138,6 +138,11 @@ writing this MSC to decrease dependencies. this MSC proposes the minimum amount of conditions to support push rules for most relations, although allowing `rel_type` on `contains_display_name` and `event_match` could be a good alternative. +- Beeper has a + [similar feature in their synapse](https://gitlab.com/beeper/synapse/-/commit/44a1728b6b021f97900c89e0c00f7d1a23ce0d43), + but it does not allow you to filter by relation type and also differs + otherwise from the usual naming of push rule condition. + ## Security considerations From fa8fc978d34b06d496a918fd940c9345e134933c Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Fri, 21 Jan 2022 03:18:40 +0100 Subject: [PATCH 03/22] Remove my misunderstanding of push rule naming in synapse. Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index a62712e50a..7acbd7b840 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -140,8 +140,8 @@ writing this MSC to decrease dependencies. `event_match` could be a good alternative. - Beeper has a [similar feature in their synapse](https://gitlab.com/beeper/synapse/-/commit/44a1728b6b021f97900c89e0c00f7d1a23ce0d43), - but it does not allow you to filter by relation type and also differs - otherwise from the usual naming of push rule condition. + but it does not allow you to filter by relation type. + ## Security considerations From b90cdb61dfb0cba3d978ad303b8d7a3628299f58 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 05:09:34 +0100 Subject: [PATCH 04/22] Rule order, clarifications and thread discussion Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 116 +++++++++++++++--- 1 file changed, 102 insertions(+), 14 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 7acbd7b840..cc02c1de5d 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -50,6 +50,11 @@ messages. and appended when testing the condition. If this is not present, it should match everything if the specific key is present. +`key` and `pattern` have exactly the same meaning as in `event_match` +conditions, the wording is taken from the current spec. The wording of that is +currently debated in https://github.com/matrix-org/matrix-doc/issues/2637 and +this MSC just follows whatever the spec does for `event_match`. + `key` and `pattern` are optional to allow you to enable or suppress all notifications for a specific event type. For example one could suppress notifications for all events in @@ -72,7 +77,10 @@ two conditions: ``` Without a `key` the push rule can be evaluated without fetching the related to -event. +event. If only `key` is present, `pattern` should be assumed to be `*`, which +allows you to match for a field being present regardless of its value. If only +`value` is present, servers should return an error when setting the rule. +Clients should ignore the `pattern` field if there is no `key` field. ### A push rule for replies @@ -89,7 +97,7 @@ which contained your display name or matrix ID. The rule should look like this: "enabled": true, "conditions": [ { - "kind": "related_event_match" + "kind": "related_event_match", "rel_type": "m.in_reply_to", "key": "sender", "pattern": "[the user's Matrix ID]" @@ -108,24 +116,104 @@ which contained your display name or matrix ID. The rule should look like this: } ``` -This should be an underride rule, since it can't be a content rule. It ensures -you get notified for replies to all events you sent. The actions are the same as -for `.m.rule.contains_display_name` and `.m.rule.contains_user_name`. +This should be an override rule, since it can't be a content rule and should +not be overriden when setting a room to mentions only. It should be places just +before `.m.rule.contains_display_name` in the list. tThis ensures you get +notified for replies to all events you sent. The actions are the same as for +`.m.rule.contains_display_name` and `.m.rule.contains_user_name`. No other rules are proposed as no other relations are in the specification as of writing this MSC to decrease dependencies. ## Potential issues -- Most push rules for relations will need a lookup into a second event. This - causes additional implementation complexity and can potentially be - expensive. Looking up one event shouldn't be that heavy, but it is overhead, - that wasn't there before and it needs to be evaluated for every event, so it - clearly is somewhat performance sensitive. -- If the related to event is not present on the homeserver, evaluating the - push rule may be delayed or fail completely. For most rules this should not - be an issue. You can assume the event was not sent by a user on your server - if the event is not present on your server. +Most push rules for relations will need a lookup into a second event. This +causes additional implementation complexity and can potentially be expensive. +Looking up one event shouldn't be that heavy, but it is overhead, that wasn't +there before and it needs to be evaluated for every event, so it clearly is +somewhat performance sensitive. + +If the related to event is not present on the homeserver, evaluating the push +rule may be delayed or fail completely. For most rules this should not be an +issue. You can assume the event was not sent by a user on your server if the +event is not present on your server. In general clients and servers should do +their best to evaluate the condition. If they fail to do so (possibly because +they can't look up the event asynchronously) in a timely manner, the condition +may be ignored/evaluated to false. This should affect only a subset of events, +because in general relations happen to events in close proximity. There is a +risk of missing notifications for replies to very old messages and similar +relations. + + +[threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies +[as a fallback](https://github.com/matrix-org/matrix-doc/pull/3440/files#diff-113727ce0257b4dc0ad6f1087b6402f2cfcb6ff93272757b947bf1ce444056aeR82). +This would cause a notification with the new `.m.rule.reply` rule. To prevent +that the threads MSC could add rules like this to suppress notifications for +threads without the `render_in` attribute: + +```json5 +{ + "rule_id": ".m.rule.suppress_reply_notify_in_threads", + "default": true, + "enabled": true, + "conditions": [ + { + "kind": "related_event_match", + "rel_type": "m.in_reply_to" + }, + { + "kind": "related_event_match", + "rel_type": "m.thread" + } + ], + "actions": [ + "notify" + ] +}, +{ + "rule_id": ".m.rule.reply_in_thread", + "default": true, + "enabled": true, + "conditions": [ + { + "kind": "related_event_match", + "rel_type": "m.in_reply_to", + "key": "sender", + "pattern": "[the user's Matrix ID]" + }, + { + "kind": "related_event_match", + "rel_type": "m.thread" + }, + { + "kind": "event_match", + "key": "m.relates_to.m.in_reply_to.render_in", + "pattern": "m.thread" + } + ], + "actions": [ + "notify", + { + "set_tweak": "sound", + "value": "default" + }, + { + "set_tweak": "highlight" + } + ] +} +``` + +This would be significantly easier if there were ways to match for fields NOT +being present and if a pattern can match a field in an array is not clearly +defined in the specification at the moment: https://github.com/matrix-org/matrix-doc/issues/3082 + +That will need a solution, but there are multiple ways to solve this and it +probably does not need to happen on this MSC? The easiest solution would be to +be able to just invert a pattern. Then you could just suppress notifications for +events without `render_in` or the threading MSC could inverts its event format +to make it easier to match with pushrules (i.e. `dont_render_in` or +`fallback_relation`). ## Alternatives From a6eebe710058111228d5266643f750dbd6d80df6 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 05:12:36 +0100 Subject: [PATCH 05/22] fix typo Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index cc02c1de5d..363acc4936 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -117,7 +117,7 @@ which contained your display name or matrix ID. The rule should look like this: ``` This should be an override rule, since it can't be a content rule and should -not be overriden when setting a room to mentions only. It should be places just +not be overridden when setting a room to mentions only. It should be places just before `.m.rule.contains_display_name` in the list. tThis ensures you get notified for replies to all events you sent. The actions are the same as for `.m.rule.contains_display_name` and `.m.rule.contains_user_name`. From 0161daabff17af925df0cb5966df1dc87730624d Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Mon, 24 Jan 2022 10:26:23 +0000 Subject: [PATCH 06/22] Update proposals/3664-notifications-for-relations.md Co-authored-by: Stuart Mumford --- proposals/3664-notifications-for-relations.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 363acc4936..ee90bc6842 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -117,8 +117,8 @@ which contained your display name or matrix ID. The rule should look like this: ``` This should be an override rule, since it can't be a content rule and should -not be overridden when setting a room to mentions only. It should be places just -before `.m.rule.contains_display_name` in the list. tThis ensures you get +not be overridden when setting a room to mentions only. It should be placed just +before `.m.rule.contains_display_name` in the list. This ensures you get notified for replies to all events you sent. The actions are the same as for `.m.rule.contains_display_name` and `.m.rule.contains_user_name`. From 242a65c52935fc3c8c0cd087f0c69ca0231a8cdb Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 13:15:49 +0100 Subject: [PATCH 07/22] value -> pattern Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index ee90bc6842..441631df0c 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -79,7 +79,7 @@ two conditions: Without a `key` the push rule can be evaluated without fetching the related to event. If only `key` is present, `pattern` should be assumed to be `*`, which allows you to match for a field being present regardless of its value. If only -`value` is present, servers should return an error when setting the rule. +`pattern` is present, servers should return an error when setting the rule. Clients should ignore the `pattern` field if there is no `key` field. ### A push rule for replies From 070babcec5817a4895b3f86fa8527691dc2f0d8c Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 13:19:11 +0100 Subject: [PATCH 08/22] Define how a client can check for support Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 441631df0c..d7f060ae94 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -82,6 +82,9 @@ allows you to match for a field being present regardless of its value. If only `pattern` is present, servers should return an error when setting the rule. Clients should ignore the `pattern` field if there is no `key` field. +A client can check for the `related_event_match` condition being supported by +testing for an existing `.m.rule.reply` in the default rules. + ### A push rule for replies To enable notifications for replies without relying on the reply fallback, but From 06b539379bb1812ccf2f70130a034a0055f56d04 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 24 Jan 2022 13:34:33 +0100 Subject: [PATCH 09/22] Clarify pattern and key more Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index d7f060ae94..5b710e665c 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -45,19 +45,15 @@ messages. - `key` (optional): The dot-separated field of the event to match, e.g. `content.body` or `sender`. If it is not present, the condition should match all events, that have a relation of type `rel_type`. -- `pattern` (optional): The glob-style pattern to match against. Patterns with - no special glob characters should be treated as having asterisks prepended - and appended when testing the condition. If this is not present, it should - match everything if the specific key is present. +- `pattern` (optional): The glob-style pattern to match against. `key` and `pattern` have exactly the same meaning as in `event_match` -conditions, the wording is taken from the current spec. The wording of that is -currently debated in https://github.com/matrix-org/matrix-doc/issues/2637 and -this MSC just follows whatever the spec does for `event_match`. +conditions. See https://github.com/matrix-org/matrix-doc/issues/2637 for a +clarification of their behaviour. `key` and `pattern` are optional to allow you to enable or suppress all -notifications for a specific event type. For example one could suppress -notifications for all events in +notifications for a specific relation type. For example one could suppress +notifications for all events with a realation from [threads](https://github.com/matrix-org/matrix-doc/pull/3440) and all [edits](https://github.com/matrix-org/matrix-doc/pull/2676) with the following two conditions: @@ -76,11 +72,11 @@ two conditions: } ``` -Without a `key` the push rule can be evaluated without fetching the related to -event. If only `key` is present, `pattern` should be assumed to be `*`, which -allows you to match for a field being present regardless of its value. If only -`pattern` is present, servers should return an error when setting the rule. -Clients should ignore the `pattern` field if there is no `key` field. +Without a `key` and `pattern` the push rule can be evaluated without fetching +the related to event. If one of those two fields is missing, a server should +prevent those rules from being added with the appropriate error code. (A client +wouldn't have a choice but to ignore those keys if the server failed to prevent +the rule from being added.) A client can check for the `related_event_match` condition being supported by testing for an existing `.m.rule.reply` in the default rules. From f2a5c57cb5fe31dbb0b55424c2e75bb8d05ad290 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 25 Jan 2022 06:01:52 +0000 Subject: [PATCH 10/22] Update proposals/3664-notifications-for-relations.md Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/3664-notifications-for-relations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 5b710e665c..0debf0b725 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -144,7 +144,7 @@ risk of missing notifications for replies to very old messages and similar relations. -[threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies +[Threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies [as a fallback](https://github.com/matrix-org/matrix-doc/pull/3440/files#diff-113727ce0257b4dc0ad6f1087b6402f2cfcb6ff93272757b947bf1ce444056aeR82). This would cause a notification with the new `.m.rule.reply` rule. To prevent that the threads MSC could add rules like this to suppress notifications for From aebe3e923c3f4e368bb9081841d5aca72da8234c Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 25 Jan 2022 14:58:14 +0000 Subject: [PATCH 11/22] Update proposals/3664-notifications-for-relations.md Co-authored-by: Matthew Hodgson --- proposals/3664-notifications-for-relations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 0debf0b725..1da9843599 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -53,7 +53,7 @@ clarification of their behaviour. `key` and `pattern` are optional to allow you to enable or suppress all notifications for a specific relation type. For example one could suppress -notifications for all events with a realation from +notifications for all events with a relation from [threads](https://github.com/matrix-org/matrix-doc/pull/3440) and all [edits](https://github.com/matrix-org/matrix-doc/pull/2676) with the following two conditions: From 1ead1da4895759454cee982f99e5c512bf1ce5f8 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 26 Jan 2022 12:01:07 +0100 Subject: [PATCH 12/22] Remove partial solution for threads in favour of a different proposal Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 70 ++----------------- 1 file changed, 4 insertions(+), 66 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 1da9843599..496263afa0 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -147,72 +147,10 @@ relations. [Threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies [as a fallback](https://github.com/matrix-org/matrix-doc/pull/3440/files#diff-113727ce0257b4dc0ad6f1087b6402f2cfcb6ff93272757b947bf1ce444056aeR82). This would cause a notification with the new `.m.rule.reply` rule. To prevent -that the threads MSC could add rules like this to suppress notifications for -threads without the `render_in` attribute: - -```json5 -{ - "rule_id": ".m.rule.suppress_reply_notify_in_threads", - "default": true, - "enabled": true, - "conditions": [ - { - "kind": "related_event_match", - "rel_type": "m.in_reply_to" - }, - { - "kind": "related_event_match", - "rel_type": "m.thread" - } - ], - "actions": [ - "notify" - ] -}, -{ - "rule_id": ".m.rule.reply_in_thread", - "default": true, - "enabled": true, - "conditions": [ - { - "kind": "related_event_match", - "rel_type": "m.in_reply_to", - "key": "sender", - "pattern": "[the user's Matrix ID]" - }, - { - "kind": "related_event_match", - "rel_type": "m.thread" - }, - { - "kind": "event_match", - "key": "m.relates_to.m.in_reply_to.render_in", - "pattern": "m.thread" - } - ], - "actions": [ - "notify", - { - "set_tweak": "sound", - "value": "default" - }, - { - "set_tweak": "highlight" - } - ] -} -``` - -This would be significantly easier if there were ways to match for fields NOT -being present and if a pattern can match a field in an array is not clearly -defined in the specification at the moment: https://github.com/matrix-org/matrix-doc/issues/3082 - -That will need a solution, but there are multiple ways to solve this and it -probably does not need to happen on this MSC? The easiest solution would be to -be able to just invert a pattern. Then you could just suppress notifications for -events without `render_in` or the threading MSC could inverts its event format -to make it easier to match with pushrules (i.e. `dont_render_in` or -`fallback_relation`). +that another MSC should add a or modify the existing push rules to not apply to +threads, when they are not a reply. This MSC does not spell out a solution for +that, because the author lacks familiarity with the design goals of the +threading proposals. ## Alternatives From 6e1d1bb61e0bd7d6f38571fd60893f117b1c3413 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 26 Jan 2022 12:06:29 +0100 Subject: [PATCH 13/22] Mention issues with unexposed notification settings Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 496263afa0..f61a8d878d 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -143,7 +143,6 @@ because in general relations happen to events in close proximity. There is a risk of missing notifications for replies to very old messages and similar relations. - [Threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies [as a fallback](https://github.com/matrix-org/matrix-doc/pull/3440/files#diff-113727ce0257b4dc0ad6f1087b6402f2cfcb6ff93272757b947bf1ce444056aeR82). This would cause a notification with the new `.m.rule.reply` rule. To prevent @@ -152,6 +151,18 @@ threads, when they are not a reply. This MSC does not spell out a solution for that, because the author lacks familiarity with the design goals of the threading proposals. +Adding a new rule, that causes notifications, forces users to change their +notification settings again. In this case, if the user disabled pings for +mentions, they will need to disable the rule manually again, if they did so to +disable replies. In the transition period clients might not have a UI to do so. +This is a risk with all push rule changes and since it allows for a much better +control over what notifies you, the trandeoff should be acceptable. Many users +disable mention based pings, because they +[can be error prone](https://github.com/matrix-org/matrix-doc/pull/3517), but +they may not actually had the intention to also disable notifications for +replies, which should only trigger for actual replies to your messages. So for a +significant chunk of people disabling mentions this should be an improvement. + ## Alternatives - One could add an optional `rel_type` key to all existing conditions. This From 3a288b66375a472ad80b8f2bb82892343475ea6b Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 1 Feb 2022 18:16:01 +0000 Subject: [PATCH 14/22] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/3664-notifications-for-relations.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index f61a8d878d..d22bb93fd7 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -3,8 +3,8 @@ Relations are very powerful and are becoming a platform to build new features like replies, edits, reactions, threads, polls and much more on. -On the other hand there is currently no way to control, what you are getting -notified for. Some people want to get notified, when someone replies to their +On the other hand there is currently no way to control what you are getting +notified for. Some people want to get notified when someone replies to their message. Some want to get notified for reactions to their message. Some people explicitly do not want that. You might want to be able to mute a thread and you may want to get notified for poll responeses or not. Some people like getting @@ -128,7 +128,7 @@ writing this MSC to decrease dependencies. Most push rules for relations will need a lookup into a second event. This causes additional implementation complexity and can potentially be expensive. -Looking up one event shouldn't be that heavy, but it is overhead, that wasn't +Looking up one event shouldn't be that heavy, but it is overhead that wasn't there before and it needs to be evaluated for every event, so it clearly is somewhat performance sensitive. @@ -151,15 +151,16 @@ threads, when they are not a reply. This MSC does not spell out a solution for that, because the author lacks familiarity with the design goals of the threading proposals. -Adding a new rule, that causes notifications, forces users to change their -notification settings again. In this case, if the user disabled pings for -mentions, they will need to disable the rule manually again, if they did so to -disable replies. In the transition period clients might not have a UI to do so. +Adding a new rule that causes notifications, forces users to change their +notification settings again. In this case, a user who disabled notifications +for mentions (or set them to silent) may be surprised to suddenly start +receiving noisy notifications for replies. Worse, in the transition period, +clients might not have a UI to disable the new notifications. This is a risk with all push rule changes and since it allows for a much better -control over what notifies you, the trandeoff should be acceptable. Many users +control over what notifies you, the tradeoff should be acceptable. Many users disable mention based pings, because they [can be error prone](https://github.com/matrix-org/matrix-doc/pull/3517), but -they may not actually had the intention to also disable notifications for +they may not actually have intended to also disable notifications for replies, which should only trigger for actual replies to your messages. So for a significant chunk of people disabling mentions this should be an improvement. From df9aab5220bc35cdd7ec1a5b9aab4bc7080a31d1 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Tue, 24 May 2022 17:11:19 +0200 Subject: [PATCH 15/22] Fix conflict with threading MSC Signed-off-by: Nicolas Werner --- proposals/3664-notifications-for-relations.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index d22bb93fd7..b2a78f81fc 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -29,6 +29,7 @@ event. Such a condition could look like this: { "kind": "related_event_match", "rel_type": "m.in_reply_to", + "include_fallbacks": false, "key": "sender", "pattern": "@me:my.server" } @@ -42,6 +43,9 @@ messages. [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) with a `rel_type` of `m.in_reply_to`. If the event has any relation of this type, the related event should be matched using `pattern` and `key`. +- `include_fallbacks` decides if the relation should be followed even for + fallbacks (i.e. relations with the `is_falling_back` property set to `true` + like for threads). Defaults to false so only actual relations are counted. - `key` (optional): The dot-separated field of the event to match, e.g. `content.body` or `sender`. If it is not present, the condition should match all events, that have a relation of type `rel_type`. @@ -146,10 +150,10 @@ relations. [Threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies [as a fallback](https://github.com/matrix-org/matrix-doc/pull/3440/files#diff-113727ce0257b4dc0ad6f1087b6402f2cfcb6ff93272757b947bf1ce444056aeR82). This would cause a notification with the new `.m.rule.reply` rule. To prevent -that another MSC should add a or modify the existing push rules to not apply to -threads, when they are not a reply. This MSC does not spell out a solution for -that, because the author lacks familiarity with the design goals of the -threading proposals. +that this MSC adds the `include_fallbacks` key to the rule, so that reply +relations only added as a fallback are ignored. (Currently `is_falling_back` key +is in a bit of a weird location. Maybe this can be amended in the threading MSC +to be a bit more generic before it is added to the spec.) Adding a new rule that causes notifications, forces users to change their notification settings again. In this case, a user who disabled notifications From 00b0ef9aa68b9d11b838a43fffc79b93a559de09 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 24 May 2022 15:15:03 +0000 Subject: [PATCH 16/22] Apply suggestions from code review Co-authored-by: David Baker --- proposals/3664-notifications-for-relations.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index b2a78f81fc..e96d3776c6 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -18,7 +18,7 @@ can adjust them to their own wishes. ### New push rule condition: `related_event_match` -Notifications for relation based features need to distinguish, what type of +Notifications for relation based features need to distinguish what type of relation was used and potentially match on the content of the related-to event. To do that we introduce a new type of condition: `related_event_match`. This is @@ -38,7 +38,7 @@ event. Such a condition could look like this: This condition can be used to notify me whenever someone sends a reply to my messages. -- `rel_type` is the relation type. For the sake of compatibility replies +- `rel_type` is the relation type. For the sake of compatibility, replies should be matched as if they were sent in the relation format from [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) with a `rel_type` of `m.in_reply_to`. If the event has any relation of this type, @@ -90,8 +90,8 @@ testing for an existing `.m.rule.reply` in the default rules. To enable notifications for replies without relying on the reply fallback, but with similar semantics we also define a new default push rule. The proposed push rule differs slightly from the old behaviour, because it only notifies you -for replies to your events, but it does not notify you for replies to events, -which contained your display name or matrix ID. The rule should look like this: +for replies to your events, but it does not notify you for replies to events +containing your display name or matrix ID. The rule should look like this: ```json5 { @@ -155,7 +155,7 @@ relations only added as a fallback are ignored. (Currently `is_falling_back` key is in a bit of a weird location. Maybe this can be amended in the threading MSC to be a bit more generic before it is added to the spec.) -Adding a new rule that causes notifications, forces users to change their +Adding a new rule that causes notifications will force users to change their notification settings again. In this case, a user who disabled notifications for mentions (or set them to silent) may be surprised to suddenly start receiving noisy notifications for replies. Worse, in the transition period, From 519b95d0f991f6d78ff1858822f2ce38a1257c25 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 24 May 2022 18:33:03 +0000 Subject: [PATCH 17/22] Update proposals/3664-notifications-for-relations.md Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/3664-notifications-for-relations.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index e96d3776c6..916a712ed0 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -45,7 +45,8 @@ messages. the related event should be matched using `pattern` and `key`. - `include_fallbacks` decides if the relation should be followed even for fallbacks (i.e. relations with the `is_falling_back` property set to `true` - like for threads). Defaults to false so only actual relations are counted. + like for [threads](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#backwards-compatibility). + Defaults to `false` so only actual relations are counted. - `key` (optional): The dot-separated field of the event to match, e.g. `content.body` or `sender`. If it is not present, the condition should match all events, that have a relation of type `rel_type`. From c9d55e2c61408c19c65146edb063ef19e40651fc Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 27 Oct 2022 17:52:48 +0200 Subject: [PATCH 18/22] Add capabilities flag --- proposals/3664-notifications-for-relations.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 916a712ed0..c5297b660f 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -197,3 +197,7 @@ While this proposal is still in progress, implementations should use the unstable prefix `im.nheko.msc3664` for the `related_event_match` condition. As a result it should be called `im.nheko.msc3664.related_event_match`. +Clients can check the capabilities for `im.nheko.msc3664.related_event_match` to +see if this MSC is implemented and enabled on the homeserver until this MSC is +included in a spec release. + From 22e67c497661c7c535513c9894399d979f842ad1 Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Thu, 27 Oct 2022 15:56:41 +0000 Subject: [PATCH 19/22] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- proposals/3664-notifications-for-relations.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index c5297b660f..d6617f2b76 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -52,9 +52,8 @@ messages. that have a relation of type `rel_type`. - `pattern` (optional): The glob-style pattern to match against. -`key` and `pattern` have exactly the same meaning as in `event_match` -conditions. See https://github.com/matrix-org/matrix-doc/issues/2637 for a -clarification of their behaviour. +`key` and `pattern` have exactly the same meaning as in +[`event_match` conditions](https://spec.matrix.org/unstable/client-server-api/#conditions-1). `key` and `pattern` are optional to allow you to enable or suppress all notifications for a specific relation type. For example one could suppress @@ -80,8 +79,8 @@ two conditions: Without a `key` and `pattern` the push rule can be evaluated without fetching the related to event. If one of those two fields is missing, a server should prevent those rules from being added with the appropriate error code. (A client -wouldn't have a choice but to ignore those keys if the server failed to prevent -the rule from being added.) +which sees a `related_event_match` condition with one, but not both, of `key` and `pattern` should +ignore the `key`/`pattern` property. A client can check for the `related_event_match` condition being supported by testing for an existing `.m.rule.reply` in the default rules. @@ -149,7 +148,7 @@ risk of missing notifications for replies to very old messages and similar relations. [Threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies -[as a fallback](https://github.com/matrix-org/matrix-doc/pull/3440/files#diff-113727ce0257b4dc0ad6f1087b6402f2cfcb6ff93272757b947bf1ce444056aeR82). +[as a fallback](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#backwards-compatibility). This would cause a notification with the new `.m.rule.reply` rule. To prevent that this MSC adds the `include_fallbacks` key to the rule, so that reply relations only added as a fallback are ignored. (Currently `is_falling_back` key From 72488260daf74d783aaeb5962c1634ed550fbf53 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 27 Oct 2022 17:59:29 +0200 Subject: [PATCH 20/22] Clarify rule ignored <-> condition evaluated to false --- proposals/3664-notifications-for-relations.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index d6617f2b76..3c932877dd 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -141,11 +141,11 @@ rule may be delayed or fail completely. For most rules this should not be an issue. You can assume the event was not sent by a user on your server if the event is not present on your server. In general clients and servers should do their best to evaluate the condition. If they fail to do so (possibly because -they can't look up the event asynchronously) in a timely manner, the condition -may be ignored/evaluated to false. This should affect only a subset of events, -because in general relations happen to events in close proximity. There is a -risk of missing notifications for replies to very old messages and similar -relations. +they can't look up the event asynchronously) in a timely manner, the rule +may be ignored/the condition evaluated to false. This should affect only a +subset of events, because in general relations happen to events in close +proximity. There is a risk of missing notifications for replies to very old +messages and similar relations. [Threads](https://github.com/matrix-org/matrix-doc/pull/3440) use replies [as a fallback](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#backwards-compatibility). From 69fe03e3fe785fc905fba60caf5c2d2b15024857 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 27 Oct 2022 18:08:34 +0200 Subject: [PATCH 21/22] Clarify how key and pattern work --- proposals/3664-notifications-for-relations.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 3c932877dd..3ef10269f4 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -42,7 +42,8 @@ messages. should be matched as if they were sent in the relation format from [MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) with a `rel_type` of `m.in_reply_to`. If the event has any relation of this type, - the related event should be matched using `pattern` and `key`. + the related event should be matched using `pattern` and `key` as if matching + that related event using an event_match rule. - `include_fallbacks` decides if the relation should be followed even for fallbacks (i.e. relations with the `is_falling_back` property set to `true` like for [threads](https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3440-threading-via-relations.md#backwards-compatibility). From b1897fa637aa6fc5631d9a82497239e1a51e3c1c Mon Sep 17 00:00:00 2001 From: "DeepBlueV7.X" Date: Tue, 1 Nov 2022 11:32:33 +0000 Subject: [PATCH 22/22] Update proposals/3664-notifications-for-relations.md Co-authored-by: Patrick Cloke --- proposals/3664-notifications-for-relations.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proposals/3664-notifications-for-relations.md b/proposals/3664-notifications-for-relations.md index 3ef10269f4..ce51d6863f 100644 --- a/proposals/3664-notifications-for-relations.md +++ b/proposals/3664-notifications-for-relations.md @@ -59,8 +59,8 @@ messages. `key` and `pattern` are optional to allow you to enable or suppress all notifications for a specific relation type. For example one could suppress notifications for all events with a relation from -[threads](https://github.com/matrix-org/matrix-doc/pull/3440) and all -[edits](https://github.com/matrix-org/matrix-doc/pull/2676) with the following +[threads](https://spec.matrix.org/v1.4/client-server-api/#threading) and all +[edits](https://spec.matrix.org/v1.4/client-server-api/#event-replacements) with the following two conditions: ```json5