Skip to content

MSC3881: Remotely toggle push notifications for another client #3881

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.

Sign up for GitHub

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

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

MSC3881: Remotely toggle push notifications for another client #3881

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
18818cd
first cut
Sep 5, 2022
1844be0
add tables
Sep 5, 2022
08c45ed
exlcude non-pusher notifs
Sep 5, 2022
62eb149
tweak headings
Sep 5, 2022
75ff4df
add msc number
Sep 5, 2022
eef3fc5
Update proposals/3881-remote-push-notification-toggling.md
Sep 9, 2022
ae4ec9f
invert is_disabled field to enabled
Sep 9, 2022
8492d15
format
Sep 9, 2022
e653dcf
make device_id nullable
Sep 13, 2022
2a843ef
remove device_id illegal in post para
Sep 22, 2022
8a78282
use optional throughout
Sep 22, 2022
f93afcb
add support flag to /versions
Sep 22, 2022
b82104c
remove copy paste mess
Sep 22, 2022
469d540
Update proposals/3881-remote-push-notification-toggling.md
Nov 21, 2022
8fc3851
Update proposals/3881-remote-push-notification-toggling.md
Nov 21, 2022
ec835f1
move migration section under unstable prefix heading
Nov 21, 2022
5da3d98
further description about how users might use remote push toggle
Nov 21, 2022
8a6bc04
Merge branch 'kerry/remote-push-toggle' of github.com:matrix-org/matr…
Nov 21, 2022
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
Prev Previous commit
Next Next commit
format
  • Loading branch information
Kerry Archibald committed Sep 9, 2022
commit 8492d1506708d0e4c052a948fd1dc406975afc00
16 changes: 9 additions & 7 deletions proposals/3881-remote-push-notification-toggling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ the value is optional and if omitted, defaults to `true`.
In [GET /_matrix/client/v3/pushers](https://spec.matrix.org/v1.3/client-server-api/#get_matrixclientv3pushers) the value
is always returned.

Pushers that are not enabled do not send
[`/notify`](https://spec.matrix.org/v1.3/push-gateway-api/#post_matrixpushv1notify) requests to push providers.
Pushers that are not enabled do not produce push notifications of any kind, either by sending
[`/notify`](https://spec.matrix.org/v1.3/push-gateway-api/#post_matrixpushv1notify) requests to push providers for
`http` pushers or otherwise.

#### Explicitly linking device and pusher
A new field `device_id` is added to the `Pusher` model as returned from [GET
Expand All @@ -44,8 +45,8 @@ A new field `device_id` is added to the `Pusher` model as returned from [GET

To be able to remove Pushers when sessions are deleted home servers must have some existing way to link a session to
pusher, so exposing the `device_id` on http pushers should be trivial. (Synapse, for instance, stores the [access
token](https://github.com/matrix-org/synapse/blob/3d201151152ca8ba9b9aae8da5b76a26044cc85f/synapse/storage/databases/main/pusher.py#L487) when adding a
pusher)
token](https://github.com/matrix-org/synapse/blob/3d201151152ca8ba9b9aae8da5b76a26044cc85f/synapse/storage/databases/main/pusher.py#L487)
when adding a pusher)

In [GET /_matrix/client/v3/pushers](https://spec.matrix.org/v1.3/client-server-api/#get_matrixclientv3pushers) the value
is required when `kind` is `http`. If `kind` is _not_ `http`, the `device_id` field is null.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for this restriction? I don't think it would be too much of an issue to generalise this to email pushers, even if we're not using it in clients at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe there was a semantic reason. For http pushers, the client will treat device_id as the device receiving the notifications whereas for email pushers that doesn't work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, though I guess technically in both cases device_id is the device that added the pusher and for http pushers clients can make the implicit assumption that that device is also the one receiving the notifications. So it's probably fine to allow device_id regardless of the pusher type.

Expand All @@ -56,12 +57,13 @@ In [POST /_matrix/client/v3/pushers/set](https://spec.matrix.org/v1.3/client-ser

### Pusher-less clients

Pausing notifications for clients that create notifications outside of the Push Gateway will not be addressed in this MSC.
Pausing notifications for clients that create notifications outside of the Push Gateway will not be addressed in this
MSC.

## Migration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should not be under the "Proposal" heading, but rather the "Unstable prefix" heading: compatibility is determined by spec version support, not by unstable flags


Clients that connect to a home server that doesn't yet support this proposal should interpret a missing `enabled`
value as `true`.
Clients that connect to a home server that doesn't yet support this proposal should interpret a missing `enabled` value
as `true`.

Home servers should migrate pushers that were registered before this proposal so that `enabled` is `true`

Expand Down