-
Notifications
You must be signed in to change notification settings - Fork 20
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
TW-504: config notifications for default push rules #568
base: main
Are you sure you want to change the base?
Changes from all commits
26219e3
7ad282d
176cd8f
efa5402
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# 5. Push rules decision | ||
Date: 2023-09-11 | ||
|
||
## Status | ||
|
||
Accepted | ||
|
||
## Context | ||
|
||
In the Matrix protocol, push rules define how messages are filtered and displayed on devices. These rules are customizable through APIs, allowing us to enable, disable, and specify actions for each rule. Furthermore, there are various types of push rules (override, content, sender, underride, etc.), each serving different purposes and enabling the configuration of push notifications. For reference, see the [Matrix Push Rules Specification](https://spec.matrix.org/v1.8/client-server-api/#push-rules). However, push rules have exhibited several limitations. For instance, notifications for group chats may still be received by users even after they have disabled them ([example](https://photos.app.goo.gl/JoaysSunovsdPtAs8)). | ||
|
||
## Decision | ||
|
||
To address these limitations, we propose the introduction of optional custom fields within the content of events. These custom fields can be utilized in the future to create custom push rules. For example, the `setName()` API for rooms lacks the capability for clients to determine whether the room already exists. To mitigate this, we suggest adding a custom field, such as `'roomState': 'created'`, to the content of `setName()` APIs. Subsequently, this `roomState` custom field can be utilized as a condition within push rules. To maintain consistency and facilitate tracking, we recommend defining metadata custom fields exclusively and documenting them in a constants file for easy reference by other developers. | ||
|
||
## Consequences | ||
|
||
- **Advantages:** | ||
- Simplifies the definition of push rules, enhancing clarity and organization. | ||
- Provides flexibility to define customized push rules. | ||
- Promotes extendability and maintainability. | ||
- Compatible with various home servers. | ||
|
||
- **Disadvantages:** | ||
- Push notifications may behave inconsistently across different client applications. In essence, push rules are optimized for Twake applications and may not offer consistent behavior in other environments. For instance, in the Element application, notifications may be displayed for actions such as message sending or group name changes, regardless of user preferences regarding name change notifications. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,34 @@ | ||||||
import 'package:matrix/matrix.dart'; | ||||||
|
||||||
extension ClientPushRulesExtension on Client { | ||||||
Future<void> setupUserDefinedPushRule({ | ||||||
String scope = 'global', | ||||||
required String ruleId, | ||||||
List<PushCondition>? conditions, | ||||||
PushRuleKind kind = PushRuleKind.override, | ||||||
String? after, | ||||||
String? before, | ||||||
}) async { | ||||||
if (!containsRule(ruleId)) { | ||||||
await setPushRule( | ||||||
scope, | ||||||
kind, | ||||||
ruleId, | ||||||
[ | ||||||
PushRuleAction.notify, | ||||||
{"set_tweak": "sound", "value": "default"} | ||||||
], | ||||||
conditions: conditions, | ||||||
after: after, | ||||||
before: before, | ||||||
); | ||||||
} | ||||||
} | ||||||
|
||||||
bool containsRule(String ruleId) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where you can put this check? Before or after sync? |
||||||
final rule = devicePushRules?.override?.firstWhere((PushRule rule) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
return rule.ruleId == ruleId; | ||||||
}); | ||||||
return rule?.ruleId == ruleId; | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -15,6 +15,7 @@ import 'package:fluffychat/domain/model/tom_server_information.dart'; | |||||
import 'package:fluffychat/domain/repository/tom_configurations_repository.dart'; | ||||||
import 'package:fluffychat/utils/client_manager.dart'; | ||||||
import 'package:fluffychat/utils/localized_exception_extension.dart'; | ||||||
import 'package:fluffychat/utils/matrix_sdk_extensions/client_push_rules_extension.dart'; | ||||||
import 'package:fluffychat/utils/platform_infos.dart'; | ||||||
import 'package:fluffychat/utils/uia_request_manager.dart'; | ||||||
import 'package:fluffychat/utils/url_launcher.dart'; | ||||||
|
@@ -448,6 +449,94 @@ class MatrixState extends State<Matrix> with WidgetsBindingObserver { | |||||
createVoipPlugin(); | ||||||
} | ||||||
|
||||||
Future<void> _setUpUserDefinedPushRules(Client client) async { | ||||||
await Future.wait([ | ||||||
client.setPushRuleEnabled( | ||||||
'global', | ||||||
PushRuleKind.override, | ||||||
'.m.rule.invite_for_me', | ||||||
false, | ||||||
), | ||||||
client.setPushRuleEnabled( | ||||||
'global', | ||||||
PushRuleKind.override, | ||||||
'.m.rule.member_event', | ||||||
false, | ||||||
), | ||||||
client.setupUserDefinedPushRule( | ||||||
ruleId: 'm.rule.invite_for_me', | ||||||
conditions: [ | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'type', | ||||||
pattern: EventTypes.RoomMember, | ||||||
), | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'content.membership', | ||||||
pattern: 'invite', | ||||||
), | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'state_key', | ||||||
pattern: '${client.userID}', | ||||||
), | ||||||
], | ||||||
), | ||||||
client.setupUserDefinedPushRule( | ||||||
ruleId: 'm.rule.set_me_as_admin', | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
conditions: [ | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'type', | ||||||
pattern: EventTypes.RoomPowerLevels, | ||||||
), | ||||||
], | ||||||
), | ||||||
client.setupUserDefinedPushRule( | ||||||
kind: PushRuleKind.underride, | ||||||
ruleId: 'm.rule.encrypted_group_chat', | ||||||
conditions: [ | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'type', | ||||||
pattern: EventTypes.Encrypted, | ||||||
), | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'content.is_direct', | ||||||
pattern: 'false', | ||||||
), | ||||||
], | ||||||
), | ||||||
]); | ||||||
|
||||||
await Future.wait([ | ||||||
client.setupUserDefinedPushRule( | ||||||
ruleId: 'm.rule.change_group_name', | ||||||
conditions: [ | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'type', | ||||||
pattern: EventTypes.RoomName, | ||||||
), | ||||||
], | ||||||
after: 'm.rule.invite_for_me', | ||||||
), | ||||||
client.setupUserDefinedPushRule( | ||||||
ruleId: 'm.rule.change_avatar_group', | ||||||
conditions: [ | ||||||
PushCondition( | ||||||
kind: 'event_match', | ||||||
key: 'type', | ||||||
pattern: EventTypes.RoomAvatar, | ||||||
), | ||||||
], | ||||||
after: 'm.rule.invite_for_me', | ||||||
), | ||||||
]); | ||||||
} | ||||||
|
||||||
void createVoipPlugin() async { | ||||||
if (await store.getItemBool(SettingKeys.experimentalVoip) == false) { | ||||||
voipPlugin = null; | ||||||
|
@@ -500,7 +589,9 @@ class MatrixState extends State<Matrix> with WidgetsBindingObserver { | |||||
} | ||||||
if (homeServer != null) { | ||||||
_setUpHomeServer(homeServer.baseUrl); | ||||||
_setUpUserDefinedPushRules(client); | ||||||
} | ||||||
|
||||||
_storeToMConfiguration(client, tomServer, identityServer); | ||||||
setUpAuthorization(client); | ||||||
} | ||||||
|
@@ -522,7 +613,7 @@ class MatrixState extends State<Matrix> with WidgetsBindingObserver { | |||||
} | ||||||
} | ||||||
|
||||||
void _setUpHomeServer(Uri homeServerUri) { | ||||||
void _setUpHomeServer(Uri homeServerUri) async { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need |
||||||
final homeServerUrlInterceptor = getIt.get<DynamicUrlInterceptors>( | ||||||
instanceName: NetworkDI.homeServerUrlInterceptorName, | ||||||
); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please show demo for new settings