Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Server notices: add an autojoin setting for the notices room #16699

Merged
merged 5 commits into from
Dec 4, 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
1 change: 1 addition & 0 deletions changelog.d/16699.feature
Copy link
Member

Choose a reason for hiding this comment

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

FWIW this is against the spec:

Servers should invite the target user rather than automatically join them to the server notice room.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, TIL server notices is specced :)

It's a SHOULD and not a MUST so I think it should be fine to have a config flag defaulting to false for that?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's probably fine?

Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I don't understand the spec's rationale and I think that auto-joining makes for a better UX.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an autojoin setting for the notices room so users get joined directly instead of receiving an invite.
3 changes: 3 additions & 0 deletions docs/server_notices.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ server_notices:
system_mxid_display_name: "Server Notices"
system_mxid_avatar_url: "mxc://server.com/oumMVlgDnLYFaPVkExemNVVZ"
room_name: "Server Notices"
auto_join: true
```

The only compulsory setting is `system_mxid_localpart`, which defines the user
Expand All @@ -55,6 +56,8 @@ room which will be created.
`system_mxid_display_name` and `system_mxid_avatar_url` can be used to set the
displayname and avatar of the Server Notices user.

`auto_join` will autojoin users to the notices room instead of sending an invite.

## Sending notices

To send server notices to users you can use the
Expand Down
4 changes: 3 additions & 1 deletion docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -3815,14 +3815,16 @@ Sub-options for this setting include:
* `system_mxid_display_name`: set the display name of the "notices" user
* `system_mxid_avatar_url`: set the avatar for the "notices" user
* `room_name`: set the room name of the server notices room

* `auto_join`: boolean. If true, the user will be automatically joined to the room instead of being invited.
Defaults to false. _Added in Synapse 1.98.0._
Example configuration:
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
```yaml
server_notices:
system_mxid_localpart: notices
system_mxid_display_name: "Server Notices"
system_mxid_avatar_url: "mxc://server.com/oumMVlgDnLYFaPVkExemNVVZ"
room_name: "Server Notices"
auto_join: true
```
---
### `enable_room_list_search`
Expand Down
2 changes: 2 additions & 0 deletions synapse/config/server_notices.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def __init__(self, *args: Any):
self.server_notices_mxid_display_name: Optional[str] = None
self.server_notices_mxid_avatar_url: Optional[str] = None
self.server_notices_room_name: Optional[str] = None
self.server_notices_auto_join: bool = False

def read_config(self, config: JsonDict, **kwargs: Any) -> None:
c = config.get("server_notices")
Expand All @@ -62,3 +63,4 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
self.server_notices_mxid_avatar_url = c.get("system_mxid_avatar_url", None)
# todo: i18n
self.server_notices_room_name = c.get("room_name", "Server Notices")
self.server_notices_auto_join = c.get("auto_join", False)
15 changes: 14 additions & 1 deletion synapse/server_notices/server_notices_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,27 @@ async def maybe_invite_user_to_room(self, user_id: str, room_id: str) -> None:
if room.room_id == room_id:
return

user_id_obj = UserID.from_string(user_id)
await self._room_member_handler.update_membership(
requester=requester,
target=UserID.from_string(user_id),
target=user_id_obj,
room_id=room_id,
action="invite",
ratelimit=False,
)

if self._config.servernotices.server_notices_auto_join:
user_requester = create_requester(
user_id, authenticated_entity=self._server_name
)
await self._room_member_handler.update_membership(
requester=user_requester,
target=user_id_obj,
room_id=room_id,
action="join",
ratelimit=False,
)

Comment on lines +236 to +247
Copy link
Contributor

Choose a reason for hiding this comment

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

I was a bit alarmed by this, because it looks like maybe_invite_user_to_room is a generic function. But I see from its docstring that it is specifically intended for server notices.

I also wondered if having the initial membership be invite might be confusing. But the room's join rules presumably require invites, so I think the invite->join transition is inevitable.

On reflection: LGTM.

async def _update_notice_user_profile_if_changed(
self,
requester: Requester,
Expand Down
27 changes: 27 additions & 0 deletions tests/rest/admin/test_server_notice.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,33 @@ def test_send_server_notice_delete_room(self) -> None:
# second room has new ID
self.assertNotEqual(first_room_id, second_room_id)

@override_config(
{"server_notices": {"system_mxid_localpart": "notices", "auto_join": True}}
)
def test_auto_join(self) -> None:
"""
Tests that the user get automatically joined to the notice room
when `auto_join` setting is used.
"""
# user has no room memberships
self._check_invite_and_join_status(self.other_user, 0, 0)

# send server notice
server_notice_request_content = {
"user_id": self.other_user,
"content": {"msgtype": "m.text", "body": "test msg one"},
}

self.make_request(
"POST",
self.url,
access_token=self.admin_user_tok,
content=server_notice_request_content,
)

# user has joined the room
self._check_invite_and_join_status(self.other_user, 0, 1)

@override_config({"server_notices": {"system_mxid_localpart": "notices"}})
def test_update_notice_user_name_when_changed(self) -> None:
"""
Expand Down
Loading