Removal: Remove experimental support for MSC3852 #17640
Conversation
There was a problem hiding this comment.
Sorry, this is complicated by the fact that matrix-org/synapse#13549 both implemented MSC3852, as well as added a new feature to the Admin API. We should try and keep the latter.
| @@ -0,0 +1 @@ | |||
| Remove experimental support for MSC 3852. | |||
There was a problem hiding this comment.
Typically MSC's are written without a space in-between "MSC" and the number:
| Remove experimental support for MSC 3852. | |
| Remove experimental support for [MSC3852](https://github.com/matrix-org/matrix-spec-proposals/pull/3852). |
| @@ -857,15 +857,13 @@ A response body like the following is returned: | |||
| "device_id": "QBUAZIFURK", | |||
| "display_name": "android", | |||
| "last_seen_ip": "1.2.3.4", | |||
| "last_seen_user_agent": "Mozilla/5.0 (X11; Linux x86_64; rv:103.0) Gecko/20100101 Firefox/103.0", | |||
There was a problem hiding this comment.
Hm, removing this field would be a backwards-incompatible change to the Admin API.
While this field was added to the Admin API in matrix-org/synapse#13549, it was not connected to the experimental_features.msc3852_enabled config option. But rather, always returned and not considered experimental.
It'd be best to keep this field in the Admin API. I could also see it as having been useful for moderation purposes.
| @@ -1131,7 +1130,6 @@ def _update_device_from_client_ips( | |||
| ip = client_ips.get((device["user_id"], device["device_id"])) | |||
| device.update( | |||
| { | |||
| "last_seen_user_agent": ip.user_agent if ip else None, | |||
There was a problem hiding this comment.
I would leave this in, but keep the following code in the client endpoints:
for device in devices:
last_seen_user_agent = device["last_seen_user_agent"]
del device["last_seen_user_agent"]such that the field isn't exposed to clients, but remains exposed to the Admin API.
| @@ -278,7 +278,6 @@ def test_devices(self) -> None: | |||
| self.assertEqual(args[0][0]["user_id"], self.user2) | |||
| self.assertIn("device_id", args[0][0]) | |||
| self.assertIsNone(args[0][0]["display_name"]) | |||
| self.assertIsNone(args[0][0]["last_seen_user_agent"]) | |||
fixes #14836 . This PR more or less reverts everything introduced in the PR matrix-org/synapse#13549
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.(run the linters)