Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 3 additions & 20 deletions routers/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
ValidationError,
field_validator,
)
from sqlalchemy import case, exists, func, or_
from sqlalchemy import exists, func, or_
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm import selectinload
from sqlmodel import Session, select
Expand Down Expand Up @@ -446,21 +446,9 @@ def _get_requested_platform_id(self) -> PlatformEnum | None:
def _platform_scope_handled_by_permissions(self) -> bool:
return self._is_platform_scoped_query() and not self._is_group_scoped_query()

def get_user_ordering(self, current_user_id: str | None = None):
"""
Deterministic ordering for user lists (keeps pagination stable):
the current admin's own entry first, then by creation date.
"""
ordering = []
if current_user_id is not None:
ordering.append(case((BiocommonsUser.id == current_user_id, 0), else_=1))
ordering.extend([BiocommonsUser.created_at, BiocommonsUser.id])
return ordering

def _get_combined_query(
self,
admin_roles: list[str],
current_user_id: str | None = None,
):
"""
Combine admin permissions and queries from the params to get the overall
Expand All @@ -472,22 +460,19 @@ def _get_combined_query(
.where(
self.get_admin_permissions_query(admin_roles),
*self.get_query_conditions(admin_roles))
.order_by(*self.get_user_ordering(current_user_id))
.order_by(BiocommonsUser.created_at, BiocommonsUser.id)
)

def get_complete_query(
self,
admin_roles: list[str],
pagination: PaginationParams = None,
current_user_id: str | None = None,
) -> SelectOfScalar[BiocommonsUser]:
"""
Return a full user query, with permissions from admin roles and pagination applied.
The current admin's own user entry (when it matches the query) is sorted to the
top of the results when current_user_id is provided.
"""
return (
self._get_combined_query(admin_roles, current_user_id=current_user_id)
self._get_combined_query(admin_roles)
.offset(pagination.start_index)
.limit(pagination.per_page)
)
Expand Down Expand Up @@ -690,13 +675,11 @@ def get_filtered_user_query(
"""
Get an SQLAlchemy query for users based on the provided filter parameters,
filtered to only return users the admin has permission to view/manage.
The admin's own user entry is included and sorted to the top of the list.
"""
admin_roles = admin_user.access_token.biocommons_roles
return user_query.get_complete_query(
admin_roles,
pagination,
current_user_id=admin_user.access_token.sub,
)


Expand Down
12 changes: 6 additions & 6 deletions tests/admin_api/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def test_get_users(test_client, as_admin_user, galaxy_platform,
assert all(u.id not in user_ids for u in invalid_users)


def test_get_users_includes_current_admin_user_first(
def test_get_users_includes_current_admin_user_sorted_by_created_at(
test_client,
as_admin_user,
galaxy_platform,
Expand All @@ -181,9 +181,10 @@ def test_get_users_includes_current_admin_user_first(
):
"""
The current admin's own user entry should be included in the user list,
sorted to the top.
sorted by created_at like any other user (no longer pinned to the top).
"""
admin_db_user = as_admin_user
admin_db_user.created_at = datetime(2024, 1, 4, tzinfo=timezone.utc)
membership = PlatformMembershipFactory.create_sync(
user_id=admin_db_user.id,
platform_id=galaxy_platform.id,
Expand All @@ -194,7 +195,6 @@ def test_get_users_includes_current_admin_user_first(
test_db_session.commit()

# Create users out of date order to check the list is sorted by created_at
# after the admin user
other_users = [
_create_user_with_platform_membership(
db_session=test_db_session,
Expand All @@ -209,9 +209,9 @@ def test_get_users_includes_current_admin_user_first(
assert resp.status_code == 200
data = resp.json()
assert len(data) == 4
assert data[0]["id"] == admin_db_user.id
expected_order_after_admin = [other_users[1].id, other_users[2].id, other_users[0].id]
assert [u["id"] for u in data[1:]] == expected_order_after_admin
# Admin (day 4) sorts last; the rest follow created_at order
expected_order = [other_users[1].id, other_users[2].id, other_users[0].id, admin_db_user.id]
assert [u["id"] for u in data] == expected_order


def test_get_users_admin_without_matching_memberships_not_included(
Expand Down