From 4834a567a883082dca6f3b3e4b4b976154978619 Mon Sep 17 00:00:00 2001 From: chengkai3 Date: Fri, 19 Jun 2026 23:25:52 +0800 Subject: [PATCH] =?UTF-8?q?[feat]:[FL-120][=E8=A7=92=E8=89=B2=E7=AE=A1?= =?UTF-8?q?=E7=90=86=E9=A1=B5=E9=9D=A2=E5=AF=B9=E9=BD=90=E7=94=A8=E6=88=B7?= =?UTF-8?q?=E7=AE=A1=E7=90=86=E5=88=86=E9=A1=B5=E4=BA=A4=E4=BA=92]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: multica-agent --- api/app/api/v1/admin.py | 8 +- api/app/services/legacy_admin_rbac_service.py | 153 ++++++++++++------ api/tests/test_role_pagination_contract.py | 53 ++++++ web/src/app/admin/roles/page.tsx | 94 +++++++---- web/src/app/admin/users/page.tsx | 27 ++-- 5 files changed, 232 insertions(+), 103 deletions(-) create mode 100644 api/tests/test_role_pagination_contract.py diff --git a/api/app/api/v1/admin.py b/api/app/api/v1/admin.py index 5a06a62..162c402 100644 --- a/api/app/api/v1/admin.py +++ b/api/app/api/v1/admin.py @@ -47,19 +47,23 @@ router = APIRouter(prefix="/admin", tags=["admin"]) @router.get("/roles", response_model=RoleListResponse) def get_roles( keyword: str | None = Query(default=None), + limit: int = Query(default=50, ge=1, le=200), + offset: int = Query(default=0, ge=0), _: CurrentUser = Depends(require_any_permission("role.read", "role.manage")), db: Session = Depends(get_db), ) -> RoleListResponse: - return list_roles(db, keyword=keyword) + return list_roles(db, keyword=keyword, limit=limit, offset=offset) @router.get("/roles-with-menus", response_model=RolesWithMenusResponse) def get_roles_with_menus( keyword: str | None = Query(default=None), + limit: int = Query(default=50, ge=1, le=200), + offset: int = Query(default=0, ge=0), _: CurrentUser = Depends(require_any_permission("role.read", "role.manage")), db: Session = Depends(get_db), ) -> RolesWithMenusResponse: - return list_roles_with_menus(db, keyword=keyword) + return list_roles_with_menus(db, keyword=keyword, limit=limit, offset=offset) @router.post("/roles", response_model=RolePublic) diff --git a/api/app/services/legacy_admin_rbac_service.py b/api/app/services/legacy_admin_rbac_service.py index 1704a08..d140f69 100644 --- a/api/app/services/legacy_admin_rbac_service.py +++ b/api/app/services/legacy_admin_rbac_service.py @@ -111,9 +111,21 @@ PROTECTED_MENU_CODES = { } -def list_roles(db: Session, keyword: str | None = None) -> RoleListResponse: +def list_roles( + db: Session, + keyword: str | None = None, + *, + limit: int | None = None, + offset: int = 0, +) -> RoleListResponse: role_source = "legacy" if _legacy_role_table_exists(db) else "modern" - rows = _load_role_rows(db, role_source=role_source, keyword=keyword) + rows, total = _load_role_page( + db, + role_source=role_source, + keyword=keyword, + limit=limit, + offset=offset, + ) role_ids = [str(row["id"]) for row in rows] role_menu_ids = _load_role_menu_ids_map(db, role_ids, role_source=role_source) menu_rows = _load_menus_map( @@ -131,20 +143,6 @@ def list_roles(db: Session, keyword: str | None = None) -> RoleListResponse: permission_codes = set(role_permission_codes.get(role_id, [])) permission_codes.update(_permission_codes_from_menu_rows(menu_rows, menu_ids)) - # Apply keyword filter on menu names if keyword provided - if keyword: - normalized_keyword = keyword.strip().lower() - # Collect menu names for this role - menu_names = " ".join( - str(menu_rows.get(menu_id, {}).get("menu_name", "")) - for menu_id in menu_ids - ) - # Build search haystack - haystack = f"{role_code} {row.get('name', '')} {menu_names}".lower() - # Skip if keyword not found - if normalized_keyword not in haystack: - continue - items.append( RolePublic( id=role_id, @@ -154,7 +152,7 @@ def list_roles(db: Session, keyword: str | None = None) -> RoleListResponse: menu_ids=menu_ids, ) ) - return RoleListResponse(items=items, total=len(items)) + return RoleListResponse(items=items, total=total) def get_role_by_id(db: Session, role_id: str) -> RolePublic | None: @@ -495,9 +493,15 @@ def list_menus(db: Session, keyword: str | None = None, status: str | None = Non return MenuListResponse(items=items, total=len(items)) -def list_roles_with_menus(db: Session, keyword: str | None = None) -> RolesWithMenusResponse: +def list_roles_with_menus( + db: Session, + keyword: str | None = None, + *, + limit: int | None = None, + offset: int = 0, +) -> RolesWithMenusResponse: """Get roles and menus in a single request to reduce network calls.""" - roles_response = list_roles(db, keyword=keyword) + roles_response = list_roles(db, keyword=keyword, limit=limit, offset=offset) menus_response = list_menus(db) return RolesWithMenusResponse( roles=roles_response.items, @@ -992,42 +996,87 @@ def _load_menus_map(db: Session, menu_ids: list[str], *, role_source: str = "leg } -def _load_role_rows(db: Session, *, role_source: str, keyword: str | None = None) -> list[dict[str, object]]: - where_clause = "" - params = {} - - if keyword: - normalized_keyword = f"%{keyword.strip().lower()}%" - where_clause = "WHERE LOWER(id) LIKE :keyword OR LOWER(name) LIKE :keyword" - params["keyword"] = normalized_keyword +def _load_role_page( + db: Session, + *, + role_source: str, + keyword: str | None = None, + limit: int | None = None, + offset: int = 0, +) -> tuple[list[dict[str, object]], int]: + normalized_offset = max(offset, 0) + normalized_limit = max(limit, 0) if limit is not None else None + params: dict[str, object] = {"offset": normalized_offset} + limit_clause = "" + if normalized_limit is not None: + limit_clause = "LIMIT :limit" + params["limit"] = normalized_limit if role_source == "legacy": - rows = db.execute( - text( - f""" - SELECT id, name - FROM user_role - {where_clause} - ORDER BY create_date DESC NULLS LAST, id ASC - """ - ), - params - ).mappings().all() + from_clause = """ + FROM user_role r + WHERE ( + :keyword IS NULL + OR LOWER(r.id) LIKE :keyword + OR LOWER(r.name) LIKE :keyword + OR EXISTS ( + SELECT 1 + FROM role_menu_rela rmr + JOIN menus m ON m.id::text = rmr.menu_id OR m.code = rmr.menu_id + WHERE rmr.role_id = r.id + AND m.code NOT IN :removed_menu_codes + AND ( + LOWER(m.code) LIKE :keyword + OR LOWER(m.name) LIKE :keyword + ) + ) + ) + """ + select_clause = "SELECT r.id, r.name" + order_clause = "ORDER BY r.create_date DESC NULLS LAST, r.id ASC" else: - if keyword: - where_clause = "WHERE LOWER(code) LIKE :keyword OR LOWER(name) LIKE :keyword" - rows = db.execute( - text( - f""" - SELECT id::text AS id, code, name - FROM roles - {where_clause} - ORDER BY id ASC - """ - ), - params - ).mappings().all() - return [dict(row) for row in rows] + from_clause = """ + FROM roles r + WHERE ( + :keyword IS NULL + OR LOWER(r.code) LIKE :keyword + OR LOWER(r.name) LIKE :keyword + OR EXISTS ( + SELECT 1 + FROM role_menus rm + JOIN menus m ON m.id = rm.menu_id + WHERE rm.role_id = r.id + AND m.code NOT IN :removed_menu_codes + AND ( + LOWER(m.code) LIKE :keyword + OR LOWER(m.name) LIKE :keyword + ) + ) + ) + """ + select_clause = "SELECT r.id::text AS id, r.code, r.name" + order_clause = "ORDER BY r.id ASC" + + trimmed_keyword = keyword.strip() if keyword else "" + params["keyword"] = f"%{trimmed_keyword.lower()}%" if trimmed_keyword else None + + total = db.scalar( + text(f"SELECT COUNT(*) {from_clause}").bindparams(bindparam("removed_menu_codes", expanding=True)), + {**params, "removed_menu_codes": tuple(REMOVED_MENU_CODES)}, + ) or 0 + rows = db.execute( + text( + f""" + {select_clause} + {from_clause} + {order_clause} + {limit_clause} + OFFSET :offset + """ + ).bindparams(bindparam("removed_menu_codes", expanding=True)), + {**params, "removed_menu_codes": tuple(REMOVED_MENU_CODES)}, + ).mappings().all() + return [dict(row) for row in rows], int(total) def _load_role_permission_codes_map( diff --git a/api/tests/test_role_pagination_contract.py b/api/tests/test_role_pagination_contract.py new file mode 100644 index 0000000..32eb593 --- /dev/null +++ b/api/tests/test_role_pagination_contract.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +from app.services import legacy_admin_rbac_service as service + + +def _role_row(index: int) -> dict[str, object]: + return {"id": f"role-{index}", "code": f"role-{index}", "name": f"Role {index}"} + + +def test_list_roles_returns_filtered_total_before_pagination(monkeypatch) -> None: + captured: dict[str, object] = {} + + monkeypatch.setattr(service, "_legacy_role_table_exists", lambda db: True) + + def load_page(db, *, role_source, keyword=None, limit=None, offset=0): + captured.update({"role_source": role_source, "keyword": keyword, "limit": limit, "offset": offset}) + return [_role_row(3), _role_row(4)], 5 + + monkeypatch.setattr(service, "_load_role_page", load_page) + monkeypatch.setattr(service, "_load_role_menu_ids_map", lambda db, role_ids, *, role_source: {role_id: [] for role_id in role_ids}) + monkeypatch.setattr(service, "_load_menus_map", lambda db, menu_ids, *, role_source: {}) + monkeypatch.setattr(service, "_load_role_permission_codes_map", lambda db, role_ids, *, role_source: {role_id: [] for role_id in role_ids}) + + response = service.list_roles(object(), keyword="role", limit=2, offset=2) + + assert response.total == 5 + assert [role.id for role in response.items] == ["role-3", "role-4"] + assert captured == {"role_source": "legacy", "keyword": "role", "limit": 2, "offset": 2} + + +def test_list_roles_with_menus_paginates_roles_only(monkeypatch) -> None: + captured: dict[str, object] = {} + menu_response = service.MenuListResponse(items=[], total=2) + + monkeypatch.setattr(service, "_legacy_role_table_exists", lambda db: False) + + def load_page(db, *, role_source, keyword=None, limit=None, offset=0): + captured.update({"role_source": role_source, "keyword": keyword, "limit": limit, "offset": offset}) + return [_role_row(2)], 3 + + monkeypatch.setattr(service, "_load_role_page", load_page) + monkeypatch.setattr(service, "_load_role_menu_ids_map", lambda db, role_ids, *, role_source: {role_id: [] for role_id in role_ids}) + monkeypatch.setattr(service, "_load_menus_map", lambda db, menu_ids, *, role_source: {}) + monkeypatch.setattr(service, "_load_role_permission_codes_map", lambda db, role_ids, *, role_source: {role_id: [] for role_id in role_ids}) + monkeypatch.setattr(service, "list_menus", lambda db: menu_response) + + response = service.list_roles_with_menus(object(), limit=1, offset=1) + + assert response.roles_total == 3 + assert [role.id for role in response.roles] == ["role-2"] + assert response.menus == [] + assert response.menus_total == 2 + assert captured == {"role_source": "modern", "keyword": None, "limit": 1, "offset": 1} diff --git a/web/src/app/admin/roles/page.tsx b/web/src/app/admin/roles/page.tsx index 1756b20..6a0aa19 100644 --- a/web/src/app/admin/roles/page.tsx +++ b/web/src/app/admin/roles/page.tsx @@ -7,6 +7,7 @@ import { Button, Card, Col, + Dropdown, Empty, Form, Input, @@ -21,7 +22,7 @@ import { type CardProps, type MenuProps, } from "antd"; -import { EditOutlined, DeleteOutlined } from "@ant-design/icons"; +import { EditOutlined, MoreOutlined } from "@ant-design/icons"; import type { ColumnsType } from "antd/es/table"; import type { CSSProperties, ComponentType } from "react"; @@ -30,12 +31,10 @@ import { useToastFeedback } from "@/hooks/use-toast-feedback"; import { useTopicSubscription } from "@/hooks/use-topic-subscription"; import { useMobileDetection } from "@/hooks/use-mobile-detection"; import { readApiError } from "@/lib/api"; -import type { MenuItem, RoleItem, RoleListResponse } from "@/types/auth"; +import type { MenuItem, RoleItem } from "@/types/auth"; const AntCard = Card as unknown as ComponentType; -type MenuListResponse = { items: MenuItem[]; total: number }; - type RolesWithMenusResponse = { roles: RoleItem[]; roles_total: number; @@ -68,6 +67,7 @@ export default function AdminRolesPage() { const [keywordInput, setKeywordInput] = useState(""); const [searchKeyword, setSearchKeyword] = useState(""); const keywordDebounceTimeoutRef = useRef(null); + const [pagination, setPagination] = useState({ current: 1, pageSize: 20 }); const [error, setError] = useState(""); const [success, setSuccess] = useState(""); @@ -85,14 +85,19 @@ export default function AdminRolesPage() { const canRead = hasPermission("role.read") || hasPermission("role.manage"); const canManage = hasPermission("role.manage"); + const { current: paginationCurrent, pageSize: paginationPageSize } = pagination; const trimmedKeyword = searchKeyword.trim(); - const rolesQueryUrl = useMemo(() => { - const url = trimmedKeyword - ? `/api/v1/admin/roles-with-menus?keyword=${encodeURIComponent(trimmedKeyword)}` - : "/api/v1/admin/roles-with-menus"; - return url; - }, [trimmedKeyword]); + const rolesQueryParams = useMemo(() => { + const params = new URLSearchParams(); + params.set("limit", String(paginationPageSize)); + params.set("offset", String((paginationCurrent - 1) * paginationPageSize)); + if (trimmedKeyword) { + params.set("keyword", trimmedKeyword); + } + return params.toString(); + }, [paginationCurrent, paginationPageSize, trimmedKeyword]); + const rolesQueryUrl = `/api/v1/admin/roles-with-menus?${rolesQueryParams}`; const loadRolesWithMenus = useCallback(async () => { const response = await fetchWithAuth(rolesQueryUrl); @@ -101,7 +106,7 @@ export default function AdminRolesPage() { }, [fetchWithAuth, rolesQueryUrl]); const rolesQuery = useQuery({ - queryKey: ["admin.roles", rolesQueryUrl], + queryKey: ["admin.roles", rolesQueryParams], queryFn: loadRolesWithMenus, enabled: !!user && canRead, }); @@ -294,6 +299,7 @@ export default function AdminRolesPage() { keywordDebounceTimeoutRef.current = setTimeout(() => { setSearchKeyword(value); + setPagination((prev) => ({ ...prev, current: 1 })); setCardViewPage(1); setAllLoadedRoles([]); }, 500); @@ -311,7 +317,11 @@ export default function AdminRolesPage() { // Update allLoadedRoles when roles data changes in card view useEffect(() => { - if (viewMode === "card" && !rolesQuery.isLoading) { + if (viewMode !== "card" || rolesQuery.isLoading) { + return; + } + + const frameId = window.requestAnimationFrame(() => { if (cardViewPage === 1) { setAllLoadedRoles(roles); } else { @@ -325,7 +335,11 @@ export default function AdminRolesPage() { }); } setIsLoadingMore(false); - } + }); + + return () => { + window.cancelAnimationFrame(frameId); + }; }, [roles, rolesQuery.isLoading, viewMode, cardViewPage]); // Handle infinite scroll for card view @@ -352,6 +366,7 @@ export default function AdminRolesPage() { if (loadedCount < total) { setIsLoadingMore(true); setCardViewPage((prev) => prev + 1); + setPagination((prev) => ({ ...prev, current: prev.current + 1 })); } } }; @@ -360,12 +375,6 @@ export default function AdminRolesPage() { return () => cardBody.removeEventListener("scroll", handleScroll); }, [viewMode, isLoadingMore, rolesQuery.isLoading, rolesQuery.data?.roles_total, allLoadedRoles.length]); - // Reset card view state when filters change - useEffect(() => { - setCardViewPage(1); - setAllLoadedRoles([]); - }, [trimmedKeyword]); - const columns = useMemo>(() => { const base: ColumnsType = [ { @@ -460,6 +469,24 @@ export default function AdminRolesPage() { const isDeleting = deletingRoleId === role.id; const isSaving = savingRoleId === role.id; const rowBusy = isDeleting || isSaving || createRoleMutation.isPending || updateRoleMutation.isPending; + const moreMenuItems: MenuProps["items"] = [ + { + key: "delete", + label: "删除", + danger: true, + disabled: rowBusy, + onClick: () => { + Modal.confirm({ + title: `确认删除角色 ${role.code} 吗？`, + content: "删除后无法恢复，请谨慎操作。", + okText: "删除", + cancelText: "取消", + okButtonProps: { danger: true, loading: isDeleting }, + onOk: () => deleteRoleMutation.mutate(role.id), + }); + }, + }, + ]; const menuLabels = role.menu_ids.length > 0 ? role.menu_ids.map((menuId) => menuNameById.get(menuId) ?? String(menuId)) @@ -489,24 +516,14 @@ export default function AdminRolesPage() { disabled={rowBusy} onClick={() => startEdit(role)} /> - deleteRoleMutation.mutate(role.id)} - disabled={rowBusy} - > +