Add joinability for groups #3045
Conversation
Adds API to set the 'joinable' flag, and corresponding flag in the table.
| requester = yield self.auth.get_user_by_req(request) | ||
| requester_user_id = requester.user.to_string() | ||
|
|
||
| content = parse_json_object_from_request(request) |
There was a problem hiding this comment.
shouldn't we evaluate that we have only have {"joinable": <bool>} as body?
|
I assume this is a flaky sytest? |
There was a problem hiding this comment.
Code wise, looks good.
API wise, it'd be good if this followed the same style of API as the visibility stuff, e.g.: https://github.com/matrix-org/synapse/blob/master/synapse/groups/groups_server.py#L838-L864. In particular we probably want to have a m.* prefix, e.g.:
{
"m.join_policy": {
"type": "invite"
}
}This would support us having values like invite, knock and open (I'd suggest not reusing "public"). If we do have multiple values the DB would need to support that.
| PATH = "/groups/(?P<group_id>[^/]*)/joinable$" | ||
|
|
||
| @defer.inlineCallbacks | ||
| def on_POST(self, origin, content, query, group_id): |
There was a problem hiding this comment.
Should this be PUT or POST? The client API is a PUT.
There was a problem hiding this comment.
Erik said it should have the same method as the CS API. @erikjohnston, can you confirm?
lukebarnard1
force-pushed
the
dbkr/group_joinable
branch
2 times, most recently
from
2a74640
to
52c2268
Compare
The API is now under
/groups/$group_id/setting/m.join_policy
and expects a JSON blob of the shape
```json
{
"m.join_policy": {
"type": "invite"
}
}
```
where "invite" could alternatively be "open".
lukebarnard1
force-pushed
the
dbkr/group_joinable
branch
from
52c2268
to
eb8d8d6
Compare
|
matrix-org/sytest#436 will need updating once these changes have landed. |
synapse/storage/group_server.py
Outdated
| @@ -29,6 +30,18 @@ | |||
|
|
|||
|
|
|||
| class GroupServerStore(SQLBaseStore): | |||
| def set_group_join_policy(self, group_id, join_policy): | |||
| return self._simple_update_one( | |||
There was a problem hiding this comment.
set_group_join_policy needs a docstring, especially as we only want to accept certain values so we should document what they are.
Might also be worth adding a assert join_policy in ('invite', 'open') (we should probably move some of those to constants somewhere at some point)
| * NULL at the python store level as necessary so that existing | ||
| * rows are given the correct default policy. | ||
| */ | ||
| ALTER TABLE groups ADD COLUMN join_policy TEXT DEFAULT NULL; |
There was a problem hiding this comment.
If we can have defaults then we may as well do
ALTER TABLE groups ADD COLUMN join_policy TEXT NON NULL DEFAULT 'invite';| content): | ||
| """Sets the join policy for a group | ||
| """ | ||
| path = PREFIX + "/groups/%s/setting/m.join_policy" % (group_id,) |
There was a problem hiding this comment.
(I was expecting it to be settings plural, but that's a matter of taste)
|
(I didn't mean to assign @richvdh there....) |
Adds API to set the 'joinable' flag, and corresponding flag in the
table.