-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, mostly just a bad merge conflict has messed things up a bit I think.
tests/rest/admin/test_admin.py
Outdated
non_admin_user_tok = self.login("nonadmin", "pass") | ||
|
||
# Attempt quarantine media APIs as non-admin | ||
url = "/_synapse/admin/v1/quarantine_media/id/example.org/abcde12345" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
url = "/_synapse/admin/v1/quarantine_media/id/example.org/abcde12345" | |
url = "/_synapse/admin/v1/quarantine_media/example.org/abcde12345" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed the endpoints to:
/_synapse/admin/v1/quarantine_media/by_id/example.org/abcde12345
/_synapse/admin/v1/quarantine_media/by_user/@bob:example.com
/_synapse/admin/v1/quarantine_media/by_room/!someroom:example.com
(with.../quarantine_media/!room...
as a legacy fallback)
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another iteration:
/_synapse/admin/v1/quarantine_media_by_id/example.org/abcde12345
/_synapse/admin/v1/quarantine_media_by_user/@bob:example.com
/_synapse/admin/v1/quarantine_media_by_room/!someroom:example.com
(with.../quarantine_media/!room...
as a legacy fallback)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will want to point these at the media repo worker, so let's update docs/workers.rst
. We might also want to run the tests against the media repo worker rather than the main homeserver. I think there is an example of it somewhere.
Mainly though, looks like we still have a dupe function
Co-Authored-By: Erik Johnston <[email protected]>
…apse into anoa/quarantine_media_ids
Erik and I tried to point these tests at the media worker, however it doesn't have servlets for creating or joining a room (which the test for quarantining media in a room needs), so we need some way of testing against multiple workers. However, we're not going to block this PR on that. |
historical_admin_path_patterns("/room/(?P<room_id>[^/]+)/media/quarantine") | ||
+ | ||
# This path kept around for legacy reasons | ||
historical_admin_path_patterns("/quarantine_media/(?P<room_id>![^/]+)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra !
snuck in here 🤦♂️
Related to: #6681, #5956, #10040 Signed-off-by: Dirk Klimpel [email protected]
* commit '1177d3f3a': Quarantine media by ID or user ID (#6681)
Fixes #5956
This PR adds some admin APIs to:
Most of the code here is test-related.