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

Add configurable room list publishing rules #4647

Merged
merged 9 commits into from
Feb 15, 2019

Conversation

erikjohnston
Copy link
Member

This allows specifying who and what is allowed to be published onto the
public room list.

This allows specifying who and what is allowed to be published onto the
public room list
@erikjohnston erikjohnston requested a review from a team February 14, 2019 16:04
@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #4647 into develop will increase coverage by <.01%.
The diff coverage is 81.13%.

@@             Coverage Diff             @@
##           develop    #4647      +/-   ##
===========================================
+ Coverage    75.27%   75.27%   +<.01%     
===========================================
  Files          338      338              
  Lines        34579    34620      +41     
  Branches      5655     5669      +14     
===========================================
+ Hits         26030    26061      +31     
- Misses        6959     6965       +6     
- Partials      1590     1594       +4

synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/handlers/directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Show resolved Hide resolved
@erikjohnston
Copy link
Member Author

Hopefully the comments make more sense now?

@erikjohnston erikjohnston requested a review from a team February 14, 2019 18:16
@erikjohnston
Copy link
Member Author

And err, woops, about how to handle multiple aliases. I think the behaviour makes the most sense.

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Generally looks sane, but I did a bunch of work recently in trying to make the default config a thing that people could understand, and I'm keen that we keep going in the right direction there.

synapse/config/room_directory.py Outdated Show resolved Hide resolved
# `alias_creation_rules`.
#
# If the room has one or more aliases associated with it, only one of
# the aliases needs to match the alias rule. If there are no aliases
Copy link
Member

Choose a reason for hiding this comment

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

what does it even mean to publish a room in the room directory when it doesn't have an alias, ooi?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a perfectly legit thing to do, while remote users wouldn't be able to join the room local users still would happily be able to.

Though I have a suspicion we might require an alias before we actually publish the room

synapse/config/room_directory.py Outdated Show resolved Hide resolved
synapse/config/room_directory.py Show resolved Hide resolved
synapse/config/room_directory.py Outdated Show resolved Hide resolved
@erikjohnston erikjohnston requested a review from a team February 15, 2019 11:00
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

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.

3 participants