Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MSC3987, fix setting Element Android notifications #3242

Merged
merged 5 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clientapi/clientapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1418,7 +1418,7 @@ func TestPushRules(t *testing.T) {
validateFunc: func(t *testing.T, respBody *bytes.Buffer) {
actions := gjson.GetBytes(respBody.Bytes(), "actions").Array()
// only a basic check
assert.Equal(t, 1, len(actions))
assert.Equal(t, 0, len(actions))
},
},
{
Expand Down
2 changes: 0 additions & 2 deletions internal/pushrules/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ func TestActionJSON(t *testing.T) {
Want Action
}{
{Action{Kind: NotifyAction}},
{Action{Kind: DontNotifyAction}},
{Action{Kind: CoalesceAction}},
{Action{Kind: SetTweakAction}},

{Action{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}},
Expand Down
11 changes: 5 additions & 6 deletions internal/pushrules/default_override.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ func defaultOverrideRules(userID string) []*Rule {
&mRuleRoomNotifDefinition,
&mRuleTombstoneDefinition,
&mRuleReactionDefinition,
&mRuleACLsDefinition,
}
}

Expand All @@ -30,7 +31,7 @@ var (
RuleID: MRuleMaster,
Default: true,
Enabled: false,
Actions: []*Action{{Kind: DontNotifyAction}},
Actions: []*Action{},
}
mRuleSuppressNoticesDefinition = Rule{
RuleID: MRuleSuppressNotices,
Expand All @@ -43,7 +44,7 @@ var (
Pattern: pointer("m.notice"),
},
},
Actions: []*Action{{Kind: DontNotifyAction}},
Actions: []*Action{},
}
mRuleMemberEventDefinition = Rule{
RuleID: MRuleMemberEvent,
Expand All @@ -56,7 +57,7 @@ var (
Pattern: pointer("m.room.member"),
},
},
Actions: []*Action{{Kind: DontNotifyAction}},
Actions: []*Action{},
}
mRuleContainsDisplayNameDefinition = Rule{
RuleID: MRuleContainsDisplayName,
Expand Down Expand Up @@ -152,9 +153,7 @@ var (
Pattern: pointer("m.reaction"),
},
},
Actions: []*Action{
{Kind: DontNotifyAction},
},
Actions: []*Action{},
}
)

Expand Down
6 changes: 3 additions & 3 deletions internal/pushrules/default_pushrules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ func TestDefaultRules(t *testing.T) {
// Default override rules
{
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,
},
{
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,
},
{
Expand All @@ -36,7 +36,7 @@ func TestDefaultRules(t *testing.T) {
},
{
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,
},
{
Expand Down
5 changes: 1 addition & 4 deletions internal/pushrules/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ func ActionsToTweaks(as []*Action) (ActionKind, map[string]interface{}, error) {

for _, a := range as {
switch a.Kind {
case DontNotifyAction:
// Don't bother processing any further
return DontNotifyAction, nil, nil

case DontNotifyAction: // Ignored
case SetTweakAction:
if tweaks == nil {
tweaks = map[string]interface{}{}
Expand Down
5 changes: 2 additions & 3 deletions internal/pushrules/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,16 @@ func TestActionsToTweaks(t *testing.T) {
{"empty", nil, UnknownAction, nil},
{"zero", []*Action{{}}, UnknownAction, 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}},
{"onlyTweakWithValue", []*Action{{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"}}, UnknownAction, map[string]interface{}{"sound": "default"}},
{
"all",
[]*Action{
{Kind: CoalesceAction},
{Kind: SetTweakAction, Tweak: HighlightTweak},
{Kind: SetTweakAction, Tweak: SoundTweak, Value: "default"},
},
CoalesceAction,
UnknownAction,
map[string]interface{}{"highlight": nil, "sound": "default"},
},
}
Expand Down
3 changes: 0 additions & 3 deletions internal/pushrules/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ func ValidateRule(kind Kind, rule *Rule) []error {
errs = append(errs, fmt.Errorf("invalid rule ID: %s", rule.RuleID))
}

if len(rule.Actions) == 0 {
errs = append(errs, fmt.Errorf("missing actions"))
}
for _, action := range rule.Actions {
errs = append(errs, validateAction(action)...)
}
Expand Down
1 change: 0 additions & 1 deletion internal/pushrules/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ func TestValidateRuleNegatives(t *testing.T) {
{Name: "emptyRuleID", Kind: OverrideKind, Rule: Rule{}, WantErrString: "invalid rule ID"},
{Name: "invalidKind", Kind: Kind("something else"), Rule: Rule{}, WantErrString: "invalid rule kind"},
{Name: "ruleIDBackslash", Kind: OverrideKind, Rule: Rule{RuleID: "#foo\\:example.com"}, WantErrString: "invalid rule ID"},
{Name: "noActions", Kind: OverrideKind, Rule: Rule{}, WantErrString: "missing actions"},
{Name: "invalidAction", Kind: OverrideKind, Rule: Rule{Actions: []*Action{{}}}, WantErrString: "invalid rule action kind"},
{Name: "invalidCondition", Kind: OverrideKind, Rule: Rule{Conditions: []*Condition{{}}}, WantErrString: "invalid rule condition kind"},
{Name: "overrideNoCondition", Kind: OverrideKind, Rule: Rule{}, WantErrString: "missing rule conditions"},
Expand Down
4 changes: 2 additions & 2 deletions userapi/consumers/roomserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ func (s *OutputRoomEventConsumer) notifyLocal(ctx context.Context, event *rstype
if err != nil {
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{
"event_id": event.EventID(),
"room_id": event.RoomID().String(),
Expand Down
10 changes: 3 additions & 7 deletions userapi/consumers/roomserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,8 @@ func Test_evaluatePushRules(t *testing.T) {
{
name: "m.reaction doesn't notify",
eventContent: `{"type":"m.reaction","room_id":"!room:example.com"}`,
wantAction: pushrules.DontNotifyAction,
wantActions: []*pushrules.Action{
{
Kind: pushrules.DontNotifyAction,
},
},
wantAction: pushrules.UnknownAction,
wantActions: []*pushrules.Action{},
},
{
name: "m.room.message notifies",
Expand Down Expand Up @@ -136,7 +132,7 @@ func Test_evaluatePushRules(t *testing.T) {
t.Fatalf("expected action to be '%s', got '%s'", tc.wantAction, gotAction)
}
// 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")
}
})
Expand Down
Loading