diff --git a/api/app/services/legacy_admin_rbac_service.py b/api/app/services/legacy_admin_rbac_service.py index d140f69..74e012d 100644 --- a/api/app/services/legacy_admin_rbac_service.py +++ b/api/app/services/legacy_admin_rbac_service.py @@ -1012,14 +1012,25 @@ def _load_role_page( limit_clause = "LIMIT :limit" params["limit"] = normalized_limit - if role_source == "legacy": - from_clause = """ - FROM user_role r - WHERE ( - :keyword IS NULL - OR LOWER(r.id) LIKE :keyword - OR LOWER(r.name) LIKE :keyword + trimmed_keyword = keyword.strip() if keyword else "" + keyword_clause = "" + if trimmed_keyword: + keyword_clause = """ + AND ( + {role_keyword_predicates} OR EXISTS ( + {menu_exists_query} + ) + ) + """ + params["keyword"] = f"%{trimmed_keyword.lower()}%" + + if role_source == "legacy": + role_keyword_predicates = """ + LOWER(r.id) LIKE :keyword + OR LOWER(r.name) LIKE :keyword + """ + menu_exists_query = """ SELECT 1 FROM role_menu_rela rmr JOIN menus m ON m.id::text = rmr.menu_id OR m.code = rmr.menu_id @@ -1029,19 +1040,20 @@ def _load_role_page( LOWER(m.code) LIKE :keyword OR LOWER(m.name) LIKE :keyword ) - ) - ) + """ + from_clause = """ + FROM user_role r + WHERE 1 = 1 + {keyword_clause} """ select_clause = "SELECT r.id, r.name" order_clause = "ORDER BY r.create_date DESC NULLS LAST, r.id ASC" else: - from_clause = """ - FROM roles r - WHERE ( - :keyword IS NULL - OR LOWER(r.code) LIKE :keyword + role_keyword_predicates = """ + LOWER(r.code) LIKE :keyword OR LOWER(r.name) LIKE :keyword - OR EXISTS ( + """ + menu_exists_query = """ SELECT 1 FROM role_menus rm JOIN menus m ON m.id = rm.menu_id @@ -1051,21 +1063,35 @@ def _load_role_page( LOWER(m.code) LIKE :keyword OR LOWER(m.name) LIKE :keyword ) - ) - ) + """ + from_clause = """ + FROM roles r + WHERE 1 = 1 + {keyword_clause} """ 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 + if keyword_clause: + keyword_clause = keyword_clause.format( + role_keyword_predicates=role_keyword_predicates, + menu_exists_query=menu_exists_query, + ) + from_clause = from_clause.format(keyword_clause=keyword_clause) + query_params = {**params, "removed_menu_codes": tuple(REMOVED_MENU_CODES)} if trimmed_keyword else params + + def role_page_stmt(sql: str): + stmt = text(sql) + if trimmed_keyword: + return stmt.bindparams(bindparam("removed_menu_codes", expanding=True)) + return stmt total = db.scalar( - text(f"SELECT COUNT(*) {from_clause}").bindparams(bindparam("removed_menu_codes", expanding=True)), - {**params, "removed_menu_codes": tuple(REMOVED_MENU_CODES)}, + role_page_stmt(f"SELECT COUNT(*) {from_clause}"), + query_params, ) or 0 rows = db.execute( - text( + role_page_stmt( f""" {select_clause} {from_clause} @@ -1073,8 +1099,8 @@ def _load_role_page( {limit_clause} OFFSET :offset """ - ).bindparams(bindparam("removed_menu_codes", expanding=True)), - {**params, "removed_menu_codes": tuple(REMOVED_MENU_CODES)}, + ), + query_params, ).mappings().all() return [dict(row) for row in rows], int(total) diff --git a/api/tests/test_role_pagination_contract.py b/api/tests/test_role_pagination_contract.py index 32eb593..7c9dd4b 100644 --- a/api/tests/test_role_pagination_contract.py +++ b/api/tests/test_role_pagination_contract.py @@ -7,6 +7,27 @@ def _role_row(index: int) -> dict[str, object]: return {"id": f"role-{index}", "code": f"role-{index}", "name": f"Role {index}"} +class _EmptyRolePageRows: + def mappings(self): + return self + + def all(self) -> list[dict[str, object]]: + return [] + + +class _RolePageCaptureDb: + def __init__(self) -> None: + self.calls: list[tuple[str, str, dict[str, object]]] = [] + + def scalar(self, stmt, params=None) -> int: + self.calls.append(("scalar", str(stmt), dict(params or {}))) + return 0 + + def execute(self, stmt, params=None) -> _EmptyRolePageRows: + self.calls.append(("execute", str(stmt), dict(params or {}))) + return _EmptyRolePageRows() + + def test_list_roles_returns_filtered_total_before_pagination(monkeypatch) -> None: captured: dict[str, object] = {} @@ -51,3 +72,40 @@ def test_list_roles_with_menus_paginates_roles_only(monkeypatch) -> None: assert response.menus == [] assert response.menus_total == 2 assert captured == {"role_source": "modern", "keyword": None, "limit": 1, "offset": 1} + + +def test_load_role_page_without_keyword_omits_nullable_keyword_predicate() -> None: + for role_source in ("legacy", "modern"): + db = _RolePageCaptureDb() + + rows, total = service._load_role_page(db, role_source=role_source, keyword=None, limit=20, offset=0) + + assert rows == [] + assert total == 0 + assert len(db.calls) == 2 + for _, sql, params in db.calls: + assert ":keyword" not in sql + assert ":keyword IS NULL" not in sql + assert "removed_menu_codes" not in params + + +def test_load_role_page_with_keyword_uses_normalized_like_filter() -> None: + role_predicates = { + "legacy": "LOWER(r.id) LIKE :keyword", + "modern": "LOWER(r.code) LIKE :keyword", + } + for role_source, role_predicate in role_predicates.items(): + db = _RolePageCaptureDb() + + rows, total = service._load_role_page(db, role_source=role_source, keyword=" Admin ", limit=20, offset=0) + + assert rows == [] + assert total == 0 + assert len(db.calls) == 2 + for _, sql, params in db.calls: + assert ":keyword IS NULL" not in sql + assert role_predicate in sql + assert "LOWER(r.name) LIKE :keyword" in sql + assert "LOWER(m.code) LIKE :keyword" in sql + assert params["keyword"] == "%admin%" + assert "removed_menu_codes" in params diff --git a/memory/2026-06-20.md b/memory/2026-06-20.md index a8f3e8d..2359941 100644 --- a/memory/2026-06-20.md +++ b/memory/2026-06-20.md @@ -509,3 +509,22 @@ - 风险与关注点： - 改动仅影响 `[data-fquiz-theme="dark"]` 下菜单管理页卡片相关 CSS，不改变亮色主题、接口、权限或页面业务逻辑。 + +# Work Log - 角色列表接口 keyword SQL 修复（FL-218） + +- 背景： + - `/api/v1/admin/roles-with-menus` 与 `/api/v1/admin/roles` 在 PostgreSQL/psycopg 下因 `_load_role_page()` 使用 `:keyword IS NULL OR ... LIKE :keyword` 触发参数类型推断失败，角色管理页列表与搜索返回 500。 + +- 本次处理： + - `legacy_admin_rbac_service._load_role_page()` 改为仅在 keyword 非空时拼接角色/菜单搜索条件，空 keyword 列表查询不再生成 `:keyword IS NULL`。 + - `removed_menu_codes` expanding bind 仅在 keyword 搜索子查询需要菜单过滤时绑定，避免空列表查询携带无用 bind。 + - `api/tests/test_role_pagination_contract.py` 补充 legacy/modern 两套角色源的 SQL 构造回归测试，覆盖空 keyword 与带 keyword 两种路径。 + +- 验证： + - 基线：`UV_CACHE_DIR=/tmp/fquiz-uv-cache UV_PYTHON_INSTALL_DIR=/tmp/fquiz-uv-python uv run --python 3.11 --with pytest --with fastapi --with pydantic-settings --with sqlalchemy --with PyJWT --with argon2-cffi --with email-validator --with python-multipart --with psycopg[binary] --with bcrypt pytest api/tests/test_role_pagination_contract.py` 通过，2 passed。 + - 修改后：同命令通过，4 passed。 + - 修改后：`python3 -m py_compile api/app/services/legacy_admin_rbac_service.py api/tests/test_role_pagination_contract.py` 通过。 + - 修改后：`git diff --check` 通过。 + +- 风险与关注点： + - 改动仅影响角色列表/搜索 SQL 生成，不改变接口路径、响应字段、分页参数、角色 CRUD 或权限语义。