Add delete room admin endpoint #7613

dklimpel · 2020-06-01T15:17:36Z

The Delete Room admin API allows server admins to remove rooms from server
and block these rooms.
DELETE /_synapse/admin/v1/rooms/<room_id>
It is a combination and improvement of "Shutdown room" and "Purge room" API.

Fixes: #6425

It also fix a bug in synapse/storage/data_stores/main/room.py in get_room_with_stats.
It should return None if the room is unknown. But it returns an IndexError.

synapse/synapse/storage/data_stores/main/room.py

Lines 99 to 105 in 901b1fa

    
               def get_room_with_stats(self, room_id: str): 
        
                   """Retrieve room with statistics. 
        
                   Args: 
        
                       room_id: The ID of the room to retrieve. 
        
                   Returns: 
        
                       A dict containing the room information, or None if the room is unknown.

Related to:

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.
Pull request includes a sign off
Code style is correct (run the linters)

Signed-off-by: Dirk Klimpel dirk@klimpel.org

richvdh

rather than cutting-and-pasting the implementation of ShutdownRoom, please could you factor out the code to a handler so that it can be shared?

By the way, please could you join #synapse-dev:matrix.org so that we can collaborate more easily on some of this stuff? It's not very high-volume.

docs/admin_api/rooms.md

dklimpel · 2020-06-05T21:22:54Z

rather than cutting-and-pasting the implementation of ShutdownRoom, please could you factor out the code to a handler so that it can be shared?

By the way, please could you join #synapse-dev:matrix.org so that we can collaborate more easily on some of this stuff? It's not very high-volume.

I have moved it to an own handler.
The room handler has not tests at the moment.

richvdh

a bunch of stylistic things here. Thanks!

docs/admin_api/rooms.md

synapse/storage/data_stores/main/room.py

docs/admin_api/rooms.md

synapse/rest/admin/rooms.py

docs/admin_api/rooms.md

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

dklimpel · 2020-06-17T05:58:34Z

@richvdh: ready for review

richvdh

nearly there!

synapse/handlers/room.py

synapse/rest/admin/rooms.py

synapse/handlers/room.py

ell1e · 2020-07-12T19:57:22Z

Just a feedback from reading this pull request as an outsider:

It is a combination and improvement of "Shutdown room" and "Purge room" API.

The "improvement" that also made it into the documentation description of this new function seems quite confusing to me. To me it looks like essentially this new "delete" just does shutdown followed by purge, is that not all it does? If the "improvement" just means the code is cleaned up as well then I suggest it should be removed from the code comment / documentation description since it seems irrelevant and misleading at best in there, it sounds like it does more than just shutdown & purge but the details are secret for some reason. Otherwise I suggest that it is clarified what exactly this "improvement" means in terms of actual behavior.

docs/admin_api/rooms.md

richvdh · 2020-07-14T11:10:27Z

@ell1e : I don't disagree: that line isn't terribly clear. That said, I don't want to hold this PR up on another round of documentation tweaking.

For the record, the new API is slightly more flexible than using the two existing APIs separately (for example: Shutdown room automatically moves all existing users to a new room. That is optional with the new APIs.) There is, of course, also a good argument that these improvements should be ported back to the shutdown room api...

(Aside: when you comment on a PR, please can you comment on a line in the "files changed" tab. That (a) helps people see which bit of the PR you're talking about, and (b) means that the conversation is threaded rather than being mixed in with all the other traffic on the PR.)

richvdh · 2020-07-14T11:12:37Z

Ah bother. @dklimpel I don't have sign-off from you. Please can you add a signed-off-by comment here as per https://github.com/matrix-org/synapse/blob/develop/CONTRIBUTING.md#sign-off?

dklimpel · 2020-07-14T11:25:03Z

Ah bother. @dklimpel I don't have sign-off from you. Please can you add a signed-off-by comment here as per https://github.com/matrix-org/synapse/blob/develop/CONTRIBUTING.md#sign-off?

@richvdh I have added it #7613 (comment)

Converts tests/rest/admin/test_room.py to have unix file endings after they were accidentally changed in #7613. Keeping the same changelog as #7613 as it hasn't gone out in a release yet.

* commit '491f0dab1': Add delete room admin endpoint (#7613)

dklimpel added 2 commits June 1, 2020 16:56

Add delete room admin endpoint

d349387

Add newsfile

47ebed9

dklimpel marked this pull request as ready for review June 1, 2020 15:39

erikjohnston requested a review from a team June 4, 2020 13:01

richvdh suggested changes Jun 5, 2020

View reviewed changes

docs/admin_api/rooms.md Outdated Show resolved Hide resolved

docs/admin_api/rooms.md Outdated Show resolved Hide resolved

richvdh added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jun 5, 2020

Move functions to own RoomShutdownHandler

3276b1a

Fix server.pyi

16b4fbb

erikjohnston removed the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jun 11, 2020

erikjohnston assigned richvdh Jun 11, 2020

erikjohnston requested a review from richvdh June 11, 2020 09:58

Merge branch 'develop' into admin_api_delete_room

72fa923

richvdh suggested changes Jun 11, 2020

View reviewed changes

Apply suggestions from code review

1015050

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

richvdh self-requested a review June 12, 2020 08:41

dklimpel added 3 commits June 16, 2020 15:30

Apply a lot of changes requested from code review

79010b5

Change defaults of block to false

8c8eb7b

Add headline to documentation

5e3da7e

dklimpel mentioned this pull request Jun 16, 2020

impossible to delete a room #7710

Closed

dklimpel added 3 commits June 16, 2020 22:24

Replace six usage with native Python

f82a0ec

Update newsfile to changed endpoint / URL

4bde87c

Merge branch 'develop' into admin_api_delete_room

4910441

richvdh suggested changes Jun 24, 2020

View reviewed changes

richvdh added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jul 2, 2020

dklimpel added 2 commits July 12, 2020 10:30

Merge branch 'develop' into admin_api_delete_room

14d972a

Apply a lot suggestions from code review

e8f8f0e

dklimpel requested a review from richvdh July 12, 2020 10:24

richvdh removed the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jul 14, 2020

richvdh reviewed Jul 14, 2020

View reviewed changes

docs/admin_api/rooms.md Outdated Show resolved Hide resolved

Update docs/admin_api/rooms.md

b90ada7

richvdh added the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jul 14, 2020

richvdh removed the X-Awaiting-Changes A contributed PR which needs changes and re-review before it can be merged label Jul 14, 2020

richvdh merged commit 491f0da into matrix-org:develop Jul 14, 2020

dklimpel mentioned this pull request Jul 14, 2020

Add button to purge rooms to room list Awesome-Technologies/synapse-admin#44

Merged

dklimpel deleted the admin_api_delete_room branch July 16, 2020 08:19

This was referenced Jul 24, 2020

make CI reject patches with DOS line endings #7943

Closed

new_room_user_id should be optional on /shutdown_room so you can remove rooms without repointing users elsewhere #3761

Closed

anoadragon453 mentioned this pull request Jul 24, 2020

Convert tests/rest/admin/test_room.py to unix file endings #7953

Merged

PaarthShah mentioned this pull request Jan 6, 2021

The Delete Room API should use the http DELETE method, not POST #9032

Closed

babolivier pushed a commit that referenced this pull request Sep 1, 2021

Merge commit '491f0dab1' into anoa/dinsic_release_1_18_x

16a44cf

* commit '491f0dab1': Add delete room admin endpoint (#7613)

hannojg mentioned this pull request Jul 8, 2020

Ability to delete rooms. matrix-org/matrix-spec#431

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add delete room admin endpoint #7613

Add delete room admin endpoint #7613

dklimpel commented Jun 1, 2020 •

edited

Loading

richvdh left a comment

dklimpel commented Jun 5, 2020

richvdh left a comment

dklimpel commented Jun 17, 2020

richvdh left a comment

ell1e commented Jul 12, 2020 •

edited

Loading

richvdh commented Jul 14, 2020

richvdh commented Jul 14, 2020

dklimpel commented Jul 14, 2020

	def get_room_with_stats(self, room_id: str):
	"""Retrieve room with statistics.

	Args:
	room_id: The ID of the room to retrieve.
	Returns:
	A dict containing the room information, or None if the room is unknown.

Add delete room admin endpoint #7613

Add delete room admin endpoint #7613

Conversation

dklimpel commented Jun 1, 2020 • edited Loading

Pull Request Checklist

richvdh left a comment

Choose a reason for hiding this comment

dklimpel commented Jun 5, 2020

richvdh left a comment

Choose a reason for hiding this comment

dklimpel commented Jun 17, 2020

richvdh left a comment

Choose a reason for hiding this comment

ell1e commented Jul 12, 2020 • edited Loading

richvdh commented Jul 14, 2020

richvdh commented Jul 14, 2020

dklimpel commented Jul 14, 2020

dklimpel commented Jun 1, 2020 •

edited

Loading

ell1e commented Jul 12, 2020 •

edited

Loading