Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC3890: Remotely silence local notifications #3890
base: main
Are you sure you want to change the base?
MSC3890: Remotely silence local notifications #3890
Changes from 6 commits
952f367
1684fba
7d92ffb
6069565
dc590b6
f456278
03bfdb4
435aa52
4f03708
4bc380d
f09deab
10be7fb
9bbcf3d
28d723d
2e25a3a
3eb4062
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
generally we prefer to phrase things in the positive:
noisy_notifications: true
(for example)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.
A consideration might be to have per-device account data similar to how we already have per-room account data: though awkward at first, the intention behind it would be to allow other devices to change the device's account data values, specifically for when cases like this matter.
Another example might be to have per-device [client] settings that might be possible to change from another client, like breadcrumbs or theme. This would be part of the overal "granular settings" idea created years ago by element-web (then vector-web).
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.
I'm not familiar with the "granular settings" idea. I'm not sure how many client level settings it would be desirable/useful to have on the server and editable from other devices. Quick setup of new devices that mirror existing ones could be one use case.
If read/write is allowed from any device in the account (which it would have to be to fit this use case) then the added utility of per device account data over profile account data seems quite small.
I've added a small note in the alternatives section which I can explore more fully if needed.
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.
It is feasible, I think speccing device-scoped push rules even maybe just allowing
override
rules to manage this would be better as otherwise you have a "local" silencing, but still causing the device to get awoken via its pusher.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.
^the proposal, like MSC3881, should mention why per-device push rules aren't suitable
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.
This doesn't apply here as the proposal specifically covers clients without pushers that generate notifications locally from
/sync
responses.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.
Pushers are not the same as per-device push rules, confusingly. Per-device push rules are a theory that push rules (which are what clients use to calculate counts locally) could be scoped to a particular device, allowing for locally-silenced notification [counts].
Pushers also use push rules to determine counts/highlights, which also means per-device push rules could also affect how devices with pushers could work.
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.
@turt2live sorry, I was replying to the comment above yours, specifically the snippet I quoted. In the context of this MSC, we're focusing on clients that use push rules but not pushers. As a result, without a pusher there is also no wake-up involved. The fact that the silencing is "local" is actually by design because the generation of notifications from
/sync
responses is also local.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.
right, but the underlying argument is still that this MSC might not be neccessary if per-device push rules were to actually exist. Account data feels like the wrong mechanism for this at the moment, regardless of having a pusher or not.
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.
Right, I'm not disagreeing with that. My point was just that needlessly waking up clients is not the proper reason to favor per-device push rules because that happens neither here nor in MSC3881.
Anyway, I think we're aligned. What's missing in both MSCs is a more elaborate description of why we think these proposals are favorable to per-device push rules. We'll be working an that. 👍