Repository navigation
feat(auth): separate account types, capabilities, and case roles - #108
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough本次变更将用户单一 Changes权限模型拆分
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant auth_service
participant case_service
participant Database
Client->>auth_service: authenticate and receive typed identity
Client->>case_service: request case operation with case_role
case_service->>Database: validate membership and capabilities
Database-->>case_service: account and membership data
case_service-->>Client: authorized case response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
migration/sql/mysql/up/0012_split_account_type_and_capabilities.sql (1)
13-24: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win重命名
role后应显式删除旧索引，避免回滚时同名索引冲突。CHANGE COLUMN role account_type会保留idx_users_role_status的索引名；随后DROP COLUMN account_type只会把该列从复合索引里移除，down再创建idx_users_role_status时会报Duplicate key name。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@migration/sql/mysql/up/0012_split_account_type_and_capabilities.sql` around lines 13 - 24, 在迁移脚本中，围绕 CHANGE COLUMN role account_type 显式删除遗留的 idx_users_role_status 索引，并确保后续创建 idx_users_account_type_status 不会保留旧索引名。同步调整回滚逻辑，使其创建 idx_users_role_status 前不会因同名索引冲突而失败。Source: Path instructions
🧹 Nitpick comments (3)
docs/openapi.yaml (1)
611-634: 🗄️ Data Integrity & Integration | 🔵 Trivial建议在变更记录中标注该破坏性 API 变更。
静态分析（oasdiff）提示 login/auth-me/case-members 响应中移除了必需字段
global_role。这是本 PR 有意为之的角色拆分（role→account_type+global_capabilities），前后端已同步更新，但对外部 API 消费者而言仍是破坏性变更，建议在 API 变更记录或版本说明中显式标注，方便第三方客户端升级。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/openapi.yaml` around lines 611 - 634, 在 API 变更记录或版本说明中明确标注 login/auth-me/case-members 响应移除必需字段 global_role 属于破坏性变更，并说明其已由 account_type 与 global_capabilities 替代，提示第三方客户端完成升级。Source: Linters/SAST tools
src/roles.rs (1)
155-177: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
InvalidRole丢弃了实际的无效值，不利于排查数据完整性问题。三个构造函数都接收
_value: &str却从未使用，导致最终报错（如account_type_from_database中拼接的ApiError::Database消息）只能说明"哪个字段类型不对"，却看不到具体是什么脏值触发的。这会削弱本 PR 目标中"未知持久化值应产生可诊断的数据完整性错误"的可观测性。♻️ 建议保留原始值以便诊断
-#[derive(Debug, Eq, Error, PartialEq)] -#[error("invalid {scope} role value")] +#[derive(Debug, Eq, Error, PartialEq)] +#[error("invalid {scope} role value {value:?}")] pub struct InvalidRole { scope: &'static str, + value: String, } impl InvalidRole { - const fn account_type(_value: &str) -> Self { - Self { - scope: "account type", - } - } - - const fn global_capability(_value: &str) -> Self { - Self { - scope: "global capability", - } - } - - const fn case(_value: &str) -> Self { - Self { scope: "case" } - } + fn account_type(value: &str) -> Self { + Self { + scope: "account type", + value: value.to_owned(), + } + } + + fn global_capability(value: &str) -> Self { + Self { + scope: "global capability", + value: value.to_owned(), + } + } + + fn case(value: &str) -> Self { + Self { + scope: "case", + value: value.to_owned(), + } + } }As per path instructions, "Review Rust/Actix code for ... error handling ... data-loss risks."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/roles.rs` around lines 155 - 177, Update InvalidRole to retain the offending role value in addition to scope, and include that value in its Display/error output. Modify account_type, global_capability, and case to store their _value argument rather than discarding it, while preserving each constructor’s existing scope label and the diagnostic propagation through callers such as account_type_from_database.Source: Path instructions
migration/sql/postgres/up/0012_split_account_type_and_capabilities.sql (1)
11-23: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff大表场景下的锁表/校验开销建议（视规模决定是否需要立即处理）。
Squawk 提示：
SET NOT NULL（第 15 行）会在校验期间阻塞读取，DROP COLUMN（第 19 行）/新增未NOT VALID的CHECK约束（第 21 行）/非并发建索引（第 23 行）在大表上都可能造成短暂锁表。对于当前项目体量可能影响有限，但如果未来users表增长，建议改为"先加可空列 + CHECK 约束校验，再补 NOT NULL"和NOT VALID+VALIDATE CONSTRAINT的模式。需要注意：docs/DATABASE.md 提到迁移在数据库支持时会整体放入事务，而
CREATE INDEX CONCURRENTLY无法在事务块内执行，因此该项建议是否可行取决于migration/src/m0012_split_account_type_and_capabilities.rs的实际事务包装方式（未在本次评审文件中提供），建议后续确认。As per path instructions, "Check SeaORM migrations for safe forward and rollback behavior... Flag destructive changes..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@migration/sql/postgres/up/0012_split_account_type_and_capabilities.sql` around lines 11 - 23, 改造 users 表迁移以降低大表锁定与校验开销：先填充可空的 account_type，添加 users_account_type_check 为 NOT VALID 后执行校验，再设置 NOT NULL；仅在迁移不处于事务时将 idx_users_account_type_status 改为并发创建，否则保留事务安全方案并确认 SeaORM 的事务包装方式。审慎处理 DROP COLUMN role，确保前向迁移不会丢失仍需使用的数据，并同步提供可安全回滚的迁移行为。Sources: Path instructions, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/API.md`:
- Around line 67-71: Update the role descriptions in the API documentation to
note that users with the family case role may also add members by inviting a
commander for initial case leadership, while they cannot invite volunteers. Keep
the existing commander permissions and other role descriptions unchanged.
In `@frontend/src/pages/DashboardPage.tsx`:
- Around line 65-68: 在 DashboardPage 的管理员空案件列表分支补充回归测试：模拟 account_type 为
member、global_capabilities 包含 admin 且案件列表为空的用户状态，断言页面显示“管理员账号不自动拥有案件权限”。沿用现有
learner 空状态测试结构，确保覆盖 user?.global_capabilities.includes('admin') 分支及其用户可见提示。
In `@migration/sql/mysql/down/0012_restore_single_user_role.sql`:
- Around line 15-21: 在对应的 up 迁移中，显式执行 DROP INDEX idx_users_role_status，再进行 users
表的列变更，确保索引不会被静默保留；随后验证该迁移与本 down 迁移可以完成 up → down 循环，且 CREATE INDEX
idx_users_role_status 不再发生同名冲突。
In `@migration/src/m0012_split_account_type_and_capabilities.rs`:
- Around line 25-33: 为 migration 的 down 回滚明确记录这是有损转换：多能力用户折叠为单一 role 时按 admin >
commander > volunteer > family 取最高优先级，其余能力会丢失。围绕
m0012_split_account_type_and_capabilities 的 down 方法补充覆盖多能力用户的回滚测试，验证该优先级及最终
role，确保行为被锁定。
---
Outside diff comments:
In `@migration/sql/mysql/up/0012_split_account_type_and_capabilities.sql`:
- Around line 13-24: 在迁移脚本中，围绕 CHANGE COLUMN role account_type 显式删除遗留的
idx_users_role_status 索引，并确保后续创建 idx_users_account_type_status
不会保留旧索引名。同步调整回滚逻辑，使其创建 idx_users_role_status 前不会因同名索引冲突而失败。
---
Nitpick comments:
In `@docs/openapi.yaml`:
- Around line 611-634: 在 API 变更记录或版本说明中明确标注 login/auth-me/case-members 响应移除必需字段
global_role 属于破坏性变更，并说明其已由 account_type 与 global_capabilities 替代，提示第三方客户端完成升级。
In `@migration/sql/postgres/up/0012_split_account_type_and_capabilities.sql`:
- Around line 11-23: 改造 users 表迁移以降低大表锁定与校验开销：先填充可空的 account_type，添加
users_account_type_check 为 NOT VALID 后执行校验，再设置 NOT NULL；仅在迁移不处于事务时将
idx_users_account_type_status 改为并发创建，否则保留事务安全方案并确认 SeaORM 的事务包装方式。审慎处理 DROP
COLUMN role，确保前向迁移不会丢失仍需使用的数据，并同步提供可安全回滚的迁移行为。
In `@src/roles.rs`:
- Around line 155-177: Update InvalidRole to retain the offending role value in
addition to scope, and include that value in its Display/error output. Modify
account_type, global_capability, and case to store their _value argument rather
than discarding it, while preserving each constructor’s existing scope label and
the diagnostic propagation through callers such as account_type_from_database.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 13b15ba1-290d-467b-9a80-39f13036c29c
📒 Files selected for processing (39)
docs/API.mddocs/DATABASE.mddocs/openapi.yamlfrontend/src/App.test.tsxfrontend/src/App.tsxfrontend/src/api/auth.tsfrontend/src/api/cases.test.tsfrontend/src/api/cases.tsfrontend/src/auth/AuthContext.test.tsxfrontend/src/components/AppShell.tsxfrontend/src/pages/CaseWorkspacePage.tsxfrontend/src/pages/DashboardPage.test.tsxfrontend/src/pages/DashboardPage.tsxmigration/sql/mysql/down/0012_restore_single_user_role.sqlmigration/sql/mysql/up/0012_split_account_type_and_capabilities.sqlmigration/sql/postgres/down/0012_restore_single_user_role.sqlmigration/sql/postgres/up/0012_split_account_type_and_capabilities.sqlmigration/sql/sqlite/down/0012_restore_single_user_role.sqlmigration/sql/sqlite/up/0012_split_account_type_and_capabilities.sqlmigration/src/lib.rsmigration/src/m0012_split_account_type_and_capabilities.rssrc/bin/angui-admin.rssrc/entities/mod.rssrc/entities/user_global_capabilities.rssrc/entities/users.rssrc/lib.rssrc/models.rssrc/roles.rssrc/routes/auth.rssrc/services/auth_service.rssrc/services/case_service.rssrc/services/intake_session_service.rstests/api/auth_login_post.rstests/api/auth_me_get.rstests/api/case_members_post.rstests/api/cases_create_post.rstests/api/cases_list_get.rstests/api/intake_sessions_create_post.rstests/support/mod.rs
| - `member`：正常业务账号类型。可创建案件；创建者在新案件中获得 `family` 角色。只有被显式加入案件后才能读取或操作该案件。 | ||
| - `learner`：长期学习账号类型；当前可登录并恢复会话，但不能创建或加入案件，学习与考核能力将在后续平台提供。 | ||
| - `commander`：可叠加的全局能力。持有者可被显式授予某案件的 `commander` 角色；该案件角色才允许查看全部线索、审核线索、改变案件状态和添加成员。 | ||
| - `volunteer`：可叠加的全局能力。持有者可被指挥显式授予某案件的 `volunteer` 角色；该案件角色只能查看已确认线索，老人资料中的健康注意字段由服务端删除。 | ||
| - `admin`：可叠加的全局能力，不自动获得业务案件访问权，也不绕过案件成员关系。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
"添加成员"权限描述与实际授权行为不完全一致
文档将"添加成员"能力仅归于持有 commander 案件角色的用户。但根据 tests/api/case_members_post.rs 中的 post_case_members_applies_invitation_role_and_duplicate_rules 测试，持有 family 案件角色的用户也可以成功邀请 commander（用于案件的初始指挥授权），只是不能邀请 volunteer。建议补充说明 family 角色的这一特例邀请权限，避免 API 使用者误以为 family 完全没有邀请能力。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/API.md` around lines 67 - 71, Update the role descriptions in the API
documentation to note that users with the family case role may also add members
by inviting a commander for initial case leadership, while they cannot invite
volunteers. Keep the existing commander permissions and other role descriptions
unchanged.
| : user?.global_capabilities.includes('admin') | ||
| ? { | ||
| title: '管理员账号不自动拥有案件权限', | ||
| description: '管理员需要先被授予具体案件成员关系，才能查看案件内容。', |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
补充管理员无案件成员关系的回归测试。
当前提供的前端测试仅覆盖 learner 空状态；global_capabilities.includes('admin') 分支没有覆盖。请模拟 account_type: 'member' 且 global_capabilities: ['admin']、案件列表为空，并断言“管理员账号不自动拥有案件权限”，避免管理员被错误展示为拥有案件访问权。
As per path instructions, frontend reviews must focus on authentication state and user-visible failure paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/DashboardPage.tsx` around lines 65 - 68, 在 DashboardPage
的管理员空案件列表分支补充回归测试：模拟 account_type 为 member、global_capabilities 包含 admin
且案件列表为空的用户状态，断言页面显示“管理员账号不自动拥有案件权限”。沿用现有 learner 空状态测试结构，确保覆盖
user?.global_capabilities.includes('admin') 分支及其用户可见提示。
Source: Path instructions
| async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { | ||
| let sql = sql_for_backend( | ||
| manager, | ||
| include_str!("../sql/sqlite/down/0012_restore_single_user_role.sql"), | ||
| include_str!("../sql/postgres/down/0012_restore_single_user_role.sql"), | ||
| include_str!("../sql/mysql/down/0012_restore_single_user_role.sql"), | ||
| ); | ||
| execute_script(manager, sql).await | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查看三种方言下 0012 回滚脚本的完整内容，确认多能力用户的 role 取值逻辑
fd '0012_restore_single_user_role.sql' migration/sql --exec cat -n {}Repository: AG-Angui/Angui
Length of output: 4231
回滚会静默丢失多能力用户的能力信息
down() 把 user_global_capabilities 折叠成单一 role，并按 admin > commander > volunteer > family 固定优先级取值；一旦用户同时拥有多个全局能力，回滚就会静默丢掉其余能力。若这是预期的有损回滚，需要显式说明，并补一条覆盖多能力用户的回滚测试，锁定该优先级。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@migration/src/m0012_split_account_type_and_capabilities.rs` around lines 25 -
33, 为 migration 的 down 回滚明确记录这是有损转换：多能力用户折叠为单一 role 时按 admin > commander >
volunteer > family 取最高优先级，其余能力会丢失。围绕 m0012_split_account_type_and_capabilities 的
down 方法补充覆盖多能力用户的回滚测试，验证该优先级及最终 role，确保行为被锁定。
Preview readyThe preview for this build is available at https://pr-108-dab40e4e071b.angui.cg8.site. Temporary demo accounts (all use the password
|
Closes #106
Summary
Verification
Summary by CodeRabbit
account_type与global_capabilities。case_role，并携带成员能力信息。