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

Depublish a room from the public rooms list when it is upgraded#6232

Merged
anoadragon453 merged 13 commits intodevelopfrom
anoa/room_upgrade_depublishing
Nov 1, 2019
Merged

Depublish a room from the public rooms list when it is upgraded#6232
anoadragon453 merged 13 commits intodevelopfrom
anoa/room_upgrade_depublishing

Conversation

@anoadragon453
Copy link
Copy Markdown
Member

@anoadragon453 anoadragon453 commented Oct 21, 2019

Fixes #4948

Remove a room from the public rooms list on a server when a user from that server joins the upgraded room, and when that user has permission to remove the listing in the old room.

Also fix an incorrect docstring.

TODO:

  • Remove the need for the user to have permission; we are the server so we should have power to do this
  • Consolidate duplicated code

richvdh
richvdh previously approved these changes Oct 23, 2019
Copy link
Copy Markdown
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.

I swear I reviewed this earlier

LGTM, though it now has conflicts :/

Comment thread synapse/handlers/room_member.py Outdated
@anoadragon453 anoadragon453 force-pushed the anoa/room_upgrade_depublishing branch from 656bbe4 to 3d5ca5a Compare October 29, 2019 16:07
Rooms on remote servers were getting the old room depublished from the room directory,
but weren't getting new rooms auto-published to the room directory.
@anoadragon453
Copy link
Copy Markdown
Member Author

anoadragon453 commented Oct 30, 2019

@richvdh This has changed quite a bit since your approval, so I'd like another review if possible.

I've changed the check to being on when a user joins a room the server already knows about and one they haven't, to only checking when a room is upgraded and upon your server seeing it for the first time.

Copy link
Copy Markdown
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.

looks good, modulo a couple of nits

Comment thread synapse/federation/federation_client.py Outdated
Comment thread synapse/handlers/federation.py Outdated
Comment thread synapse/handlers/room_member.py
Comment thread synapse/handlers/room_member.py
@anoadragon453 anoadragon453 merged commit ace947e into develop Nov 1, 2019
@anoadragon453 anoadragon453 deleted the anoa/room_upgrade_depublishing branch November 1, 2019 10:28
anoadragon453 added a commit to matrix-org/sytest that referenced this pull request Nov 1, 2019
babolivier pushed a commit that referenced this pull request Sep 1, 2021
* commit 'ace947e8d':
  Depublish a room from the public rooms list when it is upgraded (#6232)
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.

2 participants