diff --git a/routers/admin.py b/routers/admin.py index 461c70d6..aae0bef6 100644 --- a/routers/admin.py +++ b/routers/admin.py @@ -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 @@ -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 @@ -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) ) @@ -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, ) diff --git a/tests/admin_api/test_admin.py b/tests/admin_api/test_admin.py index d674d837..041f0dac 100644 --- a/tests/admin_api/test_admin.py +++ b/tests/admin_api/test_admin.py @@ -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, @@ -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, @@ -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, @@ -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(