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

Add default_power_level_content_override config option. #12535

Closed
wants to merge 3 commits into from

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Apr 23, 2022

Lets you override the default power levels for rooms created on this server, per
room creation preset.

Useful if you know that your users need special permissions in rooms
that they create (e.g. to send particular types of state events without
needing an elevated power level). This takes the same shape as the
power_level_content_override parameter in the /createRoom API, but
is applied before that parameter.

This is something of a workaround in the absence of MSC3779 or MSC3761.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

Lets you override the default power levels for rooms created on this server, per
room creation preset.

Useful if you know that your users need special permissions in rooms
that they create (e.g. to send particular types of state events without
needing an elevated power level).  This takes the same shape as the
power_level_content_override parameter in the /createRoom API, but
is applied before that parameter.

This is something of a workaround in the absence of MSC3779 or MSC3761.
@ara4n ara4n requested a review from a team as a code owner April 23, 2022 14:32
@ara4n
Copy link
Member Author

ara4n commented Apr 23, 2022

An example use of this would be something like:

default_power_level_content_override:
  private_chat: { "events": { "com.example.foo" : 0 } }
  trusted_private_chat: { "events": { "com.example.foo" : 0 } }
  public_chat: { "events": { "com.example.foo" : 0 } }

...to let users in locally created rooms set com.example.foo state events without needing PL>0.

Comment on lines 1046 to +1060
if power_level_content_override:
power_level_content.update(power_level_content_override)

# override default_power_level_content_override for this room preset, if any
default_power_level_content_override = (
self.config.room.default_power_level_content_override
)
if (
default_power_level_content_override
and default_power_level_content_override.get(preset_config)
):
power_level_content.update(
default_power_level_content_override.get(preset_config)
)

Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation for default_power_level_content_override says that it's applied before power_level_content_override, but here it is the other way around?

# This is something of a workaround in the absence of MSC3779 or MSC3761.
#
#default_power_level_content_override:
# private_chat: null
Copy link
Contributor

Choose a reason for hiding this comment

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

Some example non-null content might help admins understand what goes here:

Suggested change
# private_chat: null
# private_chat: { "events": { "com.example.foo" : 0 } }

Comment on lines +1057 to +1059
power_level_content.update(
default_power_level_content_override.get(preset_config)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

If an admin were to configure an override like: private_chat: { "events": { "com.example.foo" : 0 } }, the entire default "events": {"m.room.name": ..., ...} content from above will be replaced. Is this intended behaviour?

kind of related to matrix-org/matrix-spec#492

Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

I'd like to see a test for this change before we can merge the PR.

@DMRobertson
Copy link
Contributor

I'd like to see a test for this change before we can merge the PR.

And an entry in the config manual too, please.

@andybalaam
Copy link
Contributor

I am attempting to address the comments here in #12618

@babolivier
Copy link
Contributor

I am attempting to address the comments here in #12618

Closing in favour of #12618 then.

@babolivier babolivier closed this May 4, 2022
@ara4n
Copy link
Member Author

ara4n commented May 4, 2022

sorry for completely missing this while travelling/catching up - @andybalaam thanks for taking it over.

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

Successfully merging this pull request may close these issues.

5 participants