Skip to content

Add tests for downgrading a v12 room to one that doesn't support MSC4289 #794

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

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

Conversation

anoadragon453
Copy link
Member

Adds some tests for downgrading a v12 room. Also cleans up some existing tests and fixes a typo.

⚠️ MSC4289 does not specify what power level to set the room's creator (and additional_creators) to in the new room when downgrading to a room that does not support MSC4289.

The general consensus seems to be setting their power level(s) to the minimum needed to send m.room.power_levels in the room - aka being able to do anything you want in the room. Synapse implements this as taking the max of all fields under events, and state_default (which would apply to m.room.power_levels if it wasn't listed under events).

This PR would effectively be requiring the above of homeservers without the spec having yet agreed on it yet. We should hold off on merging this PR until that is decided.

Recommended to review commit-by-commit.

It was unused, and had a bug in it ('addition_users' should go under 'creation_content').
@anoadragon453
Copy link
Member Author

@matthias:ahouansou.cz mentioned in an internal room that it'd be good to also take all user power levels into account when calculating the creator's power level in the new room. Otherwise, you won't be able to downgrade other users' power levels, whereas you could in the old room as the creator.

They also proposed just setting the creator's power level to Int::MAX. While this would look odd in clients, clients could "mask" this by additionally adding the label "Creator" (or whatever) to users with PL Int::Max in room versions that don't support MSC4289.

In addition, this would be much easier for homeservers to implement (and thus not introduce bugs) than taking the max of a bunch of different fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant