---
req_id: REQ-USR-002
date: 2026-04-30
round: 1
reviewer: superpower-code-reviewer
---

# Review: REQ-USR-002 — round 1

## 结论
approve

## Must-fix
（无）

## Nice-to-have

- backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:134-140 — 与 USR-001 review #3 同源：permissionCategoryIds=[1,1,2] 全有效时 SQL IN 隐式去重 → countActiveByIds=2 ≠ ids.size=3 → 误抛 40023 假阴性。建议 ids = ids.stream().distinct().toList() 4 行修复，或 module-report 阶段统一处理两处。
- backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:200-229 — `putValidBody_with_jwt_returns200_andUpdates` 已断言 sPasswordHash / sCreatedBy 保留，漏 tCreateDate 与 sBrandsId 锁。spec § 验收 工程 #2 显列 'sPasswordHash / sCreatedBy / tCreateDate 在 PUT 前后字面相同'。建议 SELECT 加这两列断言保留原值。
- backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:200-229 — 类似 USR-001 review #4：响应 data 缺 sPasswordHash 不变量没断言。建议加 `assertThat(jb.get("data").has("sPasswordHash")).isFalse()`。
- backend/src/test/java/com/xly/erp/module/usr/service/UserServiceImplTest.java:305-317 — `updateWithEmptyPermissionIds_clearsExisting` 只覆盖 null 路径，未覆盖 `[]` 空 list 路径。spec § 输入末行明言 'null / 空 list → 清空' 是同等语义。建议补等价 case 或 parameterize。
- backend/src/main/java/com/xly/erp/module/usr/dto/UpdateUserDTO.java — 类头缺 JavaDoc 说明 sPasswordHash 故意剔除（spec 显列 'API 契约声明该字段不可改'）。两行注释帮助未来读者理解 DTO 与 CreateUserDTO 字段差异。
- backend/src/main/java/com/xly/erp/module/usr/service/impl/UserServiceImpl.java:120 — `update` 方法体缺 `// REQ-USR-002: 用户修改` 行内锚点（与 USR-001 review #7 同源）。建议 module-report 阶段统一补齐 USR/MOD 所有 REQ 行内锚点。
- backend/src/test/java/com/xly/erp/module/usr/controller/UserControllerIT.java:342-371 — `putWithoutJwt_permitAllStub_*` / `putTamperedJwt_*` 沿袭 stub 路径仍缺 `// REQ-MOD-001 stub: see USR-004 follow-up` 锚点；与上条统一 module-report 处理。

## 反例 / 测试覆盖缺口

Spec 验收清单（service 10 + mapperIT 1 + controllerIT 10）100% 落地，mvn 110/110 全绿。

- 重建权限组策略（先 deleteByUserId 后 batch INSERT）严格匹配 spec § 业务规则 #6，`@Transactional(rollbackFor = Exception.class)` 包整流程。
- 流程顺序（selectById 存在性 → 枚举 → staff → permission → updateById catch DupKey → deleteByUserId → INSERT × N）严格匹配 spec § 业务规则 1–7。
- updateById 抛 DupKey 后 deleteByUserId 不被调用由 `verify(userPermissionMapper, never()).deleteByUserId(any())` 单测显式锁定，事务回滚由 @Transactional 保证。
- 不可改字段（sPasswordHash / sCreatedBy / tCreateDate / sBrandsId / sSubsidiaryId）依赖 MyBatis-Plus 默认 NOT_NULL 跳过 null 字段策略；service 单测用 ArgumentCaptor 断言这些字段在 entity 上为 null。
- permissionCategoryIds null / 空 list 通过 `ids != null && !ids.isEmpty()` 短路同时跳过校验与 INSERT 但仍执行 deleteByUserId，正确实现「清空」语义。
- LocalDateTime.now() 已 hoist 出 INSERT 循环（修正 USR-001 review nice-to-have #2）。

非阻塞缺口：permissionCategoryIds 重复 id 假阴性、sPasswordHash 响应不变量锁、tCreateDate/sBrandsId 保留断言、`[]` 空 list 等价路径覆盖、行内 REQ 锚点 + stub 锚点。前 4 条可在本 REQ 后续修补，后 2 条按既定路径留 module-report 统一处理。