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

Show erasure status when listing users in the Admin API #14205

Merged
merged 15 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14205.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Show erasure status when listing users in the Admin API.
6 changes: 4 additions & 2 deletions synapse/storage/databases/main/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ async def get_users_paginate(
name: Optional[str] = None,
guests: bool = True,
deactivated: bool = False,
order_by: str = UserSortOrder.USER_ID.value,
order_by: str = UserSortOrder.NAME.value,
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
direction: str = "f",
approved: bool = True,
) -> Tuple[List[JsonDict], int]:
Expand Down Expand Up @@ -261,6 +261,7 @@ def get_users_paginate_txn(
sql_base = f"""
FROM users as u
LEFT JOIN profiles AS p ON u.name = '@' || p.user_id || ':' || ?
LEFT JOIN erased_users AS eu ON u.name = eu.user_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a significant impact on performance with an additional left join on a large list?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have a query plan here: #14205 (comment)

There is a sequential scan mentioned under the sorting section, but that mentions the profiles table only, which we already join to before this change. I have a suggestion here: will raise an issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

{where_clause}
"""
sql = "SELECT COUNT(*) as total_users " + sql_base
Expand All @@ -269,7 +270,8 @@ def get_users_paginate_txn(

sql = f"""
SELECT name, user_type, is_guest, admin, deactivated, shadow_banned,
displayname, avatar_url, creation_ts * 1000 as creation_ts, approved
displayname, avatar_url, creation_ts * 1000 as creation_ts, approved,
eu.user_id is not null as erased
{sql_base}
ORDER BY {order_by_column} {order}, u.name ASC
LIMIT ? OFFSET ?
Expand Down
17 changes: 15 additions & 2 deletions tests/rest/admin/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -1092,8 +1092,10 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:

self.other_user = self.register_user("user", "pass", displayname="User1")
self.other_user_token = self.login("user", "pass")
self.url_other_user = "/_synapse/admin/v2/users/%s" % urllib.parse.quote(
self.other_user
self.url_users = "/_synapse/admin/v2/users"
self.url_other_user = "%s/%s" % (
self.url_users,
urllib.parse.quote(self.other_user),
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
)
self.url = "/_synapse/admin/v1/deactivate/%s" % urllib.parse.quote(
self.other_user
Expand Down Expand Up @@ -1222,6 +1224,17 @@ def test_deactivate_user_erase_true(self) -> None:

self._is_erased("@user:test", True)

channel = self.make_request(
"GET",
self.url_users + "?deactivated=true",
access_token=self.admin_user_tok,
)

self.assertEqual(
1,
len(list(filter(lambda user: user["erased"], channel.json_body["users"]))),
)
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

@override_config({"max_avatar_size": 1234})
def test_deactivate_user_erase_true_avatar_nonnull_but_empty(self) -> None:
"""Check we can erase a user whose avatar is the empty string.
Expand Down