Implement MSC3987, fix setting Element Android notifications (#3242)

Should fix https://github.com/matrix-org/dendrite/issues/3183, since
Element Android already implements
[MSC3987](https://github.com/vector-im/element-android/pull/8530)

This is also part of https://github.com/matrix-org/dendrite/issues/3225
This commit is contained in:
Till 2023-10-24 11:51:08 +02:00 committed by GitHub
parent c1d6b9aa8e
commit 1b124fe9cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 18 additions and 29 deletions

View File

@ -1418,7 +1418,7 @@ func TestPushRules(t *testing.T) {
validateFunc: func(t *testing.T, respBody *bytes.Buffer) { validateFunc: func(t *testing.T, respBody *bytes.Buffer) {
actions := gjson.GetBytes(respBody.Bytes(), "actions").Array() actions := gjson.GetBytes(respBody.Bytes(), "actions").Array()
// only a basic check // only a basic check
assert.Equal(t, 1, len(actions)) assert.Equal(t, 0, len(actions))
}, },
}, },
{ {

View File

@ -13,8 +13,6 @@ func TestActionJSON(t *testing.T) {
Want Action Want Action
}{ }{
{Action{Kind: NotifyAction}}, {Action{Kind: NotifyAction}},
{Action{Kind: DontNotifyAction}},
{Action{Kind: CoalesceAction}},
{Action{Kind: SetTweakAction}}, {Action{Kind: SetTweakAction}},
{Action{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}}, {Action{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}},

View File

@ -10,6 +10,7 @@ func defaultOverrideRules(userID string) []*Rule {
&mRuleRoomNotifDefinition, &mRuleRoomNotifDefinition,
&mRuleTombstoneDefinition, &mRuleTombstoneDefinition,
&mRuleReactionDefinition, &mRuleReactionDefinition,
&mRuleACLsDefinition,
} }
} }
@ -30,7 +31,7 @@ var (
RuleID: MRuleMaster, RuleID: MRuleMaster,
Default: true, Default: true,
Enabled: false, Enabled: false,
Actions: []*Action{{Kind: DontNotifyAction}}, Actions: []*Action{},
} }
mRuleSuppressNoticesDefinition = Rule{ mRuleSuppressNoticesDefinition = Rule{
RuleID: MRuleSuppressNotices, RuleID: MRuleSuppressNotices,
@ -43,7 +44,7 @@ var (
Pattern: pointer("m.notice"), Pattern: pointer("m.notice"),
}, },
}, },
Actions: []*Action{{Kind: DontNotifyAction}}, Actions: []*Action{},
} }
mRuleMemberEventDefinition = Rule{ mRuleMemberEventDefinition = Rule{
RuleID: MRuleMemberEvent, RuleID: MRuleMemberEvent,
@ -56,7 +57,7 @@ var (
Pattern: pointer("m.room.member"), Pattern: pointer("m.room.member"),
}, },
}, },
Actions: []*Action{{Kind: DontNotifyAction}}, Actions: []*Action{},
} }
mRuleContainsDisplayNameDefinition = Rule{ mRuleContainsDisplayNameDefinition = Rule{
RuleID: MRuleContainsDisplayName, RuleID: MRuleContainsDisplayName,
@ -152,9 +153,7 @@ var (
Pattern: pointer("m.reaction"), Pattern: pointer("m.reaction"),
}, },
}, },
Actions: []*Action{ Actions: []*Action{},
{Kind: DontNotifyAction},
},
} }
) )

View File

@ -21,12 +21,12 @@ func TestDefaultRules(t *testing.T) {
// Default override rules // Default override rules
{ {
name: ".m.rule.master", name: ".m.rule.master",
inputBytes: []byte(`{"rule_id":".m.rule.master","default":true,"enabled":false,"actions":["dont_notify"]}`), inputBytes: []byte(`{"rule_id":".m.rule.master","default":true,"enabled":false,"actions":[]}`),
want: mRuleMasterDefinition, want: mRuleMasterDefinition,
}, },
{ {
name: ".m.rule.suppress_notices", name: ".m.rule.suppress_notices",
inputBytes: []byte(`{"rule_id":".m.rule.suppress_notices","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"content.msgtype","pattern":"m.notice"}],"actions":["dont_notify"]}`), inputBytes: []byte(`{"rule_id":".m.rule.suppress_notices","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"content.msgtype","pattern":"m.notice"}],"actions":[]}`),
want: mRuleSuppressNoticesDefinition, want: mRuleSuppressNoticesDefinition,
}, },
{ {
@ -36,7 +36,7 @@ func TestDefaultRules(t *testing.T) {
}, },
{ {
name: ".m.rule.member_event", name: ".m.rule.member_event",
inputBytes: []byte(`{"rule_id":".m.rule.member_event","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.member"}],"actions":["dont_notify"]}`), inputBytes: []byte(`{"rule_id":".m.rule.member_event","default":true,"enabled":true,"conditions":[{"kind":"event_match","key":"type","pattern":"m.room.member"}],"actions":[]}`),
want: mRuleMemberEventDefinition, want: mRuleMemberEventDefinition,
}, },
{ {

View File

@ -16,10 +16,7 @@ func ActionsToTweaks(as []*Action) (ActionKind, map[string]interface{}, error) {
for _, a := range as { for _, a := range as {
switch a.Kind { switch a.Kind {
case DontNotifyAction: case DontNotifyAction: // Ignored
// Don't bother processing any further
return DontNotifyAction, nil, nil
case SetTweakAction: case SetTweakAction:
if tweaks == nil { if tweaks == nil {
tweaks = map[string]interface{}{} tweaks = map[string]interface{}{}

View File

@ -17,17 +17,16 @@ func TestActionsToTweaks(t *testing.T) {
{"empty", nil, UnknownAction, nil}, {"empty", nil, UnknownAction, nil},
{"zero", []*Action{{}}, UnknownAction, nil}, {"zero", []*Action{{}}, UnknownAction, nil},
{"onlyPrimary", []*Action{{Kind: NotifyAction}}, NotifyAction, nil}, {"onlyPrimary", []*Action{{Kind: NotifyAction}}, NotifyAction, nil},
{"onlyPrimaryDontNotify", []*Action{{Kind: DontNotifyAction}}, DontNotifyAction, nil}, {"onlyPrimaryDontNotify", []*Action{}, UnknownAction, nil},
{"onlyTweak", []*Action{{Kind: SetTweakAction, Tweak: HighlightTweak}}, UnknownAction, map[string]interface{}{"highlight": nil}}, {"onlyTweak", []*Action{{Kind: SetTweakAction, Tweak: HighlightTweak}}, UnknownAction, map[string]interface{}{"highlight": nil}},
{"onlyTweakWithValue", []*Action{{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}}, UnknownAction, map[string]interface{}{"sound": "default"}}, {"onlyTweakWithValue", []*Action{{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}}, UnknownAction, map[string]interface{}{"sound": "default"}},
{ {
"all", "all",
[]*Action{ []*Action{
{Kind: CoalesceAction},
{Kind: SetTweakAction, Tweak: HighlightTweak}, {Kind: SetTweakAction, Tweak: HighlightTweak},
{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}, {Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"},
}, },
CoalesceAction, UnknownAction,
map[string]interface{}{"highlight": nil, "sound": "default"}, map[string]interface{}{"highlight": nil, "sound": "default"},
}, },
} }

View File

@ -18,7 +18,7 @@ func ValidateRule(kind Kind, rule *Rule) []error {
errs = append(errs, fmt.Errorf("invalid rule ID: %s", rule.RuleID)) errs = append(errs, fmt.Errorf("invalid rule ID: %s", rule.RuleID))
} }
if len(rule.Actions) == 0 { if rule.Actions == nil {
errs = append(errs, fmt.Errorf("missing actions")) errs = append(errs, fmt.Errorf("missing actions"))
} }
for _, action := range rule.Actions { for _, action := range rule.Actions {

View File

@ -538,8 +538,8 @@ func (s *OutputRoomEventConsumer) notifyLocal(ctx context.Context, event *rstype
if err != nil { if err != nil {
return fmt.Errorf("pushrules.ActionsToTweaks: %w", err) return fmt.Errorf("pushrules.ActionsToTweaks: %w", err)
} }
// TODO: support coalescing.
if a != pushrules.NotifyAction && a != pushrules.CoalesceAction { if a != pushrules.NotifyAction {
log.WithFields(log.Fields{ log.WithFields(log.Fields{
"event_id": event.EventID(), "event_id": event.EventID(),
"room_id": event.RoomID().String(), "room_id": event.RoomID().String(),

View File

@ -81,12 +81,8 @@ func Test_evaluatePushRules(t *testing.T) {
{ {
name: "m.reaction doesn't notify", name: "m.reaction doesn't notify",
eventContent: `{"type":"m.reaction","room_id":"!room:example.com"}`, eventContent: `{"type":"m.reaction","room_id":"!room:example.com"}`,
wantAction: pushrules.DontNotifyAction, wantAction: pushrules.UnknownAction,
wantActions: []*pushrules.Action{ wantActions: []*pushrules.Action{},
{
Kind: pushrules.DontNotifyAction,
},
},
}, },
{ {
name: "m.room.message notifies", name: "m.room.message notifies",
@ -136,7 +132,7 @@ func Test_evaluatePushRules(t *testing.T) {
t.Fatalf("expected action to be '%s', got '%s'", tc.wantAction, gotAction) t.Fatalf("expected action to be '%s', got '%s'", tc.wantAction, gotAction)
} }
// this is taken from `notifyLocal` // this is taken from `notifyLocal`
if tc.wantNotify && gotAction != pushrules.NotifyAction && gotAction != pushrules.CoalesceAction { if tc.wantNotify && gotAction != pushrules.NotifyAction {
t.Fatalf("expected to notify but didn't") t.Fatalf("expected to notify but didn't")
} }
}) })